llvm dev - Feb 2018 - how to simplify FP ops with an undef operand?

%y = fadd float %x, undef

Can we simplify this?

Currently in IR, we do nothing for fadd/fsub/fmul. For fdiv/frem, we
propagate undef. The code comment for fdiv/frem says:
"the undef could be a snan"

If that's correct, then shouldn't it be the same for fadd/fsub/fmul? But
this can't be correct because we support targets that don't raise
exceptions...and even targets that raise exceptions do not trap by default
on snan?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180227/a5c4d756/attachment.html>

I’m not sure the transformation happening with fdiv is correct. If we have “%y =
fdiv float %x, undef” and %x is a NaN then the result will be NaN for any value
of the undef, right? So if I understand the undef rules correctly (never a
certainty) then we can’t safely replace the expression with undef. We could, I
think, replace it with “%y = %x” though. I think the same is true for fadd,
fsub, fmul, and frem.

-Andy

From: Sanjay Patel [mailto:spatel at rotateright.com]
Sent: Tuesday, February 27, 2018 6:29 PM
To: llvm-dev <llvm-dev at lists.llvm.org>
Cc: Nuno Lopes <nunoplopes at sapo.pt>; Stephen Canon <scanon at
apple.com>; David Majnemer <david.majnemer at gmail.com>; John Regehr
<regehr at cs.utah.edu>; Kaylor, Andrew <andrew.kaylor at
intel.com>; Sanjoy Das <sanjoy at playingwithpointers.com>; Friedman,
Eli <efriedma at codeaurora.org>; Matt Arsenault <arsenm2 at
gmail.com>
Subject: how to simplify FP ops with an undef operand?

%y = fadd float %x, undef

Can we simplify this?

Currently in IR, we do nothing for fadd/fsub/fmul. For fdiv/frem, we propagate
undef. The code comment for fdiv/frem says:
"the undef could be a snan"

If that's correct, then shouldn't it be the same for fadd/fsub/fmul? But
this can't be correct because we support targets that don't raise
exceptions...and even targets that raise exceptions do not trap by default on
snan?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180228/0c5a43b1/attachment-0001.html>

Yes, if %x is a NaN, we should expect that NaN is propagated.

I'm still not sure what to do here. We can take comfort in knowing that
whatever we do is likely an improvement over the current situation though.
:)
That's because the code in InstSimplify is inconsistent with the LangRef:
http://llvm.org/docs/LangRef.html#undefined-values (UB for fdiv by 0?)
...and both of those are inconsistent with undef handling in SDAG.

Let me propose an alternate interpretation:

1. The meaning of snan as written in IEEE754-2008 is: "Signaling NaNs
afford representations for uninitialized variables..."
2. That matches our intent with 'undef' here in IR as written in the
LangRef: "unspecified bit-pattern".
3. The current fdiv transform is actually correct (any SNaN UB/trapping
commentary is irrelevant because we assume exceptions are off by default).

The undef operand represents an uninitialized variable, and the result of
any FP op with that uninitialized variable is well-defined: it's another
NaN which is just 'undef' in IR.





On Wed, Feb 28, 2018 at 11:43 AM, Kaylor, Andrew <andrew.kaylor at
intel.com>
wrote:
> I’m not sure the transformation happening with fdiv is correct. If we have
> “%y = fdiv float %x, undef” and %x is a NaN then the result will be NaN for
> any value of the undef, right? So if I understand the undef rules correctly
> (never a certainty) then we can’t safely replace the expression with undef.
> We could, I think, replace it with “%y = %x” though. I think the same is
> true for fadd, fsub, fmul, and frem.
>
>
>
> -Andy
>
>
>
>
> %y = fadd float %x, undef
>
>
>
> Can we simplify this?
>
> Currently in IR, we do nothing for fadd/fsub/fmul. For fdiv/frem, we
> propagate undef. The code comment for fdiv/frem says:
> "the undef could be a snan"
>
>
>
> If that's correct, then shouldn't it be the same for
fadd/fsub/fmul? But
> this can't be correct because we support targets that don't raise
> exceptions...and even targets that raise exceptions do not trap by default
> on snan?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180228/d373d40e/attachment.html>

On 02/27/2018 06:29 PM, Sanjay Patel via llvm-dev wrote:
> %y = fadd float %x, undef
>
> Can we simplify this?
>
> Currently in IR, we do nothing for fadd/fsub/fmul. For fdiv/frem, we 
> propagate undef. The code comment for fdiv/frem says:
> "the undef could be a snan"
>
> If that's correct, then shouldn't it be the same for
fadd/fsub/fmul?
> But this can't be correct because we support targets that don't
raise
> exceptions...and even targets that raise exceptions do not trap by 
> default on snan?
Fair warning, the following is a tangent from the actual topic under 
discussion...  It's only vaguely connected to the example at hand and is 
mostly a reaction to a collection of comments in various previous 
floating point discussion threads.

The reasoning here worries me.  The semantics of LLVM IR is specified 
independently of what any particular target does. Referring to target 
behavior to decide on appropriate semantics for a particular 
optimization seems inherently problematic. We should instead use LLVM 
IR's stated semantics to decide legality.  We should only use target 
semantics to help shape proposals to change the IR semantics.

To be specific, something along the lines of the following seems 
entirely reasonable: "LLVM IR allows us to perform the following 
optimization here, but doing that would radically complicate the 
lowering on target X which doesn't support Y.  Should we change the IR 
specification to Z?"  On the other hand, reasoning like "Target X 
doesn't do Y, so this optimization can't be legal" are problematic.

If the code is more specific than the LangRef, we should propose a 
clarification to the LangRef.  The specification for floating point 
semantics is admittedly a weakness in the current version.  Rather than 
working around it, we should fix that.

End vaguely related rant, thanks for reading!

Philip