<div dir="ltr">Is this going to handle -infinity correctly on some (power) platforms?<div><br></div><div>When hashing <span style="color:rgb(32,33,36);font-family:Roboto,Arial,sans-serif;font-size:13px;letter-spacing:0.185714px;white-space:pre-wrap">-std::numeric_limits<long double>::infinity() we get:</span></div><div><br></div><div><span style="color:rgb(32,33,36);font-family:Roboto,Arial,sans-serif;font-size:13px;letter-spacing:0.185714px;white-space:pre-wrap">good version:
0x0000: 01
0x0000: 0100 0000
bad version:
0x0000: 00
0x0000: 0100 0000
<br></span></div><div><span style="color:rgb(32,33,36);font-family:Roboto,Arial,sans-serif;font-size:13px;letter-spacing:0.185714px;white-space:pre-wrap">which has exactly the sign bit of difference and generates the same hash value as </span><span style="color:rgb(32,33,36);font-family:Roboto,Arial,sans-serif;font-size:13px;letter-spacing:0.185714px;white-space:pre-wrap">std::numeric_limits<long double>::infinity(). Possibly a bug in the hasher here, but wanted to bring it up.</span><span style="color:rgb(32,33,36);font-family:Roboto,Arial,sans-serif;font-size:13px;letter-spacing:0.185714px;white-space:pre-wrap">
</span></div><div><span style="color:rgb(32,33,36);font-family:Roboto,Arial,sans-serif;font-size:13px;letter-spacing:0.185714px;white-space:pre-wrap"><br></span></div><div><font color="#202124" face="Roboto, Arial, sans-serif"><span style="letter-spacing:0.185714px;white-space:pre-wrap">Thoughts?</span></font></div><div><font color="#202124" face="Roboto, Arial, sans-serif"><span style="letter-spacing:0.185714px;white-space:pre-wrap"><br></span></font></div><div><font color="#202124" face="Roboto, Arial, sans-serif"><span style="letter-spacing:0.185714px;white-space:pre-wrap">-eric</span></font></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Mar 28, 2020 at 6:47 AM Sanjay Patel via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Alive2 confirms the -0.0 problem:</div><div><a href="http://volta.cs.utah.edu:8080/z/f39Rw-" target="_blank">http://volta.cs.utah.edu:8080/z/f39Rw-</a></div><div><br></div><div>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 ( <a href="https://bugs.llvm.org/show_bug.cgi?id=38086" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=38086</a> ).<br></div><div>(It would be interesting to have Alive2 command-line options to disallow -0.0 or NaN values similar to the -disable-undef-input option.)<br></div><div><br></div><div>We do have this transform for fcmp already:</div><div> // fcmp (fpext X), C -> fcmp X, (fptrunc C) if fptrunc is lossless</div><div><br></div><div>And I think that's as far as we can take it - there is no fptrunc sibling for that because truncation loses bits:</div><div></div><div><a href="http://volta.cs.utah.edu:8080/z/zTheyA" target="_blank">http://volta.cs.utah.edu:8080/z/zTheyA</a></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Mar 28, 2020 at 4:38 AM Roman Lebedev <<a href="mailto:lebedev.ri@gmail.com" target="_blank">lebedev.ri@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Ah yes, indeed. Sorry for the noise.<br>
<br>
On Sat, Mar 28, 2020 at 10:31 AM Craig Topper <<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>> wrote:<br>
><br>
> Fp compare wouldn’t handle negative 0 correctly would it?<br>
><br>
> On Fri, Mar 27, 2020 at 11:27 PM Roman Lebedev via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> On Sat, Mar 28, 2020 at 12:34 AM Sanjay Patel via llvm-commits<br>
>> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>> ><br>
>> ><br>
>> > Author: Sanjay Patel<br>
>> > Date: 2020-03-27T17:33:59-04:00<br>
>> > New Revision: 0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0<br>
>> ><br>
>> > URL: <a href="https://github.com/llvm/llvm-project/commit/0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0</a><br>
>> > DIFF: <a href="https://github.com/llvm/llvm-project/commit/0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/0f56bbc1a5b2ddc881d1c55c9024b9c473dac6f0.diff</a><br>
>> ><br>
>> > LOG: [InstCombine] reduce FP-casted and bitcasted signbit check<br>
>> ><br>
>> > PR45305:<br>
>> > <a href="https://bugs.llvm.org/show_bug.cgi?id=45305" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=45305</a><br>
>> ><br>
>> > Alive2 proofs:<br>
>> > <a href="http://volta.cs.utah.edu:8080/z/bVyrko" rel="noreferrer" target="_blank">http://volta.cs.utah.edu:8080/z/bVyrko</a><br>
>> > <a href="http://volta.cs.utah.edu:8080/z/Vxpz9q" rel="noreferrer" target="_blank">http://volta.cs.utah.edu:8080/z/Vxpz9q</a><br>
>> ><br>
>> > Added:<br>
>> ><br>
>> ><br>
>> > Modified:<br>
>> > llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
>> > llvm/test/Transforms/InstCombine/icmp.ll<br>
>> ><br>
>> > Removed:<br>
>> ><br>
>> ><br>
>> ><br>
>> > ################################################################################<br>
>> > diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
>> > index 63bf3462faac..62b99e8b8b5b 100644<br>
>> > --- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
>> > +++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
>> > @@ -2760,6 +2760,30 @@ static Instruction *foldICmpBitCast(ICmpInst &Cmp,<br>
>> > if (match(BCSrcOp, m_UIToFP(m_Value(X))))<br>
>> > if (Cmp.isEquality() && match(Op1, m_Zero()))<br>
>> > return new ICmpInst(Pred, X, ConstantInt::getNullValue(X->getType()));<br>
>> > +<br>
>> > + // If this is a sign-bit test of a bitcast of a casted FP value, eliminate<br>
>> > + // the FP cast because the FP cast does not change the sign-bit.<br>
>> > + const APInt *C;<br>
>> > + bool TrueIfSigned;<br>
>> > + if (match(Op1, m_APInt(C)) && Bitcast->hasOneUse() &&<br>
>> > + isSignBitCheck(Pred, *C, TrueIfSigned)) {<br>
>> > + if (match(BCSrcOp, m_FPExt(m_Value(X))) ||<br>
>> > + match(BCSrcOp, m_FPTrunc(m_Value(X)))) {<br>
>> > + // (bitcast (fpext/fptrunc X)) to iX) < 0 --> (bitcast X to iY) < 0<br>
>> > + // (bitcast (fpext/fptrunc X)) to iX) > -1 --> (bitcast X to iY) > -1<br>
>> > + Type *XType = X->getType();<br>
>> > + Type *NewType = Builder.getIntNTy(XType->getScalarSizeInBits());<br>
>> > + if (XType->isVectorTy())<br>
>> > + NewType = VectorType::get(NewType, XType->getVectorNumElements());<br>
>> > + Value *NewBitcast = Builder.CreateBitCast(X, NewType);<br>
>> > + if (TrueIfSigned)<br>
>> > + return new ICmpInst(ICmpInst::ICMP_SLT, NewBitcast,<br>
>> > + ConstantInt::getNullValue(NewType));<br>
>> > + else<br>
>> > + return new ICmpInst(ICmpInst::ICMP_SGT, NewBitcast,<br>
>> > + ConstantInt::getAllOnesValue(NewType));<br>
>> > + }<br>
>> > + }<br>
>> > }<br>
>> ><br>
>> > // Test to see if the operands of the icmp are casted versions of other<br>
>> ><br>
>> > diff --git a/llvm/test/Transforms/InstCombine/icmp.ll b/llvm/test/Transforms/InstCombine/icmp.ll<br>
>> > index b2c6213399b0..41bf661b092e 100644<br>
>> > --- a/llvm/test/Transforms/InstCombine/icmp.ll<br>
>> > +++ b/llvm/test/Transforms/InstCombine/icmp.ll<br>
>> > @@ -3609,9 +3609,8 @@ define <2 x i32> @Op1Negated_Vec(<2 x i32> %x) {<br>
>> ><br>
>> > define i1 @signbit_bitcast_fpext(float %x) {<br>
>> > ; CHECK-LABEL: @signbit_bitcast_fpext(<br>
>> > -; CHECK-NEXT: [[F:%.*]] = fpext float [[X:%.*]] to double<br>
>> > -; CHECK-NEXT: [[B:%.*]] = bitcast double [[F]] to i64<br>
>> > -; CHECK-NEXT: [[R:%.*]] = icmp slt i64 [[B]], 0<br>
>> > +; CHECK-NEXT: [[TMP1:%.*]] = bitcast float [[X:%.*]] to i32<br>
>> > +; CHECK-NEXT: [[R:%.*]] = icmp slt i32 [[TMP1]], 0<br>
>> > ; CHECK-NEXT: ret i1 [[R]]<br>
>> Not that i disagree with the transform, but why do we want to keep bitcast<br>
>> instead of producing fp comparison?<br>
>><br>
>> We then would want to split this fold:<br>
>> (bitcast (fpext/fptrunc X)) to iX) < 0 --> (fpext/fptrunc X) < 0<br>
>> (fpext/fptrunc X) < 0 --> X < 0<br>
>><br>
>> > %f = fpext float %x to double<br>
>> > @@ -3622,9 +3621,8 @@ define i1 @signbit_bitcast_fpext(float %x) {<br>
>> ><br>
>> > define <2 x i1> @signbit_bitcast_fpext_vec(<2 x half> %x) {<br>
>> > ; CHECK-LABEL: @signbit_bitcast_fpext_vec(<br>
>> > -; CHECK-NEXT: [[F:%.*]] = fpext <2 x half> [[X:%.*]] to <2 x float><br>
>> > -; CHECK-NEXT: [[B:%.*]] = bitcast <2 x float> [[F]] to <2 x i32><br>
>> > -; CHECK-NEXT: [[R:%.*]] = icmp slt <2 x i32> [[B]], zeroinitializer<br>
>> > +; CHECK-NEXT: [[TMP1:%.*]] = bitcast <2 x half> [[X:%.*]] to <2 x i16><br>
>> > +; CHECK-NEXT: [[R:%.*]] = icmp slt <2 x i16> [[TMP1]], zeroinitializer<br>
>> > ; CHECK-NEXT: ret <2 x i1> [[R]]<br>
>> > ;<br>
>> > %f = fpext <2 x half> %x to <2 x float><br>
>> > @@ -3635,9 +3633,8 @@ define <2 x i1> @signbit_bitcast_fpext_vec(<2 x half> %x) {<br>
>> ><br>
>> > define i1 @signbit_bitcast_fptrunc(float %x) {<br>
>> > ; CHECK-LABEL: @signbit_bitcast_fptrunc(<br>
>> > -; CHECK-NEXT: [[F:%.*]] = fptrunc float [[X:%.*]] to half<br>
>> > -; CHECK-NEXT: [[B:%.*]] = bitcast half [[F]] to i16<br>
>> > -; CHECK-NEXT: [[R:%.*]] = icmp sgt i16 [[B]], -1<br>
>> > +; CHECK-NEXT: [[TMP1:%.*]] = bitcast float [[X:%.*]] to i32<br>
>> > +; CHECK-NEXT: [[R:%.*]] = icmp sgt i32 [[TMP1]], -1<br>
>> > ; CHECK-NEXT: ret i1 [[R]]<br>
>> > ;<br>
>> > %f = fptrunc float %x to half<br>
>> > @@ -3648,9 +3645,8 @@ define i1 @signbit_bitcast_fptrunc(float %x) {<br>
>> ><br>
>> > define <2 x i1> @signbit_bitcast_fptrunc_vec(<2 x double> %x) {<br>
>> > ; CHECK-LABEL: @signbit_bitcast_fptrunc_vec(<br>
>> > -; CHECK-NEXT: [[F:%.*]] = fptrunc <2 x double> [[X:%.*]] to <2 x half><br>
>> > -; CHECK-NEXT: [[B:%.*]] = bitcast <2 x half> [[F]] to <2 x i16><br>
>> > -; CHECK-NEXT: [[R:%.*]] = icmp sgt <2 x i16> [[B]], <i16 -1, i16 -1><br>
>> > +; CHECK-NEXT: [[TMP1:%.*]] = bitcast <2 x double> [[X:%.*]] to <2 x i64><br>
>> > +; CHECK-NEXT: [[R:%.*]] = icmp sgt <2 x i64> [[TMP1]], <i64 -1, i64 -1><br>
>> > ; CHECK-NEXT: ret <2 x i1> [[R]]<br>
>> > ;<br>
>> > %f = fptrunc <2 x double> %x to <2 x half><br>
>> > @@ -3672,6 +3668,19 @@ define i1 @signbit_bitcast_fpext_wrong_cmp(float %x) {<br>
>> > ret i1 %r<br>
>> > }<br>
>> ><br>
>> > +define <4 x i1> @signbit_bitcast_fpext_vec_wrong_bitcast(<2 x half> %x) {<br>
>> > +; CHECK-LABEL: @signbit_bitcast_fpext_vec_wrong_bitcast(<br>
>> > +; CHECK-NEXT: [[F:%.*]] = fpext <2 x half> [[X:%.*]] to <2 x float><br>
>> > +; CHECK-NEXT: [[B:%.*]] = bitcast <2 x float> [[F]] to <4 x i16><br>
>> > +; CHECK-NEXT: [[R:%.*]] = icmp sgt <4 x i16> [[B]], <i16 -1, i16 -1, i16 -1, i16 -1><br>
>> > +; CHECK-NEXT: ret <4 x i1> [[R]]<br>
>> > +;<br>
>> > + %f = fpext <2 x half> %x to <2 x float><br>
>> > + %b = bitcast <2 x float> %f to <4 x i16><br>
>> > + %r = icmp sgt <4 x i16> %b, <i16 -1, i16 -1, i16 -1, i16 -1><br>
>> > + ret <4 x i1> %r<br>
>> > +}<br>
>> > +<br>
>> > declare void @use_i64(i64)<br>
>> ><br>
>> > define i1 @signbit_bitcast_fpext_extra_use(float %x, i64* %p) {<br>
>> ><br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > llvm-commits mailing list<br>
>> > <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
>> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
><br>
> --<br>
> ~Craig<br>
</blockquote></div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>