Restore `#![no_builtins]` crates participation in LTO. by dianqk · Pull Request #113923 · rust-lang/rust

dianqk · 2023-07-21T15:53:22Z

After #113716, we can make #![no_builtins] crates participate in LTO again.

#![no_builtins] with LTO does not result in undefined references to the error. I believe this type of issue won't happen again.

Fixes #72140. Fixes #112245. Fixes #110606. Fixes #105734. Fixes #96486. Fixes #108853. Fixes #108893. Fixes #78744. Fixes #91158. Fixes rust-lang/cargo#10118. Fixes rust-lang/compiler-builtins#347.

The nightly-2023-07-20 version does not always reproduce problems due to changes in compiler-builtins, core, and user code. That's why this issue recurs and disappears.
Some issues were not tested due to the difficulty of reproducing them.

r? pnkfelix

cc @bjorn3 @japaric @alexcrichton @Amanieu

compiler/rustc_codegen_ssa/src/base.rs

dianqk · 2023-07-22T01:06:25Z

Removed opaque pointers for LLVM 14.

compiler/rustc_codegen_ssa/src/base.rs

dianqk · 2023-07-29T15:02:17Z

ping? @pnkfelix

dianqk · 2023-08-10T11:09:40Z

Rebase and remove the DisableSimplifyLibCalls parameter.

dianqk · 2023-08-15T03:57:16Z

@rustbot ready

pnkfelix · 2023-08-17T14:28:11Z

@bors r+ rollup=never

bors · 2023-08-17T14:28:13Z

📌 Commit c1ec76c has been approved by pnkfelix

It is now in the queue for this repository.

bors · 2023-08-17T16:16:02Z

⌛ Testing commit c1ec76c with merge f4589297857113e2cc3f6437f112d567b4da3694...

bors · 2023-08-17T16:49:28Z

💔 Test failed - checks-actions

dianqk · 2023-08-18T01:35:57Z

Based on the error message, I believe #78744, #91158 amd rust-lang/cargo#10118 are also not participating in the LTO issue.
But let me fix the no_builtins issue with wasm first.

It show

  (import "env" "__muloti4" (func $__muloti4 (type 0)))

dianqk · 2023-08-18T01:40:11Z

@rustbot author

dianqk · 2023-08-19T08:57:56Z

TL;DR
Using built-in functions like __multi3 does not generate calls during IR optimization.
This call is not generated until the LLVM code generation phase.
https://llvm.godbolt.org/z/aoMK4KTWT is an example.

