SIL: Remove special meaning for @_semantics("stdlib_binary_only") #12237
Conversation
|
Also @milseman for the string stuff |
|
I haven't even built this locally, so of course all tests will pass! |
|
@swift-ci Please smoke test |
|
@swift-ci Please clean smoke test Linux |
slavapestov
force-pushed
the
kill-stdlib-binary-only
branch
2 times, most recently
from
3b12ae4 to
fd60335
Compare
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke benchmark |
1 similar comment
|
@swift-ci Please smoke benchmark |
Build comment file:Build failed before running benchmark. |
docs/HighLevelSILOptimizations.rst
Outdated
|
|
||
| Notice that this annotation is similar to the resilient annotation that will | ||
| disallow the cloning of code into the user application. | ||
| application for functions marked @_inlinable. This allows the optimizer to |
There was a problem hiding this comment.
s/inlinable/inlineable It's not codable =)
docs/HighLevelSILOptimizations.rst
Outdated
| disallow the cloning of code into the user application. | ||
| application for functions marked @_inlinable. This allows the optimizer to | ||
| inline calls from the stdlib and improve the performance of code that uses | ||
| common operators such as '++' or basic containers such as Array. However, |
There was a problem hiding this comment.
I know it's not your words, but you touched it, so... ++ does not exit any more.
| @inline(never) | ||
| @_inlineable |
There was a problem hiding this comment.
Wait, I thought @inline(never) and @_inlineable don't play well together, and already wanted to ask for a warning.
There was a problem hiding this comment.
@_inlineable is not about inlining, so they are actually orthogonal.
There was a problem hiding this comment.
But, @swiftix told me that @inline(__always) implies @_inlineable. I'm confused now...
There was a problem hiding this comment.
@moiseev IIRC, the current rules in the compiler work in such a way that @inline(__always) and @_transparent implies @_inlineable.
@slavapestov That's how it works today, or?
There was a problem hiding this comment.
@swiftix and @milseman are correct. I know it's a bit confusing but this is where we are at today:
@_transparentand@inline(__always)imply@_inlineable, but only if the function is public.@_inlineablejust marks the SIL function body for serialization, it's ignored by the optimizer.@inline(never)is an optimizer directive and is orthogonal to@_inlineable.
With -sil-serialize-all gone, this no longer means anything; just don't declare the function as @_inlineable instead. Fixes <rdar://problem/34564380>.
slavapestov
force-pushed
the
kill-stdlib-binary-only
branch
from
fd60335 to
0fad13e
Compare
|
@swift-ci Please smoke test |
| @_versioned // FIXME(sil-serialize-all) | ||
| @inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency | ||
| @inline(never) // Hide the CF dependency |
There was a problem hiding this comment.
IIRC The comment is solely from the lack of @_inlineable. I don't think it goes on @inline(never), but I don't know where to put it. Perhaps under the new paradigm it's just not an interesting distinction. Thoughts?
There was a problem hiding this comment.
I think the comment is still useful for future maintainers in case they decide to put @_inlineable back in?
There was a problem hiding this comment.
In theory we shouldn't need the inline-never anymore either, right?
There was a problem hiding this comment.
Without inline-never it can be inlined inside the stdlib itself.
There was a problem hiding this comment.
Right, but that should be fine now.
There was a problem hiding this comment.
Outline-and-inline-never is still a useful, albeit niche, tool inside the stdlib when Builtin.expect is insufficient.
|
@swift-ci Please clean smoke test Linux |
|
I'm not sure why testing didn't catch this, but StdlibUnittest is still using this. Reverting for now. |
With -sil-serialize-all gone, this no longer means anything; just
don't declare the function as @_inlineable instead.