Bug 134056

Summary: [ftlopt] LICM should be able to hoist CheckStructure even if the loop clobbers structures so long as the structures being checked are watchable
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED INVALID    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 133229    
Attachments:
Description Flags
the patch none

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.