Function addresses are significant - #162209
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Function addresses are significant
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (ed03c85): comparison URL. Overall result: ❌ regressions - BENCHMARK(S) FAILEDBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf ❗ ❗ ❗ ❗ ❗
❗ ❗ ❗ ❗ ❗ Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.4%, secondary 3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 2.4%, secondary 3.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: missing data |
98ca4e3 to
f234a14
Compare
|
Looks like rustc-perf has some trouble running rustc benchmark at the moment (not specific to this pull request):
Let's try again. @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Function addresses are significant
|
The job Click to see the possible cause of the failure (guessed by this bot) |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (86f1ad5): comparison URL. Overall result: ❌ regressions - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.7%, secondary -0.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.0%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 2.3%, secondary 3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 477.338s -> 498.097s (4.35%) |
|
That is some significant bnary size regressions. And that is even without linker ICF which we currently don't do, but without unnamed_addr would never be able to safely enable. |
|
(fat fingered) |
|
Could it be done only for cases where someone does But this wouldn't perfect, in particular for cross-crate, but better a best-effort than nothing? |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
For generic functions I think we really need the attribute. We could consider removing it for functions that are monomorphized only once. Historically that was all functions with only lifetime generics, but these days we mark some functions as cross-crate-inlinable and then they get monomorphzed downstream... |
|
Placing a method in a compiler generated vtable doesn't actually make its address significant, right? This is an aspect that LLVM doesn't understand at the moment, that would prevent it from inferring |
It's up to us to define what makes the address significant... it is unclear which user-visible behavior you are trying to achieve. But, we say that it is UB to read the memory that stores a vtable, so whatever is stored there is not observable by the program. So in that sense the address cannot be significant, yeah. |
|
☔ The latest upstream changes (presumably #162333) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
View all comments
A perf experiment to measure the impact of making function addresses significant.