[BuildLibCalls] Properly set ABI attributes on arguments

[aeubanks]

Summary

Some floating point lib calls have ABI attributes that need to be set on
the caller. Found via D103412.

[aeubanks]

this is kinda ugly, but I'm not sure of a nicer way to do it. Maybe a new method on AttributeList that merges attributes?
ldexp() and friends are the only functions that have ABI attributes set (setSignExtendedArg())

[rnk]

I'd suggest something like:

CI->setAttributes(CI->getCalledFunction()->getAttributes().removeAttributes(Ctx, llvm::AttributeList::FunctionIndex)));

That should take the callee's attribute list, throw away all function index attributes (things like SSE level, noreturn, noinline, alwaysinline, optnone), and apply the return and parameter attributes. It should also create fewer immortal, immutable, but temporary attribute lists.

[aeubanks]

In D103415#2794636, @rnk wrote:

I'd suggest something like:

CI->setAttributes(CI->getCalledFunction()->getAttributes().removeAttributes(Ctx, llvm::AttributeList::FunctionIndex)));

That should take the callee's attribute list, throw away all function index attributes (things like SSE level, noreturn, noinline, alwaysinline, optnone), and apply the return and parameter attributes. It should also create fewer immortal, immutable, but temporary attribute lists.

The problem is that right above, we already call

CI->setAttributes()

and a second one would overwrite that.

I was thinking of adding something new to merge two AttributeLists, but idk

[rnk]

You're right, I don't see a way to directly merge two AttributeList objects. I looked at AttrBuilder::merge, but AttrBuilder makes AttributeSet objects. I guess it makes sense to have an AttributeList method to do this.

[aeubanks]

add merge function

[rnk]

lgtm

There's some horrifying index manipulation involving unsigned overflow that I had to avoid by checking if either AttributeList is empty.

By horrifying, surely you mean far too clever for anyone's best interests. =D

It'd be nice to not have AttributeList::AttrIndex::FunctionIndex = ~0U.

I had a patch to do that back in 2017, but the reviewers at the time felt it was too risky:
https://reviews.llvm.org/D32819#749163
https://reviews.llvm.org/D32819?id=98222#inline-286125

We could try again, but it has the potential to break frontends in ways that are hard to debug.

[aeubanks]

use existing AttributeList merge method
I completely missed that this one already existed

[rnk]

lgtm

llvm/lib/Transforms/Utils/BuildLibCalls.cpp

