Skip to content

Conversation

jckarter
Copy link
Contributor

The resilience model forces inlinable code to use resiliently conservative access paths for properties in the same module, but some stdlib methods by design escapes storage of a global variable. The additional Builtin.addressof validation introduced in #15608 exposed the latent issue here.

The resilience model forces inlinable code to use resiliently conservative access paths for properties in the same module, but some stdlib methods by design escapes storage of a global variable. The additional `Builtin.addressof` validation introduced in swiftlang#15608 exposed the latent issue here.
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter
Copy link
Contributor Author

@milseman Is this acceptable, or should we make a way for stdlib code to promise global variables will be permanently stored? As is, I would at least expect us doing one dylib call to get the address would be slightly better than going through the conservative property access pattern.

@milseman
Copy link
Member

Eep, so @_inlineable on a computed value sends accesses to stored properties in its body down a resilient slow path? How should we reason about whether to mark something @_inlineable?

The goal here is to have a unique bit pattern to serve as an empty string without spare capacity. An address was just one means, but we'd like to be able to inline String.isEmpty's fast-path into user code. @lorentey tried some other approaches, but I don't remember his results. I'm otherwise indifferent how this is done.

@slavapestov
Copy link
Contributor

You can promise a global variable remains stored with @_fixed_layout. Then inlinable function bodies will address it directly. The more resilient approach is to make the globals non-public and only use them from non-inlinable function bodies.

@slavapestov
Copy link
Contributor

The reason for the restriction is that if the global is changed from stored to computed, other clients may have inlined the inlinable function, so we have no choice but to be conservative here.

Also, using a materializeForSet-like accessor for globals will solve the problem - @rjmccall was working on that IIRC.

@jckarter jckarter merged commit d4fa141 into swiftlang:swift-4.2-branch Mar 31, 2018
@milseman
Copy link
Member

milseman commented Apr 1, 2018

Did this have a corresponding master PR?

edit: yes: #15632

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