[llvm] r258428 - [LibCallSimplifier] don't get fooled by a fake fmin()

[Hans Wennborg via llvm-commits]

Sanjay, it was pointed out by Joerg that this should probably also be
merged to 3.8?

On Thu, Jan 21, 2016 at 12:19 PM, Sanjay Patel via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: spatel
> Date: Thu Jan 21 14:19:54 2016
> New Revision: 258428
>
> URL: http://llvm.org/viewvc/llvm-project?rev=258428&view=rev
> Log:
> [LibCallSimplifier] don't get fooled by a fake fmin()
>
> This is similar to the bug/fix:
> https://llvm.org/bugs/show_bug.cgi?id=26211
> http://reviews.llvm.org/rL258325
>
> The fmin() test case reveals another bug caused by sloppy
> code duplication. It will crash without this patch because
> fp128 is a valid floating-point type, but we would think
> that we had matched a function that used doubles.
>
> The new helper function can be used to replace similar
> checks that are used in several other places in this file.
>
>
> Modified:
>     llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
>     llvm/trunk/test/Transforms/InstCombine/double-float-shrink-1.ll
>
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp?rev=258428&r1=258427&r2=258428&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp Thu Jan 21 14:19:54 2016
> @@ -953,13 +953,34 @@ static Value *valueHasFloatPrecision(Val
>    return nullptr;
>  }
>
> +/// Any floating-point library function that we're trying to simplify will have
> +/// a signature of the form: fptype foo(fptype param1, fptype param2, ...).
> +/// CheckDoubleTy indicates that 'fptype' must be 'double'.
> +static bool matchesFPLibFunctionSignature(const Function *F, unsigned NumParams,
> +                                          bool CheckDoubleTy) {
> +  FunctionType *FT = F->getFunctionType();
> +  if (FT->getNumParams() != NumParams)
> +    return false;
> +
> +  // The return type must match what we're looking for.
> +  Type *RetTy = FT->getReturnType();
> +  if (CheckDoubleTy ? !RetTy->isDoubleTy() : !RetTy->isFloatingPointTy())
> +    return false;
> +
> +  // Each parameter must match the return type, and therefore, match every other
> +  // parameter too.
> +  for (const Type *ParamTy : FT->params())
> +    if (ParamTy != RetTy)
> +      return false;
> +
> +  return true;
> +}
> +
>  /// Shrink double -> float for unary functions like 'floor'.
>  static Value *optimizeUnaryDoubleFP(CallInst *CI, IRBuilder<> &B,
>                                      bool CheckRetType) {
>    Function *Callee = CI->getCalledFunction();
> -  FunctionType *FT = Callee->getFunctionType();
> -  if (FT->getNumParams() != 1 || !FT->getReturnType()->isDoubleTy() ||
> -      !FT->getParamType(0)->isDoubleTy())
> +  if (!matchesFPLibFunctionSignature(Callee, 1, true))
>      return nullptr;
>
>    if (CheckRetType) {
> @@ -997,12 +1018,7 @@ static Value *optimizeUnaryDoubleFP(Call
>  /// Shrink double -> float for binary functions like 'fmin/fmax'.
>  static Value *optimizeBinaryDoubleFP(CallInst *CI, IRBuilder<> &B) {
>    Function *Callee = CI->getCalledFunction();
> -  FunctionType *FT = Callee->getFunctionType();
> -  // Just make sure this has 2 arguments of the same FP type, which match the
> -  // result type.
> -  if (FT->getNumParams() != 2 || FT->getReturnType() != FT->getParamType(0) ||
> -      FT->getParamType(0) != FT->getParamType(1) ||
> -      !FT->getParamType(0)->isFloatingPointTy())
> +  if (!matchesFPLibFunctionSignature(Callee, 2, true))
>      return nullptr;
>
>    // If this is something like 'fmin((double)floatval1, (double)floatval2)',
>
> Modified: llvm/trunk/test/Transforms/InstCombine/double-float-shrink-1.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/double-float-shrink-1.ll?rev=258428&r1=258427&r2=258428&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/double-float-shrink-1.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/double-float-shrink-1.ll Thu Jan 21 14:19:54 2016
> @@ -364,6 +364,26 @@ define float @max1(float %a, float %b) {
>  ; CHECK-NEXT:  ret
>  }
>
> +; A function can have a name that matches a common libcall,
> +; but with the wrong type(s). Let it be.
> +
> +define float @fake_fmin(float %a, float %b) {
> +  %c = fpext float %a to fp128
> +  %d = fpext float %b to fp128
> +  %e = call fp128 @fmin(fp128 %c, fp128 %d)
> +  %f = fptrunc fp128 %e to float
> +  ret float %f
> +
> +; CHECK-LABEL: fake_fmin(
> +; CHECK-NEXT:  %c = fpext float %a to fp128
> +; CHECK-NEXT:  %d = fpext float %b to fp128
> +; CHECK-NEXT:  %e = call fp128 @fmin(fp128 %c, fp128 %d)
> +; CHECK-NEXT:  %f = fptrunc fp128 %e to float
> +; CHECK-NEXT:  ret float %f
> +}
> +
> +declare fp128 @fmin(fp128, fp128) ; This is not the 'fmin' you're looking for.
> +
>  declare double @fmax(double, double)
>
>  declare double @tanh(double)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits