[PATCH] D122013: [InstCombine] Fold abs of known negative operand when source is sub

[Sanjay Patel via Phabricator via llvm-commits]

spatel requested changes to this revision.
spatel added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:794-796
+  if (const OverflowingBinaryOperator *I =
+          dyn_cast<OverflowingBinaryOperator>(Op)) {
+    if (I->getOpcode() == Instruction::Sub && I->hasNoSignedWrap())
----------------
This code would be shorter using 'match':
  Value *X, *Y;
  if (match(Op, m_NSWSub(m_Value(X), m_Value(Y))))
    return isImpliedByDomCondition(ICmpInst::ICMP_SLT, X, Y, CxtI, DL);



================
Comment at: llvm/test/Transforms/InstCombine/abs-intrinsic.ll:429
+; Test from https://github.com/llvm/llvm-project/issues/54132.
+define i32 @sub_abs(i32 %x, i32 %y) {
+; CHECK-LABEL: @sub_abs(
----------------
We should have at least 3 more tests:
1. Check when the dominating condition is 'false' - change the icmp predicate to 'slt'?
2. A negative test (no transform should be made) for wrong predicate - change the icmp predicate to 'ugt' or some other value to make the transform invalid.
3. A negative test (no transform should be made) when there is no 'nsw' on the sub.


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

https://reviews.llvm.org/D122013