Page MenuHomePhabricator

[InstCombine] Preserve FMA opportunities when combining PHI nodes
Needs RevisionPublic

Authored by nemanjai on Jul 22 2020, 4:52 AM.

Details

Reviewers
majnemer
spatel
efriedma
craig.topper
lebedev.ri
Group Reviewers
Restricted Project
Summary

We currently unconditionally combine multiple FAdd instructions that feed a PHI into one. However, if at least some of those are fed by FMul's and the instructions allow contraction, this takes away opportunities to produce FMA's.

This patch just adds a check for this situation and prevents combining if such a situation is encountered.

Diff Detail

Event Timeline

nemanjai created this revision.Jul 22 2020, 4:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2020, 4:52 AM

Avoiding the canonicalization seems unlikely to be a universal win and won't solve the problem if the source was already in the form that we want to avoid.
I think we need to reverse the transform in CodeGenPrepare. There we can use TTI/TLI to decide if it is profitable (at the least, the target should have custom/legal FMA lowering for the value type in question?).

Avoiding the canonicalization seems unlikely to be a universal win and won't solve the problem if the source was already in the form that we want to avoid.
I think we need to reverse the transform in CodeGenPrepare. There we can use TTI/TLI to decide if it is profitable (at the least, the target should have custom/legal FMA lowering for the value type in question?).

+1, this direction does not seem like the right one for instcombine.

Sounds like agreement. I'll move this to CGP. Thanks for the quick response.

lebedev.ri requested changes to this revision.Jul 24 2020, 1:52 PM

(as per the disscussion)

This revision now requires changes to proceed.Jul 24 2020, 1:52 PM