forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 342
Cherry pick debug info fixes for single-location debug values #7486
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
Merged
felipepiovezan
merged 8 commits into
swiftlang:stable/20230725
from
felipepiovezan:felipe/cherry-pick-stable
Sep 18, 2023
Merged
Cherry pick debug info fixes for single-location debug values #7486
felipepiovezan
merged 8 commits into
swiftlang:stable/20230725
from
felipepiovezan:felipe/cherry-pick-stable
Sep 18, 2023
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
…form when producing DWARF" This reverts commit 7c6af65.
…t location kinds" This reverts commit c1db101.
Only a subset of the fields of DbgVariable are meaningful at any time, and some fields are re-used for multiple purposes (for example FrameIndexExprs is used with a throw-away frame-index of 0 to hold a single DIExpression without needing to add another member). The exact invariants must be reverse-engineered by inspecting the actual use of the class, its imprecise/outdated doc-comment, and some asserts. Refactor DbgVariable into a sum type by inheriting from std::variant. This makes the active fields for any given state explicit and removes the need to re-use fields in disparate contexts. As a bonus, it seems to reduce the size on my x86_64 linux box from 144 bytes to 96 bytes. There is some potential cost to `std::get` as it must check the active alternative even when context or an assert obviates it. To try to help ensure the compiler can optimize out the checks the patch also adds a helper `get` method which uses the noexcept `std::get_if`. Some of the extra cost would also be avoided more cleanly with a refactor that exposes the alternative types in the public interface, which will come in another patch. Differential Revision: https://reviews.llvm.org/D158675 (cherry picked from commit 58c108c)
…copeVariable Differential Revision: https://reviews.llvm.org/D158676 (cherry picked from commit 414ceff)
Differential Revision: https://reviews.llvm.org/D158677 (cherry picked from commit 35e621f)
This potentially has a slightly positive performance impact, as std::visit can be implemented as a `switch`-like jump rather than a series of `if`s. More importantly, the reader can be confident is no overlap between the cases. Differential Revision: https://reviews.llvm.org/D158678 (cherry picked from commit 4333146)
…on kinds The AsmPrinter currently assumes that a Debug Variable will have all of its fragments with the same "kind" of location (i.e. all in the stack or all in entry values). This is not enforced by the verifier, so it needs to be handled properly. Until we do so, we conservatively drop one of the fragments. Differential Revision: https://reviews.llvm.org/D159468 (cherry picked from commit bc5dac1)
…n producing DWARF Revision c383f4d enabled using variadic-form debug values to represent single-location, non-stack-value debug values, and a further patch made all DBG_INSTR_REFs use variadic form. Not all code paths were updated correctly to handle the new syntax however, with entry values in still expecting an expression that begins exactly DW_OP_LLVM_entry_value, 1. A function already exists to select non-variadic-like expressions; this patch adds an extra function to cheaply simplify such cases to non-variadic form, which we use prior to any entry-value processing to put DBG_INSTR_REFs and DBG_VALUEs down the same code path. We also use it for a few DIExpression functions that check for whether the first element(s) of a DIExpression match a particular pattern, so that they will return the same result for DIExpression(DW_OP_LLVM_arg, 0, <ops>) as for DIExpression(<ops>). Differential Revision: https://reviews.llvm.org/D158185 (cherry picked from commit 9811ffe)
e26bc4e
to
e086990
Compare
@swift-ci please test llvm |
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.
We merged D158185 to this branch prior to it being merged upstream. The commit changed slightly in the meantime, so we revert and cherry-pick again to reflect what is actually upstream.
The same is done with D159468. This one also requires the 4 NFC patches included here.