SimplifyLibCalls: Replace copysign calls with intrinsic

[arsenm]

Make sure source types match

[arsenm]

ping

[arsenm]

ping

[hfinkel]

I think that most of this looks good, except as noted below...

lib/Transforms/InstCombine/InstCombineCasts.cpp

1423

I think this should be done in DAGCombine. It is not clear to me that this is always beneficial nor does it clearly make a useful canonical form. I think we should canonicalize the other way: it seems simpler.

[arsenm]

In D28499#664355, @hfinkel wrote:

I think that most of this looks good, except as noted below...

In D28499#664355, @hfinkel wrote:

I think that most of this looks good, except as noted below...

I only did this to keep what it was already doing for the libcall. Should I just rip out all of those tests and drop this?

[arsenm]

In D28499#664916, @arsenm wrote:

In D28499#664355, @hfinkel wrote:

I think that most of this looks good, except as noted below...

In D28499#664355, @hfinkel wrote:

I think that most of this looks good, except as noted below...

I only did this to keep what it was already doing for the libcall. Should I just rip out all of those tests and drop this?

DAGCombiner does something similar and more interesting because it can mismatch the source types.

If I drop this all of the interesting tested cases work if I drop that part, except for test19 in float-shrink-compare (added by D6495, which apparently was a reported bug). I could special case this in the fcmp handling

lib/Transforms/InstCombine/InstCombineCalls.cpp

if (match(Src, m_FNeg(m_Value(SrcSrc))) ||
       // cos(-x) -> cos(x)
       // cos(fabs(x)) -> cos(x)
       II->setArgOperand(0, SrcSrc);
       return II;
     }
 
     break;
   }
+  case Intrinsic::copysign: {
+    Value *ExtSrc0;
+    Value *ExtSrc1;
+
+    // copysign (fpext x), (fpext y) -> copysign x, y
+    if (match(II->getArgOperand(0), m_FPExt(m_Value(ExtSrc0))) &&
+        match(II->getArgOperand(1), m_FPExt(m_Value(ExtSrc1))) &&
+        ExtSrc0->getType() == ExtSrc1->getType()) {
+      Value *F = Intrinsic::getDeclaration(II->getModule(), II->getIntrinsicID(),
+                                           { ExtSrc0->getType() });
+      CallInst *NewCall = Builder->CreateCall(F, { ExtSrc0, ExtSrc1 });
+      NewCall->copyFastMathFlags(II);
+      NewCall->takeName(II);
+      return new FPExtInst(NewCall, II->getType());
+    }
+
+    break;
+  }
   case Intrinsic::ppc_altivec_lvx:
   case Intrinsic::ppc_altivec_lvxl:
     // Turn PPC lvx -> load if the pointer is known aligned.
     if (getOrEnforceKnownAlignment(II->getArgOperand(0), 16, DL, II, &AC,
                                    &DT) >= 16) {
       Value *Ptr = Builder->CreateBitCast(II->getArgOperand(0),
                                          PointerType::getUnqual(II->getType()));
       return new LoadInst(Ptr);

lib/Transforms/InstCombine/InstCombineCasts.cpp

case Intrinsic::trunc: {
       SmallVector<OperandBundleDef, 1> OpBundles;
       II->getOperandBundlesAsDefs(OpBundles);
 
       Value *Args[] = { InnerTrunc };
       CallInst *NewCI =  CallInst::Create(Overload, Args,
                                           OpBundles, II->getName());
       NewCI->copyFastMathFlags(II);
       return NewCI;
-    }
-    }
+  case Intrinsic::copysign: {
+    Type *Ty = CI.getType();
+    // Do binary FP operation on smaller type.
+    // (fptrunc (copysign x, y)) -> (copysign (fptrunc x), (fptrunc y))
+    Value *Trunc0 = Builder->CreateFPTrunc(II->getArgOperand(0), Ty);
+    Value *Trunc1 = Builder->CreateFPTrunc(II->getArgOperand(1), Ty);
+
+    Function *Overload = Intrinsic::getDeclaration(
+      CI.getModule(), II->getIntrinsicID(), Ty);
+
+    SmallVector<OperandBundleDef, 1> OpBundles;
+    II->getOperandBundlesAsDefs(OpBundles);
+
+    CallInst *NewCI =  CallInst::Create(Overload, { Trunc0, Trunc1 },
+                                        OpBundles, II->getName());
+    NewCI->copyFastMathFlags(II);
+    return NewCI;
+  }
   }
 
   return nullptr;
 }
 
 Instruction *InstCombiner::visitFPExt(CastInst &CI) {
   return commonCastTransforms(CI);
 }

lib/Transforms/Utils/SimplifyLibCalls.cpp

static Value *replaceUnaryCall(CallInst *CI, IRBuilder<> &B, Intrinsic::ID IID) {
   Module *M = CI->getModule();
   Value *V = CI->getArgOperand(0);
   Function *F = Intrinsic::getDeclaration(M, IID, CI->getType());
   CallInst *NewCall = B.CreateCall(F, V);
   NewCall->takeName(CI);
   return NewCall;
 }
 
