[PATCH] D150612: AMDGPU: Expand casted f16 fmed3 pattern to fmin/fmax on gfx8

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp:391-393
+    if (MRI.getType(SrcReg) != LLT::scalar(16))
+      return false;
+    return true;
----------------



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:5949
+
+    // FIXME: This is to workaround the inability of tablegen match combiners to
+    // match intrinsics in patterns.
----------------
nit: also add a complementary comment in the tablegen file (e.g. TODO: match intrinsics, currently we replace the intrinsic in LegalizerInfo to work around it), that way if we add intrinsic matching later, we don't forget to remove this workaround when updating the pattern 


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:11181
+  if (TruncSrc.getOpcode() != AMDGPUISD::FMED3 ||
+      TruncSrc.getValueType() != MVT::f32 || !TruncSrc.hasOneUse() ||
+      !isNullConstant(N->getOperand(1)))
----------------
Here you check for `f32` explicitly, but I think in the GISel combine you don't enforce it, why?


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

https://reviews.llvm.org/D150612