[PATCH] D114996: [InstSimplify] Add logic `or` fold

[Mehrnoosh Heidarpour via Phabricator via llvm-commits]

MehrHeidar marked 2 inline comments as done.
MehrHeidar added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:2229-2230
 
+  // (A ^ B) | (~A | B) --> -1
+  // (A ^ B) | (~B | A) --> -1
+  if (match(X, m_c_Or(m_Not(m_Value(A)), m_Value(B))) &&
----------------
spatel wrote:
> 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.
I did change the comments to 4 lines and only A is `not` in the comments to match with the logic in code.


================
Comment at: llvm/test/Transforms/InstSimplify/or.ll:904
+  %xor = xor i32 %a, %b
+  %notb = xor i32  %b, -1
+  %or = or i32  %a, %notb
----------------
spatel wrote:
> 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.
I changed the test cases based on your suggestions; it totally make sense to follow same rule across the tests.
Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114996/new/

https://reviews.llvm.org/D114996