[llvm] a253a2a - [SDAG] fold fsub -0.0, undef to undef rather than NaN

[Roman Lebedev via llvm-commits]

On Mon, Feb 24, 2020 at 4:39 PM Sanjay Patel <spatel at rotateright.com> wrote:
>
>
>
> On Sun, Feb 23, 2020 at 11:41 AM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>>
>> On Sun, Feb 23, 2020 at 7:37 PM Sanjay Patel via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> >
>> >
>> > Author: Sanjay Patel
>> > Date: 2020-02-23T11:36:53-05:00
>> > New Revision: a253a2a793cda34d1f6421ee9b7ca76a03fdfc59
>> >
>> > URL: https://github.com/llvm/llvm-project/commit/a253a2a793cda34d1f6421ee9b7ca76a03fdfc59
>> > DIFF: https://github.com/llvm/llvm-project/commit/a253a2a793cda34d1f6421ee9b7ca76a03fdfc59.diff
>> >
>> > LOG: [SDAG] fold fsub -0.0, undef to undef rather than NaN
>> >
>> > A question about this behavior came up on llvm-dev:
>> > http://lists.llvm.org/pipermail/llvm-dev/2020-February/139003.html
>> > ...and as part of backend improvements in D73978.
>>
>> > We decided not to implement a more general change that would have
>> > folded any FP binop with nearly arbitrary constant + undef operand
>> > to undef because that is not theoretically correct (even if it is
>> > practically correct).
>> This is a bit too pessimistic. Alive experiments show that it would be fine
>> in general at least for the fsub, as long as we don't have NaN's
>
>
> Maybe I'm not seeing it correctly. We said this:
>     fsub float 4.000000, undef --> undef
>
> ...is invalid in D74713, and that's independent of "nnan". Ie, we can't produce some tiny number like a denormal given a known constant like "4.0". So it would be wrong to say the output is fully undef - there is some range of values that can never be produced given (just about?) any fixed FP constant and any of the FP opcodes.
Hm. As per current alive, it's only invalid if we are required to handle NaN's:

----------------------------------------
Name: t0
 %r = fsub nnan half %x, undef
 ret %r
=>
 ret undef
 %r = fsub nnan half %x, undef

Done: 1
Optimization is correct!

----------------------------------------
Name: t1
 %r = fsub nnan half undef, %x
 ret %r
=>
 ret undef
 %r = fsub nnan half undef, %x

Done: 1
Optimization is correct!

----------------------------------------
Name: t1
 %r = fsub nnan half 4.000000, undef
 ret %r
=>
 ret undef
 %r = fsub nnan half 4.000000, undef

Done: 1
Optimization is correct!

----------------------------------------
Name: t1
 %r = fsub half 4.000000, undef
 ret %r
=>
 ret undef
 %r = fsub half 4.000000, undef

ERROR: Value mismatch

Example:
half %r = NaN   [based on undef value]
Source value: NaN
Target value: #x8500 (-0.000076293945?)


@Nuno: is this a bug in alive?

>>
>> > This is the SDAG-equivalent to the IR change in D74713.
>> >
>> > Added:
>> >
>> >
>> > Modified:
>> >     llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> >     llvm/test/CodeGen/X86/vec_fneg.ll
>> >
>> > Removed:
>> >
>> >
>> >
>> > ################################################################################
>> > diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> > index e809816d68be..d2db4bccc3ac 100644
>> > --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> > +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> > @@ -5112,8 +5112,13 @@ SDValue SelectionDAG::foldConstantFPMath(unsigned Opcode, const SDLoc &DL,
>> >    }
>> >
>> >    switch (Opcode) {
>> > -  case ISD::FADD:
>> >    case ISD::FSUB:
>> > +    // -0.0 - undef --> undef (consistent with "fneg undef")
>> > +    if (N1CFP && N1CFP->getValueAPF().isNegZero() && N2.isUndef())
>> > +      return getUNDEF(VT);
>> > +    LLVM_FALLTHROUGH;
>> > +
>> > +  case ISD::FADD:
>> >    case ISD::FMUL:
>> >    case ISD::FDIV:
>> >    case ISD::FREM:
>> >
>> > diff  --git a/llvm/test/CodeGen/X86/vec_fneg.ll b/llvm/test/CodeGen/X86/vec_fneg.ll
>> > index 4d5539feef3c..c3c1932c2311 100644
>> > --- a/llvm/test/CodeGen/X86/vec_fneg.ll
>> > +++ b/llvm/test/CodeGen/X86/vec_fneg.ll
>> > @@ -76,12 +76,10 @@ define <4 x float> @fneg_undef(<4 x float> %Q) nounwind {
>> >  define <4 x float> @fsub_neg0_undef_elts_undef(<4 x float> %x) {
>> >  ; X32-SSE-LABEL: fsub_neg0_undef_elts_undef:
>> >  ; X32-SSE:       # %bb.0:
>> > -; X32-SSE-NEXT:    movaps {{.*#+}} xmm0 = <NaN,u,u,NaN>
>> >  ; X32-SSE-NEXT:    retl
>> >  ;
>> >  ; X64-SSE-LABEL: fsub_neg0_undef_elts_undef:
>> >  ; X64-SSE:       # %bb.0:
>> > -; X64-SSE-NEXT:    movaps {{.*#+}} xmm0 = <NaN,u,u,NaN>
>> >  ; X64-SSE-NEXT:    retq
>> >    %r = fsub <4 x float> <float -0.0, float undef, float undef, float -0.0>, undef
>> >    ret <4 x float> %r
>> >
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits