[llvm] 0f56bbc - [InstCombine] reduce FP-casted and bitcasted signbit check

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 28 00:31:47 PDT 2020


Fp compare wouldn’t handle negative 0 correctly would it?

On Fri, Mar 27, 2020 at 11:27 PM Roman Lebedev via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On Sat, Mar 28, 2020 at 12:34 AM Sanjay Patel via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> >
> >
> > Author: Sanjay Patel
> > Date: 2020-03-27T17:33:59-04:00
> > New Revision: 0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0
> > DIFF:
> https://github.com/llvm/llvm-project/commit/0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0.diff
> >
> > LOG: [InstCombine] reduce FP-casted and bitcasted signbit check
> >
> > PR45305:
> > https://bugs.llvm.org/show_bug.cgi?id=45305
> >
> > Alive2 proofs:
> > http://volta.cs.utah.edu:8080/z/bVyrko
> > http://volta.cs.utah.edu:8080/z/Vxpz9q
> >
> > Added:
> >
> >
> > Modified:
> >     llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
> >     llvm/test/Transforms/InstCombine/icmp.ll
> >
> > Removed:
> >
> >
> >
> >
> ################################################################################
> > diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
> b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
> > index 63bf3462faac..62b99e8b8b5b 100644
> > --- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
> > +++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
> > @@ -2760,6 +2760,30 @@ static Instruction *foldICmpBitCast(ICmpInst &Cmp,
> >      if (match(BCSrcOp, m_UIToFP(m_Value(X))))
> >        if (Cmp.isEquality() && match(Op1, m_Zero()))
> >          return new ICmpInst(Pred, X,
> ConstantInt::getNullValue(X->getType()));
> > +
> > +    // If this is a sign-bit test of a bitcast of a casted FP value,
> eliminate
> > +    // the FP cast because the FP cast does not change the sign-bit.
> > +    const APInt *C;
> > +    bool TrueIfSigned;
> > +    if (match(Op1, m_APInt(C)) && Bitcast->hasOneUse() &&
> > +        isSignBitCheck(Pred, *C, TrueIfSigned)) {
> > +      if (match(BCSrcOp, m_FPExt(m_Value(X))) ||
> > +          match(BCSrcOp, m_FPTrunc(m_Value(X)))) {
> > +        // (bitcast (fpext/fptrunc X)) to iX) < 0 --> (bitcast X to iY)
> < 0
> > +        // (bitcast (fpext/fptrunc X)) to iX) > -1 --> (bitcast X to
> iY) > -1
> > +        Type *XType = X->getType();
> > +        Type *NewType = Builder.getIntNTy(XType->getScalarSizeInBits());
> > +        if (XType->isVectorTy())
> > +          NewType = VectorType::get(NewType,
> XType->getVectorNumElements());
> > +        Value *NewBitcast = Builder.CreateBitCast(X, NewType);
> > +        if (TrueIfSigned)
> > +          return new ICmpInst(ICmpInst::ICMP_SLT, NewBitcast,
> > +                              ConstantInt::getNullValue(NewType));
> > +        else
> > +          return new ICmpInst(ICmpInst::ICMP_SGT, NewBitcast,
> > +                              ConstantInt::getAllOnesValue(NewType));
> > +      }
> > +    }
> >    }
> >
> >    // Test to see if the operands of the icmp are casted versions of
> other
> >
> > diff  --git a/llvm/test/Transforms/InstCombine/icmp.ll
> b/llvm/test/Transforms/InstCombine/icmp.ll
> > index b2c6213399b0..41bf661b092e 100644
> > --- a/llvm/test/Transforms/InstCombine/icmp.ll
> > +++ b/llvm/test/Transforms/InstCombine/icmp.ll
> > @@ -3609,9 +3609,8 @@ define <2 x i32> @Op1Negated_Vec(<2 x i32> %x) {
> >
> >  define i1 @signbit_bitcast_fpext(float %x) {
> >  ; CHECK-LABEL: @signbit_bitcast_fpext(
> > -; CHECK-NEXT:    [[F:%.*]] = fpext float [[X:%.*]] to double
> > -; CHECK-NEXT:    [[B:%.*]] = bitcast double [[F]] to i64
> > -; CHECK-NEXT:    [[R:%.*]] = icmp slt i64 [[B]], 0
> > +; CHECK-NEXT:    [[TMP1:%.*]] = bitcast float [[X:%.*]] to i32
> > +; CHECK-NEXT:    [[R:%.*]] = icmp slt i32 [[TMP1]], 0
> >  ; CHECK-NEXT:    ret i1 [[R]]
> Not that i disagree with the transform, but why do we want to keep bitcast
> instead of producing fp comparison?
>
> We then would want to split this fold:
>   (bitcast (fpext/fptrunc X)) to iX) < 0 --> (fpext/fptrunc X) < 0
>   (fpext/fptrunc X) < 0   --> X < 0
>
> >    %f = fpext float %x to double
> > @@ -3622,9 +3621,8 @@ define i1 @signbit_bitcast_fpext(float %x) {
> >
> >  define <2 x i1> @signbit_bitcast_fpext_vec(<2 x half> %x) {
> >  ; CHECK-LABEL: @signbit_bitcast_fpext_vec(
> > -; CHECK-NEXT:    [[F:%.*]] = fpext <2 x half> [[X:%.*]] to <2 x float>
> > -; CHECK-NEXT:    [[B:%.*]] = bitcast <2 x float> [[F]] to <2 x i32>
> > -; CHECK-NEXT:    [[R:%.*]] = icmp slt <2 x i32> [[B]], zeroinitializer
> > +; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <2 x half> [[X:%.*]] to <2 x
> i16>
> > +; CHECK-NEXT:    [[R:%.*]] = icmp slt <2 x i16> [[TMP1]],
> zeroinitializer
> >  ; CHECK-NEXT:    ret <2 x i1> [[R]]
> >  ;
> >    %f = fpext <2 x half> %x to <2 x float>
> > @@ -3635,9 +3633,8 @@ define <2 x i1> @signbit_bitcast_fpext_vec(<2 x
> half> %x) {
> >
> >  define i1 @signbit_bitcast_fptrunc(float %x) {
> >  ; CHECK-LABEL: @signbit_bitcast_fptrunc(
> > -; CHECK-NEXT:    [[F:%.*]] = fptrunc float [[X:%.*]] to half
> > -; CHECK-NEXT:    [[B:%.*]] = bitcast half [[F]] to i16
> > -; CHECK-NEXT:    [[R:%.*]] = icmp sgt i16 [[B]], -1
> > +; CHECK-NEXT:    [[TMP1:%.*]] = bitcast float [[X:%.*]] to i32
> > +; CHECK-NEXT:    [[R:%.*]] = icmp sgt i32 [[TMP1]], -1
> >  ; CHECK-NEXT:    ret i1 [[R]]
> >  ;
> >    %f = fptrunc float %x to half
> > @@ -3648,9 +3645,8 @@ define i1 @signbit_bitcast_fptrunc(float %x) {
> >
> >  define <2 x i1> @signbit_bitcast_fptrunc_vec(<2 x double> %x) {
> >  ; CHECK-LABEL: @signbit_bitcast_fptrunc_vec(
> > -; CHECK-NEXT:    [[F:%.*]] = fptrunc <2 x double> [[X:%.*]] to <2 x
> half>
> > -; CHECK-NEXT:    [[B:%.*]] = bitcast <2 x half> [[F]] to <2 x i16>
> > -; CHECK-NEXT:    [[R:%.*]] = icmp sgt <2 x i16> [[B]], <i16 -1, i16 -1>
> > +; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <2 x double> [[X:%.*]] to <2 x
> i64>
> > +; CHECK-NEXT:    [[R:%.*]] = icmp sgt <2 x i64> [[TMP1]], <i64 -1, i64
> -1>
> >  ; CHECK-NEXT:    ret <2 x i1> [[R]]
> >  ;
> >    %f = fptrunc <2 x double> %x to <2 x half>
> > @@ -3672,6 +3668,19 @@ define i1 @signbit_bitcast_fpext_wrong_cmp(float
> %x) {
> >    ret i1 %r
> >  }
> >
> > +define <4 x i1> @signbit_bitcast_fpext_vec_wrong_bitcast(<2 x half> %x)
> {
> > +; CHECK-LABEL: @signbit_bitcast_fpext_vec_wrong_bitcast(
> > +; CHECK-NEXT:    [[F:%.*]] = fpext <2 x half> [[X:%.*]] to <2 x float>
> > +; CHECK-NEXT:    [[B:%.*]] = bitcast <2 x float> [[F]] to <4 x i16>
> > +; CHECK-NEXT:    [[R:%.*]] = icmp sgt <4 x i16> [[B]], <i16 -1, i16 -1,
> i16 -1, i16 -1>
> > +; CHECK-NEXT:    ret <4 x i1> [[R]]
> > +;
> > +  %f = fpext <2 x half> %x to <2 x float>
> > +  %b = bitcast <2 x float> %f to <4 x i16>
> > +  %r = icmp sgt <4 x i16> %b, <i16 -1, i16 -1, i16 -1, i16 -1>
> > +  ret <4 x i1> %r
> > +}
> > +
> >  declare void @use_i64(i64)
> >
> >  define i1 @signbit_bitcast_fpext_extra_use(float %x, i64* %p) {
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200328/0c744aa5/attachment.html>


More information about the llvm-commits mailing list