Bug 134056 - [ftlopt] LICM should be able to hoist CheckStructure even if the loop clobbers structures so long as the structures being checked are watchable
Summary: [ftlopt] LICM should be able to hoist CheckStructure even if the loop clobber...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 133229
  Show dependency treegraph
 
Reported: 2014-06-18 22:00 PDT by Filip Pizlo
Modified: 2014-06-19 13:44 PDT (History)
7 users (show)

See Also:


Attachments
the patch (10.44 KB, patch)
2014-06-18 22:24 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2014-06-18 22:00:51 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2014-06-18 22:24:10 PDT
Created attachment 233349 [details]
the patch
Comment 2 Filip Pizlo 2014-06-19 13:30:42 PDT
Comment on attachment 233349 [details]
the patch

This is totally wrong.
Comment 3 Filip Pizlo 2014-06-19 13:44:09 PDT
It's not right to hoist a CheckStructure just because the structure is watchable.  Consider a loop like:

var o = ...; // when assigned here, o does not have a field f.
for (;;) {
    if (p1)
        o.f = 42; // adds a field f
    if (p2) // p2 becomes true only after p1 had become true
        ... = o.f; // say that after adding a field f to o, we won't add any more things to o, so the CheckStructure here will be on a watchable structure set.
}

If we hoist the CheckStructure then the CheckStructure will be sure to fail.  Therefore, the rules on the hoistability of CheckStructures need to be substantially more subtle.  In the example above, at a minimum we need to have some profiling of the type that o has at the loop pre-header to make sure that hoisting the CheckStructure to that location won't be guaranteed to fail.