[PATCH] D67446: [ConstProp] allow folding for fma that produces NaN

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2242
+          V.fusedMultiplyAdd(Op2->getValueAPF(), Op3->getValueAPF(),
+                             APFloat::rmNearestTiesToEven);
+          return ConstantFP::get(Ty->getContext(), V);
----------------
reames wrote:
> Does opInvalidOp always imply Nan?  If so, then the name should be updated or at least clarifying comments added to the APFloat header.  If not, then this code may be incorrect.
>From the perspective/usage of fusedMultiplyAdd() (and I think for any of the calls that produces an FP result), APFloat::opInvalidOp implies producing a NaN.

In the general case, APFloat::opInvalidOp does not necessarily imply NaN because we use that status for non-FP APIs. For example, we have this for IEEEFloat::convertToInteger():
"we provide deterministic values in case of an invalid operation exception, namely zero for NaNs and the minimal or maximal value respectively for underflow or overflow."

So I think this code is correct. Add something like the above text to the header comment?


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

https://reviews.llvm.org/D67446