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

Filip Pizlo
Reported 2014-06-18 22:00:51 PDT
Patch forthcoming.
Attachments
the patch (10.44 KB, patch)
2014-06-18 22:24 PDT, Filip Pizlo
no flags
Filip Pizlo
Comment 1 2014-06-18 22:24:10 PDT
Created attachment 233349 [details] the patch
Filip Pizlo
Comment 2 2014-06-19 13:30:42 PDT
Comment on attachment 233349 [details] the patch This is totally wrong.
Filip Pizlo
Comment 3 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.
Note You need to log in before you can comment on or make changes to this bug.