[PATCH] D102672: [ConstantFolding] Use APFloat for constant folding. NFC

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added inline comments.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2146
     case LibFunc_logf_finite:
-      if (V > 0.0 && TLI->has(Func))
-        return ConstantFoldFP(log, V, Ty);
+      if (!APF.isNegative() && !APF.isZero() && TLI->has(Func))
+        return ConstantFoldFP(log, APF, Ty);
----------------
efriedma wrote:
> sepavloff wrote:
> > efriedma wrote:
> > > sepavloff wrote:
> > > > efriedma wrote:
> > > > > This isn't precisely equivalent to the existing code.
> > > > Could you please point out the case when this code behaves differently from the existing one?
> > > This handles NaNs differently.
> > Strictly speaking you are right. Comparison in C is ordered, so `V > 0.0` gives `false for NaN and `ConstantFoldFP` is not called. However all these functions (`log*` and `sqrt*`) have specified behavior when their argument is NaN, so the final result must be the same. Other similar functions like `sin` are called for NaN arguments.
> > 
> If you want to change this, please add test coverage.
Things are even simpler. The code above contains a check:
```
    if (!U.isFinite())
      return nullptr;
```
So NaNs cannot reach this code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102672