[PATCH] D90050: AMDGPU/GlobalISel: Add integer med3 combines

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Looks pretty good, just minor comments inline.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp:97
+        MI, MRI,
+        m_CommutativeBinOp(
+            m_CommutativeBinOp(MMMOpc.Max, m_Reg(Val), m_Cst(K0)), m_Cst(K1)));
----------------
I don't think you really need the form of m_CommutativeBinOp that does not check the opcode. This whole function could be written as:
```
return mi_match(MI, MRI, m_any_of(m_CommutativeBinOp(Max, m_CommutativeBinOp(Min, Val, K1), K0)),
                                  m_CommutativeBinOp(Min, m_CommutativeBinOp(Max, Val, K0), K1)));
```


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp:115
+  LLT Ty = MRI.getType(Dst);
+  if (Ty != LLT::scalar(16) && Ty != LLT::scalar(32))
+    return false;
----------------
foad wrote:
> Can we just assert the type is one of these, since we're running on legalized GMIR and these are the only types that we have integer min/max instructions for? (Or do we need to exclude v2s16?)
Ping? Maybe just `if (Ty.isVector()) return false;`?


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

https://reviews.llvm.org/D90050