Bug 156327 – [JSC] Do a better job a comparing unsigned integers

• Component: New Bugs
• Status: NEW

[Benjamin Poulain]

[JSC] Do a better job a comparing unsigned integers

[Benjamin Poulain]

Created attachment 275847 [details]
Patch

[WebKit Commit Bot]

Attachment 275847 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:143:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:181:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:301:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 275847 [details]
Patch

This change is unsound.  It's valid for a phase to perform optimizations based on the fact that UInt32ToNumber will OSR exit for certain values.  For example, it would be valid for a phase to claim that after UInt32ToNumber(@x) has executed, @x cannot possibly be negative.

This is the way our IR works for all other cases where we perform any kind of speculation.  This patch could already be wrong, if there is anything that already does UInt32ToNumber-based optimizations.  Probably, there is no other such optimization, but I don't see how we would prevent such an optimization from being written.  It would be awkward for UInt32ToNumber's speculation to be the only one exempted from the usual rule.

The correct way to make this work is to use Int52.  This is a feature of the DFG that we already have.  The correct behavior of UInt32ToNumber would be to return an Int52.  If you did this, then you'd get all of the other stuff for free.