AMDGPU/SI: Implement sendmsghalt intrinsic

[arsenm]

New intrinsics should go in include/llvm/IR/IntrinsicsAMDGPU.td, and have an amdgcn prefix. I also think it probably should not have a separate parameter for m0, and instead rely on llvm.write_register setting m0

[jvesely]

In D23511#515604, @arsenm wrote:

New intrinsics should go in include/llvm/IR/IntrinsicsAMDGPU.td, and have an amdgcn prefix. I also think it probably should not have a separate parameter for m0, and instead rely on llvm.write_register setting m0

I'd like it to keep it as close to sendmsg as possible. Do you know if there are compatibility issues if I rename SI_sendmsg to amdgcn_sendmsg?

[arsenm]

In D23511#515722, @jvesely wrote:

In D23511#515604, @arsenm wrote:

New intrinsics should go in include/llvm/IR/IntrinsicsAMDGPU.td, and have an amdgcn prefix. I also think it probably should not have a separate parameter for m0, and instead rely on llvm.write_register setting m0

I'd like it to keep it as close to sendmsg as possible. Do you know if there are compatibility issues if I rename SI_sendmsg to amdgcn_sendmsg?

SI.sendmsg also needs to be changed and replaced (as well as the rest of the intrinsics in the backend). The goal is to eventually fix any intrinsic design issues and fix the names when moving them to the public intrinsics

[jvesely]

rename and expose both sendmsg intrinsics

[jvesely]

In D23511#515739, @arsenm wrote:

In D23511#515722, @jvesely wrote:

In D23511#515604, @arsenm wrote:

New intrinsics should go in include/llvm/IR/IntrinsicsAMDGPU.td, and have an amdgcn prefix. I also think it probably should not have a separate parameter for m0, and instead rely on llvm.write_register setting m0

I'd like it to keep it as close to sendmsg as possible. Do you know if there are compatibility issues if I rename SI_sendmsg to amdgcn_sendmsg?

SI.sendmsg also needs to be changed and replaced (as well as the rest of the intrinsics in the backend). The goal is to eventually fix any intrinsic design issues and fix the names when moving them to the public intrinsics

I understand that, although having a list of approaches considered deprecated would reduce some wasted effort.
My question was whether there are users of the old sendmsg intrinsic name that would break.

I'm not sure how to unbundle writing m0 if we want to expose sendmsg as __builtin.function, is there a generic way to write to m0 from high level language?

[arsenm]

In D23511#516775, @jvesely wrote:

In D23511#515739, @arsenm wrote:

In D23511#515722, @jvesely wrote:

In D23511#515604, @arsenm wrote:

New intrinsics should go in include/llvm/IR/IntrinsicsAMDGPU.td, and have an amdgcn prefix. I also think it probably should not have a separate parameter for m0, and instead rely on llvm.write_register setting m0

I'd like it to keep it as close to sendmsg as possible. Do you know if there are compatibility issues if I rename SI_sendmsg to amdgcn_sendmsg?

SI.sendmsg also needs to be changed and replaced (as well as the rest of the intrinsics in the backend). The goal is to eventually fix any intrinsic design issues and fix the names when moving them to the public intrinsics

I understand that, although having a list of approaches considered deprecated would reduce some wasted effort.
My question was whether there are users of the old sendmsg intrinsic name that would break.

I'm not sure how to unbundle writing m0 if we want to expose sendmsg as __builtin.function, is there a generic way to write to m0 from high level language?

We can keep the old intrinsic working while adding the new one. A builtin would be needed for emitting the write to m0 since read/write_register are not directly exposed. My concern about this is what happens if you have something like:

llvm.write_register(m0)
%foo = load i32, i32 addrspace(3)*
llvm.amdgcn.s.sendmsg()

The lowering of the LDS access will insert initialization of m0 to -1, clobbering the old value. I'm not sure if it's better to either switch the M0 initialization lowering to copy the pre-existing value and restore after. I'm also not sure if we should keep considering m0 as an allocatable register

[jvesely]

In D23511#517122, @arsenm wrote:

We can keep the old intrinsic working while adding the new one.

If we don't know of any users now, we probably won't know more in the future. I don't mind updating mine.
Let me know your preference, I have an alternative patch that keeps the old name ready.

A builtin would be needed for emitting the write to m0 since read/write_register are not directly exposed. My concern about this is what happens if you have something like:

llvm.write_register(m0)
%foo = load i32, i32 addrspace(3)*
llvm.amdgcn.s.sendmsg()

The lowering of the LDS access will insert initialization of m0 to -1, clobbering the old value. I'm not sure if it's better to either switch the M0 initialization lowering to copy the pre-existing value and restore after. I'm also not sure if we should keep considering m0 as an allocatable register

