[PATCH] D34582: AMDGPU: Fix emitting encoded calls

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
Herald added subscribers: t-tye, tpr, dstuttard, yaxunl, nhaehnle, wdng.

This was failing on out of bounds access to the extra operands
on the s_swappc_b64 beyond those in the instruction definition.

      

This was working, but somehow regressed within the past few weeks,
although I don't see any obvious commit.


https://reviews.llvm.org/D34582

Files:
  lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
  lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
  test/CodeGen/AMDGPU/call-encoding.ll


Index: test/CodeGen/AMDGPU/call-encoding.ll
===================================================================
--- /dev/null
+++ test/CodeGen/AMDGPU/call-encoding.ll
@@ -0,0 +1,19 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=fiji -amdgpu-function-calls -filetype=obj -verify-machineinstrs < %s | llvm-objdump -triple amdgcn--amdhsa -mcpu=fiji -d - | FileCheck -check-prefixes=GCN,VI %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -amdgpu-function-calls -filetype=obj -verify-machineinstrs < %s | llvm-objdump -triple amdgcn--amdhsa -mcpu=gfx900 -d - | FileCheck -check-prefixes=GCN,GFX9 %s
+; XUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=hawaii -amdgpu-function-calls -filetype=obj -verify-machineinstrs < %s | llvm-objdump -triple amdgcn--amdhsa -mcpu=hawaii -d - | FileCheck -check-prefixes=GCN,CI %s
+
+; GCN: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GCN-NEXT: s_setpc_b64
+define void @void_func_void() #1 {
+  ret void
+}
+
+; GCN: s_getpc_b64
+; GCN: s_swappc_b64
+define amdgpu_kernel void @test_call_void_func_void() {
+  call void @void_func_void()
+  ret void
+}
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind noinline }
Index: lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
===================================================================
--- lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
+++ lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
@@ -278,7 +278,7 @@
     return;
 
   // Check for additional literals in SRC0/1/2 (Op 1/2/3)
-  for (unsigned i = 0, e = MI.getNumOperands(); i < e; ++i) {
+  for (unsigned i = 0, e = Desc.getNumOperands(); i < e; ++i) {
 
     // Check if this operand should be encoded as [SV]Src
     if (!AMDGPU::isSISrcOperand(Desc, i))
Index: lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
===================================================================
--- lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
+++ lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
@@ -129,19 +129,26 @@
 
 void AMDGPUMCInstLower::lower(const MachineInstr *MI, MCInst &OutMI) const {
   unsigned Opcode = MI->getOpcode();
+  const auto *TII = ST.getInstrInfo();
 
   // FIXME: Should be able to handle this with emitPseudoExpansionLowering. We
   // need to select it to the subtarget specific version, and there's no way to
   // do that with a single pseudo source operation.
   if (Opcode == AMDGPU::S_SETPC_B64_return)
     Opcode = AMDGPU::S_SETPC_B64;
   else if (Opcode == AMDGPU::SI_CALL) {
     // SI_CALL is just S_SWAPPC_B64 with an additional operand to track the
-    // called function.
-    Opcode = AMDGPU::S_SWAPPC_B64;
+    // called function (which we need to remove here).
+    OutMI.setOpcode(TII->pseudoToMCOpcode(AMDGPU::S_SWAPPC_B64));
+    MCOperand Dest, Src;
+    lowerOperand(MI->getOperand(0), Dest);
+    lowerOperand(MI->getOperand(1), Src);
+    OutMI.addOperand(Dest);
+    OutMI.addOperand(Src);
+    return;
   }
 
-  int MCOpcode = ST.getInstrInfo()->pseudoToMCOpcode(Opcode);
+  int MCOpcode = TII->pseudoToMCOpcode(Opcode);
   if (MCOpcode == -1) {
     LLVMContext &C = MI->getParent()->getParent()->getFunction()->getContext();
     C.emitError("AMDGPUMCInstLower::lower - Pseudo instruction doesn't have "


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34582.103804.patch
Type: text/x-patch
Size: 3185 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170623/d83e2352/attachment.bin>