-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AArch64] Restore Z-registers before P-registers #79623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
73803bf
3a4ed50
f93078f
48e3e6b
c62ecd8
4c6aa9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3186,11 +3186,6 @@ bool AArch64FrameLowering::restoreCalleeSavedRegisters( | |
return MIB->getIterator(); | ||
}; | ||
|
||
// SVE objects are always restored in reverse order. | ||
for (const RegPairInfo &RPI : reverse(RegPairs)) | ||
if (RPI.isScalable()) | ||
EmitMI(RPI); | ||
|
||
if (homogeneousPrologEpilog(MF, &MBB)) { | ||
auto MIB = BuildMI(MBB, MBBI, DL, TII.get(AArch64::HOM_Epilog)) | ||
.setMIFlag(MachineInstr::FrameDestroy); | ||
|
@@ -3201,11 +3196,19 @@ bool AArch64FrameLowering::restoreCalleeSavedRegisters( | |
return true; | ||
} | ||
|
||
// For performance reasons restore SVE register in increasing order | ||
auto IsPPR = [](const RegPairInfo &c) { return c.Type == RegPairInfo::PPR; }; | ||
auto PPRBegin = std::find_if(RegPairs.begin(), RegPairs.end(), IsPPR); | ||
auto PPREnd = std::find_if(RegPairs.rbegin(), RegPairs.rend(), IsPPR); | ||
std::reverse(PPRBegin, PPREnd.base()); | ||
auto IsZPR = [](const RegPairInfo &c) { return c.Type == RegPairInfo::ZPR; }; | ||
auto ZPRBegin = std::find_if(RegPairs.begin(), RegPairs.end(), IsZPR); | ||
auto ZPREnd = std::find_if(RegPairs.rbegin(), RegPairs.rend(), IsZPR); | ||
std::reverse(ZPRBegin, ZPREnd.base()); | ||
|
||
if (ReverseCSRRestoreSeq) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If That said, I think this option is never really used other than in some of the tests. Which makes me wonder, can we remove it? @francisvm There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can remove it! I added it for experiments at first and it didn't turn out useful in the end. |
||
MachineBasicBlock::iterator First = MBB.end(); | ||
for (const RegPairInfo &RPI : reverse(RegPairs)) { | ||
if (RPI.isScalable()) | ||
continue; | ||
MachineBasicBlock::iterator It = EmitMI(RPI); | ||
if (First == MBB.end()) | ||
First = It; | ||
|
@@ -3214,8 +3217,6 @@ bool AArch64FrameLowering::restoreCalleeSavedRegisters( | |
MBB.splice(MBBI, &MBB, First); | ||
} else { | ||
for (const RegPairInfo &RPI : RegPairs) { | ||
if (RPI.isScalable()) | ||
continue; | ||
(void)EmitMI(RPI); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing a crash here in a downstream project, running on Windows compiled via MSVC.
attached
.bc
and.ll
output fromllvm-dis [file.bc]
: 79623_repro_files.zipIf I revert 3f0404a, the crash goes away.