[llvm] r266452 - [InstCombine] Don't transform compares of calls to functions named fabs{f, l, }

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "David Majnemer via llvm-commits" <llvm-commits at lists.llvm.org>
> To: llvm-commits at lists.llvm.org
> Sent: Friday, April 15, 2016 12:21:03 PM
> Subject: [llvm] r266452 - [InstCombine] Don't transform compares of calls to functions named fabs{f, l, }
> 
> Author: majnemer
> Date: Fri Apr 15 12:21:03 2016
> New Revision: 266452
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=266452&view=rev
> Log:
> [InstCombine] Don't transform compares of calls to functions named
> fabs{f,l,}
> 
> InstCombine wants to optimize compares of calls to fabs with zero.
> However, we didn't have the necessary legality checking to verify
> that
> the function call had the same behavior as fabs.

And so you limited the transformation only to the intrinsic? Why? Shouldn't we just have the same checks here that we have in LibCallSimplifier::optimizeFabs:

Value *LibCallSimplifier::optimizeFabs(CallInst *CI, IRBuilder<> &B) {
  Function *Callee = CI->getCalledFunction();
  if (!matchesFPLibFunctionSignature(Callee, 1, false))
    return nullptr;

specifically the matchesFPLibFunctionSignature check.

Thanks again,
Hal

> 
> Modified:
>     llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>     llvm/trunk/test/Transforms/InstCombine/pr27332.ll
> 
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=266452&r1=266451&r2=266452&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Fri
> Apr 15 12:21:03 2016
> @@ -26,6 +26,7 @@
>  #include "llvm/Support/CommandLine.h"
>  #include "llvm/Support/Debug.h"
>  #include "llvm/Analysis/TargetLibraryInfo.h"
> +#include "llvm/Analysis/VectorUtils.h"
>  
>  using namespace llvm;
>  using namespace PatternMatch;
> @@ -4564,39 +4565,33 @@ Instruction *InstCombiner::visitFCmpInst
>            break;
>  
>          CallInst *CI = cast<CallInst>(LHSI);
> -        const Function *F = CI->getCalledFunction();
> -        if (!F)
> +        Intrinsic::ID IID = getIntrinsicIDForCall(CI, TLI);
> +        if (IID != Intrinsic::fabs)
>            break;
>  
>          // Various optimization for fabs compared with zero.
> -        LibFunc::Func Func;
> -        if (F->getIntrinsicID() == Intrinsic::fabs ||
> -            (TLI->getLibFunc(F->getName(), Func) && TLI->has(Func)
> &&
> -             (Func == LibFunc::fabs || Func == LibFunc::fabsf ||
> -              Func == LibFunc::fabsl))) {
> -          switch (I.getPredicate()) {
> -          default:
> -            break;
> -            // fabs(x) < 0 --> false
> -          case FCmpInst::FCMP_OLT:
> -            llvm_unreachable("handled by SimplifyFCmpInst");
> -            // fabs(x) > 0 --> x != 0
> -          case FCmpInst::FCMP_OGT:
> -            return new FCmpInst(FCmpInst::FCMP_ONE,
> CI->getArgOperand(0), RHSC);
> -            // fabs(x) <= 0 --> x == 0
> -          case FCmpInst::FCMP_OLE:
> -            return new FCmpInst(FCmpInst::FCMP_OEQ,
> CI->getArgOperand(0), RHSC);
> -            // fabs(x) >= 0 --> !isnan(x)
> -          case FCmpInst::FCMP_OGE:
> -            return new FCmpInst(FCmpInst::FCMP_ORD,
> CI->getArgOperand(0), RHSC);
> -            // fabs(x) == 0 --> x == 0
> -            // fabs(x) != 0 --> x != 0
> -          case FCmpInst::FCMP_OEQ:
> -          case FCmpInst::FCMP_UEQ:
> -          case FCmpInst::FCMP_ONE:
> -          case FCmpInst::FCMP_UNE:
> -            return new FCmpInst(I.getPredicate(),
> CI->getArgOperand(0), RHSC);
> -          }
> +        switch (I.getPredicate()) {
> +        default:
> +          break;
> +        // fabs(x) < 0 --> false
> +        case FCmpInst::FCMP_OLT:
> +          llvm_unreachable("handled by SimplifyFCmpInst");
> +        // fabs(x) > 0 --> x != 0
> +        case FCmpInst::FCMP_OGT:
> +          return new FCmpInst(FCmpInst::FCMP_ONE,
> CI->getArgOperand(0), RHSC);
> +        // fabs(x) <= 0 --> x == 0
> +        case FCmpInst::FCMP_OLE:
> +          return new FCmpInst(FCmpInst::FCMP_OEQ,
> CI->getArgOperand(0), RHSC);
> +        // fabs(x) >= 0 --> !isnan(x)
> +        case FCmpInst::FCMP_OGE:
> +          return new FCmpInst(FCmpInst::FCMP_ORD,
> CI->getArgOperand(0), RHSC);
> +        // fabs(x) == 0 --> x == 0
> +        // fabs(x) != 0 --> x != 0
> +        case FCmpInst::FCMP_OEQ:
> +        case FCmpInst::FCMP_UEQ:
> +        case FCmpInst::FCMP_ONE:
> +        case FCmpInst::FCMP_UNE:
> +          return new FCmpInst(I.getPredicate(),
> CI->getArgOperand(0), RHSC);
>          }
>        }
>        }
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/pr27332.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/pr27332.ll?rev=266452&r1=266451&r2=266452&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/pr27332.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/pr27332.ll Fri Apr 15
> 12:21:03 2016
> @@ -9,3 +9,15 @@ entry:
>  }
>  ; CHECK-LABEL: define <4 x i1> @test1(
>  ; CHECK:   ret <4 x i1> zeroinitializer
> +
> +declare float @fabsf()
> +
> +define i1 @test2() {
> +  %call = call float @fabsf()
> +  %cmp = fcmp olt float %call, 0.000000e+00
> +  ret i1 %cmp
> +}
> +; CHECK-LABEL: define i1 @test2(
> +; CHECK:  %[[call:.*]] = call float @fabsf()
> +; CHECK:  %[[cmp:.*]] = fcmp olt float %[[call]], 0.000000e+00
> +; CHECK:  ret i1 %[[cmp]]
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory