From 526ce9c9299ad1f8c2c5971204066dd02e528199 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 13 Oct 2022 09:18:31 +0100 Subject: [PATCH] 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 --- llvm/lib/CodeGen/MachineInstr.cpp | 8 ++++++++ llvm/lib/CodeGen/ModuloSchedule.cpp | 11 ----------- llvm/test/CodeGen/Thumb2/pipeliner-inlineasm.mir | 4 ++-- llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir | 16 ++++++++-------- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp index 18d11e6b..19a1ede 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 d689e8e..c7fde45 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 25f8be7..59758c1 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 d656db2..08f08c4 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) -- 2.7.4