[PATCH] D135434: Propagate tied operands when copying a MachineInstr.

Fri Oct 7 02:54:40 PDT 2022

simon_tatham created this revision.
simon_tatham added reviewers: dmgreen, dpenry.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
simon_tatham requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135434

Files:
  llvm/lib/CodeGen/MachineInstr.cpp
  llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir


Index: llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
===================================================================

--- llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
+++ llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
@@ -188,16 +188,16 @@
   ; 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 @@
   ; 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)
Index: llvm/lib/CodeGen/MachineInstr.cpp
===================================================================
--- llvm/lib/CodeGen/MachineInstr.cpp
+++ llvm/lib/CodeGen/MachineInstr.cpp
@@ -129,6 +129,14 @@
   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);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135434.466017.patch
Type: text/x-patch
Size: 5394 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221007/8da34c20/attachment.bin>
