Skip to content

Commit 54732a3

Browse files
c-rhodesc-rhodes
committed
[AArch64] Use TargetRegisterClass::hasSubClassEq in tryToFindRegisterToRename
When renaming store operands for pairing in the load/store optimizer it tries to find an available register from the minimal physical register class of the original register. For each register it compares the equality of minimal physical register class of all sub/super registers with the minimal physical register class of the original register. Simply checking for register class equality can break once additional register classes are added, as was the case when adding: def foo : RegisterClass<"AArch64", [i32], 32, (sequence "W%u", 12, 15)> which broke: CodeGen/AArch64/stp-opt-with-renaming-reserved-regs.mir CodeGen/AArch64/stp-opt-with-renaming.mir Since the introduction of the register class above, the rename register in test1 of the reserved regs test changed from x12 to x18. The reason for this is the minimal physical register class of x12 (as well as x13-x15) and its sub/super registers no longer matches that of x9 (GPR64noip_and_tcGPR64). Rather than selecting a matching register based on a comparison of the minimal physical register classes of the original and rename registers, this patch selects based on `MachineInstr::getRegClassConstraint` for the original register. It's worth mentioning the parameter passing registers (r0-r7) could be now be used as rename registers since the GPR32arg and GPR64arg register classes are subclasses of the minimal physical register class for x8 for example. I'm not entirely sure if we want to exclude those registers, if so maybe we could explicitly exclude those register classes. Reviewed By: efriedma, paulwalker-arm Differential Revision: https://reviews.llvm.org/D88663
1 parent b72732c commit 54732a3

File tree

4 files changed

+57
-21
lines changed

4 files changed

+57
-21
lines changed

‎llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,16 @@ static bool isMergeableLdStUpdate(MachineInstr &MI) {
723723
}
724724
}
725725

