[PATCH] D94874: [InstCombine] Transform abs pattern using multiplication to abs intrinsic (PR45691)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:375
+    Abs->takeName(&I);
+    return replaceInstUsesWith(I, Abs);
+  }
----------------
Doesn't just `return Builder.CreateBinaryIntrinsic` work here?


================
Comment at: llvm/test/Transforms/InstCombine/ashr-or-mul-abs.ll:37
-; CHECK-NEXT:    [[I1:%.*]] = or i32 [[I]], 1
-; CHECK-NEXT:    [[I2:%.*]] = mul i32 [[I1]], [[X]]
 ; CHECK-NEXT:    ret i32 [[I2]]
----------------
`git grep thwart llvm/test/Transforms/InstCombine`. Your current test is effectively equivalent to the previous one due to complexity canonicalization.


================
Comment at: llvm/test/Transforms/InstCombine/ashr-or-mul-abs.ll:100
   ret i32 %i2
 }
----------------
Missing a test where the two `%X` are not the same.


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

https://reviews.llvm.org/D94874