static Value *emitBinaryFloatFnCallHelper(Value *Op1, Value *Op2,
   CallInst *CI = B.CreateCall(Callee, { Op1, Op2 }, Name);
 
   // The incoming attribute set may have come from a speculatable intrinsic, but
   // is being replaced with a library call which is not allowed to be
   // speculatable.
   CI->setAttributes(Attrs.removeAttribute(B.getContext(),
                                           AttributeList::FunctionIndex,
                                           Attribute::Speculatable));
+  for (int I = 0, E = Callee.getFunctionType()->getNumParams(); I != E; ++I) {
+    for (const auto &A :
+         CI->getCalledFunction()->getAttributes().getParamAttributes(I)) {
+      CI->addParamAttr(I, A);
+    }
+  }
   if (const Function *F =
           dyn_cast<Function>(Callee.getCallee()->stripPointerCasts()))
     CI->setCallingConv(F->getCallingConv());
 
   return CI;
 }
 
 Value *llvm::emitBinaryFloatFnCall(Value *Op1, Value *Op2, StringRef Name,

llvm/test/Transforms/InstCombine/pow_fp_int.ll

 ;
   %subfp = uitofp i31 %x to double
   %pow = tail call afn double @llvm.pow.f64(double 7.000000e+00, double %subfp)
   ret double %pow
 }
 
 define double @pow_sitofp_double_const_base_2_fast(i32 %x) {
 ; CHECK-LABEL: @pow_sitofp_double_const_base_2_fast(
-; CHECK-NEXT:    [[LDEXPF:%.*]] = call afn float @ldexpf(float 1.000000e+00, i32 [[X:%.*]])
+; CHECK-NEXT:    [[LDEXPF:%.*]] = call afn float @ldexpf(float 1.000000e+00, i32 signext [[X:%.*]])
 ; CHECK-NEXT:    [[RES:%.*]] = fpext float [[LDEXPF]] to double
 ; CHECK-NEXT:    ret double [[RES]]
 ;
   %subfp = sitofp i32 %x to float
   %pow = tail call afn float @llvm.pow.f32(float 2.000000e+00, float %subfp)
   %res = fpext float %pow to double
   ret double %res
 }
;
   %pow = tail call afn float @llvm.pow.f32(float 16.000000e+00, float %subfp)
   %res = fpext float %pow to double
   ret double %res
 }
 
 define double @pow_uitofp_const_base_2_fast(i31 %x) {
 ; CHECK-LABEL: @pow_uitofp_const_base_2_fast(
 ; CHECK-NEXT:    [[TMP1:%.*]] = zext i31 [[X:%.*]] to i32
-; CHECK-NEXT:    [[LDEXPF:%.*]] = call afn float @ldexpf(float 1.000000e+00, i32 [[TMP1]])
+; CHECK-NEXT:    [[LDEXPF:%.*]] = call afn float @ldexpf(float 1.000000e+00, i32 signext [[TMP1]])
 ; CHECK-NEXT:    [[RES:%.*]] = fpext float [[LDEXPF]] to double
 ; CHECK-NEXT:    ret double [[RES]]
 ;
   %subfp = uitofp i31 %x to float
   %pow = tail call afn float @llvm.pow.f32(float 2.000000e+00, float %subfp)
   %res = fpext float %pow to double
   ret double %res
 }
;
   %subfp = uitofp i32 %x to float
   %pow = tail call float @llvm.pow.f32(float 7.000000e+00, float %subfp)
   %res = fpext float %pow to double
   ret double %res
 }
 
 define double @pow_sitofp_const_base_2_no_fast(i32 %x) {
 ; CHECK-LABEL: @pow_sitofp_const_base_2_no_fast(
-; CHECK-NEXT:    [[LDEXPF:%.*]] = call float @ldexpf(float 1.000000e+00, i32 [[X:%.*]])
+; CHECK-NEXT:    [[LDEXPF:%.*]] = call float @ldexpf(float 1.000000e+00, i32 signext [[X:%.*]])
 ; CHECK-NEXT:    [[RES:%.*]] = fpext float [[LDEXPF]] to double
 ; CHECK-NEXT:    ret double [[RES]]
 ;
   %subfp = sitofp i32 %x to float
   %pow = tail call float @llvm.pow.f32(float 2.000000e+00, float %subfp)
   %res = fpext float %pow to double
   ret double %res
 }

llvm/test/Transforms/InstCombine/simplify-libcalls.ll

 ; CHECK-NEXT:    [[Y:%.*]] = call double @exp2(double %x)
 ; CHECK-NEXT:    ret double [[Y]]
 
   %y = call inreg double @pow(double inreg 2.0, double inreg %x)
   ret double %y
 }
 define double @fake_ldexp(i32 %x) {
 ; CHECK-LABEL: @fake_ldexp(
-; CHECK-NEXT:    [[Z:%.*]] = call double @ldexp(double 1.0{{.*}}, i32 %x)
+; CHECK-NEXT:    [[Z:%.*]] = call double @ldexp(double 1.0{{.*}}, i32 signext %x)
 ; CHECK-NEXT:    ret double [[Z]]
 
   %y = sitofp i32 %x to double
   %z = call inreg double @exp2(double %y)
   ret double %z
 }
 
+; CHECK: declare double @ldexp(double, i32 signext)
+
 
 attributes #0 = { nobuiltin }
 attributes #1 = { builtin }