Skip to content

Commit b6ee831

Browse files
davemgreendavemgreen
committed
[AArch64] Load/store optimizer fixes and cleanup.
This includes a couple of fixes after #71908 for bundles and some cleanup for the debug output. One was an iterator type that asserted on bundles, the second a rather subtle issue where forAllMIsUntilDef would hit the LdStLimit when renaming registers, meaning the last instruction was not updated leaving an invalid `ldp x6, x6` instruction.
1 parent d345cfb commit b6ee831

File tree

2 files changed

+129
-24
lines changed

2 files changed

+129
-24
lines changed

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

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -941,11 +941,11 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
941941
}
942942
}
943943
}
944-
LLVM_DEBUG(dbgs() << "Renamed " << MI << "\n");
944+
LLVM_DEBUG(dbgs() << "Renamed " << MI);
945945
return true;
946946
};
947947
forAllMIsUntilDef(MergeForward ? *I : *std::prev(Paired), RegToRename, TRI,
948-
LdStLimit, UpdateMIs);
948+
UINT32_MAX, UpdateMIs);
949949

950950
#if !defined(NDEBUG)
951951
// For forward merging store:
@@ -1464,7 +1464,7 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween,
14641464
MOP.isImplicit() && MOP.isKill() &&
14651465
TRI->regsOverlap(RegToRename, MOP.getReg());
14661466
})) {
1467-
LLVM_DEBUG(dbgs() << " Operand not killed at " << FirstMI << "\n");
1467+
LLVM_DEBUG(dbgs() << " Operand not killed at " << FirstMI);
14681468
return false;
14691469
}
14701470

@@ -1476,11 +1476,11 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween,
14761476
// * collect the registers used and required register classes for RegToRename.
14771477
std::function<bool(MachineInstr &, bool)> CheckMIs = [&](MachineInstr &MI,
14781478
bool IsDef) {
1479-
LLVM_DEBUG(dbgs() << "Checking " << MI << "\n");
1479+
LLVM_DEBUG(dbgs() << "Checking " << MI);
14801480
// Currently we do not try to rename across frame-setup instructions.
14811481
if (MI.getFlag(MachineInstr::FrameSetup)) {
1482-
LLVM_DEBUG(dbgs() << " Cannot rename framesetup instructions currently ("
1483-
<< MI << ")\n");
1482+
LLVM_DEBUG(dbgs() << " Cannot rename framesetup instructions "
1483+
<< "currently\n");
14841484
return false;
14851485
}
14861486

@@ -1500,8 +1500,7 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween,
15001500
// 1. Insert an extra copy, to materialize the def.
15011501
// 2. Skip pseudo-defs until we find an non-pseudo def.
15021502
if (MI.isPseudo()) {
1503-
LLVM_DEBUG(dbgs() << " Cannot rename pseudo instruction " << MI
1504-
<< "\n");
1503+
LLVM_DEBUG(dbgs() << " Cannot rename pseudo/bundle instruction\n");
15051504
return false;
15061505
}
15071506

@@ -1510,8 +1509,7 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween,
15101509
!TRI->regsOverlap(MOP.getReg(), RegToRename))
15111510
continue;
15121511
if (!canRenameMOP(MOP, TRI)) {
1513-
LLVM_DEBUG(dbgs()
1514-
<< " Cannot rename " << MOP << " in " << MI << "\n");
1512+
LLVM_DEBUG(dbgs() << " Cannot rename " << MOP << " in " << MI);
15151513
return false;
15161514
}
15171515
RequiredClasses.insert(TRI->getMinimalPhysRegClass(MOP.getReg()));
@@ -1524,8 +1522,7 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween,
15241522
continue;
15251523

