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

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added inline comments.


================
Comment at: llvm/test/Transforms/InstSimplify/fast-math-strictfp.ll:65
+;
+  %negx = call float @llvm.experimental.constrained.fsub.f32(float -0.0, float %x, metadata !"round.tonearest", metadata !"fpexcept.ignore") #0
+  %r = call nnan float @llvm.experimental.constrained.fadd.f32(float %negx, float %x, metadata !"round.tonearest", metadata !"fpexcept.ignore") #0
----------------
spatel wrote:
> Not part of this patch, but is the plan to convert the constrained op(s) into regular ops in this situation?
No. Mixing of regular FP ops and constrained ops in a single function is not allowed since we can't prevent harmful reordering. So the only case where it might be legal is where an entire function uses the constrained intrinsics in the default FP environment. In that case ... why are the constrained intrinsics being used, anyway?


================
Comment at: llvm/test/Transforms/InstSimplify/fp-undef-poison-strictfp.ll:74
 ; CHECK-LABEL: @fadd_poison_op0_defaultfp(
-; CHECK-NEXT:    [[R:%.*]] = call float @llvm.experimental.constrained.fadd.f32(float poison, float [[X:%.*]], metadata !"round.tonearest", metadata !"fpexcept.ignore") #[[ATTR0]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    ret float 0x7FF8000000000000
 ;
----------------
spatel wrote:
> There was a bug in poison propagation -- hopefully fixed with 4ec7c021970d -- but that doesn't explain why this and similar tests below are not returning poison. Update/regenerate the CHECK lines?
Will do, tomorrow.


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

https://reviews.llvm.org/D103169