shouldn't register allocation (picking the same phys m0) and instruction scheduling figure out the hazard and either schedule the instructions correctly or spill?

anyway, It looks like proper handling of m0 would need a separate patch. I'd prefer to leave that for another time. This change just adds a halting copy of sendmsg, so both can be modified at the same time if necessary.

[jvesely]

keep the old intrinsic around

[arsenm]

include/llvm/IR/IntrinsicsAMDGPU.td

107–110 ↗

(On Diff #71473)

These should have a comments explaining the arguments, at least mentioning that the implicit m0 argument is the last one

test/CodeGen/AMDGPU/amdgcn.sendmsg.ll

23 ↗

(On Diff #71473)

better name would be test_sendmsg or something

31–35 ↗

(On Diff #71473)

I would split each of these sets into its own function

[jvesely]

add comment
improve test

[arsenm]

LGTM

test/CodeGen/AMDGPU/amdgcn.sendmsg.ll

1–2 ↗

(On Diff #82416)

Can you change these to use GCN as the check prefix

lib/Target/AMDGPU/AMDGPUISelLowering.h

enum NodeType : unsigned {
   /// T0|v.x| | | |
   /// T1|v.y| | | |
   /// T2|v.z| | | |
   /// T3|v.w| | | |
   BUILD_VERTICAL_VECTOR,
   /// Pointer to the start of the shader's constant data.
   CONST_DATA_PTR,
   SENDMSG,
+  SENDMSGHALT,
   INTERP_MOV,
   INTERP_P1,
   INTERP_P2,
   PC_ADD_REL_OFFSET,
   KILL,
   FIRST_MEM_OPCODE_NUMBER = ISD::FIRST_TARGET_MEMORY_OPCODE,
   STORE_MSKOR,
   LOAD_CONSTANT,

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

const char* AMDGPUTargetLowering::getTargetNodeName(unsigned Opcode) const {
   NODE_NAME_CASE(CVT_F32_UBYTE2)
   NODE_NAME_CASE(CVT_F32_UBYTE3)
   NODE_NAME_CASE(BUILD_VERTICAL_VECTOR)
   NODE_NAME_CASE(CONST_DATA_PTR)
   NODE_NAME_CASE(PC_ADD_REL_OFFSET)
   NODE_NAME_CASE(KILL)
   case AMDGPUISD::FIRST_MEM_OPCODE_NUMBER: break;
   NODE_NAME_CASE(SENDMSG)
+  NODE_NAME_CASE(SENDMSGHALT)
   NODE_NAME_CASE(INTERP_MOV)
   NODE_NAME_CASE(INTERP_P1)
   NODE_NAME_CASE(INTERP_P2)
   NODE_NAME_CASE(STORE_MSKOR)
   NODE_NAME_CASE(LOAD_CONSTANT)
   NODE_NAME_CASE(TBUFFER_STORE_FORMAT)
   NODE_NAME_CASE(ATOMIC_CMP_SWAP)
   NODE_NAME_CASE(ATOMIC_INC)

lib/Target/AMDGPU/AMDGPUInstrInfo.td

 >;
 
 def AMDGPUfmed3 : SDNode<"AMDGPUISD::FMED3", SDTFPTernaryOp, []>;
 
 def AMDGPUsendmsg : SDNode<"AMDGPUISD::SENDMSG",
                     SDTypeProfile<0, 1, [SDTCisInt<0>]>,
                     [SDNPHasChain, SDNPInGlue]>;
 
+def AMDGPUsendmsghalt : SDNode<"AMDGPUISD::SENDMSGHALT",
+                    SDTypeProfile<0, 1, [SDTCisInt<0>]>,
+                    [SDNPHasChain, SDNPInGlue]>;
+
 def AMDGPUinterp_mov : SDNode<"AMDGPUISD::INTERP_MOV",
                         SDTypeProfile<1, 3, [SDTCisFP<0>]>,
                         [SDNPInGlue]>;
 
 def AMDGPUinterp_p1 : SDNode<"AMDGPUISD::INTERP_P1",
                       SDTypeProfile<1, 3, [SDTCisFP<0>]>,
                       [SDNPInGlue, SDNPOutGlue]>;
 

lib/Target/AMDGPU/SIISelLowering.cpp

SDValue SITargetLowering::LowerINTRINSIC_VOID(SDValue Op,
 
   switch (IntrinsicID) {
   case AMDGPUIntrinsic::SI_sendmsg: {
     Chain = copyToM0(DAG, Chain, DL, Op.getOperand(3));
     SDValue Glue = Chain.getValue(1);
     return DAG.getNode(AMDGPUISD::SENDMSG, DL, MVT::Other, Chain,
                        Op.getOperand(2), Glue);
   }
+  case AMDGPUIntrinsic::SI_sendmsghalt: {
+    Chain = copyToM0(DAG, Chain, DL, Op.getOperand(3));
+    SDValue Glue = Chain.getValue(1);
+    return DAG.getNode(AMDGPUISD::SENDMSGHALT, DL, MVT::Other, Chain,
+                       Op.getOperand(2), Glue);
+  }
   case AMDGPUIntrinsic::SI_tbuffer_store: {
     SDValue Ops[] = {
       Chain,
       Op.getOperand(2),
       Op.getOperand(3),
       Op.getOperand(4),
       Op.getOperand(5),
       Op.getOperand(6),

lib/Target/AMDGPU/SIInsertWaits.cpp

 }
 
 void SIInsertWaits::handleSendMsg(MachineBasicBlock &MBB,
                                   MachineBasicBlock::iterator I) {
   if (ST->getGeneration() < SISubtarget::VOLCANIC_ISLANDS)
     return;
 
   // There must be "S_NOP 0" between an instruction writing M0 and S_SENDMSG.
-  if (LastInstWritesM0 && I->getOpcode() == AMDGPU::S_SENDMSG) {
+  if (LastInstWritesM0 && (I->getOpcode() == AMDGPU::S_SENDMSG || I->getOpcode() == AMDGPU::S_SENDMSGHALT)) {
     BuildMI(MBB, I, DebugLoc(), TII->get(AMDGPU::S_NOP)).addImm(0);
     LastInstWritesM0 = false;
     return;
   }
 
   // Set whether this instruction sets M0
   LastInstWritesM0 = false;
 
for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end();
       Counters Required;
 
       // Wait for everything before a barrier.
       //
       // S_SENDMSG implicitly waits for all outstanding LGKM transfers to finish,
       // but we also want to wait for any other outstanding transfers before
       // signalling other hardware blocks
       if (I->getOpcode() == AMDGPU::S_BARRIER ||
-          I->getOpcode() == AMDGPU::S_SENDMSG)
+          I->getOpcode() == AMDGPU::S_SENDMSG ||
+          I->getOpcode() == AMDGPU::S_SENDMSGHALT)
         Required = LastIssued;
       else
         Required = handleOperands(*I);
 
       Counters Increment = getHwCounts(*I);
 
       if (countersNonZero(Required) || countersNonZero(Increment))
         increaseCounters(Required, DelayedWaitOn);

lib/Target/AMDGPU/SIInstructions.td

 
 def S_SETPRIO : SOPP <0x0000000f, (ins i16imm:$simm16), "s_setprio $simm16">;
 
 let Uses = [EXEC, M0] in {
   // FIXME: Should this be mayLoad+mayStore?
   def S_SENDMSG : SOPP <0x00000010, (ins SendMsgImm:$simm16), "s_sendmsg $simm16",
       [(AMDGPUsendmsg (i32 imm:$simm16))]
   >;
-} // End Uses = [EXEC, M0]
 
-def S_SENDMSGHALT : SOPP <0x00000011, (ins SendMsgImm:$simm16), "s_sendmsghalt $simm16">;
+  def S_SENDMSGHALT : SOPP <0x00000011, (ins SendMsgImm:$simm16), "s_sendmsghalt $simm16",
+      [(AMDGPUsendmsghalt (i32 imm:$simm16))]
+  >;
+} // End Uses = [EXEC, M0]
 def S_TRAP : SOPP <0x00000012, (ins i16imm:$simm16), "s_trap $simm16">;
 def S_ICACHE_INV : SOPP <0x00000013, (ins), "s_icache_inv"> {
   let simm16 = 0;
 }
 def S_INCPERFLEVEL : SOPP <0x00000014, (ins i16imm:$simm16), "s_incperflevel $simm16">;
 def S_DECPERFLEVEL : SOPP <0x00000015, (ins i16imm:$simm16), "s_decperflevel $simm16">;
 def S_TTRACEDATA : SOPP <0x00000016, (ins), "s_ttracedata"> {
   let simm16 = 0;

lib/Target/AMDGPU/SIIntrinsics.td

def int_SI_buffer_load_dword : Intrinsic <
      llvm_i32_ty,     // offen(imm)
      llvm_i32_ty,     // idxen(imm)
      llvm_i32_ty,     // glc(imm)
      llvm_i32_ty,     // slc(imm)
      llvm_i32_ty],    // tfe(imm)
     [IntrReadMem, IntrArgMemOnly]>;
 
   def int_SI_sendmsg : Intrinsic <[], [llvm_i32_ty, llvm_i32_ty], []>;
+  def int_SI_sendmsghalt : Intrinsic <[], [llvm_i32_ty, llvm_i32_ty], []>;
 
   // Fully-flexible SAMPLE instruction.
   class SampleRaw : Intrinsic <
     [llvm_v4f32_ty],    // vdata(VGPR)
     [llvm_anyint_ty,    // vaddr(VGPR)
      llvm_v8i32_ty,     // rsrc(SGPR)
      llvm_v4i32_ty,     // sampler(SGPR)
      llvm_i32_ty,       // dmask(imm)

test/CodeGen/AMDGPU/llvm.SI.sendmsg-m0.ll

 ; RUN: llc -march=amdgcn -mcpu=verde -verify-machineinstrs < %s | FileCheck -check-prefix=SI -check-prefix=GCN %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck -check-prefix=VI -check-prefix=GCN %s
 
 ; GCN-LABEL: {{^}}main:
 ; GCN: s_mov_b32 m0, s0
 ; VI-NEXT: s_nop 0
 ; GCN-NEXT: sendmsg(MSG_GS_DONE, GS_OP_NOP)
 ; GCN-NEXT: s_endpgm
 
 define amdgpu_gs void @main(i32 inreg %a) #0 {
   call void @llvm.SI.sendmsg(i32 3, i32 %a)
   ret void
 }
 
+; GCN-LABEL: {{^}}main_halt:
+; GCN: s_mov_b32 m0, s0
+; VI-NEXT: s_nop 0
+; GCN-NEXT: s_sendmsghalt sendmsg(MSG_INTERRUPT)
+; GCN-NEXT: s_endpgm
+
+define  void @main_halt(i32 inreg %a) #0 {
+  call void @llvm.SI.sendmsghalt(i32 1, i32 %a)
+  ret void
+}
+
 declare void @llvm.SI.sendmsg(i32, i32) #0
+declare void @llvm.SI.sendmsghalt(i32, i32) #0
 
 attributes #0 = { nounwind }

test/CodeGen/AMDGPU/llvm.SI.sendmsg.ll

 ;RUN: llc < %s -march=amdgcn -mcpu=verde -verify-machineinstrs | FileCheck %s
 ;RUN: llc < %s -march=amdgcn -mcpu=tonga -verify-machineinstrs | FileCheck %s
 
 ; CHECK-LABEL: {{^}}main:
 ; CHECK: s_mov_b32 m0, 0
 ; CHECK-NOT: s_mov_b32 m0
+; CHECK: s_sendmsg sendmsg(MSG_INTERRUPT)
 ; CHECK: s_sendmsg sendmsg(MSG_GS, GS_OP_EMIT, 0)
 ; CHECK: s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 1)
 ; CHECK: s_sendmsg sendmsg(MSG_GS, GS_OP_EMIT_CUT, 2)
 ; CHECK: s_sendmsg sendmsg(MSG_GS_DONE, GS_OP_NOP)
+; CHECK: s_sendmsghalt sendmsg(MSG_INTERRUPT)
+; CHECK: s_sendmsghalt sendmsg(MSG_GS, GS_OP_EMIT, 0)
+; CHECK: s_sendmsghalt sendmsg(MSG_GS, GS_OP_CUT, 1)
+; CHECK: s_sendmsghalt sendmsg(MSG_GS, GS_OP_EMIT_CUT, 2)
+; CHECK: s_sendmsghalt sendmsg(MSG_GS_DONE, GS_OP_NOP)
 
 define void @main() {
 main_body:
+  call void @llvm.SI.sendmsg(i32 1, i32 0);
   call void @llvm.SI.sendmsg(i32 34, i32 0);
   call void @llvm.SI.sendmsg(i32 274, i32 0);
   call void @llvm.SI.sendmsg(i32 562, i32 0);
   call void @llvm.SI.sendmsg(i32 3, i32 0);
+
+  call void @llvm.SI.sendmsghalt(i32 1, i32 0);
+  call void @llvm.SI.sendmsghalt(i32 34, i32 0);
+  call void @llvm.SI.sendmsghalt(i32 274, i32 0);
+  call void @llvm.SI.sendmsghalt(i32 562, i32 0);
+  call void @llvm.SI.sendmsghalt(i32 3, i32 0);
   ret void
 }
 
 ; Function Attrs: nounwind
 declare void @llvm.SI.sendmsg(i32, i32) #0
+declare void @llvm.SI.sendmsghalt(i32, i32) #0
 
 attributes #0 = { nounwind }