[InstSimplify] Fold abs(abs(x)) -> abs(x)

[craig.topper]

Summary

It's always safe to pick the earlier abs regardless of the nsw flag. We'll just lose it if it is on the outer abs but not the inner abs.

[arsenm]

There's an isidempotent function somewhere that catches this

[craig.topper]

In D85053#2188490, @arsenm wrote:

There's an isidempotent function somewhere that catches this

Yeah its called from visitUnaryIntrinsic. This intrinsic is binary because of the nsw argument. I could add it to IsIdempotentand add another call to isIdempotent to the top of visitBinaryIntrinsic, but I thought with the oddity of nsw handling abs directly might be more clear.

[nikic]

LG. This has some overlap with D85043, which will already catch the cases where the inner abs has poison=true, but not those with poison=false, so this seems reasonable as an explicit fold. One could make an argument that this should live in InstCombine and try to preserve the outer poison flag, but this doesn't seem particularly important to me.

[spatel]

Will need update after:
rG04b99a4d18cf

llvm/lib/Analysis/InstructionSimplify.cpp

5262

Could use match here to make this slightly more obvious:

if (match(Op0, m_Intrinsic<Intrinsic::abs>(m_Value(), m_Value())))
  return Op0;

llvm/lib/Analysis/InstructionSimplify.cpp

 
 static Value *simplifyBinaryIntrinsic(Function *F, Value *Op0, Value *Op1,
                                       const SimplifyQuery &Q) {
   Intrinsic::ID IID = F->getIntrinsicID();
   Type *ReturnType = F->getReturnType();
   unsigned BitWidth = ReturnType->getScalarSizeInBits();
   switch (IID) {
   case Intrinsic::abs:
+    // abs(abs(x)) -> abs(x). We don't need to worry about the nsw arg here.
+    // It is always ok to pick the earlier abs. We'll just lose nsw if its only
+    // on the outer abs.
+    if (match(Op0, m_Intrinsic<Intrinsic::abs>(m_Value(), m_Value())))
+      return Op0;
     // If the sign bit is clear already, then abs does not do anything.
     if (isKnownNonNegative(Op0, Q.DL, 0, Q.AC, Q.CxtI, Q.DT))
       return Op0;
     break;
 
   case Intrinsic::smax:
   case Intrinsic::smin:
   case Intrinsic::umax:

llvm/test/Transforms/InstSimplify/call.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instsimplify -S | FileCheck %s
 ; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
 
 declare i32 @llvm.abs.i32(i32, i1)
 declare <3 x i82> @llvm.abs.v3i82(<3 x i82>, i1)
 
+define i32 @test_abs_abs_0(i32 %x) {
+; CHECK-LABEL: @test_abs_abs_0(
+; CHECK-NEXT:    [[A:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false)
+; CHECK-NEXT:    ret i32 [[A]]
+;
+  %a = call i32 @llvm.abs.i32(i32 %x, i1 false)
+  %b = call i32 @llvm.abs.i32(i32 %a, i1 false)
+  ret i32 %b
+}
+
+define i32 @test_abs_abs_1(i32 %x) {
+; CHECK-LABEL: @test_abs_abs_1(
+; CHECK-NEXT:    [[A:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 true)
+; CHECK-NEXT:    ret i32 [[A]]
+;
+  %a = call i32 @llvm.abs.i32(i32 %x, i1 true)
+  %b = call i32 @llvm.abs.i32(i32 %a, i1 false)
+  ret i32 %b
+}
+
+define i32 @test_abs_abs_2(i32 %x) {
+; CHECK-LABEL: @test_abs_abs_2(
+; CHECK-NEXT:    [[A:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false)
+; CHECK-NEXT:    ret i32 [[A]]
+;
+  %a = call i32 @llvm.abs.i32(i32 %x, i1 false)
+  %b = call i32 @llvm.abs.i32(i32 %a, i1 true)
+  ret i32 %b
+}
+
+define i32 @test_abs_abs_3(i32 %x) {
+; CHECK-LABEL: @test_abs_abs_3(
+; CHECK-NEXT:    [[A:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 true)
+; CHECK-NEXT:    ret i32 [[A]]
+;
+  %a = call i32 @llvm.abs.i32(i32 %x, i1 true)
+  %b = call i32 @llvm.abs.i32(i32 %a, i1 true)
+  ret i32 %b
+}
+
 ; If the sign bit is known zero, the abs is not needed.
 
 define i32 @zext_abs(i31 %x) {
 ; CHECK-LABEL: @zext_abs(
 ; CHECK-NEXT:    [[ZEXT:%.*]] = zext i31 [[X:%.*]] to i32
 ; CHECK-NEXT:    ret i32 [[ZEXT]]
 ;
   %zext = zext i31 %x to i32