naked functions: respect function-sections
#147811
Conversation
|
Some changes occurred in compiler/rustc_codegen_ssa |
rustbot
added
S-waiting-on-review
folkertdev
changed the title
function-sections
folkertdev
force-pushed
the
naked-function-sections
branch
from
2ebd197 to
ac4b179
Compare
folkertdev
force-pushed
the
naked-function-sections
branch
from
ac4b179 to
6580a5f
Compare
| if let Some(section) = &link_section { | ||
| writeln!(begin, ".pushsection {section},\"xr\"").unwrap() | ||
| } else if function_sections { | ||
| writeln!(begin, ".pushsection .text${asm_name},\"xr\"").unwrap() |
There was a problem hiding this comment.
On -msvc targets, function_sections is ignored. .text$sym is only used on -gnu targets.
There was a problem hiding this comment.
Also LLVM currently generates the following line on COFF targets:
.section {section},"xr",one_only,{sym},unique,0`
Should we be doing the same here?
There was a problem hiding this comment.
On -msvc targets, function_sections is ignored.
Meaning that it's just always assumed to be on? https://godbolt.org/z/a6ofW49YK
.text$symis only used on -gnu targets.
It should still work for msvc, no?
Should we be doing the same here?
I don't think we can reliably emit the unique,id bit of that line because how do we make that ID?
There was a problem hiding this comment.
Actually using -Zfunction-sections=no on msvc does have an effect. So I'm not really sure what it being ignored means then. Maybe this is a more recent LLVM change?
There was a problem hiding this comment.
No, it's always assumed to be off on msvc. You can see it always uses .text.
There was a problem hiding this comment.
It does always use .text but normally it emits the line you quoted:
.section .text,"xr",one_only,foo,unique,0
i.e. it uses a subsection which, as far as I understand, does allow DCE, with -Zfunction-sections=no it emits just
.text
So then DCE is impossible
|
☔ The latest upstream changes (presumably #148305) made this pull request unmergeable. Please resolve the merge conflicts. |
folkertdev
force-pushed
the
naked-function-sections
branch
from
6580a5f to
b12830c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Looks like windows will also be using function sections by default soon #148669 |
For `gnu` function_sections is off by default.
folkertdev
force-pushed
the
naked-function-sections
branch
from
b12830c to
77de006
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This is unlikely to happen, as you can see crashes in failed tests in that PR. That is what I was talking about in #147789 (comment) |
7 participants
fixes #147789
r? @Amanieu