[PATCH] D70422: [APFloat] Fix fusedMultiplyAdd when `this` equals to `Addend`

[Ehud Katz via Phabricator via llvm-commits]

ekatz marked 2 inline comments as done.
ekatz added inline comments.


================
Comment at: llvm/lib/Support/APFloat.cpp:989
   bool ignored;
+  IEEEFloat extendedAddend(*semantics, 0);
 
----------------
foad wrote:
> Marginally more efficient to omit the `, 0` here, to use the one-argument constructor.
> 
> Since we are unconditionally constructing the addend here anyway, would it make more sense to force all callers to pass in an addend (using zero if they're not interested in it)?
Correct. I'll change the constructor to the single parameter version.

Regarding the construction of the addend, that is a good idea.
I suggest making addend passed by value, with default to zero (keep in mind that this function is private, so there is no API change).
What do you think?


================
Comment at: llvm/lib/Support/APFloat.cpp:1031
 
-  if (addend && addend->isNonZero()) {
+  if (extendedAddend.isNonZero()) {
     // The intermediate result of the multiplication has "2 * precision"
----------------
foad wrote:
> Is it safe to skip this code when the addend is zero? If the result of the multiply is also zero, adding zero could affect the sign of the result, couldn't it?
Good observation, but adding/subtracting a negative zero, doesn't affect the sign (it is considered the same as positive zero).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70422