726+
static bool isRewritableImplicitDef(unsigned Opc) {
727+
switch (Opc) {
728+
default:
729+
return false;
730+
case AArch64::ORRWrs:
731+
case AArch64::ADDWri:
732+
return true;
733+
}
734+
}
735+
726736
MachineBasicBlock::iterator
727737
AArch64LoadStoreOpt::mergeNarrowZeroStores(MachineBasicBlock::iterator I,
728738
MachineBasicBlock::iterator MergeMI,
@@ -871,20 +881,22 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
871881

872882
// Return the sub/super register for RenameReg, matching the size of
873883
// OriginalReg.
874-
auto GetMatchingSubReg = [this,
875-
RenameReg](MCPhysReg OriginalReg) -> MCPhysReg {
876-
for (MCPhysReg SubOrSuper : TRI->sub_and_superregs_inclusive(*RenameReg))
877-
if (TRI->getMinimalPhysRegClass(OriginalReg) ==
878-
TRI->getMinimalPhysRegClass(SubOrSuper))
884+
auto GetMatchingSubReg =
885+
[this, RenameReg](const TargetRegisterClass *C) -> MCPhysReg {
886+
for (MCPhysReg SubOrSuper :
887+
TRI->sub_and_superregs_inclusive(*RenameReg)) {
888+
if (C->contains(SubOrSuper))
879889
return SubOrSuper;
890+
}
880891
llvm_unreachable("Should have found matching sub or super register!");
881892
};
882893

883894
std::function<bool(MachineInstr &, bool)> UpdateMIs =
884895
[this, RegToRename, GetMatchingSubReg](MachineInstr &MI, bool IsDef) {
885896
if (IsDef) {
886897
bool SeenDef = false;
887-
for (auto &MOP : MI.operands()) {
898+
for (unsigned OpIdx = 0; OpIdx < MI.getNumOperands(); ++OpIdx) {
899+
MachineOperand &MOP = MI.getOperand(OpIdx);
888900
// Rename the first explicit definition and all implicit
889901
// definitions matching RegToRename.
890902
if (MOP.isReg() && !MOP.isDebug() && MOP.getReg() &&
@@ -893,18 +905,33 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
893905
assert((MOP.isImplicit() ||
894906
(MOP.isRenamable() && !MOP.isEarlyClobber())) &&
895907
"Need renamable operands");
896-
MOP.setReg(GetMatchingSubReg(MOP.getReg()));
908+
Register MatchingReg;
909+
if (const TargetRegisterClass *RC =
910+
MI.getRegClassConstraint(OpIdx, TII, TRI))
911+
MatchingReg = GetMatchingSubReg(RC);
912+
else {
913+
if (!isRewritableImplicitDef(MI.getOpcode()))
914+
continue;
915+
MatchingReg = GetMatchingSubReg(
916+
TRI->getMinimalPhysRegClass(MOP.getReg()));
917+
}
918+
MOP.setReg(MatchingReg);
897919
SeenDef = true;
898920
}
899921
}
900922
} else {
901-
for (auto &MOP : MI.operands()) {
923+
for (unsigned OpIdx = 0; OpIdx < MI.getNumOperands(); ++OpIdx) {
924+
MachineOperand &MOP = MI.getOperand(OpIdx);
902925
if (MOP.isReg() && !MOP.isDebug() && MOP.getReg() &&
903926
TRI->regsOverlap(MOP.getReg(), RegToRename)) {
904927
assert((MOP.isImplicit() ||
905928
(MOP.isRenamable() && !MOP.isEarlyClobber())) &&
906929
"Need renamable operands");
907-
MOP.setReg(GetMatchingSubReg(MOP.getReg()));
930+
const TargetRegisterClass *RC =
931+
MI.getRegClassConstraint(OpIdx, TII, TRI);
932+
if (!RC)
933+
continue;
934+
MOP.setReg(GetMatchingSubReg(RC));
908935
}
909936
}
910937
}
@@ -1404,6 +1431,16 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween,
14041431
<< MOP << ")\n");
14051432
return false;
14061433
}
1434+
1435+
// We cannot rename arbitrary implicit-defs, the specific rule to rewrite
1436+
// them must be known. For example, in ORRWrs the implicit-def
1437+
// corresponds to the result register.
1438+
if (MOP.isImplicit() && MOP.isDef()) {
1439+
if (!isRewritableImplicitDef(MOP.getParent()->getOpcode()))
1440+
return false;
1441+
return TRI->isSuperOrSubRegisterEq(
1442+
MOP.getParent()->getOperand(0).getReg(), MOP.getReg());
1443+
}
14071444
}
14081445
return MOP.isImplicit() ||
14091446
(MOP.isRenamable() && !MOP.isEarlyClobber() && !MOP.isTied());
@@ -1511,10 +1548,9 @@ static std::optional<MCPhysReg> tryToFindRegisterToRename(
15111548
// required register classes.
15121549
auto CanBeUsedForAllClasses = [&RequiredClasses, TRI](MCPhysReg PR) {
15131550
return all_of(RequiredClasses, [PR, TRI](const TargetRegisterClass *C) {
1514-
return any_of(TRI->sub_and_superregs_inclusive(PR),
1515-
[C, TRI](MCPhysReg SubOrSuper) {
1516-
return C == TRI->getMinimalPhysRegClass(SubOrSuper);
1517-
});
1551+
return any_of(
1552+
TRI->sub_and_superregs_inclusive(PR),
1553+
[C](MCPhysReg SubOrSuper) { return C->contains(SubOrSuper); });
15181554
});
15191555
};
15201556

‎llvm/test/CodeGen/AArch64/stp-opt-with-renaming-reserved-regs.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ body: |
4747
...
4848
# CHECK-LABEL: name: test2
4949
# CHECK: bb.0:
50-
# CHECK-NEXT: liveins: $x0, $x1, $x10, $x11, $x12, $x13
50+
# CHECK-NEXT: liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6, $x7, $x9, $x10, $x11, $x12, $x13
5151
# CHECK: renamable $w19 = LDRWui renamable $x0, 0 :: (load (s64))
5252
# PRESERVED-NEXT: renamable $x9, renamable $x8 = LDPXi renamable $x0, 1 :: (load (s64))
5353
# NOPRES-NEXT: renamable $x9, renamable $x8 = LDPXi renamable $x0, 1 :: (load (s64))
@@ -74,7 +74,7 @@ frameInfo:
7474
machineFunctionInfo: {}
7575
body: |
7676
bb.0:
77-
liveins: $x0, $x1, $x10, $x11, $x12, $x13
77+
liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6, $x7, $x9, $x10, $x11, $x12, $x13
7878
renamable $w19 = LDRWui renamable $x0, 0 :: (load (s64))
7979
renamable $x9, renamable $x8 = LDPXi renamable $x0, 1 :: (load (s64))
8080
STRXui renamable killed $x9, renamable $x0, 11 :: (store (s64), align 4)

‎llvm/test/CodeGen/AArch64/stp-opt-with-renaming-undef-assert.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
# CHECK-NEXT: liveins: $x0, $x17, $x18
1818
# CHECK: renamable $q13_q14_q15 = LD3Threev16b undef renamable $x17 :: (load (s384) from `ptr undef`, align 64)
1919
# CHECK-NEXT: renamable $q23_q24_q25 = LD3Threev16b undef renamable $x18 :: (load (s384) from `ptr undef`, align 64)
20-
# CHECK-NEXT: $q16 = EXTv16i8 renamable $q23, renamable $q23, 8
20+
# CHECK-NEXT: $q0 = EXTv16i8 renamable $q23, renamable $q23, 8
2121
# CHECK-NEXT: renamable $q20 = EXTv16i8 renamable $q14, renamable $q14, 8
2222
# CHECK-NEXT: STRQui killed renamable $q20, $sp, 4 :: (store (s128))
2323
# CHECK-NEXT: renamable $d6 = ZIP2v8i8 renamable $d23, undef renamable $d16
2424
# CHECK-NEXT: STRDui killed renamable $d6, $sp, 11 :: (store (s64))
2525
# CHECK-NEXT: renamable $q6 = EXTv16i8 renamable $q13, renamable $q13, 8
26-
# CHECK-NEXT: STPQi killed renamable $q6, killed $q16, $sp, 6 :: (store (s128))
26+
# CHECK-NEXT: STPQi killed renamable $q6, killed $q0, $sp, 6 :: (store (s128))
2727
# CHECK-NEXT: RET undef $lr
2828

2929
---

‎llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ body: |
361361
# ($x14 in this example)
362362
# CHECK-LABEL: name: test11
363363
# CHECK: bb.0:
364-
# CHECK-NEXT: liveins: $x0, $x1, $x11, $x12, $x13
364+
# CHECK-NEXT: liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6, $x7, $x11, $x12, $x13
365365

366366
# CHECK: renamable $w10 = LDRWui renamable $x0, 0 :: (load (s64))
367367
# CHECK-NEXT: renamable $x9, renamable $x8 = LDPXi renamable $x0, 1 :: (load (s64))
@@ -387,7 +387,7 @@ frameInfo:
387387
machineFunctionInfo: {}
388388
body: |
389389
bb.0:
390-
liveins: $x0, $x1, $x11, $x12, $x13
390+
liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6, $x7, $x11, $x12, $x13
391391
renamable $w10 = LDRWui renamable $x0, 0 :: (load (s64))
392392
renamable $x9, renamable $x8 = LDPXi renamable $x0, 1 :: (load (s64))
393393
STRXui renamable killed $x9, renamable $x0, 11 :: (store (s64), align 4)
@@ -445,7 +445,7 @@ body: |
445445
# paired store. ($x14 in this example)
446446
# CHECK-LABEL: name: test13
447447
# CHECK: bb.0:
448-
# CHECK-NEXT: liveins: $x0, $x1, $x10, $x11, $x12, $x13
448+
# CHECK-NEXT: liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6, $x7, $x10, $x11, $x12, $x13
449449
# CHECK: renamable $x9, renamable $x8 = LDPXi renamable $x0, 0 :: (load (s64))
450450
# CHECK-NEXT: renamable $x14 = LDRXui renamable $x0, 4 :: (load (s64))
451451
# CHECK-NEXT: STRXui killed renamable $x14, renamable $x0, 100 :: (store (s64), align 4)
@@ -467,7 +467,7 @@ frameInfo:
467467
machineFunctionInfo: {}
468468
body: |
469469
bb.0:
470-
liveins: $x0, $x1, $x10, $x11, $x12, $x13
470+
liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6, $x7, $x10, $x11, $x12, $x13
471471
renamable $x9, renamable $x8 = LDPXi renamable $x0, 0 :: (load (s64))
472472
renamable $x14 = LDRXui renamable $x0, 4 :: (load (s64))
473473
STRXui renamable killed $x14, renamable $x0, 100 :: (store (s64), align 4)

0 commit comments

Comments
 (0)