[PartialInlineLibCalls] Teach PartialInlineLibCalls to honor nobuiltin, properly check the function signature, and check TLI::has

[craig.topper]

Summary

We shouldn't do this transformation if the function is marked nobuitlin.

We were only checking that the return type is floating point, we really should be checking the argument types and argument count as well. This can be accomplished by using the other version of getLibFunc that takes the Function and not just the name.

We should also be checking TLI::has since sqrtf is a macro on Windows.

Happy to split this up if we want.

Fixes PR32559.

[davide]

I guess this is fine, as this was particularly broken before, but please wait for another opinion.

[efriedma]

LGTM

[davide]

Forgot to click accept

llvm/trunk/lib/Transforms/Scalar/PartiallyInlineLibCalls.cpp

 
 static bool optimizeSQRT(CallInst *Call, Function *CalledFunc,
                          BasicBlock &CurrBB, Function::iterator &BB) {
   // There is no need to change the IR, since backend will emit sqrt
   // instruction if the call has already been marked read-only.
   if (Call->onlyReadsMemory())
     return false;
 
-  // The call must have the expected result type.
-  if (!Call->getType()->isFloatingPointTy())
-    return false;
-
   // Do the following transformation:
   //
   // (before)
   // dst = sqrt(src)
   //
   // (after)
   // v0 = sqrt_noreadmem(src) # native sqrt instruction.
   // if (v0 is a NaN)
for (Function::iterator BB = F.begin(), BE = F.end(); BB != BE;) {
     for (BasicBlock::iterator II = CurrBB->begin(), IE = CurrBB->end();
          II != IE; ++II) {
       CallInst *Call = dyn_cast<CallInst>(&*II);
       Function *CalledFunc;
 
       if (!Call || !(CalledFunc = Call->getCalledFunction()))
         continue;
 
+      if (Call->isNoBuiltin())
+        continue;
+
       // Skip if function either has local linkage or is not a known library
       // function.
       LibFunc LF;
-      if (CalledFunc->hasLocalLinkage() || !CalledFunc->hasName() ||
-          !TLI->getLibFunc(CalledFunc->getName(), LF))
+      if (CalledFunc->hasLocalLinkage() ||
+          !TLI->getLibFunc(*CalledFunc, LF) || !TLI->has(LF))
         continue;
 
       switch (LF) {
       case LibFunc_sqrtf:
       case LibFunc_sqrt:
         if (TTI->haveFastSqrt(Call->getType()) &&
             optimizeSQRT(Call, CalledFunc, *CurrBB, BB))
           break;

llvm/trunk/test/Transforms/PartiallyInlineLibCalls/bad-prototype.ll

 ; RUN: opt -S -partially-inline-libcalls < %s | FileCheck %s
 ; RUN: opt -S -passes=partially-inline-libcalls < %s | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
 
 declare i32 @sqrt()
+declare float @sqrtf()
 
 ; CHECK-LABEL: @foo
 define i32 @foo() {
   ; CHECK: call{{.*}}@sqrt
   ; CHECK-NOT: call{{.*}}@sqrt
   %r = call i32 @sqrt()
   ret i32 %r
 }
+
+; CHECK-LABEL: @bar
+define float @bar() {
+  ; CHECK: call{{.*}}@sqrtf
+  ; CHECK-NOT: call{{.*}}@sqrtf
+  %r = call float @sqrtf()
+  ret float %r
+}

llvm/trunk/test/Transforms/PartiallyInlineLibCalls/nobuiltin.ll

+; RUN: opt -S -partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+; RUN: opt -S -passes=partially-inline-libcalls -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+
+define float @f(float %val) {
+; CHECK-LABEL: @f
+; CHECK: call{{.*}}@sqrtf
+; CHECK-NOT: call{{.*}}@sqrtf
+  %res = tail call float @sqrtf(float %val) nobuiltin
+  ret float %res
+}
+
+declare float @sqrtf(float)