Skip to content

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

Conversation

felipepiovezan
Copy link

@felipepiovezan felipepiovezan commented Sep 18, 2023

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.

felipepiovezan and others added 8 commits September 18, 2023 06:47
…form when producing DWARF"

This reverts commit 7c6af65.
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)
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)
@felipepiovezan felipepiovezan force-pushed the felipe/cherry-pick-stable branch from e26bc4e to e086990 Compare September 18, 2023 17:26
@felipepiovezan
Copy link
Author

@swift-ci please test llvm

@felipepiovezan felipepiovezan merged commit 2037e2a into swiftlang:stable/20230725 Sep 18, 2023
@felipepiovezan felipepiovezan deleted the felipe/cherry-pick-stable branch September 18, 2023 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants