[PATCH] D135447: [AMDGPU] Add llvm.is.fpclass intrinsic to existing SelectionDAG fp class support and introduce GlobalISel implementation for AMDGPU

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.

LGTM with nits. you have some dead checks and dead code



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstrInfo.td:132
 def AMDGPUfp_to_f16 : SDNode<"AMDGPUISD::FP_TO_FP16" , SDTFPToIntOp>;
 
 def AMDGPUfp_class_impl : SDNode<"AMDGPUISD::FP_CLASS", AMDGPUFPClassOp>;
----------------
Whole file is now whitespace only changes which can be dropped


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:913-922
+static int getV_CMP_CLASSOpcode(unsigned size, bool hasTrue16BitInsts) {
+  switch(size) {
+  default: return -1;
+  case 16:
+    return hasTrue16BitInsts ? AMDGPU::V_CMP_CLASS_F16_t16_e64
+                             : AMDGPU::V_CMP_CLASS_F16_e64;
+  case 32: return AMDGPU::V_CMP_CLASS_F32_e64;
----------------
Dead code


================
Comment at: llvm/test/CodeGen/AMDGPU/llvm.is.fpclass.f16.ll:94-95
+; GFX7GLISEL-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX7GLISEL-NEXT:    v_cvt_f32_f16_e32 v0, v0
+; GFX7GLISEL-NEXT:    v_cmp_class_f32_e64 s[4:5], v0, 3
+; GFX7GLISEL-NEXT:    v_cndmask_b32_e64 v0, 0, 1, s[4:5]
----------------
This is broken for signaling nans. You dropped this from the patch but left these dead checks around 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135447