[PATCH] D123693: Transform illegal intrinsics to V_ILLEGAL

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:327
+        if (VT.isFloatingPoint()) {
+          return CurDAG->getConstantFP(0.0, DL, VT);
+        }
----------------
Just replace with undef regardless of the type. You also should have just done this when the node was initially created, no need to use the DAG preprocess


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h:539
 
+  ILLEGAL,
+
----------------
I don't think it's worth introducing a wrapper node for this


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:7701
+  auto VTs = Op.getNode()->getVTList();
+  return DAG.getNode(AMDGPUISD::ILLEGAL, DL, VTs);
 }
----------------
You can select directly to the instruction with getMachineNode, no need for the intermediate AMDGPUISD::ILLEGAL


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:3266-3270
+def V_ILLEGAL : Enc32, InstSI<(outs), (ins), "v_illegal"> {
+  let Inst{31-0} = 0;
+  let FixedSize = 1;
+  let Uses = [EXEC];
+}
----------------
This is more complicated. It's actually defined on gfx10 (and I believe can also be encoded as both 32 and 64-bit). For gfx6-9, all 0s does have an interpretation as an almost valid instruction. It decodes fine but violates the constant bus restriction. I would prefer to define a separate V_ILLEGAL that uses all 1s pre gfx10


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123693