[llvm] 526ce9c - Propagate tied operands when copying a MachineInstr.

Simon Tatham via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 13 01:40:52 PDT 2022


Author: Simon Tatham
Date: 2022-10-13T09:40:35+01:00
New Revision: 526ce9c9299ad1f8c2c5971204066dd02e528199

URL: https://github.com/llvm/llvm-project/commit/526ce9c9299ad1f8c2c5971204066dd02e528199
DIFF: https://github.com/llvm/llvm-project/commit/526ce9c9299ad1f8c2c5971204066dd02e528199.diff

LOG: Propagate tied operands when copying a MachineInstr.

MachineInstr's copy constructor works by calling the addOperand method
to add each operand of the old MachineInstr to the new one, one by
one. But addOperand deliberately avoids trying to replicate ties
between operands, on the grounds that the tie refers to operands by
index, and the indices aren't necessarily finalized yet.

This led to a code generation fault when the machine pipeliner cloned
an Arm conditional instruction, and lost the tie between the output
register and the input value to be used when the condition failed to
execute.

Reviewed By: dmgreen

Differential Revision: https://reviews.llvm.org/D135434

Added: 
    

Modified: 
    llvm/lib/CodeGen/MachineInstr.cpp
    llvm/lib/CodeGen/ModuloSchedule.cpp
    llvm/test/CodeGen/Thumb2/pipeliner-inlineasm.mir
    llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 18d11e6b29908..19a1ede00896d 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -129,6 +129,14 @@ MachineInstr::MachineInstr(MachineFunction &MF, const MachineInstr &MI)
   for (const MachineOperand &MO : MI.operands())
     addOperand(MF, MO);
 
+  // Replicate ties between the operands, which addOperand was not
+  // able to do reliably.
+  for (unsigned i = 0, e = getNumOperands(); i < e; ++i) {
+    MachineOperand &NewMO = getOperand(i);
+    const MachineOperand &OrigMO = MI.getOperand(i);
+    NewMO.TiedTo = OrigMO.TiedTo;
+  }
+
   // Copy all the sensible flags.
   setFlags(MI.Flags);
 }

diff  --git a/llvm/lib/CodeGen/ModuloSchedule.cpp b/llvm/lib/CodeGen/ModuloSchedule.cpp
index d689e8e9d2f20..c7fde45eba6a6 100644
--- a/llvm/lib/CodeGen/ModuloSchedule.cpp
+++ b/llvm/lib/CodeGen/ModuloSchedule.cpp
@@ -998,17 +998,6 @@ MachineInstr *ModuloScheduleExpander::cloneInstr(MachineInstr *OldMI,
                                                  unsigned CurStageNum,
                                                  unsigned InstStageNum) {
   MachineInstr *NewMI = MF.CloneMachineInstr(OldMI);
-  // Check for tied operands in inline asm instructions. This should be handled
-  // elsewhere, but I'm not sure of the best solution.
-  if (OldMI->isInlineAsm())
-    for (unsigned i = 0, e = OldMI->getNumOperands(); i != e; ++i) {
-      const auto &MO = OldMI->getOperand(i);
-      if (MO.isReg() && MO.isUse())
-        break;
-      unsigned UseIdx;
-      if (OldMI->isRegTiedToUseOperand(i, &UseIdx))
-        NewMI->tieOperands(i, UseIdx);
-    }
   updateMemOperands(*NewMI, *OldMI, CurStageNum - InstStageNum);
   return NewMI;
 }

diff  --git a/llvm/test/CodeGen/Thumb2/pipeliner-inlineasm.mir b/llvm/test/CodeGen/Thumb2/pipeliner-inlineasm.mir
index 25f8be7ad4645..59758c1d3747c 100644
--- a/llvm/test/CodeGen/Thumb2/pipeliner-inlineasm.mir
+++ b/llvm/test/CodeGen/Thumb2/pipeliner-inlineasm.mir
@@ -85,7 +85,7 @@ body:             |
   ; CHECK-NEXT:   [[VLDRS2:%[0-9]+]]:spr = VLDRS [[COPY4]], 1, 14 /* CC::al */, $noreg :: (load (s32) from %ir.scevgep7)
   ; CHECK-NEXT:   [[t2ADDri1:%[0-9]+]]:rgpr = t2ADDri [[COPY3]], 4, 14 /* CC::al */, $noreg, $noreg
   ; CHECK-NEXT:   [[VLDRS3:%[0-9]+]]:spr = VLDRS [[COPY3]], 1, 14 /* CC::al */, $noreg :: (load (s32) from %ir.scevgep3)
