Bug 159388 - DFG LICM needs to go all-in on the idea that some loops can't be LICMed
Summary: DFG LICM needs to go all-in on the idea that some loops can't be LICMed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-02 20:53 PDT by Filip Pizlo
Modified: 2016-07-03 10:48 PDT (History)
5 users (show)

See Also:


Attachments
the patch (4.55 KB, patch)
2016-07-02 21:00 PDT, Filip Pizlo
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2016-07-02 20:53:55 PDT
Some time ago I acknowledged that LICM required loops to meet certain requirements that may get broken by the time we do LICM, like that the terminal of the pre-header is ExitOK.  It used to be that we just ignored that requirement and would hoist anyway, but since r189126 we've stopped hoisting out of loops that don't have ExitOK.  We also added tests for the case that the pre-header doesn't exist or is invalid.

It turns out that this patch didn't go far enough: even though it made LICM avoid loops that had an invalid pre-header, the part that updated the AI state in nested loops still assumed that these loops had valid pre-headers.  We would crash in null dereference in that loop if a nested loop had an invalid pre-header.

The fix is simple: don't update the AI state of nested loops that don't have pre-headers, since we won't try to hoist out of those loops anyway.
Comment 1 Filip Pizlo 2016-07-02 21:00:49 PDT
Created attachment 282653 [details]
the patch
Comment 2 Mark Lam 2016-07-03 06:15:24 PDT
Comment on attachment 282653 [details]
the patch

r=me.