Fix fcmp + fabs instcombines when using the intrinsic

[arsenm]

Summary

This was only handling the libcall. This is another example
of why only the intrinsic should ever be used when it exists.

[arsenm]

ping

[mehdi_amini]

LGTM.

One comment: I also wonder if other comparisons could be folded. I think for example about FCMP_UGT/FCMP_UGE which are always true?

[arsenm]

r225465

lib/Transforms/InstCombine/InstCombineCompares.cpp

if (Instruction *LHSI = dyn_cast<Instruction>(Op0))
           if (GlobalVariable *GV = dyn_cast<GlobalVariable>(GEP->getOperand(0)))
             if (GV->isConstant() && GV->hasDefinitiveInitializer() &&
                 !cast<LoadInst>(LHSI)->isVolatile())
               if (Instruction *Res = FoldCmpLoadFromIndexedGlobal(GEP, GV, I))
                 return Res;
         }
         break;
       case Instruction::Call: {
+        if (!RHSC->isNullValue())
+          break;
+
         CallInst *CI = cast<CallInst>(LHSI);
-        LibFunc::Func Func;
+        const Function *F = CI->getCalledFunction();
+        if (!F)
+          break;
+
         // Various optimization for fabs compared with zero.
-        if (RHSC->isNullValue() && CI->getCalledFunction() &&
-            TLI->getLibFunc(CI->getCalledFunction()->getName(), Func) &&
-            TLI->has(Func)) {
-          if (Func == LibFunc::fabs || Func == LibFunc::fabsf ||
-              Func == LibFunc::fabsl) {
-            switch (I.getPredicate()) {
-            default: break;
+        LibFunc::Func Func;
+        if (F->getIntrinsicID() == Intrinsic::fabs ||
+            (TLI->getLibFunc(F->getName(), Func) && TLI->has(Func) &&
+             (Func == LibFunc::fabs || Func == LibFunc::fabsf ||
+              Func == LibFunc::fabsl))) {
+          default:
+            break;
             // fabs(x) < 0 --> false
-            case FCmpInst::FCMP_OLT:
-              return ReplaceInstUsesWith(I, Builder->getFalse());
             // fabs(x) > 0 --> x != 0
-            case FCmpInst::FCMP_OGT:
-              return new FCmpInst(FCmpInst::FCMP_ONE, CI->getArgOperand(0),
-                                  RHSC);
+            return new FCmpInst(FCmpInst::FCMP_ONE, CI->getArgOperand(0), RHSC);
             // fabs(x) <= 0 --> x == 0
-            case FCmpInst::FCMP_OLE:
-              return new FCmpInst(FCmpInst::FCMP_OEQ, CI->getArgOperand(0),
-                                  RHSC);
+            return new FCmpInst(FCmpInst::FCMP_OEQ, CI->getArgOperand(0), RHSC);
             // fabs(x) >= 0 --> !isnan(x)
-            case FCmpInst::FCMP_OGE:
-              return new FCmpInst(FCmpInst::FCMP_ORD, CI->getArgOperand(0),
-                                  RHSC);
+            return new FCmpInst(FCmpInst::FCMP_ORD, CI->getArgOperand(0), RHSC);
             // fabs(x) == 0 --> x == 0
             // fabs(x) != 0 --> x != 0
-            case FCmpInst::FCMP_OEQ:
-            case FCmpInst::FCMP_UEQ:
-            case FCmpInst::FCMP_ONE:
-            case FCmpInst::FCMP_UNE:
-              return new FCmpInst(I.getPredicate(), CI->getArgOperand(0),
-                                  RHSC);
-            }
+            return new FCmpInst(I.getPredicate(), CI->getArgOperand(0), RHSC);
           }
         }
       }
       }
   }
 
   // fcmp pred (fneg x), (fneg y) -> fcmp swap(pred) x, y
   Value *X, *Y;

test/Transforms/InstCombine/fcmp.ll

 ; RUN: opt -S -instcombine < %s | FileCheck %s
 
