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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h:266
+    return (L.match(MRI, Op0) && R.match(MRI, Op1)) ||
+           (Commutable && (R.match(MRI, Op0) && L.match(MRI, Op1)));
+  }
----------------
Nit: drop the parens around `(R.match(MRI, Op0) && L.match(MRI, Op1))`.


================
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;
----------------
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?)


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp:124
+
+  const APInt &KO_Imm = getConstantIntVRegVal(K0, MRI)->getValue();
+  const APInt &K1_Imm = getConstantIntVRegVal(K1, MRI)->getValue();
----------------
Nit: make this "K0_Imm" not "KO_Imm".


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

https://reviews.llvm.org/D90050