Skip to content

Commit c98ea57

Browse files
committed
[CodeGen] commuteInstruction should update implicit-def
When the RegisterCoalescer adds an implicit-def when coalescing a SUBREG_TO_REG (llvm#123632), this causes issues when removing other COPY nodes by commuting the instruction because it doesn't take the implicit-def into consideration. This PR fixes that.
1 parent 100df3a commit c98ea57

File tree

2 files changed

+23
-2
lines changed

2 files changed

+23
-2
lines changed

llvm/lib/CodeGen/TargetInstrInfo.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ MachineInstr *TargetInstrInfo::commuteInstructionImpl(MachineInstr &MI,
200200
Reg1.isPhysical() ? MI.getOperand(Idx1).isRenamable() : false;
201201
bool Reg2IsRenamable =
202202
Reg2.isPhysical() ? MI.getOperand(Idx2).isRenamable() : false;
203+
203204
// If destination is tied to either of the commuted source register, then
204205
// it must be updated.
205206
if (HasDef && Reg0 == Reg1 &&
@@ -214,6 +215,24 @@ MachineInstr *TargetInstrInfo::commuteInstructionImpl(MachineInstr &MI,
214215
SubReg0 = SubReg1;
215216
}
216217

218+
// For a case like this:
219+
// %0.sub = INST %0.sub(tied), %1.sub, implicit-def %0
220+
// we need to update the implicit-def after commuting to result in:
221+
// %1.sub = INST %1.sub(tied), %0.sub, implicit-def %1
222+
SmallVector<unsigned> UpdateImplicitDefIdx;
223+
if (HasDef && MI.hasImplicitDef() && MI.getOperand(0).getReg() != Reg0) {
224+
const TargetRegisterInfo *TRI =
225+
MI.getMF()->getSubtarget().getRegisterInfo();
226+
Register OrigReg0 = MI.getOperand(0).getReg();
227+
for (auto [OpNo, MO] : llvm::enumerate(MI.implicit_operands())) {
228+
Register ImplReg = MO.getReg();
229+
if ((ImplReg.isVirtual() && ImplReg == OrigReg0) ||
230+
(ImplReg.isPhysical() && OrigReg0.isPhysical() &&
231+
TRI->isSubRegisterEq(ImplReg, OrigReg0)))
232+
UpdateImplicitDefIdx.push_back(OpNo + MI.getNumExplicitOperands());
233+
}
234+
}
235+
217236
MachineInstr *CommutedMI = nullptr;
218237
if (NewMI) {
219238
// Create a new instruction.
@@ -226,6 +245,8 @@ MachineInstr *TargetInstrInfo::commuteInstructionImpl(MachineInstr &MI,
226245
if (HasDef) {
227246
CommutedMI->getOperand(0).setReg(Reg0);
228247
CommutedMI->getOperand(0).setSubReg(SubReg0);
248+
for (unsigned Idx : UpdateImplicitDefIdx)
249+
CommutedMI->getOperand(Idx).setReg(Reg0);
229250
}
230251
CommutedMI->getOperand(Idx2).setReg(Reg1);
231252
CommutedMI->getOperand(Idx1).setReg(Reg2);

llvm/test/CodeGen/X86/coalesce-commutative-implicit-def.mir

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ body: |
1010
; CHECK-LABEL: name: implicit_def_dst
1111
; CHECK: [[MOV64rm:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
1212
; CHECK-NEXT: [[MOV64rm1:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
13-
; CHECK-NEXT: [[MOV64rm:%[0-9]+]].sub_32bit:gr64_with_sub_8bit = AND32rr [[MOV64rm]].sub_32bit, [[MOV64rm1]].sub_32bit, implicit-def dead $eflags, implicit-def [[MOV64rm1]]
13+
; CHECK-NEXT: [[MOV64rm:%[0-9]+]].sub_32bit:gr64_with_sub_8bit = AND32rr [[MOV64rm]].sub_32bit, [[MOV64rm1]].sub_32bit, implicit-def dead $eflags, implicit-def [[MOV64rm]]
1414
; CHECK-NEXT: RET 0, implicit [[MOV64rm]]
1515
%0:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
1616
%1:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
@@ -27,7 +27,7 @@ body: |
2727
; CHECK-LABEL: name: two_implicit_defs_dst
2828
; CHECK: [[MOV64rm:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
2929
; CHECK-NEXT: [[MOV64rm1:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
30-
; CHECK-NEXT: [[MOV64rm:%[0-9]+]].sub_32bit:gr64_with_sub_8bit = AND32rr [[MOV64rm]].sub_32bit, [[MOV64rm1]].sub_32bit, implicit-def dead $eflags, implicit-def [[MOV64rm1]], implicit-def [[MOV64rm1]]
30+
; CHECK-NEXT: [[MOV64rm:%[0-9]+]].sub_32bit:gr64_with_sub_8bit = AND32rr [[MOV64rm]].sub_32bit, [[MOV64rm1]].sub_32bit, implicit-def dead $eflags, implicit-def [[MOV64rm]], implicit-def [[MOV64rm]]
3131
; CHECK-NEXT: RET 0, implicit [[MOV64rm]]
3232
%0:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
3333
%1:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)

0 commit comments

Comments
 (0)