Summary: | The DFG should fold ~~x just like x|0. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||
Component: | JavaScriptCore | Assignee: | Andreas Kling <kling> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | atrick, barraclough, dtc-llvm, ggaren, juergen, kling, mark.lam, mhahnenberg, msaboff, nrotem, oliver, sam | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Filip Pizlo
2014-04-06 20:03:23 PDT
Created attachment 241748 [details]
Patch
First DFG patch, please don't be gentle.
Comment on attachment 241748 [details]
Patch
Would be nice to use the convertToIdentityOverChild* helper, by changing the helper to accept an explicit Node* argument.
Created attachment 241752 [details]
Patch II
Tweak the helper per Geoff's suggestion.
Comment on attachment 241752 [details]
Patch II
I bet you this is wrong for this code:
var tmp = ~x
var y = tmp + 1
var z = ~tmp
print(y + ", " + z)
And also it's wrong for this case:
var tmp = ~x
o.f // make the DFG optimize this for some structure and then call with a different structure
var y = ~tmp
print(y)
I think the right answer here is to change the current XOR node to an identity node over its XOR child's child, but to leave its XOR child node unchanged, in case that child has other uses in the graph. (In reply to comment #5) > I think the right answer here is to change the current XOR node to an > identity node over its XOR child's child, but to leave its XOR child node > unchanged, in case that child has other uses in the graph. Yup, that would do it. It's worth noting that in the FTL, ~~ will get folded by LLVM, and LLVM will not have this problem since the at-exit use of the first Xor will be explicit. So the value of this optimization on our side is mainly canonicalization: being able to see that ~~x is actually the same as x after an int check. And leaving the original Xor is a fine way to approach this. |