[PATCH] D110937: [AMDGPU][GlobalISel] Select v_fma_mix_f32 and v_mad_mix_f32

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h:595
 
+// Helper for ignoring a copy when matching
+template <typename SubPatternT> struct StripCopy_match {
----------------
mbrkusanin wrote:
> foad wrote:
> > mbrkusanin wrote:
> > > Does this need to be a separate patch with a unit test?
> > Does this match exactly one copy? Would it be more useful to skip zero or more copies?
> m_Copy matches exactly one copy. This matches zero or one copy. Renamed to m_IgnoreCopy, StripCopy was a little misleading.
> 
> It could be easily changed to zero or more, but I did not run into a case where that was necessary.
> In this patch we start from G_FMA/FMAD src operand which is vpgr and match until a constant (most likely a sgpr), so sooner or later a copy will appear.
The practical case where you would want to use this is when you know you can ignore cross bank copies, which I think should only ever see 1 copy


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3512
+               m_IgnoreCopy(m_GTrunc(m_GLShr(
+                   m_Reg(LShlSrc), m_IgnoreCopy(m_SpecificICst(16))))))) {
+    Out = LShlSrc;
----------------
Really this copy shouldn't be here, but regbankselect and the post-regbank combiner don't optimally handle constants. For an inline constant we should always have the constant appear directly with the result regbank. Can you add a fix to remove this?


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

https://reviews.llvm.org/D110937