-  ; CHECK-NEXT:   INLINEASM &nop, 0 /* attdialect */, 196618 /* regdef:SPR */, def %30, 2147483657 /* reguse tiedto:$0 */, [[VLDRS2]]
+  ; CHECK-NEXT:   INLINEASM &nop, 0 /* attdialect */, 196618 /* regdef:SPR */, def %30, 2147483657 /* reguse tiedto:$0 */, [[VLDRS2]](tied-def 3)
   ; CHECK-NEXT:   [[t2SUBri2:%[0-9]+]]:rgpr = t2SUBri [[COPY]], 1, 14 /* CC::al */, $noreg, def $cpsr
   ; CHECK-NEXT:   [[COPY5:%[0-9]+]]:gprnopc = COPY [[t2SUBri2]]
   ; CHECK-NEXT:   [[COPY6:%[0-9]+]]:gprnopc = COPY [[t2ADDri1]]
@@ -101,7 +101,7 @@ body:             |
   ; CHECK-NEXT:   [[VLDRS4:%[0-9]+]]:spr = VLDRS [[COPY7]], 1, 14 /* CC::al */, $noreg :: (load unknown-size from %ir.scevgep7, align 4)
   ; CHECK-NEXT:   [[t2ADDri3:%[0-9]+]]:rgpr = t2ADDri [[COPY6]], 4, 14 /* CC::al */, $noreg, $noreg
   ; CHECK-NEXT:   [[VLDRS5:%[0-9]+]]:spr = VLDRS [[COPY6]], 1, 14 /* CC::al */, $noreg :: (load unknown-size from %ir.scevgep3, align 4)
-  ; CHECK-NEXT:   INLINEASM &nop, 0 /* attdialect */, 196618 /* regdef:SPR */, def %40, 2147483657 /* reguse tiedto:$0 */, [[VLDRS4]]
+  ; CHECK-NEXT:   INLINEASM &nop, 0 /* attdialect */, 196618 /* regdef:SPR */, def %40, 2147483657 /* reguse tiedto:$0 */, [[VLDRS4]](tied-def 3)
   ; CHECK-NEXT:   [[t2SUBri3:%[0-9]+]]:rgpr = t2SUBri [[COPY5]], 1, 14 /* CC::al */, $noreg, def $cpsr
   ; CHECK-NEXT:   [[COPY8:%[0-9]+]]:gpr = COPY [[t2SUBri3]]
   ; CHECK-NEXT:   [[COPY9:%[0-9]+]]:gpr = COPY [[t2ADDri3]]

diff  --git a/llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir b/llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
index d656db256c9da..08f08c41917b1 100644
--- a/llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
+++ b/llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
@@ -188,16 +188,16 @@ body:             |
   ; CHECK-NEXT:   t2STRHi12 [[t2MLS]], [[t2LDRSH_PRE1]], 14, 14 /* CC::al */, $noreg :: (store (s16) into %ir.uglygep4, !tbaa !7)
   ; CHECK-NEXT:   [[t2LDRHi12_:%[0-9]+]]:gprnopc = t2LDRHi12 [[t2LDRSH_PRE1]], 4, 14 /* CC::al */, $noreg :: (load (s16) from %ir.uglygep5, !tbaa !8)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_]], [[t2UXTH]], 14 /* CC::al */, $noreg, implicit-def $cpsr
-  ; CHECK-NEXT:   [[t2ADDri:%[0-9]+]]:rgpr = t2ADDri [[COPY]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[COPY]]
+  ; CHECK-NEXT:   [[t2ADDri:%[0-9]+]]:rgpr = t2ADDri [[COPY]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[COPY]](tied-def 0)
   ; CHECK-NEXT:   [[t2LDRHi12_1:%[0-9]+]]:gprnopc = t2LDRHi12 [[t2LDRSH_PRE1]], 6, 14 /* CC::al */, $noreg :: (load (s16) from %ir.uglygep6, !tbaa !9)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_1]], [[t2UXTH1]], 14 /* CC::al */, $noreg, implicit-def $cpsr
