Bug 131288 - The DFG should fold ~~x just like x|0.
Summary: The DFG should fold ~~x just like x|0.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-06 20:03 PDT by Filip Pizlo
Modified: 2015-03-23 08:30 PDT (History)
12 users (show)

See Also:


Attachments
Patch (4.68 KB, patch)
2014-11-17 16:33 PST, Andreas Kling
no flags Details | Formatted Diff | Diff
Patch II (6.69 KB, patch)
2014-11-17 16:57 PST, Andreas Kling
fpizlo: review-
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-04-06 20:03:23 PDT
...

(Yes, srsly, this is a thing.)
Comment 1 Andreas Kling 2014-11-17 16:33:46 PST
Created attachment 241748 [details]
Patch

First DFG patch, please don't be gentle.
Comment 2 Geoffrey Garen 2014-11-17 16:51:32 PST
Comment on attachment 241748 [details]
Patch

Would be nice to use the convertToIdentityOverChild* helper, by changing the helper to accept an explicit Node* argument.
Comment 3 Andreas Kling 2014-11-17 16:57:36 PST
Created attachment 241752 [details]
Patch II

Tweak the helper per Geoff's suggestion.
Comment 4 Filip Pizlo 2014-11-17 17:12:53 PST
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)
Comment 5 Geoffrey Garen 2014-11-18 11:17:48 PST
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.
Comment 6 Filip Pizlo 2015-03-23 08:30:37 PDT
(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.