[PATCH] D11689: Fix the addition of implicit register operands in incorrect order in SiInstrInfo.

[Alex Lorenz]

arphaman created this revision.
arphaman added reviewers: dexonsmith, tstellarAMD, arsenm.
arphaman added a subscriber: llvm-commits.
arphaman set the repository for this revision to rL LLVM.

This patch fixes a bug in SIInstrInfo where the implicit register operands were added to a machine instruction
in incorrect order - The implicit uses were added before the implicit defs.
I found this bug while trying to enable the implicit register operand verification in machine verifier.

Before:
  V_ADD_I32_e32 killed %13, killed %12, implicit %exec, implicit-def %vcc
After:
  V_ADD_I32_e32 killed %13, killed %12, implicit-def %vcc, implicit %exec

This patch also makes the method 'addImplicitDefUseOperands' in the machine instruction class public so that we can reuse it in SIInstrInfo.

Repository:
  rL LLVM

http://reviews.llvm.org/D11689

Files:
  include/llvm/CodeGen/MachineInstr.h
  lib/Target/AMDGPU/SIInstrInfo.cpp
  test/CodeGen/AMDGPU/si-instr-info-correct-implicit-operands.ll

Index: test/CodeGen/AMDGPU/si-instr-info-correct-implicit-operands.ll
===================================================================
--- /dev/null
+++ test/CodeGen/AMDGPU/si-instr-info-correct-implicit-operands.ll
@@ -0,0 +1,17 @@
+; RUN: llc -o /dev/null %s -march=amdgcn -mcpu=verde -verify-machineinstrs -stop-after expand-isel-pseudos 2>&1 | FileCheck %s
+; This test verifies that the instruction selection will add the implicit
+; register operands in the correct order when modifying the opcode of an
+; instruction to V_ADD_I32_e32.
+
+; CHECK: name: entry
+; CHECK: %19 = V_ADD_I32_e32 killed %13, killed %12, implicit-def %vcc, implicit %exec
+
+define void @test(i32 addrspace(1)* %out, i32 addrspace(1)* %in) {
+entry:
+  %b_ptr = getelementptr i32, i32 addrspace(1)* %in, i32 1
+  %a = load i32, i32 addrspace(1)* %in
+  %b = load i32, i32 addrspace(1)* %b_ptr
+  %result = add i32 %a, %b
+  store i32 %result, i32 addrspace(1)* %out
+  ret void
+}
Index: lib/Target/AMDGPU/SIInstrInfo.cpp
===================================================================
--- lib/Target/AMDGPU/SIInstrInfo.cpp
+++ lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -2595,20 +2595,9 @@
 
 void SIInstrInfo::addDescImplicitUseDef(const MCInstrDesc &NewDesc,
                                         MachineInstr *Inst) const {
+  assert(Inst->getDesc().getOpcode() == NewDesc.getOpcode());
   // Add the implict and explicit register definitions.
-  if (NewDesc.ImplicitUses) {
-    for (unsigned i = 0; NewDesc.ImplicitUses[i]; ++i) {
-      unsigned Reg = NewDesc.ImplicitUses[i];
-      Inst->addOperand(MachineOperand::CreateReg(Reg, false, true));
-    }
-  }
-
-  if (NewDesc.ImplicitDefs) {
-    for (unsigned i = 0; NewDesc.ImplicitDefs[i]; ++i) {
-      unsigned Reg = NewDesc.ImplicitDefs[i];
-      Inst->addOperand(MachineOperand::CreateReg(Reg, true, true));
-    }
-  }
+  Inst->addImplicitDefUseOperands(*Inst->getParent()->getParent());
 }
 
 unsigned SIInstrInfo::findUsedSGPR(const MachineInstr *MI,
Index: include/llvm/CodeGen/MachineInstr.h
===================================================================
--- include/llvm/CodeGen/MachineInstr.h
+++ include/llvm/CodeGen/MachineInstr.h
@@ -1180,16 +1180,15 @@
     }
   }
 
+  /// Add all implicit def and use operands to this instruction.
+  void addImplicitDefUseOperands(MachineFunction &MF);
 
 private:
   /// If this instruction is embedded into a MachineFunction, return the
   /// MachineRegisterInfo object for the current function, otherwise
   /// return null.
   MachineRegisterInfo *getRegInfo();
 
-  /// Add all implicit def and use operands to this instruction.
-  void addImplicitDefUseOperands(MachineFunction &MF);
-
   /// Unlink all of the register operands in this instruction from their
   /// respective use lists.  This requires that the operands already be on their
   /// use lists.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11689.31152.patch
Type: text/x-patch
Size: 2871 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150731/6e402ca4/attachment.bin>