[PATCH] D134354: [AMDGPU][GlobalISel] Support mad/fma_mix selection

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUGISel.td:156
 
+def gi_vop3_mad_mix_mods :
+    GIComplexOperandMatcher<s64, "selectVOP3PMadMixMods">,
----------------
Pierre-vh wrote:
> foad wrote:
> > Pierre-vh wrote:
> > > foad wrote:
> > > > Pierre-vh wrote:
> > > > > foad wrote:
> > > > > > Is this required? I am confused about whether you are now matching VOP3PMadMixMods in TableGen patterns, or only doing it from the C++ code in selectG_FMA.
> > > > > I'm doing it exclusively in C++.
> > > > > This is needed to tell the GlobalISelEmitter that, when it sees "VOP3PMadMixMods" it needs to use the "selectVOP3PMadMixMods" function from AMDGPUInstructionSelector.
> > > > > Without that, it wouldn't import patterns that use it.
> > > > I'm still confused. If we are now importing patterns that use VOP3PMadMixMods, why do you need to do the selection in C++ code?
> > > It's a ComplexPattern
> > > ```
> > > def VOP3PMadMixMods  : ComplexPattern<untyped, 2, "SelectVOP3PMadMixMods">;
> > > ```
> > > Those functions work with DAG nodes and must be rewritten for GISel so the patterns can be imported
> > > https://llvm.org/docs/GlobalISel/InstructionSelect.html#complexpatterns
> > Right, but why do you need to write code in selectG_FMA_FMAD to manually select MAD_MIX/FMA_MIX? Why can't the imported patterns do this automatically?
> Because there are no patterns for MIX, only for MIXHI/MIXLO. MIX selection is still manual in the DAG as well (see SelectFMAD_FMA)
OK, thanks for explaining. Out of curiosity, is there a good reason why mix selection cannot be done with patterns?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3398
 
-  if (MI && MI->getOpcode() == AMDGPU::G_FNEG) {
+  if (MI->getOpcode() == AMDGPU::G_FNEG) {
     Src = MI->getOperand(1).getReg();
----------------
I've also done this as part of D136238.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134354