Skip to content

Add @_used and @_section attributes for global variables and top-level functions #65901

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 26, 2023
Merged

Add @_used and @_section attributes for global variables and top-level functions #65901

merged 5 commits into from
May 26, 2023

Conversation

kubamracek
Copy link
Contributor

Add @_used and @_section attributes for global variables and top-level functions

This adds:

  • @_used attribute that flags as a global variable or a top-level function as "do not dead-strip" via llvm.used, roughly the equivalent of __attribute__((used)) in C/C++.
  • @_section("...") attribute that places a global variable or a top-level function into a section with that name, roughly the equivalent of __attribute__((section("..."))) in C/C++.

@kubamracek kubamracek requested a review from eeckstein May 13, 2023 05:06
eeckstein
eeckstein reviewed May 15, 2023
View reviewed changes
include/swift/SIL/SILFunction.h Outdated

/// Returns a SectionAttr if this function belongs to a declaration that
/// has `@_section` attribute.
SectionAttr *getSectionAttr() const {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to think about inlining behavior. What should happen if a function with a section attribute is inlined into a function without (or a different) section attribute? Should inlining be prevented in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. My sense is that we should mirror what __attribute__((section)) does in Clang: It does not prevent inlining or interact with it in any way. What the attribute does is that it makes sure that the standalone/top-level function is emitted into that section, but other functions are still free to inline the code from it (as long as we also emit the standalone function that is in the right section). If the user wants to prevent inlining, they can use another attribute for that, __attribute__((noinline)) / @inline(never).

WDYT?

zoecarver
zoecarver reviewed May 16, 2023
View reviewed changes
@kubamracek kubamracek marked this pull request as ready for review May 19, 2023 23:43
@kubamracek kubamracek requested a review from jckarter May 19, 2023 23:44
@kubamracek kubamracek force-pushed the used-and-section branch 7 times, most recently from 7d2e1e5 to 888f675 Compare May 22, 2023 14:52
@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

As @jckarter pointed out to me, it's needed to use different section names for different platforms / object file formats, but luckily this can already be done by conditionalizing the attribute:

#if os(...)
@_section("...")
#elif os(...)
@_section("...")
#endif
func foo()

@kubamracek
Copy link
Contributor Author

Notes to self: I need to also update (1) UnderscoredAttributes.md, (2) the attribute list in SwiftSyntax.

@hborla
Copy link
Member

hborla commented May 24, 2023

I don't see a new experimental feature for these attributes. Can you please gate them behind a new EXPERIMENTAL_FEATURE identifier (declared in Features.def)?

eeckstein
eeckstein approved these changes May 24, 2023
View reviewed changes
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.

lgtm!

@kubamracek kubamracek requested a review from tshortli as a code owner May 24, 2023 23:29
tshortli
tshortli reviewed May 24, 2023
View reviewed changes
@kubamracek
Copy link
Contributor Author

I don't see a new experimental feature for these attributes. Can you please gate them behind a new EXPERIMENTAL_FEATURE identifier (declared in Features.def)?

Added.

@kubamracek
Copy link
Contributor Author

@swift-ci please test

2 similar comments
@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

@swift-ci please test

kubamracek added 5 commits May 26, 2023 07:10
@kubamracek
Add @_used and @_section attributes for global variables and top-leve…
80222b8 
…l functions

This adds:
- @_used attribute that flags as a global variable or a top-level function as
  "do not dead-strip" via llvm.used, roughly the equivalent of
  __attribute__((used)) in C/C++.
- @_section("...") attribute that places a global variable or a top-level
  function into a section with that name, roughly the equivalent of
  __attribute__((section("..."))) in C/C++.
@kubamracek
Add EXPERIMENTAL_FEATURE(SymbolLinkageMarkers, true)
6f60b67
@kubamracek
Disallow non-constant globals with @_section attribute
cccc281
@kubamracek
Implement usesFeatureSymbolLinkageMarkers
0789ce9
@kubamracek
Adjust section.swift test
a8e8c41
@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek kubamracek merged commit 2d5f33e into swiftlang:main May 26, 2023
@kubamracek kubamracek deleted the used-and-section branch May 26, 2023 21:02
@finagolfin finagolfin mentioned this pull request May 27, 2023
Merged
@kubamracek kubamracek mentioned this pull request May 28, 2023
Closed
@dreampiggy
Copy link

Finally😂

@thedavidharris thedavidharris mentioned this pull request Jun 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zoecarver zoecarver zoecarver left review comments

@jckarter jckarter jckarter approved these changes

@eeckstein eeckstein eeckstein approved these changes

@tshortli tshortli tshortli approved these changes

@hborla hborla Awaiting requested review from hborla hborla is a code owner

@slavapestov slavapestov Awaiting requested review from slavapestov slavapestov is a code owner

@xedin xedin Awaiting requested review from xedin xedin is a code owner

@ahoppen ahoppen Awaiting requested review from ahoppen ahoppen is a code owner

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

@CodaFi CodaFi Awaiting requested review from CodaFi CodaFi is a code owner

@DougGregor DougGregor Awaiting requested review from DougGregor DougGregor is a code owner

@rintaro rintaro Awaiting requested review from rintaro rintaro is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@kubamracek @hborla @dreampiggy @jckarter @zoecarver @eeckstein @tshortli