[llvm] 0f56bbc - [InstCombine] reduce FP-casted and bitcasted signbit check
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 6 15:53:39 PDT 2020
Is this going to handle -infinity correctly on some (power) platforms?
When hashing -std::numeric_limits<long double>::infinity() we get:
good version: 0x0000: 01 0x0000: 0100 0000 bad version: 0x0000: 00 0x0000:
0100 0000
which has exactly the sign bit of difference and generates the same hash
value as std::numeric_limits<long double>::infinity(). Possibly a bug in
the hasher here, but wanted to bring it up.
Thoughts?
-eric
On Sat, Mar 28, 2020 at 6:47 AM Sanjay Patel via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Alive2 confirms the -0.0 problem:
> http://volta.cs.utah.edu:8080/z/f39Rw-
>
> NaN inputs could also be wrong. We could do better given fast-math-flags,
> but I'm avoiding that without strong motivation...because FMF on fcmp is a
> mess ( https://bugs.llvm.org/show_bug.cgi?id=38086 ).
> (It would be interesting to have Alive2 command-line options to disallow
> -0.0 or NaN values similar to the -disable-undef-input option.)
>
> We do have this transform for fcmp already:
> // fcmp (fpext X), C -> fcmp X, (fptrunc C) if fptrunc is lossless
>
> And I think that's as far as we can take it - there is no fptrunc sibling
> for that because truncation loses bits:
> http://volta.cs.utah.edu:8080/z/zTheyA
>
>
> On Sat, Mar 28, 2020 at 4:38 AM Roman Lebedev <lebedev.ri at gmail.com>
> wrote:
>
>> Ah yes, indeed. Sorry for the noise.
>>
>> On Sat, Mar 28, 2020 at 10:31 AM Craig Topper <craig.topper at gmail.com>
>> wrote:
>> >
>> > 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
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200406/68ae811c/attachment.html>
More information about the llvm-commits
mailing list