[llvm] r371518 - [InstCombine] Use SimplifyFMulInst to simplify multiply in fma.

[Philip Reames via llvm-commits]

On 9/10/19 1:29 PM, Florian Hahn wrote:
> On Tue, Sep 10, 2019 at 8:39 PM Philip Reames <listmail at philipreames.com> wrote:
>> Wait, is this correct?  I thought that FMA had different rounding
>> behavior than fadd/fmul.  Is it legal to take the simplified mul and
>> simple feed it as an operand to a fadd?
>>
> Yes, I think if SimplifyFMulInst would return a new `simplified` fmul
> instruction, than the transform as it is would introduce additional
> rounding error. For example, a few lines above, we have the following
> transform: fma fabs(x), fabs(x), z -> fma x, x, z. Here it would be
> wrong to replace that with fadd z, fmul x, x.
>
> But SimplifyFMul won't create any new instructions as far as I am
> aware, it will only return an existing value if it could simplify the
> fmul to it. In all those cases, we skip the multiply as part of the
> FMA and there is no extra rounding error introduced. And I think
> SimplifyFMulInst should only return a value if we can use it to
> replace the FMul.
>
> Does this make sense?

Not sure, what about the following cornercase:

fma z, x, y where x and y are constants chose such that the result of 
fmul x, y is rounded, but the interior mul in the fma is not?  
SimplifyFMul should invoke the constant folder, which would round the 
result *appropriately for the fmul* (not the fma). Isn't that a bug?



>
> Cheers,
> Florian