[PATCH] D73939: [AMDGPU] Fix infinite loop with fma combines

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp:742
+  switch (Op.getOpcode()) {
+    case ISD::FMA: {
+      // Negating a fma is not free if it has users without source mods.
----------------
kerbowa wrote:
> arsenm wrote:
> > kerbowa wrote:
> > > arsenm wrote:
> > > > This is incomplete and FMAD will have the same problem at minimum
> > > The hang at least doesn't effect FMAD. The combine that triggers this wont happen.
> > Where is the combine? It's also suspicious if an fneg combine triggered for FMA but not FMAD. 
> It's a loop so there are combines in two directions.
> 
> The FNeg combine -fma(a, b, c) => fma(a, -b, -c), triggers for FMA/FMAD and is correct.
> 
> AFAICS the unprofitable combine, fma(a, -b, -c) => -fma(a, b, c), only triggers for FMA.
> https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L12652
This should be just as valid for fmad, but we probably don't want it. This should handle fmad as well.

I am somewhat concerned about splitting much of the same logic between isNegatibleForFree and the target specific fneg combines. Is there now more overlap in what happens in the AMDGPU performFNegCombine and DAGCombiner than there used to be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73939