Bug 68716 - Add a bunch of unhandled node types to the propagator
Summary: Add a bunch of unhandled node types to the propagator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-23 12:39 PDT by Oliver Hunt
Modified: 2011-09-23 12:50 PDT (History)
0 users

See Also:

Attachments
Patch (2.28 KB, patch)
2011-09-23 12:41 PDT, Oliver Hunt 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2011-09-23 12:39:56 PDT 
Add a bunch of unhandled node types to the propagator
Comment 1 Oliver Hunt 2011-09-23 12:41:50 PDT 
Created attachment 108510 [details]
Patch
Comment 2 Darin Adler 2011-09-23 12:43:28 PDT 
Comment on attachment 108510 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108510&action=review

> Source/JavaScriptCore/dfg/DFGPropagator.cpp:-610
> -            ASSERT_NOT_REACHED();

Might still be nice to have an ASSERT_NOT_REACHED when the node type is not a legal enum value. Sometimes we do that by using return instead of break, and then asserting outside the switch statement.
Comment 3 Gavin Barraclough 2011-09-23 12:46:09 PDT 
Comment on attachment 108510 [details]
Patch

r+ with or without ValueToDouble doing anything.
Comment 4 Oliver Hunt 2011-09-23 12:47:38 PDT 
Committed r95851: <http://trac.webkit.org/changeset/95851>
Comment 5 Filip Pizlo 2011-09-23 12:50:37 PDT 
Comment on attachment 108510 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108510&action=review

> Source/JavaScriptCore/dfg/DFGPropagator.cpp:404
> +            changed |= mergePrediction(makePrediction(PredictNumber, StrongPrediction));

This should be PredictDouble.  PredictNumber means that it could be either double or int32.  ValueToDouble only returns doubles.

But also, this is dead code: ValueToDouble should never come up at this stage of compilation, since it is only introduced during post-propagation fixup just following this fixpoint.  We could put ASSERT_NOT_REACHED here.