AArch64: support extended spills in SEH on WoS #166849
Open
+79
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-backend-aarch64 Author: Saleem Abdulrasool (compnerd) ChangesWhen lowering code for Windows, we might be using a non-standard calling convention (e.g. This adds support for the handling for these spills but is insufficient on its own. The encoded results are incorrect due to the expectation that the pair wise spills are always 16-byte aligned which we currently do not enforce. Fixing that is beyond the scope of emitting the SEH directives for the spill. Full diff: https://github.com/llvm/llvm-project/pull/166849.diff 6 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index c31a090bba77f..e8766bc1b8c62 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -3364,6 +3364,22 @@ void AArch64AsmPrinter::emitInstruction(const MachineInstr *MI) {
TS->emitARM64WinCFIPACSignLR();
return;
+ case AArch64::SEH_SaveAnyRegI:
+ assert(MI->getOperand(1).getImm() <= 1008 &&
+ "SaveAnyRegQP SEH opcode offset must fit into 6 bits");
+ TS->emitARM64WinCFISaveAnyRegI(MI->getOperand(0).getImm(),
+ MI->getOperand(1).getImm());
+ return;
+
+ case AArch64::SEH_SaveAnyRegIP:
+ assert(MI->getOperand(1).getImm() - MI->getOperand(0).getImm() == 1 &&
+ "Non-consecutive registers not allowed for save_any_reg");
+ assert(MI->getOperand(2).getImm() <= 1008 &&
+ "SaveAnyRegQP SEH opcode offset must fit into 6 bits");
+ TS->emitARM64WinCFISaveAnyRegIP(MI->getOperand(0).getImm(),
+ MI->getOperand(2).getImm());
+ return;
+
case AArch64::SEH_SaveAnyRegQP:
assert(MI->getOperand(1).getImm() - MI->getOperand(0).getImm() == 1 &&
"Non-consecutive registers not allowed for save_any_reg");
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 3ee4d58ca892c..70c5c29149288 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -1082,14 +1082,24 @@ AArch64FrameLowering::insertSEH(MachineBasicBlock::iterator MBBI,
case AArch64::LDPXi: {
Register Reg0 = MBBI->getOperand(0).getReg();
Register Reg1 = MBBI->getOperand(1).getReg();
+
+ int SEHReg0 = RegInfo->getSEHRegNum(Reg0);
+ int SEHReg1 = RegInfo->getSEHRegNum(Reg1);
+
if (Reg0 == AArch64::FP && Reg1 == AArch64::LR)
MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveFPLR))
.addImm(Imm * 8)
.setMIFlag(Flag);
- else
+ else if (SEHReg0 >= 19 && SEHReg1 >= 19)
MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveRegP))
- .addImm(RegInfo->getSEHRegNum(Reg0))
- .addImm(RegInfo->getSEHRegNum(Reg1))
+ .addImm(SEHReg0)
+ .addImm(SEHReg1)
+ .addImm(Imm * 8)
+ .setMIFlag(Flag);
+ else
+ MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveAnyRegIP))
+ .addImm(SEHReg0)
+ .addImm(SEHReg1)
.addImm(Imm * 8)
.setMIFlag(Flag);
break;
@@ -1097,10 +1107,16 @@ AArch64FrameLowering::insertSEH(MachineBasicBlock::iterator MBBI,
case AArch64::STRXui:
case AArch64::LDRXui: {
int Reg = RegInfo->getSEHRegNum(MBBI->getOperand(0).getReg());
- MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveReg))
- .addImm(Reg)
- .addImm(Imm * 8)
- .setMIFlag(Flag);
+ if (Reg >= 19)
+ MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveReg))
+ .addImm(Reg)
+ .addImm(Imm * 8)
+ .setMIFlag(Flag);
+ else
+ MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveAnyRegI))
+ .addImm(Reg)
+ .addImm(Imm * 8)
+ .setMIFlag(Flag);
break;
}
case AArch64::STRDui:
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index ccc8eb8a9706d..4b4073365483e 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1217,6 +1217,8 @@ bool AArch64InstrInfo::isSEHInstruction(const MachineInstr &MI) {
case AArch64::SEH_EpilogStart:
case AArch64::SEH_EpilogEnd:
case AArch64::SEH_PACSignLR:
+ case AArch64::SEH_SaveAnyRegI:
+ case AArch64::SEH_SaveAnyRegIP:
case AArch64::SEH_SaveAnyRegQP:
case AArch64::SEH_SaveAnyRegQPX:
case AArch64::SEH_AllocZ:
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 2871a20e28b65..e6954f75b1a6a 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -5666,6 +5666,8 @@ let isPseudo = 1 in {
def SEH_EpilogStart : Pseudo<(outs), (ins), []>, Sched<[]>;
def SEH_EpilogEnd : Pseudo<(outs), (ins), []>, Sched<[]>;
def SEH_PACSignLR : Pseudo<(outs), (ins), []>, Sched<[]>;
+ def SEH_SaveAnyRegI : Pseudo<(outs), (ins i32imm:$reg0, i32imm:$offs), []>, Sched<[]>;
+ def SEH_SaveAnyRegIP : Pseudo<(outs), (ins i32imm:$reg0, i32imm:$reg1, i32imm:$offs), []>, Sched<[]>;
def SEH_SaveAnyRegQP : Pseudo<(outs), (ins i32imm:$reg0, i32imm:$reg1, i32imm:$offs), []>, Sched<[]>;
def SEH_SaveAnyRegQPX : Pseudo<(outs), (ins i32imm:$reg0, i32imm:$reg1, i32imm:$offs), []>, Sched<[]>;
def SEH_AllocZ : Pseudo<(outs), (ins i32imm:$offs), []>, Sched<[]>;
diff --git a/llvm/lib/Target/AArch64/AArch64PrologueEpilogue.cpp b/llvm/lib/Target/AArch64/AArch64PrologueEpilogue.cpp
index 4df4d54e60c95..965585f40571b 100644
--- a/llvm/lib/Target/AArch64/AArch64PrologueEpilogue.cpp
+++ b/llvm/lib/Target/AArch64/AArch64PrologueEpilogue.cpp
@@ -253,6 +253,8 @@ static void fixupSEHOpcode(MachineBasicBlock::iterator MBBI,
case AArch64::SEH_SaveReg:
case AArch64::SEH_SaveFRegP:
case AArch64::SEH_SaveFReg:
+ case AArch64::SEH_SaveAnyRegI:
+ case AArch64::SEH_SaveAnyRegIP:
case AArch64::SEH_SaveAnyRegQP:
case AArch64::SEH_SaveAnyRegQPX:
ImmOpnd = &MBBI->getOperand(ImmIdx);
diff --git a/llvm/test/MC/AArch64/seh-extended-spills.ll b/llvm/test/MC/AArch64/seh-extended-spills.ll
new file mode 100644
index 0000000000000..a27808531396c
--- /dev/null
+++ b/llvm/test/MC/AArch64/seh-extended-spills.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple aarch64-unknown-windows-msvc -filetype asm -o - %s | FileCheck %s
+
+declare dso_local void @g(ptr noundef)
+define dso_local preserve_mostcc void @f(ptr noundef %p) #0 {
+entry:
+ %p.addr = alloca ptr, align 8
+ store ptr %p, ptr %p.addr, align 8
+ %0 = load ptr, ptr %p.addr, align 8
+ call void @g(ptr noundef %0)
+ ret void
+}
+
+attributes #0 = { nounwind uwtable(sync) }
+
+; CHECK: stp x9, x10, [sp, #[[OFFSET_0:[0-9]+]]]
+; CHECK-NEXT: .seh_save_any_reg_p x9, [[OFFSET_0]]
+; CHECK: stp x11, x12, [sp, #[[OFFSET_1:[0-9]+]]]
+; CHECK-NEXT: .seh_save_any_reg_p x11, [[OFFSET_1]]
+; CHECK: stp x13, x14, [sp, #[[OFFSET_2:[0-9]+]]]
+; CHECK-NEXT: .seh_save_any_reg_p x13, [[OFFSET_2]]
+; CHECK: str x15, [sp, #[[OFFSET_3:[0-9]+]]]
+; CHECK-NEXT: .seh_save_any_reg x15, [[OFFSET_3]]
|
|
Can you clarify the "WoS" acronym here? |
| @@ -0,0 +1,22 @@ | |||
| ; RUN: llc -mtriple aarch64-unknown-windows-msvc -filetype asm -o - %s | FileCheck %s | |||
There was a problem hiding this comment.
IR tests belong in llvm/test/Codegen/AArch64; MC is reserved for assembler tests (like checking if .seh_save_any_reg_p is encoded correctly in the object file).
I think I'd prefer to use update_llc_test_checks.py here, even if that ends up hardcoding the offsets. We probably want to be aware if some code change modifies the offsets we use for a simple testcase like this.
There was a problem hiding this comment.
I can certainly update the test to use update_llc_test_checks.py but that feels like it is less clear what it is testing. The current incantation explicitly shows that we are looking at the spills and associated SEH opcodes.
There was a problem hiding this comment.
If we don't write specific offsets, we also can't tell if the offset is a multiple of 16.
In any case, please also add the corresponding checks for the epilogue.
There was a problem hiding this comment.
Quite so, but that I think should be a follow up change (I need to change how the stack spills occur). That is going to be a bit more invasive.
I can absolutely add the epilogue checks though!
When lowering code for Windows, we might be using a non-standard calling convention (e.g. `preserve_most`). In such a case, we might be spilling registers which are unexpected (i.e. x9-x15). Use the ARM64EC opcodes to indicate such spills. This adds support for the handling for these spills but is insufficient on its own. The encoded results are incorrect due to the expectation that the pair wise spills are always 16-byte aligned which we currently do not enforce. Fixing that is beyond the scope of emitting the SEH directives for the spill.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When lowering code for Windows, we might be using a non-standard calling convention (e.g.
preserve_most). In such a case, we might be spilling registers which are unexpected (i.e. x9-x15). Use the ARM64EC opcodes to indicate such spills.This adds support for the handling for these spills but is insufficient on its own. The encoded results are incorrect due to the expectation that the pair wise spills are always 16-byte aligned which we currently do not enforce. Fixing that is beyond the scope of emitting the SEH directives for the spill.