[PATCH] D93708: [AMDGPU] Add a new Clamp Pattern to the GlobalISel Path.

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla marked 9 inline comments as done.
tsymalla added a comment.

Previously this was implemented in the PostLegalizer. The SMAX / SMIN MIR gets lowered to a pattern of G_SELECT / G_ICMP, so the state after PostLegalizing is equivalent to what I've implemented in the tests before matching this to the min/max instructions. Disabling the optimization in the implementation here yields a result similar to the following:

  %62:_(s1) = G_ICMP intpred(slt), %53:_(s32), %25:_
  %63:_(s64) = G_CONSTANT i64 0
  %28:_(s64) = G_SELECT %62:_(s1), %63:_, %61:_
  %29:_(s64) = G_CONSTANT i64 32767
  %37:_(s1) = G_ICMP intpred(slt), %28:_(s64), %29:_
  %30:_(s64) = G_SELECT %37:_(s1), %28:_, %29:_
  %31:_(s64) = G_CONSTANT i64 -32768
  %36:_(s1) = G_ICMP intpred(sgt), %30:_(s64), %31:_
  %32:_(s64) = G_SELECT %36:_(s1), %30:_, %31:_
  %34:_(s32) = G_TRUNC %32:_(s64)

So this is why I moved the implementation to the PreLegalize step where the G_SMIN / G_SMAX IR still exists:

  %30:_(s64) = G_SMIN %28:_, %29:_
  %32:_(s64) = G_SMAX %30:_, %31:_
  %33:_(s16) = G_TRUNC %32:_(s64)

I don't exactly understand why the min / max instructions get lowered to this pattern so for the moment, the only way to get this producing the code I wish seems to match the pattern in the PreLegalize step. By the way, the goal here is to check explicitly if an long should get clamped to short boundaries (or inbetween).  Instead of producing some comparisons, this should yield these two instructions as I've figured out with Nicolai.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp:134
+
+  constexpr unsigned int CvtOpcode = AMDGPU::V_CVT_PK_I16_I32_e64;
+  assert(MI.getOpcode() != CvtOpcode);
----------------
arsenm wrote:
> It would be better to use a generic operation pre-regbank select rather than forcing register classes here
How would I go for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93708