[PATCH] D139128: [AMDGPU] G_IS_FPCLASS lower() support for IEEE fp types

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

LGTM except for some nits



================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:7243
+// size. Only considering the IEEE semantics for now.
+static const fltSemantics &getIEEEfltSemantics(unsigned NrOfBits) {
+  switch (NrOfBits) {
----------------
We already have getFltSemanticForLLT


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:7265
+  uint64_t Mask = MI.getOperand(2).getImm();
+  MIRBuilder.setDebugLoc(MI.getDebugLoc());
+
----------------
I thought this was set already before?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:7341
+          MIRBuilder.buildICmp(CmpInst::Predicate::ICMP_EQ, DstTy, Abs, ZeroC));
+    else // ISD::fcNegZero
+      appendToRes(MIRBuilder.buildICmp(CmpInst::Predicate::ICMP_EQ, DstTy,
----------------
No ISD::


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:7371
+                                       InfWithQnanBitC));
+    } else { // ISD::fcSNan
+      // issignaling(V) ==> abs(V) u> unsigned(Inf) &&
----------------
No ISD::


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:992
+        .widenScalarToNextPow2(1)
+        .scalarize(0);
 
----------------
Ought to have a fallback lower at the end of each too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139128