Bug 157502 – [JSC] FTL can produce GetByVal nodes without proper bounds checking

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[JSC] FTL can produce GetByVal nodes without proper bounds checking

[Benjamin Poulain]

Created attachment 278473 [details]
Patch

[Filip Pizlo]

Comment on attachment 278473 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGPlan.cpp:422
> +
> +        performLivenessAnalysis(dfg);
> +        performIntegerRangeOptimization(dfg);

Can you file a bug about this very strange behavior of the IR?  Ideally, the IR would be able to tell LICM if it's safe to hoist GetByVal.  I think having a bug about this, and referencing it in a FIXME here, would be a good way of not forgetting about this very surprising behavior.

[Filip Pizlo]

(In reply to comment #2)
> Comment on attachment 278473 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=278473&action=review
> 
> > Source/JavaScriptCore/dfg/DFGPlan.cpp:422
> > +
> > +        performLivenessAnalysis(dfg);
> > +        performIntegerRangeOptimization(dfg);
> 
> Can you file a bug about this very strange behavior of the IR?  Ideally, the
> IR would be able to tell LICM if it's safe to hoist GetByVal.  I think
> having a bug about this, and referencing it in a FIXME here, would be a good
> way of not forgetting about this very surprising behavior.

It's sort of crazy that LICM for GetByVal is only correct if it *also* hoists the CheckInBounds.  That's nuts!

Please definitely put a FIXME linking a bug somewhere.  We don't want to forget how insane this is.

[Benjamin Poulain]

Created attachment 278528 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 278528 [details]
Patch for landing

Clearing flags on attachment: 278528

Committed r200645: <http://trac.webkit.org/changeset/200645>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.