Add [nonatomic] attribute to all SIL reference counting instructions.

[swiftix]

What's in this pull request?

Add [nonatomic] attribute to all SIL reference counting instructions.

Support serialization/deserialization, printing and parsing of this attribute.
Update all affected places to explicitly use the new attribute.

THIS IS WORK IN PROGRESS. DO NOT MERGE YET.

Resolved bug number: (SR-249)

---

Before merging this pull request to apple/swift repository:

• Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform	Comment
All supported platforms	@swift-ci Please smoke test
OS X platform	@swift-ci Please smoke test OS X platform
Linux platform	@swift-ci Please smoke test Linux platform

Validation Testing

Platform	Comment
All supported platforms	@swift-ci Please test
OS X platform	@swift-ci Please test OS X platform
Linux platform	@swift-ci Please test Linux platform

Note: Only members of the Apple organization can trigger swift-ci.

[swiftix]

@swift-ci Please smoke test Linux platform

[swiftix]

@swift-ci Please smoke test Linux platform

[swiftix]

@swift-ci Please smoke test Linux platform

[swiftix]

@jckarter Joe, do you mind taking a look?

This PR contains a lot of almost mechanical changes and boiler plate code to introduce the new [nonatomic] attribute for all reference counting SIL instructions. This includes:

• serialization/deserialization of SIL with this attribute
• parsing and printing SIL with this attribute
• Updating all constructors/builders to take an explicit parameter to set this attribute and updating all the relevant call sites to pass this parameter
• propagating the nonatomic flag from a SIL instruction through the IRGen internals down to the places, where the actual reference counting runtime function is picked based on this flag.

[rjmccall]

Please make the isAtomic flag an enum class : bool instead of a bare bool, either in IRGen.h or somewhere in AST/SIL.

Do you really need to pass this to the in-memory operations? You're not doing anything in SIL that would allow IRGen to ever use anything except atomic refcounting for them.

[swiftix]

@rjmccall

Please make the isAtomic flag an enum class : bool instead of a bare bool, either in IRGen.h or somewhere in AST/SIL.

Sure. Good point!

Do you really need to pass this to the in-memory operations? You're not doing anything in SIL that would allow IRGen to ever use anything except atomic refcounting for them.

I'm not sure I understand your question. Can you elaborate?

My overall plan is that there will be a SIL optimization pass which marks some of SIL refcounting instructions as nonatomic. IRGen then needs to lower them properly.

So far, I added this "isAtomic" flag to some APIs in a couple of IRGen base classes. As a result a bunch of classes derived from those had to be adjusted as well to add this additional parameter to their corresponding derived functions.

Do you mean that I changed too many classes in IRGen and some of those changes could be avoided, because they are not necessary?

[rjmccall]

I mean that you've changed the SIL ref-counting operations (retain, release, etc.), but not the SIL memory access operations (load, store, copy_addr, destroy_addr); and yet you've added the isAtomic flag to all the IRGen load/store operations.

[swiftix]

Based on @rjmccall's comment, I introduced a enum class Atomicity : bool, which is now used instead of a simple bool flag. This is definitely safer than using a boolean.

In the next update, I'm going to address the comment about the in-memory operations.

[swiftix]

@swift-ci Please smoke test Linux platform

[jckarter]

Looks good.

[swiftix]

@rjmccall I removed the atomicity flag from destroy and loadAsCopy methods.

But

• LoadableTypeInfo::copy is used by visitRetainValueInst in IRGenSIL.cpp
• LoadableTypeInfo::consume is used by visitReleaseValueInst in IRGenSIL.cpp

So, I guess consume and copy methods still need this atomicity parameter, or?

[swiftix]

@swift-ci Please smoke test Linux platform

[rjmccall]

That makes sense to me.

[swiftix]

@rjmccall OK. Thank for reviewing this PR, John! I'll test and merge.

[swiftix]

@swift-ci Please smoke test