I renamed wasm-spurious-import to wasm-builtins-import. Because we have found the essence of the problem, and it is the same issue that this PR is trying to solve. (That's why it magically disappeared.)
Now we use this test to check that the built-in functions are not missing.

An interesting attempt: I can get correct results using LLVM's LTO in lld.
I just need to change the main.main.7fc14dcaa7e3fa65-cgu.0.rcgu.o to main.main.7fc14dcaa7e3fa65-cgu.0.rcgu.lto.input.bc.

"lld" "-flavor" "wasm" "--rsp-quoting=posix" \
"--export" "multer" "--export=__heap_base" "--export=__data_end" \
"-z" "stack-size=1048576" "--stack-first" "--allow-undefined" "--fatal-warnings" "--no-demangle" "--no-entry" \
"main.main.7fc14dcaa7e3fa65-cgu.0.rcgu.lto.input.bc" \
"-o" "main.wasm" "--gc-sections" "--no-entry" "-O3"

I investigated along with this result why lld's LTO could be successful.
Then I found llvm/llvm-project@e06bac4.

Two issues in LLVM (or maybe not).

The first is InternalizePass should consider built-in functions. InternalizePass should consider built-in functions. llvm/llvm-project#65965
The second is the no-builtins should work with __multi3. https://llvm.godbolt.org/z/9vcj8dqaY "no-builtins" should work on __multi3? llvm/llvm-project#65031

nnethercote · 2023-12-02T10:22:19Z

Right, those compile-time regressions are really bad :( Lots of ~5% binary size increases, too. What can be done?

bjorn3 · 2023-12-02T10:43:52Z

I believe I saw something around symbols being marked as "used". If this is currently linker used, maybe we can downgrade it to compiler used?

dianqk · 2023-12-02T10:46:04Z

Right, those compile-time regressions are really bad :( Lots of ~5% binary size increases, too. What can be done?

I can verify that this indeed preserves builtin functions.

dianqk · 2023-12-02T10:50:31Z

It seems that linking has become 10-20ms slower and binaries ~260kb bigger. Maybe we no longer gc unused compiler builtins functions?

This should be the right way to go.

I believe I saw something around symbols being marked as "used". If this is currently linker used, maybe we can downgrade it to compiler used?

I will start debugging with your analysis this weekend. :)

Avoid adding builtin functions to `symbols.o` We found performance regressions in rust-lang#113923. The problem seems to be that `--gc-sections` does not remove these symbols. I tested that lld removes these symbols, but ld and gold do not. I found that `used` adds symbols to `symbols.o` at https://github.com/rust-lang/rust/blob/3e202ead604be31f4c1a5798a296953d3159da7e/compiler/rustc_codegen_ssa/src/back/linker.rs#L1786-L1791. The PR removes builtin functions. Note that under LTO, ld still preserves these symbols. (lld will still remove them.) The first commit also fixes rust-lang#118559. But I think the second commit also makes sense.

rylev · 2023-12-05T11:18:14Z

#118568 seems to reverse these perf regressions.

@rustbot label: +perf-regression-triaged

Avoid adding builtin functions to `symbols.o` We found performance regressions in rust-lang#113923. The problem seems to be that `--gc-sections` does not remove these symbols. I tested that lld removes these symbols, but ld and gold do not. I found that `used` adds symbols to `symbols.o` at https://github.com/rust-lang/rust/blob/3e202ead604be31f4c1a5798a296953d3159da7e/compiler/rustc_codegen_ssa/src/back/linker.rs#L1786-L1791. The PR removes builtin functions. Note that under LTO, ld still preserves these symbols. (lld will still remove them.) The first commit also fixes rust-lang#118559. But I think the second commit also makes sense.

…to, r=pnkfelix" This reverts commit 8c2b577, reversing changes made to 9cf18e9.

…enkov Revert rust-lang#113923 Per [#t-compiler/meetings > [weekly] 2024-01-11](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11) discussion, revert rust-lang#113923. Also revert associated rust-lang#118568. The PR rust-lang#113923 causes the regression issue rust-lang#118609. We need more time to find a proper solution. Discussions start at [412365838](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412365838) and continue to [412369643](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412369643). Fixes rust-lang#118609. r? compiler

…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#119817 (Remove special-casing around `AliasKind::Opaque` when structurally resolving in new solver) - rust-lang#119819 (Check rust lints when an unknown lint is detected) - rust-lang#119872 (Give me a way to emit all the delayed bugs as errors (add `-Zeagerly-emit-delayed-bugs`)) - rust-lang#119877 (Add more information to `visit_projection_elem`) - rust-lang#119884 (Rename `--env` option flag to `--env-set`) - rust-lang#119885 (Revert rust-lang#113923) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#119885 - DianQK:revert-pr-113923, r=petrochenkov Revert rust-lang#113923 Per [#t-compiler/meetings > [weekly] 2024-01-11](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11) discussion, revert rust-lang#113923. Also revert associated rust-lang#118568. The PR rust-lang#113923 causes the regression issue rust-lang#118609. We need more time to find a proper solution. Discussions start at [412365838](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412365838) and continue to [412369643](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412369643). Fixes rust-lang#118609. r? compiler

[beta] Revert rust-lang#113923 Per [#t-compiler/meetings > [weekly] 2024-01-11](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11) discussion, revert rust-lang#113923. Also revert associated rust-lang#118568. The PR rust-lang#113923 causes the regression issue rust-lang#118609. We need more time to find a proper solution. Discussions start at [412365838](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412365838) and continue to [412369643](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412369643). Fixes rust-lang#118609. Same as rust-lang#119885 but backported to beta. r? compiler

Since rust-lang/rust#113923 no_builtins crates participate in LTO too.

rustbot assigned pnkfelix Jul 21, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 21, 2023

This comment has been minimized.

Sign in to view

petrochenkov reviewed Jul 21, 2023

View reviewed changes

compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved

dianqk force-pushed the restore-no-builtins-lto branch from 5896550 to beaa1ea Compare July 21, 2023 23:49

This comment has been minimized.

Sign in to view

dianqk force-pushed the restore-no-builtins-lto branch from beaa1ea to e04af60 Compare July 22, 2023 01:04

petrochenkov reviewed Jul 22, 2023

View reviewed changes

compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved

dianqk force-pushed the restore-no-builtins-lto branch from e04af60 to 8799b43 Compare July 22, 2023 11:08

dianqk force-pushed the restore-no-builtins-lto branch from 8799b43 to c1ec76c Compare August 10, 2023 11:07

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 17, 2023

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 17, 2023

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 18, 2023

byeongkeunahn mentioned this pull request Dec 3, 2023

Windows x64 no-std build broken with errors related to compiler-builtins #118559

Closed

dianqk mentioned this pull request Dec 3, 2023

Avoid adding builtin functions to symbols.o #118568

Merged

ithinuel mentioned this pull request Dec 4, 2023

🐛 Symbol multiply defined since nightly-2023-12-02 with custom builtins. #118609

Closed

rustbot added the perf-regression-triaged The performance regression has been triaged. label Dec 5, 2023

mauricelam mentioned this pull request Dec 6, 2023

error: Did not find ELF magic number on latest nightly japaric/cargo-call-stack#109

Open

byeongkeunahn mentioned this pull request Dec 8, 2023

Shorten the generated code with ./release-rs.sh --features short boj-rs/basm-rs#40

Merged

RalfJung mentioned this pull request Dec 8, 2023

Several functions perform out-of-bounds memory accesses (which is UB) rust-lang/compiler-builtins#559

Closed

dianqk added a commit to dianqk/rust that referenced this pull request Jan 12, 2024

Revert "Auto merge of rust-lang#113923 - DianQK:restore-no-builtins-l…

aa874c5

…to, r=pnkfelix" This reverts commit 8c2b577, reversing changes made to 9cf18e9.

dianqk added a commit to dianqk/rust that referenced this pull request Jan 12, 2024

Revert "Auto merge of rust-lang#113923 - DianQK:restore-no-builtins-l…

251322e

…to, r=pnkfelix" This reverts commit 8c2b577, reversing changes made to 9cf18e9.

This was referenced Jan 12, 2024

Revert #113923 #119885

Merged

[beta] Revert #113923 #119886

Merged

GuillaumeGomez mentioned this pull request Jan 12, 2024

Rollup of 6 pull requests #119889

Merged

bjorn3 added a commit to bjorn3/c-ward that referenced this pull request Feb 16, 2024

Remove outdated statement from the readme

386448b

Since rust-lang/rust#113923 no_builtins crates participate in LTO too.

bjorn3 mentioned this pull request Feb 16, 2024

Remove outdated statement from the readme sunfishcode/c-ward#119

Merged

sunfishcode pushed a commit to sunfishcode/c-ward that referenced this pull request Feb 17, 2024

Remove outdated statement from the readme (#119)

c2efa1b

Since rust-lang/rust#113923 no_builtins crates participate in LTO too.

Zalathar mentioned this pull request Oct 11, 2024

compiletest: Remove the magic hacks for finding output with lto=thin #131524

Merged

maurer mentioned this pull request Jun 10, 2025

rustc-LLVM ERROR of ControlFlowIntegrit #142284

Open

tgross35 mentioned this pull request Jun 13, 2025

Make lto and linker-plugin-lto work the same for compiler_builtins #142323

Closed

davidsemakula mentioned this pull request Sep 2, 2025

Re-enable rustc overflow checks use-ink/cargo-contract#2116

Merged

7 tasks

Uh oh!

Conversation

dianqk commented Jul 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

dianqk commented Jul 22, 2023

Uh oh!

Uh oh!

dianqk commented Jul 29, 2023

Uh oh!

dianqk commented Aug 10, 2023

Uh oh!

dianqk commented Aug 15, 2023

Uh oh!

pnkfelix commented Aug 17, 2023

Uh oh!

bors commented Aug 17, 2023

Uh oh!

bors commented Aug 17, 2023

Uh oh!

This comment has been minimized.

bors commented Aug 17, 2023

Uh oh!

dianqk commented Aug 18, 2023

Uh oh!

dianqk commented Aug 18, 2023

Uh oh!

dianqk commented Aug 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nnethercote commented Dec 2, 2023

Uh oh!

bjorn3 commented Dec 2, 2023

Uh oh!

dianqk commented Dec 2, 2023

Uh oh!

dianqk commented Dec 2, 2023

Uh oh!

rylev commented Dec 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

dianqk commented Jul 21, 2023 •

edited

Loading

dianqk commented Aug 19, 2023 •

edited

Loading