Skip to content

SIL: fix memory behavior of global_addr for globals with non-fixed layout. #31205

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
merged 1 commit into from
Apr 22, 2020

Conversation

eeckstein
Copy link
Contributor

@eeckstein eeckstein commented Apr 22, 2020

Global variables with resilient types might be allocated into a buffer and not statically in the data segment.
In this case, the global_addr depends on alloc_global being executed first.
Letting global_addr have a side effect ensures that this dependency is kept.
It prevents e.g. LICM to move a global_addr out of a loop while keeping the alloc_global inside the loop.

rdar://problem/61602640

…yout.

Global variables with resilient types might be allocated into a buffer and not statically in the data segment.
In this case, the global_addr depends on alloc_global being executed first.
We model this by letting global_addr have a side effect.
It prevents e.g. LICM to move a global_addr out of a loop while keeping the alloc_global inside the loop.

rdar://problem/61602640
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein eeckstein requested a review from slavapestov April 22, 2020 14:40
Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

Nice catch!

// It prevents e.g. LICM to move a global_addr out of a loop while keeping
// the alloc_global inside the loop.
SILModule &M = ga->getFunction()->getModule();
auto expansion = TypeExpansionContext::maximal(M.getAssociatedContext(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Since IRGen makes a similar calculation when lowering the instruction to decide if it should allocate the box or not, should we factor it out somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IRGen uses the TypeInfo machinery for this, which is only available in IRgen

@eeckstein eeckstein merged commit ceae3dc into swiftlang:master Apr 22, 2020
@eeckstein eeckstein deleted the fix-global_addr branch April 22, 2020 19:31
@compnerd
Copy link
Member

FYI: This broke the Android CI

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