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

[Sanjay Patel via Phabricator via llvm-commits]

spatel 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))) &&
----------------
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.


================
Comment at: llvm/test/Transforms/InstSimplify/or.ll:904
+  %xor = xor i32 %a, %b
+  %notb = xor i32  %b, -1
+  %or = or i32  %a, %notb
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114996