This also corrects the description to match what was actually implemented. The old comment said X^(C1|C2), but it implemented X^((C1|C2)&~(C1&C2)). I believe ((C1|C2)&~(C1&C2)) is equivalent to (C1^C2).
Details
Diff Detail
Event Timeline
LGTM.
lib/Transforms/InstCombine/InstCombineAndOrXor.cpp | ||
---|---|---|
2393–2394 | Nit / potential clean-up: I like to make the variable names match the comment names for easier reading when possible, so I'd pick either 'X' or 'V' and stick with it throughout this block. | |
test/Transforms/InstCombine/select-with-bitwise-ops.ll | ||
271 | Are we just changing the mask constant in these 3 tests to prove that MaskedValueIsZero() is working as expected? Might be more interesting to use a shift or zext to clear some bits instead? |
test/Transforms/InstCombine/select-with-bitwise-ops.ll | ||
---|---|---|
271 | These are all tests for select combines. Test65 tests an and of a larger type with a power of 2 constant that fits in the smaller type. Until ver recently we tried to truncate first then shift. So it only worked for the smaller constant. I don't think these tests were intentionally trying to test the xor combine we have here. It just happens that they trigger it. But the fact that they trigger it and the vector versions didn't is why I even noticed in the first place. |
Nit / potential clean-up: I like to make the variable names match the comment names for easier reading when possible, so I'd pick either 'X' or 'V' and stick with it throughout this block.