[PATCH] D48535: [InstCombine] (A + 1) + (B ^ -1) --> A - B

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 25 05:42:45 PDT 2018


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM - see inline for some test suggestions.



================
Comment at: test/Transforms/InstCombine/add.ll:801-802
+;
+  %C = xor i32 -1, %B
+  %D = add i32 1, %A
+  ; E = ~B + (1 + A) = A - B
----------------
1. Commuting the constants in "%C" and "%D" distracts from the meaningful commute of "%E", so don't do that.
2. It would provide more coverage if one of the tests used vector types (or add a dedicated test for vectors).
3. It's a matter of taste, but I prefer to have the code comment above the function rather than embedded in the code.
4. Similarly, give the test a meaningful name: "@add_not_increment"?
5. Always prefer to commit the tests with baseline CHECKs as a preliminary step to the patched tests.


https://reviews.llvm.org/D48535





More information about the llvm-commits mailing list