[llvm] 0f56bbc - [InstCombine] reduce FP-casted and bitcasted signbit check
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 7 03:48:04 PDT 2020
Thanks for catching that.
I forgot about the PPC_FP128 type, and I'm not sure what it means to
bitcast that type to an integer. The safe thing will be to just exclude
that type from the transform. There's precedence for that kind of
restriction in other places in LLVM.
AFAIK, that's the only problematic FP type in LLVM [1]. But let me know if
you see other issues. I'll post a patch for review shortly.
Side note - Alive2 says that ppc_fp128 is an unsupported type, so we can't
use it to say anything about this example:
define i1 @src(float %x) {
%s2 = fpext float %x to ppc_fp128
%s3 = bitcast ppc_fp128 %s2 to i128
%s4 = icmp slt i128 %s3, 0
ret i1 %s4
}
[1] http://llvm.org/docs/LangRef.html#floating-point-types
On Mon, Apr 6, 2020 at 6:53 PM Eric Christopher <echristo at gmail.com> wrote:
> 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/20200407/591ea4d5/attachment.html>
More information about the llvm-commits
mailing list