+// Replace a libcall \p CI with a call to intrinsic \p IID
+static Value *replaceBinaryCall(CallInst *CI, IRBuilder<> &B, Intrinsic::ID IID) {
+  // Propagate fast-math flags from the existing call to the new call.
+  IRBuilder<>::FastMathFlagGuard Guard(B);
+  B.setFastMathFlags(CI->getFastMathFlags());
+
+  Module *M = CI->getModule();
+  Value *V0 = CI->getArgOperand(0);
+  Value *V1 = CI->getArgOperand(1);
+  Function *F = Intrinsic::getDeclaration(M, IID, CI->getType());
+  CallInst *NewCall = B.CreateCall(F, { V0, V1 });
+  NewCall->takeName(CI);
+  return NewCall;
+}
+
 /// Shrink double -> float for binary functions like 'fmin/fmax'.
 static Value *optimizeBinaryDoubleFP(CallInst *CI, IRBuilder<> &B) {
   Function *Callee = CI->getCalledFunction();
   // We know this libcall has a valid prototype, but we don't know which.
   if (!CI->getType()->isDoubleTy())
     return nullptr;
 
   // If this is something like 'fmin((double)floatval1, (double)floatval2)',
if (TLI->getLibFunc(*Callee, Func) && TLI->has(Func)) {
     case LibFunc::expm1:
     case LibFunc::sin:
     case LibFunc::sinh:
     case LibFunc::tanh:
       if (UnsafeFPShrink && hasFloatVersion(FuncName))
         return optimizeUnaryDoubleFP(CI, Builder, true);
       return nullptr;
     case LibFunc::copysign:
-      if (hasFloatVersion(FuncName))
-        return optimizeBinaryDoubleFP(CI, Builder);
-      return nullptr;
+      return replaceBinaryCall(CI, Builder, Intrinsic::copysign);
     case LibFunc::fminf:
     case LibFunc::fmin:
     case LibFunc::fminl:
     case LibFunc::fmaxf:
     case LibFunc::fmax:
     case LibFunc::fmaxl:
       return optimizeFMinFMax(CI, Builder);
     default:

test/Transforms/InstCombine/copysign.ll

 
 ; CHECK-LABEL: @constant_fold_copysign_f64_03
 ; CHECK-NEXT: ret double -1.000000e+00
 define double @constant_fold_copysign_f64_03() #0 {
   %x = call double @llvm.copysign.f64(double -1.0, double -2.0) #0
   ret double %x
 }
 
+; CHECK-LABEL: @reduce_precision(
+; CHECK: %copysign = call float @llvm.copysign.f32(float %x, float %y)
+; CHECK-NEXT: ret float %copysign
+define float @reduce_precision(float %x, float %y) {
+  %x.ext = fpext float %x to double
+  %y.ext = fpext float %y to double
+  %copysign = call double @llvm.copysign.f64(double %x.ext, double %y.ext)
+  %trunc = fptrunc double %copysign to float
+  ret float %trunc
+}
+
+; CHECK-LABEL: @reduce_precision_fmf(
+; CHECK: %copysign = call nnan float @llvm.copysign.f32(float %x, float %y)
+; CHECK-NEXT: ret float %copysign
+define float @reduce_precision_fmf(float %x, float %y) {
+  %x.ext = fpext float %x to double
+  %y.ext = fpext float %y to double
+  %copysign = call nnan double @llvm.copysign.f64(double %x.ext, double %y.ext)
+  %trunc = fptrunc double %copysign to float
+  ret float %trunc
+}
+
+; CHECK-LABEL: @reduce_precision_constant(
+; CHECK: %trunc = call float @llvm.copysign.f32(float %x, float 4.000000e+00)
+; CHECK-NEXT: ret float %trunc
+define float @reduce_precision_constant(float %x) {
+  %x.ext = fpext float %x to double
+  %copysign = call double @llvm.copysign.f64(double %x.ext, double 4.0)
+  %trunc = fptrunc double %copysign to float
+  ret float %trunc
+}
+
+; CHECK-LABEL: @reduce_precision_multi_use_src0(
+; CHECK: %x.ext = fpext float %x to double
+; CHECK-NEXT: %copysign = call float @llvm.copysign.f32(float %x, float %y)
+; CHECK-NEXT: store volatile double %x.ext,
+; CHECK-NEXT: ret float %copysign
+define float @reduce_precision_multi_use_src0(float %x, float %y) {
+  %x.ext = fpext float %x to double
+  %y.ext = fpext float %y to double
+  %copysign = call double @llvm.copysign.f64(double %x.ext, double %y.ext)
+  %trunc = fptrunc double %copysign to float
+  store volatile double %x.ext, double* undef
+  ret float %trunc
+}
+
+; CHECK-LABEL: @reduce_precision_multi_use_src1(
+; CHECK: %y.ext = fpext float %y to double
+; CHECK-NEXT: %copysign = call float @llvm.copysign.f32(float %x, float %y)
+; CHECK-NEXT: store volatile double %y.ext
+; CHECK-NEXT: ret float %copysign
+define float @reduce_precision_multi_use_src1(float %x, float %y) {
+  %x.ext = fpext float %x to double
+  %y.ext = fpext float %y to double
+  %copysign = call double @llvm.copysign.f64(double %x.ext, double %y.ext)
+  %trunc = fptrunc double %copysign to float
+  store volatile double %y.ext, double* undef
+  ret float %trunc
+}
+
+; CHECK-LABEL: @reduce_precision_multi_use_src0_src1(
+; CHECK-NEXT: %x.ext = fpext float %x to double
+; CHECK-NEXT: %y.ext = fpext float %y to double
+; CHECK-NEXT: %copysign = call float @llvm.copysign.f32(float %x, float %y)
+; CHECK-NEXT: store volatile double %x.ext,
+; CHECK-NEXT: store volatile double %y.ext,
+; CHECK-NEXT: ret float %copysign
+define float @reduce_precision_multi_use_src0_src1(float %x, float %y) {
+  %x.ext = fpext float %x to double
+  %y.ext = fpext float %y to double
+  %copysign = call double @llvm.copysign.f64(double %x.ext, double %y.ext)
+  %trunc = fptrunc double %copysign to float
+  store volatile double %x.ext, double* undef
+  store volatile double %y.ext, double* undef
+  ret float %trunc
+}
+
+; CHECK-LABEL: @reduce_precision_mixed_src_types_0(
+; CHECK: %y.ext = fpext half %y to double
+; CHECK-NEXT: %1 = fptrunc double %y.ext to float
+; CHECK-NEXT: %trunc = call float @llvm.copysign.f32(float %x, float %1)
+; CHECK-NEXT: ret float %trunc
+define float @reduce_precision_mixed_src_types_0(float %x, half %y) {
+  %x.ext = fpext float %x to double
+  %y.ext = fpext half %y to double
+  %copysign = call double @llvm.copysign.f64(double %x.ext, double %y.ext)
+  %trunc = fptrunc double %copysign to float
+  ret float %trunc
+}
+
+; CHECK-LABEL: @reduce_precision_mixed_src_types_1(
+; CHECK: %x.ext = fpext half %x to double
+; CHECK-NEXT: %1 = fptrunc double %x.ext to float
+; CHECK-NEXT: %trunc = call float @llvm.copysign.f32(float %1, float %y)
+; CHECK-NEXT: ret float %trunc
+define float @reduce_precision_mixed_src_types_1(half %x, float %y) {
+  %x.ext = fpext half %x to double
+  %y.ext = fpext float %y to double
+  %copysign = call double @llvm.copysign.f64(double %x.ext, double %y.ext)
+  %trunc = fptrunc double %copysign to float
+  ret float %trunc
+}
 
 attributes #0 = { nounwind readnone }

test/Transforms/InstCombine/float-shrink-compare.ll

define i32 @test18(float %x, float %y, float %z) nounwind uwtable {
   %6 = zext i1 %5 to i32
   ret i32 %6
 ; CHECK-LABEL: @test18(
 ; CHECK-NEXT: %fmaxf = call float @fmaxf(float %x, float %y)
 ; CHECK-NEXT: fcmp oeq float %fmaxf, %z
 }
 
 define i32 @test19(float %x, float %y, float %z) nounwind uwtable {
-  %1 = fpext float %x to double
-  %2 = fpext float %y to double
-  %3 = call double @copysign(double %1, double %2) nounwind
-  %4 = fpext float %z to double
-  %5 = fcmp oeq double %3, %4
-  %6 = zext i1 %5 to i32
-  ret i32 %6
+  %x.ext = fpext float %x to double
+  %y.ext = fpext float %y to double
+  %copysign = call double @copysign(double %x.ext, double %y.ext) nounwind
+  %z.ext = fpext float %z to double
+  %cmp = fcmp oeq double %copysign, %z.ext
+  %cmp.ext = zext i1 %cmp to i32
+  ret i32 %cmp.ext
 ; CHECK-LABEL: @test19(
-; CHECK-NEXT: %copysignf = call float @copysignf(float %x, float %y)
-; CHECK-NEXT: fcmp oeq float %copysignf, %z
+; CHECK-NEXT: %copysign = call float @llvm.copysign.f32(float %x, float %y)
+; CHECK-NEXT: fcmp oeq float %copysign, %z
 }
 
 define i32 @test20(float %x, float %y) nounwind uwtable {
   %1 = fpext float %y to double
   %2 = fpext float %x to double
   %3 = call double @fmin(double 1.000000e+00, double %2) nounwind
   %4 = fcmp oeq double %1, %3
   %5 = zext i1 %4 to i32