[PATCH] D16198: [LibCallSimplifier] don't get fooled by a fake sqrt()

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 14 10:11:00 PST 2016


spatel created this revision.
spatel added reviewers: joerg, majnemer, davide.
spatel added a subscriber: llvm-commits.
Herald added a subscriber: aemerson.

The test case will crash without this patch because the subsequent call to hasUnsafeAlgebra() assumes that the call instruction is an FPMathOperator (ie, returns an FP type). 

This part of the function signature check was omitted for the sqrt() case, but seems to be in place for all other transforms.

Before:
http://reviews.llvm.org/rL257400
...I think we would have needlessly continued execution in optimizeSqrt(), but the bug was harmless because we'd eventually fail some other check and return without damage.

If this is a sufficient fix for the problem, I will push to get it included in the 3.8 branch.

http://reviews.llvm.org/D16198

Files:
  lib/Transforms/Utils/SimplifyLibCalls.cpp
  test/Transforms/InstCombine/cos-2.ll

Index: test/Transforms/InstCombine/cos-2.ll
===================================================================
--- test/Transforms/InstCombine/cos-2.ll
+++ test/Transforms/InstCombine/cos-2.ll
@@ -1,17 +1,27 @@
-; Test that the cos library call simplifier works correctly.
-;
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 declare float @cos(double)
+declare signext i8 @sqrt(...)
 
-; Check that cos functions with the wrong prototype aren't simplified.
+; Check that functions with the wrong prototype aren't simplified.
 
 define float @test_no_simplify1(double %d) {
 ; CHECK-LABEL: @test_no_simplify1(
   %neg = fsub double -0.000000e+00, %d
   %cos = call float @cos(double %neg)
 ; CHECK: call float @cos(double %neg)
   ret float %cos
 }
+
+
+define i8 @bogus_sqrt() {
+  %fake_sqrt = call signext i8 (...) @sqrt()
+  ret i8 %fake_sqrt
+
+; CHECK-LABEL: bogus_sqrt(
+; CHECK-NEXT:  %fake_sqrt = call signext i8 (...) @sqrt()
+; CHECK-NEXT:  ret i8 %fake_sqrt
+}
+
Index: lib/Transforms/Utils/SimplifyLibCalls.cpp
===================================================================
--- lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -1394,12 +1394,21 @@
 
 Value *LibCallSimplifier::optimizeSqrt(CallInst *CI, IRBuilder<> &B) {
   Function *Callee = CI->getCalledFunction();
-  
+
   Value *Ret = nullptr;
   if (TLI->has(LibFunc::sqrtf) && (Callee->getName() == "sqrt" ||
                                    Callee->getIntrinsicID() == Intrinsic::sqrt))
     Ret = optimizeUnaryDoubleFP(CI, B, true);
 
+  // FIXME: Refactor - this check is repeated all over this file and even in the
+  // preceding call to shrink double -> float.
+
+  // Make sure this has 1 argument of FP type, which matches the result type.
+  FunctionType *FT = Callee->getFunctionType();
+  if (FT->getNumParams() != 1 || FT->getReturnType() != FT->getParamType(0) ||
+      !FT->getParamType(0)->isFloatingPointTy())
+    return Ret;
+
   if (!CI->hasUnsafeAlgebra())
     return Ret;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16198.44898.patch
Type: text/x-patch
Size: 2191 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160114/93a95360/attachment.bin>


More information about the llvm-commits mailing list