15261524
if (!canRenameMOP(MOP, TRI)) {
1527-
LLVM_DEBUG(dbgs()
1528-
<< " Cannot rename " << MOP << " in " << MI << "\n");
1525+
LLVM_DEBUG(dbgs() << " Cannot rename " << MOP << " in " << MI);
15291526
return false;
15301527
}
15311528
RequiredClasses.insert(TRI->getMinimalPhysRegClass(MOP.getReg()));
@@ -1565,15 +1562,12 @@ static bool canRenameUntilSecondLoad(
15651562
auto RegToRename = getLdStRegOp(FirstLoad).getReg();
15661563
bool Success = std::all_of(
15671564
FirstLoad.getIterator(), SecondLoad.getIterator(),
1568-
[&](MachineBasicBlock::iterator MBBI) {
1569-
MachineInstr &MI = *MBBI;
1570-
1571-
LLVM_DEBUG(dbgs() << "Checking " << MI << "\n");
1565+
[&](MachineInstr &MI) {
1566+
LLVM_DEBUG(dbgs() << "Checking " << MI);
15721567
// Currently we do not try to rename across frame-setup instructions.
15731568
if (MI.getFlag(MachineInstr::FrameSetup)) {
1574-
LLVM_DEBUG(dbgs()
1575-
<< " Cannot rename framesetup instructions currently ("
1576-
<< MI << ")\n");
1569+
LLVM_DEBUG(dbgs() << " Cannot rename framesetup instructions "
1570+
<< "currently\n");
15771571
return false;
15781572
}
15791573

@@ -1582,8 +1576,7 @@ static bool canRenameUntilSecondLoad(
15821576
!TRI->regsOverlap(MOP.getReg(), RegToRename))
15831577
continue;
15841578
if (!canRenameMOP(MOP, TRI)) {
1585-
LLVM_DEBUG(dbgs()
1586-
<< " Cannot rename " << MOP << " in " << MI << "\n");
1579+
LLVM_DEBUG(dbgs() << " Cannot rename " << MOP << " in " << MI);
15871580
return false;
15881581
}
15891582
RequiredClasses.insert(TRI->getMinimalPhysRegClass(MOP.getReg()));

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

Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ body: |
622622
STRQui killed renamable $q0, $sp, 2 :: (store 16, align 128)
623623
RET undef $lr
624624
625-
...
625+
...
626626
---
627627
#
628628
# CHECK-LABEL: name: ldst-no-reg-available
@@ -650,7 +650,7 @@ body: |
650650
STRQui killed renamable $q0, $sp, 2 :: (store 16, align 32)
651651
RET undef $lr
652652
653-
...
653+
...
654654
---
655655
#
656656
# CHECK-LABEL: name: ldst-basereg-modified
@@ -677,7 +677,7 @@ body: |
677677
STRQui killed renamable $q0, $sp, 2 :: (store 16, align 32)
678678
RET undef $lr
679679
680-
...
680+
...
681681
---
682682
#
683683
# CHECK-LABEL: name: ldr-dest-reg-implicit-killed
@@ -700,3 +700,115 @@ body: |
700700
renamable $q0 = LDRQui $sp, 0 :: (load 16, align 32)
701701
STRSui renamable $s0, $sp, 10, implicit killed $q0 :: (store (s32))
702702
RET undef $lr
703+
704+
...
705+
---
706+
#
707+
# CHECK-LABEL: name: bundled
708+
# CHECK: renamable $q0, $q1 = LDPQi $sp, 0 :: (load (s128)), (load (s128), align 32)
709+
# CHECK-NEXT: STPSi $s1, renamable $s0, $sp, 9 :: (store (s32))
710+
# CHECK-NEXT: BUNDLE implicit-def $z3
711+
# CHECK-NEXT: $z3 = MOVPRFX_ZZ $z19
712+
# CHECK-NEXT: $z3 = FMUL_ZPmZ_S renamable $p0, killed $z3, renamable $z16
713+
# CHECK-NEXT: }
714+
# CHECK-NEXT: RET undef $lr
715+
#
716+
name: bundled
717+
alignment: 4
718+
tracksRegLiveness: true
719+
frameInfo:
720+
maxAlignment: 1
721+
maxCallFrameSize: 0
722+
machineFunctionInfo:
723+
hasRedZone: false
724+
body: |
725+
bb.0.entry:
726+
liveins: $z3, $z19, $p0, $z16
727+
renamable $q0 = LDRQui $sp, 1 :: (load 16)
728+
STRSui renamable $s0, $sp, 9, implicit killed $q0 :: (store (s32))
729+
BUNDLE implicit-def $z3, implicit-def $q3, implicit-def $d3, implicit-def $s3, implicit-def $h3, implicit-def $b3, implicit $z19, implicit $p0, implicit $z16 {
730+
$z3 = MOVPRFX_ZZ $z19
731+
$z3 = FMUL_ZPmZ_S renamable $p0, killed $z3, renamable $z16
732+
}
733+
renamable $q0 = LDRQui $sp, 0 :: (load 16, align 32)
734+
STRSui renamable $s0, $sp, 10, implicit killed $q0 :: (store (s32))
735+
RET undef $lr
736+
...
737+
---
738+
#
739+
# CHECK-LABEL: name: bundled_use
740+
# CHECK: renamable $q0 = LDRQui $sp, 1
741+
# CHECK-NEXT: STRQui renamable $q0, $sp, 3, implicit-def $z0
742+
# CHECK-NEXT: BUNDLE implicit-def $z3
743+
# CHECK-NEXT: $z3 = MOVPRFX_ZZ $z19
744+
# CHECK-NEXT: $z3 = FMUL_ZPmZ_S renamable $p0, killed $z3, renamable $z0
745+
# CHECK-NEXT: }
746+
# CHECK-NEXT: renamable $q0 = LDRQui $sp, 0
747+
# CHECK-NEXT: STRQui killed renamable $q0, $sp, 2
748+
# CHECK-NEXT: RET undef $lr
749+
#
750+
name: bundled_use
751+
alignment: 4
752+
tracksRegLiveness: true
753+
frameInfo:
754+
maxAlignment: 1
755+
maxCallFrameSize: 0
756+
machineFunctionInfo:
757+
hasRedZone: false
758+
body: |
759+
bb.0.entry:
760+
liveins: $z3, $z19, $p0, $z16
761+
renamable $q0 = LDRQui $sp, 1 :: (load 16)
762+
STRQui renamable $q0, $sp, 3, implicit-def $z0 :: (store 16, basealign 128)
763+
BUNDLE implicit-def $z3, implicit-def $q3, implicit-def $d3, implicit-def $s3, implicit-def $h3, implicit-def $b3, implicit $z19, implicit $p0, implicit $z0, implicit $q0 {
764+
$z3 = MOVPRFX_ZZ $z19
765+
$z3 = FMUL_ZPmZ_S renamable $p0, killed $z3, renamable $z0
766+
}
767+
renamable $q0 = LDRQui $sp, 0 :: (load 16, align 128)
768+
STRQui killed renamable $q0, $sp, 2 :: (store 16, align 128)
769+
RET undef $lr
770+
771+
...
772+
---
773+
#
774+
# CHECK-LABEL: name: bundle_at_limit
775+
# CHECK: renamable $x3, $x4 = LDPXi $sp, 1 :: (load (s64))
776+
#
777+
name: bundle_at_limit
778+
alignment: 4
779+
tracksRegLiveness: true
780+
frameInfo:
781+
maxAlignment: 1
782+
maxCallFrameSize: 0
783+
machineFunctionInfo:
784+
hasRedZone: false
785+
body: |
786+
bb.0.entry:
787+
liveins: $x0, $x1, $x2, $z0, $p0, $z3, $z19
788+
renamable $x3 = LDRXui $sp, 2 :: (load (s64))
789+
$x2 = ADDXri renamable $x2, 1, 0
790+
$x2 = ADDXri renamable $x3, 1, 0
791+
$x2 = ADDXri renamable $x2, 1, 0
792+
$x2 = ADDXri renamable $x2, 1, 0
793+
$x2 = ADDXri renamable $x2, 1, 0
794+
$x2 = ADDXri renamable $x2, 1, 0
795+
$x2 = ADDXri renamable $x2, 1, 0
796+
$x2 = ADDXri renamable $x2, 1, 0
797+
$x2 = ADDXri renamable $x2, 1, 0
798+
$x2 = ADDXri renamable $x2, 1, 0
799+
$x2 = ADDXri renamable $x2, 1, 0
800+
$x2 = ADDXri renamable $x2, 1, 0
801+
$x2 = ADDXri renamable $x2, 1, 0
802+
$x2 = ADDXri renamable $x2, 1, 0
803+
$x2 = ADDXri renamable $x2, 1, 0
804+
$x2 = ADDXri renamable $x2, 1, 0
805+
$x2 = ADDXri renamable $x3, 1, 0
806+
BUNDLE implicit-def $z3, implicit-def $q3, implicit-def $d3, implicit-def $s3, implicit-def $h3, implicit-def $b3, implicit $z19, implicit $p0, implicit $z0 {
807+
$z3 = MOVPRFX_ZZ $z19
808+
$z3 = FMUL_ZPmZ_S renamable $p0, killed $z3, renamable $z0
809+
}
810+
$x2 = ADDXri renamable $x3, 1, 0
811+
renamable $x3 = LDRXui $sp, 1 :: (load (s64))
812+
RET undef $lr
813+
814+
...

0 commit comments

Comments
 (0)