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

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 20 19:20:03 PDT 2016


On Wed, Apr 20, 2016 at 9:08 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- 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?


No, I have not.

The call to getIntrinsicIDForCall will return Intrinsic::fabs for either
the abs intrinsic or an explicit call to the library functions fabsf, fabs,
fabsl.

By utilizing getIntrinsicIDForCall to test if we have a call to
Intrinsic::fabs we nicely abstract the same checks over and over again out
to a single, canonical, location.


> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160420/976ee41d/attachment.html>


More information about the llvm-commits mailing list