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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:5049-5050
 
-  if (Constant *C = simplifyFPOp({Op0, Op1}, FMF, Q))
-    return C;
+  if (!IsDefaultFPEnvironment(ExBehavior, Rounding))
+    return nullptr;
 
----------------
kpn wrote:
> spatel wrote:
> > This and similar changes are opening the door to every existing FP fold. That seems safe enough given that we're in the default environment, but there are no tests for this unless I've missed it? 
> > 
> > Can we make that a small follow-on patch with some test coverage that includes FMF, so we know it's all getting piped through as expected.
> You want some tests to verify that the below folds work correctly when given constrained intrinsics with FMF? I think that's what you are asking. Since we have no tests for that currently, yes, we can do that. Unless I misunderstood?
Yes, I'd add at least 1 test for each intrinsic, and sprinkle some FMF-required folds for more better coverage. For example, I think we want these to fold?

```
define float @fdiv_by 1(float %x) {
  %r = call float @llvm.experimental.constrained.fdiv.f32(float %x, float 1.0, metadata !"round.tonearest", metadata !"fpexcept.ignore")
  ret float %r
}

define float @fdiv_same_op_nnan(float %x) {
  %r = call nnan float @llvm.experimental.constrained.fdiv.f32(float %x, float %x, metadata !"round.tonearest", metadata !"fpexcept.ignore")
  ret float %r
}

```


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

https://reviews.llvm.org/D103169