| 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: | JavaScriptCore | Assignee: | 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
Filip Pizlo
2014-06-18 22:00:51 PDT
Created attachment 233349 [details]
the patch
Comment on attachment 233349 [details]
the patch
This is totally wrong.
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.
|