+declare double @llvm.fabs.f64(double) nounwind readnone
+
 define i1 @test1(float %x, float %y) nounwind {
   %ext1 = fpext float %x to double
   %ext2 = fpext float %y to double
   %cmp = fcmp ogt double %ext1, %ext2
   ret i1 %cmp
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT: fcmp ogt float %x, %y
 }
define i32 @test9(double %a) nounwind {
   %cmp = fcmp olt double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test9(
 ; CHECK-NOT: fabs
 ; CHECK: ret i32 0
 }
 
+define i32 @test9_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp olt double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test9_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: ret i32 0
+}
+
 define i32 @test10(double %a) nounwind {
   %call = tail call double @fabs(double %a) nounwind
   %cmp = fcmp ole double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test10(
 ; CHECK-NOT: fabs
 ; CHECK: fcmp oeq double %a, 0.000000e+00
 }
 
+define i32 @test10_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp ole double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test10_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: fcmp oeq double %a, 0.000000e+00
+}
+
 define i32 @test11(double %a) nounwind {
   %call = tail call double @fabs(double %a) nounwind
   %cmp = fcmp ogt double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test11(
 ; CHECK-NOT: fabs
 ; CHECK: fcmp one double %a, 0.000000e+00
 }
 
+define i32 @test11_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp ogt double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test11_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: fcmp one double %a, 0.000000e+00
+}
+
 define i32 @test12(double %a) nounwind {
   %call = tail call double @fabs(double %a) nounwind
   %cmp = fcmp oge double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test12(
 ; CHECK-NOT: fabs
 ; CHECK: fcmp ord double %a, 0.000000e+00
 }
 
+define i32 @test12_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp oge double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test12_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: fcmp ord double %a, 0.000000e+00
+}
+
 define i32 @test13(double %a) nounwind {
   %call = tail call double @fabs(double %a) nounwind
   %cmp = fcmp une double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test13(
 ; CHECK-NOT: fabs
 ; CHECK: fcmp une double %a, 0.000000e+00
 }
 
+define i32 @test13_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp une double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test13_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: fcmp une double %a, 0.000000e+00
+}
+
 define i32 @test14(double %a) nounwind {
   %call = tail call double @fabs(double %a) nounwind
   %cmp = fcmp oeq double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test14(
 ; CHECK-NOT: fabs
 ; CHECK: fcmp oeq double %a, 0.000000e+00
 }
 
+define i32 @test14_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp oeq double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test14_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: fcmp oeq double %a, 0.000000e+00
+}
+
 define i32 @test15(double %a) nounwind {
   %call = tail call double @fabs(double %a) nounwind
   %cmp = fcmp one double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test15(
 ; CHECK-NOT: fabs
 ; CHECK: fcmp one double %a, 0.000000e+00
 }
 
+define i32 @test15_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp one double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test15_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: fcmp one double %a, 0.000000e+00
+}
+
 define i32 @test16(double %a) nounwind {
   %call = tail call double @fabs(double %a) nounwind
   %cmp = fcmp ueq double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 ; CHECK-LABEL: @test16(
 ; CHECK-NOT: fabs
 ; CHECK: fcmp ueq double %a, 0.000000e+00
 }
 
+define i32 @test16_intrinsic(double %a) nounwind {
+  %call = tail call double @llvm.fabs.f64(double %a) nounwind
+  %cmp = fcmp ueq double %call, 0.000000e+00
+  %conv = zext i1 %cmp to i32
+  ret i32 %conv
+; CHECK-LABEL: @test16_intrinsic(
+; CHECK-NOT: fabs
+; CHECK: fcmp ueq double %a, 0.000000e+00
+}
+
 ; Don't crash.
 define i32 @test17(double %a, double (double)* %p) nounwind {
   %call = tail call double %p(double %a) nounwind
   %cmp = fcmp ueq double %call, 0.000000e+00
   %conv = zext i1 %cmp to i32
   ret i32 %conv
 }