[PATCH] D136448: [AMDGPU][GISel] Add llvm.amdgcn.icmp selection

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1079
+  if (Size == 16 && !ST.has16BitInsts())
     return -1;
+
----------------
Pierre-vh wrote:
> The test won't pass on, say, hawaii because of lack of 16 bit insts. It'll fail to select.
> I guess it's a separate issue where we should probably legalize the intrinsic (= widen its input to i32) for those archs?
I think failing to select is the correct behaviour - or diagnose it as "unsupported" in the legalizer (we do this in some cases but not consistently). Generally intrinsics like this map one-to-one onto a machine instruction, and are not designed to be supported on subtargets that do not have the instruction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136448