Skip to content

[SIL] Added lexical flag to alloc_stack. #39291

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

nate-chandler
Copy link
Contributor

The new flag will be used to track whether a borrow scope corresponds to a source-level lexical scope. Here, the flag is just documented, added to the instruction, represented in textual and serialized SIL, and cloned.

The new flag will be used to track whether a borrow scope corresponds to
a source-level lexical scope.  Here, the flag is just documented, added
to the instruction, represented in textual and serialized SIL, and
cloned.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - b57b222

@nate-chandler
Copy link
Contributor Author

@swift-ci please clean test linux platform

@nate-chandler nate-chandler merged commit c3e7626 into swiftlang:main Sep 14, 2021
@nate-chandler nate-chandler deleted the lexical_lifetimes/alloc_stack/lexical_flag branch September 14, 2021 15:47
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nate-chandler my post-merge review

@@ -3102,6 +3102,9 @@ The ``dynamic_lifetime`` attribute specifies that the initialization and
destruction of the stored value cannot be verified at compile time.
This is the case, e.g. for conditionally initialized objects.

The optional ``lexical`` attribute specifies that the operand corresponds to a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alloc_stack does not have an operand.

How is lexical different to debug-var-attr? Doesn't alloc_stack correspond to a variable if debug-var-attr is present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fixed at #39484 .

Is the debug-var-attr always present? If it is, @atrick may know the answer. At worst, this might be duplicative and we'll need to tweak the change to Mem2Reg.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general rule, debug information is not supposed to affect semantics. If we decide to reuse the same information for both purposes, then we need to specify in the class and API that it is not simply for debug information, audit, and verify the implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, I would prefer to not have redundant information in the instruction attributes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adrian-prantl do you know in which cases debug-var-attr is present?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug-var-attr is present iff the alloc_stack is the only storage location for a (part of a) source variable. It is there regardless of whether -g is on or not. There can be alloc_stack instructions that don't have a debug-var-attr, in purely compiler-generated code (thunks, etc).

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.

5 participants