[PATCH] D89317: [InstructionSimplify] icmp simplification

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:2870-2871
+  // Canonicalize nsw add as RHS.
+  if (!match(RHS, m_NSWAdd(m_Value(), m_Value())))
+    std::swap(LHS, RHS);
+  if (!match(RHS, m_NSWAdd(m_Value(), m_Value())))
----------------
I think this works, but I'm worried that it could break if we are not careful.
If we are swapping the operands, should we also swap the predicate, and then check if it is SLT after the swap?


================
Comment at: llvm/test/Transforms/InstSimplify/compare.ll:1821
+;
+  %add5 = add i16 %V, 5
+  %add6 = add nsw i16 %V, 6
----------------
This is not what I was expecting. To test that the commuted pattern matchibng is working, the first operand on one or both of the adds should be the constant value.

Please do go ahead with pre-committing the extra tests. No review needed unless something is still not clear.


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

https://reviews.llvm.org/D89317