[PATCH] Refactor and enhance FMA combine

[Mehdi Amini]

> On Apr 2, 2015, at 12:55 PM, Fiona Glaser <fglaser at apple.com> wrote:
> 
> 
>> On Apr 2, 2015, at 12:41 PM, Olivier Sallenave <ohsallen at us.ibm.com> wrote:
>> 
>> Hi Mehdi,
>> 
>>> I’d rather see the duplicated code (the one made obsolete by a correct canonicalization) removed from your patch (i.e. do not build technical debt), and a separate commits that implement the canonicalization part.
>> 
>> 
>> Makes perfect sense, thanks for your feedback. I was able to do the canonicalization you suggest by adding the following transforms:
>> 
>> (fsub (fneg A), B) -> (fneg (fadd A, B))
>> (fpext (fneg x)) -> (fneg (fpext x)
> 
> This second transform makes me nervous; I recently implemented the exact opposite transform in an out-of-tree backend, specifically fold (fneg (fpext x)) to (fpext (fneg x)) if the first fneg cannot be folded down as a modifier and the fpext hasOneUse. This matters if the user of the fneg cannot fold in the fneg, but can fold in the fpext, while the subject of the fpext can fold in a negate. I suspect other GPU backends like R600 won’t like it either.

I’m not sure backend specific transform can rely on the generic LLVM combiner not to canonicalize in a way that they “don’t like”. I think they have to handle the canonical form whatever it is. 
I suspect that many canonicalizations in general can go in the wrong direction in specific cases for specific targets.

— 
Mehdi