[PATCH] D116441: [AMDGPU][GlobalISel] Select op_sel modifiers for VOP3P

[Mirko Brkusanin via Phabricator via llvm-commits]

mbrkusanin added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3441-3448
+    else if (mi_match(Src, MRI,
+                      m_GOr(m_GAnd(m_Reg(LoSrc), m_SpecificICst(0xffff)),
+                            m_GShl(m_Reg(HiSrc), m_SpecificICst(16))))) {
+      bool NegLo =
+          mi_match(LoSrc, MRI, m_GAnyExt(m_GFNeg(m_GTrunc(m_Reg(LoSrc)))));
+      bool LoIsShifted = isShiftHiToLo(MRI, LoSrc, LoSrc);
+      LoSrc = stripBitcast(MRI, LoSrc);
----------------
arsenm wrote:
> This matching is a bit heavy. Seems like we're missing some combines?
> 
> This at least could use some comments for the pattern being matched
The Or+Shl+And come from RegBankSelect when it breaks down G_BUILD_VECTOR(_TRUNC) instructions that form <2 x 16> operands.
A combine could help but only in cases where those instructions would be redundant and no op_sel modifiers would need to be used.

There are other cases where such combine could help but it's a separate issue.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3458-3459
+      } else {
+        bool NegHi =
+            mi_match(HiSrc, MRI, m_GAnyExt(m_GFNeg(m_GTrunc(m_Reg(HiSrc)))));
+        bool HiIsShifted = isShiftHiToLo(MRI, HiSrc, HiSrc);
----------------
arsenm wrote:
> Why would this pattern appear?
Added comment to clarify. Register is s32 but fneg that we want to match is for s16.


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

https://reviews.llvm.org/D116441