Skip to content

[IRGenDebugInfo] Fix usage of dbg.declare #73769

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 23, 2024
Merged

[IRGenDebugInfo] Fix usage of dbg.declare #73769

merged 2 commits into from
May 23, 2024

Conversation

Snowy1803
Copy link
Member

@Snowy1803 Snowy1803 commented May 21, 2024 •
edited
Loading

  • Allow multiple dbg.declare on a single alloca: This is unnecessarily dropping debug info. The sharing of stack slot can happen because of AllocStackHoisting, which is run at -Onone too.
  • Disable usage of dbg.declare in optimized code: It is incorrect in some cases to emit a dbg.declare, as the same variable appearing in both a dbg.declare and another debug intrinsic is not allowed, which could happen in optimized code.

@Snowy1803
[IRGenDebugInfo] Allow multiple dbg.declare on an alloca
2726d83 
This is unnecessarily dropping debug info, as there is currently
no assertion in LLVM. The sharing of stack slot can happen
because of AllocStackHoisting, which is run at -Onone too.
@Snowy1803 Snowy1803 requested a review from adrian-prantl as a code owner May 21, 2024 00:01
@Snowy1803
Copy link
Member Author

@swift-ci please test

@Snowy1803
Copy link
Member Author

@swift-ci please test source compatibility

@adrian-prantl
Copy link
Contributor

We're getting

Assertion failed: (FragmentOffset >= OffsetInBits && "overlapping or duplicate fragments"), function addFragmentOffset, file DwarfExpression.cpp, line 686.

This is something I've been meaning to fix at the LLVM level (rdar://124625723) — LLVM should be able to just split two overlapping fragments into two.

@Snowy1803
Copy link
Member Author

In SwiftArgumentParser, we also have a

Assertion failed: ((FrameIndexExprs.size() == 1 || llvm::all_of(FrameIndexExprs, [](FrameIndexExpr &FIE) { return FIE.Expr && FIE.Expr->isFragment(); })) && "conflicting locations for variable"), function addFrameIndexExpr, file DwarfDebug.cpp, line 322.

which is the same as rdar://128155050

@Snowy1803 Snowy1803 mentioned this pull request May 22, 2024
Closed
@Snowy1803
Copy link
Member Author

@swift-ci please test

@Snowy1803
Copy link
Member Author

@swift-ci please test source compatibility

@Snowy1803
[IRGenDebugInfo] Disable usage of dbg.declare in optimized code
5606fbc 
Except for the async context, where it is needed (arguments
within an async function).

We don't support dbg.declare in optimized code, as variables can
be moved by SIL optimization passes. If a partial store is
eliminated, we want a dbg.value on the allocation, and another
dbg.value with a fragment in place of the partial store.

rdar://128155050
@Snowy1803 Snowy1803 force-pushed the irgen-declare-all branch from 425dbc0 to 5606fbc Compare May 22, 2024 16:36
@Snowy1803
Copy link
Member Author

Source compatibility tests are passing, fixed the one failing AutoDiff test

@Snowy1803
Copy link
Member Author

@swift-ci please smoke test

@Snowy1803 Snowy1803 changed the title [IRGenDebugInfo] Allow multiple dbg.declare on an alloca [IRGenDebugInfo] Fix usage of dbg.declare May 22, 2024
@Snowy1803 Snowy1803 requested a review from adrian-prantl May 22, 2024 16:43
@Snowy1803
Copy link
Member Author

@swift-ci please test

@adrian-prantl adrian-prantl enabled auto-merge May 22, 2024 17:06
@Snowy1803 Snowy1803 disabled auto-merge May 22, 2024 17:32
@Snowy1803
Copy link
Member Author

@swift-ci please test macOS platform

@adrian-prantl adrian-prantl merged commit ece1256 into swiftlang:main May 23, 2024
4 of 5 checks passed
@hamishknight hamishknight mentioned this pull request May 23, 2024
Merged
@Snowy1803 Snowy1803 mentioned this pull request May 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrian-prantl adrian-prantl Awaiting requested review from adrian-prantl adrian-prantl is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Snowy1803 @adrian-prantl