[PATCH] D103169: [FPEnv][InstSimplify] Constrained FP support for NaN

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/Transforms/InstSimplify/fp-undef-poison-strictfp.ll:45
+; CHECK-LABEL: @fadd_poison_op0_strict(
+; CHECK-NEXT:    [[R:%.*]] = call float @llvm.experimental.constrained.fadd.f32(float poison, float [[X:%.*]], metadata !"round.dynamic", metadata !"fpexcept.strict") #[[ATTR0]]
+; CHECK-NEXT:    ret float [[R]]
----------------
kpn wrote:
> sepavloff wrote:
> > According to documentation (https://llvm.org/docs/LangRef.html#poison-values) "An instruction that depends on a poison value, produces a poison value itself". So making call here is incorrect, it would result in lost of poison.
> Yes, but the very next sentence is "A poison value may be relaxed into an undef value, ...".
> 
> Using the regular instructions we have this difference:
> add x, poison -> poison
> fadd x, poison -> NaN
> 
> I don't know exactly why, but that's the existing practice. The constrained intrinsics should be the same as the regular FP instructions.
Sorry for not seeing this and possibly other inline comments sooner.

The lack of poison propagation here is a TODO item (inconsistency bug), so let's try to get this out of the way before it gets more complicated. I'll post a patch for review shortly.

The history is that poison did not exist when I added the fold for undef. Undef behavior is more complicated than poison, (and that's why we're phasing it out).

We can't do "fadd x, undef --> undef" because if we deduce something about the variable input x, the result may not take on *any* possible FP value. This is similar to why "and x, undef --> 0" (not undef).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103169/new/

https://reviews.llvm.org/D103169