Skip to content

Commit d7abb47

Browse files
committed
[CodeGen] commuteInstruction should update implicit-def
When the RegisterCoalescer adds an implicit-def when coalescing a SUBREG_TO_REG (#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 8d23eec commit d7abb47

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
@@ -214,6 +214,7 @@ MachineInstr *TargetInstrInfo::commuteInstructionImpl(MachineInstr &MI,
214214
Reg1.isPhysical() ? MI.getOperand(Idx1).isRenamable() : false;
215215
bool Reg2IsRenamable =
216216
Reg2.isPhysical() ? MI.getOperand(Idx2).isRenamable() : false;
217+
217218
// If destination is tied to either of the commuted source register, then
218219
// it must be updated.
219220
if (HasDef && Reg0 == Reg1 &&
@@ -228,6 +229,24 @@ MachineInstr *TargetInstrInfo::commuteInstructionImpl(MachineInstr &MI,
228229
SubReg0 = SubReg1;
229230
}
230231

232+
// For a case like this:
233+
// %0.sub = INST %0.sub(tied), %1.sub, implicit-def %0
234+
// we need to update the implicit-def after commuting to result in:
235+
// %1.sub = INST %1.sub(tied), %0.sub, implicit-def %1
236+
SmallVector<unsigned> UpdateImplicitDefIdx;
237+
if (HasDef && MI.hasImplicitDef() && MI.getOperand(0).getReg() != Reg0) {
238+
const TargetRegisterInfo *TRI =
239+
MI.getMF()->getSubtarget().getRegisterInfo();
240+
Register OrigReg0 = MI.getOperand(0).getReg();
241+
for (auto [OpNo, MO] : llvm::enumerate(MI.implicit_operands())) {
242+
Register ImplReg = MO.getReg();
243+
if ((ImplReg.isVirtual() && ImplReg == OrigReg0) ||
244+
(ImplReg.isPhysical() && OrigReg0.isPhysical() &&
245+
TRI->isSubRegisterEq(ImplReg, OrigReg0)))
246+
UpdateImplicitDefIdx.push_back(OpNo + MI.getNumExplicitOperands());
247+
}
248+
}
249+
231250
MachineInstr *CommutedMI = nullptr;
232251
if (NewMI) {
233252
// Create a new instruction.
@@ -240,6 +259,8 @@ MachineInstr *TargetInstrInfo::commuteInstructionImpl(MachineInstr &MI,
240259
if (HasDef) {
241260
CommutedMI->getOperand(0).setReg(Reg0);
242261
CommutedMI->getOperand(0).setSubReg(SubReg0);
262+
for (unsigned Idx : UpdateImplicitDefIdx)
263+
CommutedMI->getOperand(Idx).setReg(Reg0);
243264
}
244265
CommutedMI->getOperand(Idx2).setReg(Reg1);
245266
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)