[PATCH] D21417: Fix Side-Conditions in SimplifyCFG for Creating Switches from InstCombine And Mask'd Comparisons
Thomas Jablin via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 15 15:52:43 PDT 2016
tjablin created this revision.
tjablin added reviewers: chandlerc, cycheng, hfinkel, majnemer.
tjablin added a subscriber: llvm-commits.
This is just the correctness fix from D21397.
SimplifyCFG is able to detect the pattern:
(i == 5334 || i == 5335)
((i & -2) == 5334)
This transformation has some incorrect side conditions. Specifically, the transformation is only applied when the right-hand side constant (5334 in the example) is a power of two not equal and not equal to the negated mask. These side conditions were added in r258904 to fix PR26323. The correct side condition is that: ((Constant & Mask) == Constant)[(5334 & -2) == 5334].
It's a little bit hard to see why these transformations are correct and what the side conditions ought to be. Here is a CVC3 program to verify them for 64-bit values:
ONE : BITVECTOR(64) = BVZEROEXTEND(0bin1, 63);
x : BITVECTOR(64);
y : BITVECTOR(64);
z : BITVECTOR(64);
mask : BITVECTOR(64) = BVSHL(ONE, z);
QUERY( (y & ~mask = y) =>
((x & ~mask = y) <=> (x = y OR x = (y | mask)))
Please note that each pattern must be a dual implication (<--> or iff). One directional implication can create spurious matches. If the implication is only one-way, an unsatisfiable condition on the left side can imply a satisfiable condition on the right side. Dual implication ensures that satisfiable conditions are transformed to other satisfiable conditions and unsatisfiable conditions are transformed to other unsatisfiable conditions.
Here is a concrete example of a unsatisfiable condition on the left implying a satisfiable condition on the right:
mask = (1 << z)
(x & ~mask) == y --> (x == y || x == (y | mask))
Substituting y = 3, z = 0 yields:
(x & -2) == 3 --> (x == 3 || x == 2)
The version of this code before r258904 had no side-conditions and incorrectly justified itself in comments through one-directional implication.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 4103 bytes
Desc: not available
More information about the llvm-commits