-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[win][x64] SetFrame does not count as a stack alloc for unwind v2 #154235
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
Closed
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
Member
|
@llvm/pr-subscribers-backend-x86 Author: Daniel Paoliello (dpaoliello) ChangesFull diff: https://github.com/llvm/llvm-project/pull/154235.diff 2 Files Affected:
diff --git a/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp b/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp
index ea8b88f41bb87..0edff5333a36b 100644
--- a/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp
+++ b/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp
@@ -130,12 +130,10 @@ bool X86WinEHUnwindV2::runOnMachineFunction(MachineFunction &MF) {
break;
case X86::SEH_StackAlloc:
- case X86::SEH_SetFrame:
if (State != FunctionState::InProlog)
- llvm_unreachable("SEH_StackAlloc or SEH_SetFrame outside of prolog");
+ llvm_unreachable("SEH_StackAlloc outside of prolog");
// Assume a large alloc...
- ApproximatePrologCodeCount +=
- (MI.getOpcode() == X86::SEH_StackAlloc) ? 3 : 1;
+ ApproximatePrologCodeCount += 3;
HasStackAlloc = true;
break;
@@ -147,9 +145,10 @@ bool X86WinEHUnwindV2::runOnMachineFunction(MachineFunction &MF) {
ApproximatePrologCodeCount += 3;
break;
+ case X86::SEH_SetFrame:
case X86::SEH_PushFrame:
if (State != FunctionState::InProlog)
- llvm_unreachable("SEH_PushFrame outside of prolog");
+ llvm_unreachable("SEH_SetFrame or SEH_PushFrame outside of prolog");
ApproximatePrologCodeCount++;
break;
diff --git a/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll b/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll
index 326127a919f3a..2629b887d3228 100644
--- a/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll
+++ b/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll
@@ -171,6 +171,25 @@ define dso_local void @large_aligned_alloc() align 16 {
; CHECK-NEXT: retq
; CHECK-NEXT: .seh_endproc
+define dso_local void @set_frame_only() local_unnamed_addr {
+ tail call i64 @llvm.x86.flags.read.u64()
+ ret void
+}
+
+; CHECK-LABEL: set_frame_only:
+; CHECK: .seh_unwindversion 2
+; CHECK: .seh_pushreg %rbp
+; CHECK: .seh_setframe %rbp, 0
+; CHECK: .seh_endprologue
+; CHECK-NOT: .seh_endproc
+; CHECK: .seh_startepilogue
+; CHECK-NEXT: .seh_unwindv2start
+; CHECK-NEXT: popq %rbp
+; CHECK-NEXT: .seh_endepilogue
+; CHECK-NEXT: retq
+; CHECK-NEXT: .seh_endproc
+
+declare i64 @llvm.x86.flags.read.u64()
declare void @a() local_unnamed_addr
declare i32 @b() local_unnamed_addr
declare i32 @c(i32) local_unnamed_addr
|
70b2f1e to
2f2a902
Compare
* SetFrame does not imply that there was a stack allocation. * `mov` is used in the epilog to set the frame back, not to deallocate. It is also optional. * If the frame is set back in the epilog, then it must be done before deallocation and register popping.
Contributor
Author
|
Still working on this... scope has expanded. |
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/MC/MCWin64EH.cpp llvm/lib/Target/X86/X86WinEHUnwindV2.cppView the diff from clang-format here.diff --git a/llvm/lib/MC/MCWin64EH.cpp b/llvm/lib/MC/MCWin64EH.cpp
index 2638b9528..c198ecb71 100644
--- a/llvm/lib/MC/MCWin64EH.cpp
+++ b/llvm/lib/MC/MCWin64EH.cpp
@@ -40,10 +40,9 @@ public:
static MCUnwindV2EpilogTargetExpr *
create(const WinEH::FrameInfo &FrameInfo,
const WinEH::FrameInfo::Epilog &Epilog, uint8_t EpilogSize_,
- StringRef FunctionName_,
- MCContext &Ctx) {
+ StringRef FunctionName_, MCContext &Ctx) {
return new (Ctx) MCUnwindV2EpilogTargetExpr(FrameInfo, Epilog, EpilogSize_,
- FunctionName_);
+ FunctionName_);
}
void printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const override {
@@ -339,8 +338,8 @@ static void EmitUnwindInfo(MCStreamer &streamer, WinEH::FrameInfo *info) {
// Each epilog is emitted as a fixup, since we can't measure the distance
// between the start of the epilog and the end of the function until
// layout has been completed.
- auto *MCE = MCUnwindV2EpilogTargetExpr::create(*info, Epilog.second,
- EpilogSize, info->Function->getName(), context);
+ auto *MCE = MCUnwindV2EpilogTargetExpr::create(
+ *info, Epilog.second, EpilogSize, info->Function->getName(), context);
OS->addFixup(MCE, FK_Data_2);
OS->appendContents(2, 0);
}
@@ -386,7 +385,8 @@ bool MCUnwindV2EpilogTargetExpr::evaluateAsRelocatableImpl(
auto Offset = GetOptionalAbsDifference(*Asm, FunctionEnd, UnwindV2Start);
if (!Offset) {
Asm->getContext().reportError(
- Loc, "Failed to evaluate epilog offset for Unwind v2 in " + FunctionName);
+ Loc,
+ "Failed to evaluate epilog offset for Unwind v2 in " + FunctionName);
return false;
}
assert(*Offset > 0);
@@ -394,8 +394,9 @@ bool MCUnwindV2EpilogTargetExpr::evaluateAsRelocatableImpl(
if (*Offset > MaxEpilogOffset) {
dbgs() << "Unwindv2: " << UnwindV2Start->getSection().getName();
dbgs() << "FunctionEnd: " << FunctionEnd->getSection().getName();
- Asm->getContext().reportError(Loc,
- "Epilog offset is too large (0x" + Twine::utohexstr(*Offset) + ") for Unwind v2 in " + FunctionName);
+ Asm->getContext().reportError(
+ Loc, "Epilog offset is too large (0x" + Twine::utohexstr(*Offset) +
+ ") for Unwind v2 in " + FunctionName);
return false;
}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.