Add logic or fold: (A ^ B) | (~A | B) --> -1
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
2231–2232 | This is more like the folds at lines 2205 / 2213, right? If there are 2 commutative ops, then we are handling 4 distinct patterns. And that means there should be 8 tests after we account for the swapped operands handled by the caller. |
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
2231–2232 | Thank you! |
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
2229–2230 | The code comment doesn't line up with the match logic. We always match the 'not' of A, so there shouldn't be a ~B here. There should be 4 lines in this list to make it symmetric with the other comments in this function. | |
llvm/test/Transforms/InstSimplify/or.ll | ||
904 | I think the tests are covering all the possibilities, but (similar to my previous comment) I'd prefer that we keep the 'not' on 'A' only, so it's easier to see how things are moving around in these comments/tests. |
Change the test cases and format of the comments to be more consistent with existing ones.
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
2229–2230 | I did change the comments to 4 lines and only A is not in the comments to match with the logic in code. | |
llvm/test/Transforms/InstSimplify/or.ll | ||
904 | I changed the test cases based on your suggestions; it totally make sense to follow same rule across the tests. |
LGTM - thanks! See inline for one very minor comment.
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
2229 | One more tiny nit - the 'X' in all of these formulas is the first operand of the 'or', but this set of formulas is showing 'X' as the second operand. That is, the inner 'or' that includes the 'not A' operand should be on the left side and the 'A ^ B' should be on the right to match what the code does. |
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
2229 | Thank you! |
One more tiny nit - the 'X' in all of these formulas is the first operand of the 'or', but this set of formulas is showing 'X' as the second operand. That is, the inner 'or' that includes the 'not A' operand should be on the left side and the 'A ^ B' should be on the right to match what the code does.