-  ; CHECK-NEXT:   [[t2ADDri1:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri]]
+  ; CHECK-NEXT:   [[t2ADDri1:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri]](tied-def 0)
   ; CHECK-NEXT:   [[t2LDRHi12_2:%[0-9]+]]:gprnopc = t2LDRHi12 [[t2LDRSH_PRE1]], 8, 14 /* CC::al */, $noreg :: (load (s16) from %ir.uglygep7, !tbaa !10)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_2]], [[t2UXTH]], 14 /* CC::al */, $noreg, implicit-def $cpsr
-  ; CHECK-NEXT:   [[t2ADDri2:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri1]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri1]]
+  ; CHECK-NEXT:   [[t2ADDri2:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri1]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri1]](tied-def 0)
   ; CHECK-NEXT:   [[t2LDRHi12_3:%[0-9]+]]:gprnopc = t2LDRHi12 [[t2LDRSH_PRE1]], 10, 14 /* CC::al */, $noreg :: (load (s16) from %ir.uglygep8, !tbaa !11)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_3]], [[t2UXTH1]], 14 /* CC::al */, $noreg, implicit-def $cpsr
-  ; CHECK-NEXT:   [[t2ADDri3:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri2]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri2]]
+  ; CHECK-NEXT:   [[t2ADDri3:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri2]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri2]](tied-def 0)
   ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:gpr = COPY [[t2ADDri3]]
   ; CHECK-NEXT:   [[t2LDRHi12_4:%[0-9]+]]:gprnopc = t2LDRHi12 [[t2LDRSH_PRE1]], 18, 14 /* CC::al */, $noreg :: (load (s16) from %ir.uglygep9, !tbaa !5)
   ; CHECK-NEXT:   t2CMPri [[t2LDRHi12_4]], 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
@@ -224,13 +224,13 @@ body:             |
   ; CHECK-NEXT:   [[t2LDRHi12_9:%[0-9]+]]:gprnopc = t2LDRHi12 [[t2LDRSH_PRE3]], 6, 14 /* CC::al */, $noreg :: (load unknown-size from %ir.uglygep6, align 2, !tbaa !9)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_8]], [[t2UXTH2]], 14 /* CC::al */, $noreg, implicit-def $cpsr
   ; CHECK-NEXT:   [[t2UXTH3:%[0-9]+]]:rgpr = t2UXTH [[t2MLS1]], 0, 14 /* CC::al */, $noreg
-  ; CHECK-NEXT:   [[t2ADDri4:%[0-9]+]]:rgpr = t2ADDri [[PHI1]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[PHI1]]
+  ; CHECK-NEXT:   [[t2ADDri4:%[0-9]+]]:rgpr = t2ADDri [[PHI1]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[PHI1]](tied-def 0)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_9]], [[t2UXTH3]], 14 /* CC::al */, $noreg, implicit-def $cpsr
-  ; CHECK-NEXT:   [[t2ADDri5:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri4]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri4]]
+  ; CHECK-NEXT:   [[t2ADDri5:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri4]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri4]](tied-def 0)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_6]], [[t2UXTH2]], 14 /* CC::al */, $noreg, implicit-def $cpsr
-  ; CHECK-NEXT:   [[t2ADDri6:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri5]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri5]]
+  ; CHECK-NEXT:   [[t2ADDri6:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri5]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri5]](tied-def 0)
   ; CHECK-NEXT:   t2CMPrr [[t2LDRHi12_7]], [[t2UXTH3]], 14 /* CC::al */, $noreg, implicit-def $cpsr
-  ; CHECK-NEXT:   [[t2ADDri7:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri6]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri6]]
+  ; CHECK-NEXT:   [[t2ADDri7:%[0-9]+]]:rgpr = t2ADDri [[t2ADDri6]], 1, 1 /* CC::ne */, $cpsr, $noreg, implicit [[t2ADDri6]](tied-def 0)
   ; CHECK-NEXT:   t2CMPri [[t2LDRHi12_5]], 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
   ; CHECK-NEXT:   [[COPY3:%[0-9]+]]:gpr = COPY [[t2ADDri7]]
   ; CHECK-NEXT:   t2STRHi12 [[t2MLS1]], [[t2LDRSH_PRE3]], 14, 14 /* CC::al */, $noreg :: (store unknown-size into %ir.uglygep4, align 2, !tbaa !7)


        


More information about the llvm-commits mailing list