[PATCH] D108619: [DAGCombine] Allow FMA combine with both FMA and FMAD

[Carl Ritson via Phabricator via llvm-commits]

critson marked an inline comment as done.
critson added a comment.

In D108619#2962032 <https://reviews.llvm.org/D108619#2962032>, @foad wrote:

>> will not be merged post legalization as FMAC becomes
>> the preferred fusion opcode
>
> I'm not sure wha you mean about FMAD "becoming the preferred fusion opcode". Is that just an AMDGPU quirk? Or is there something in generic code that prefers FMA before legalization and FMAD after??



  bool HasFMAD = (LegalOperations && TLI.isFMADLegal(DAG, N));
  
  bool HasFMA =
      TLI.isFMAFasterThanFMulAndFAdd(DAG.getMachineFunction(), VT) &&
      (!LegalOperations || TLI.isOperationLegalOrCustom(ISD::FMA, VT));
  
  unsigned PreferredFusedOpcode = HasFMAD ? ISD::FMAD : ISD::FMA;

This is most probably an AMDGPU quirk.
FMAD is only available when LegalOperations is true, i.e. post legalize vector ops.  Where as FMA is always legal if backend sets it to be Legal or Custom.  I don't know the origin of this behaviour; however, the only backend that marks ISD::FMAD legal appears to be AMDGPU (SI and R600). 
So preferred fusion opcode will only be FMAD on AMDGPU.



================
Comment at: llvm/test/CodeGen/AMDGPU/dagcombine-fma-fmad.ll:5
+; GCN-LABEL: {{^}}_amdgpu_ps_main:
+; GCN:     v_fma
+; GCN:     v_fma
----------------
foad wrote:
> Could use `GCN-COUNT-13: v_fma`.
> 
> But maybe it would be better to generate the checks for this file? It's not immediately obvious (to me) why 13 is the optimal number of fmas. There are more than 13 fmuls and fadds and fsubs in the IR.
I'll generate the checks for this file and precommit.
This example was specifically reduced to take the code paths changed by this patch.
With the patch the number of instructions falls, number of FMAs goes from 14 to 13.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108619