Skip to content

Force code generation in assembly generation smoke-tests #135088

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 4, 2025
Merged

Force code generation in assembly generation smoke-tests #135088

merged 1 commit into from
Jan 4, 2025

Conversation

maurer
Copy link
Contributor

@maurer maurer commented Jan 3, 2025

In llvm/llvm-project@7b23f41 , .text started being suppressed from LLVM assembly in cases where it wasn't strictly necessary. Currently, the sample functions in these two tests are frequently decided to be IR-only functions, resulting in no code generation, so LLVM drops the .text directive.

Adding #[no_mangle] forces these tests back to their original intent - assembly code is generated, and so a .text directive is generated as well.

@rustbot label: +llvm-main
r? @workingjubilee

I'll attach a buildbot link once it finishes consuming this PR

@maurer
Force code generation in assembly generation smoke-tests
cf679e4 
In llvm/llvm-project@7b23f41 , `.text`
started being suppressed from LLVM assembly in cases where it wasn't
strictly necessary. Currently, the sample functions in these two tests
are frequently decided to be IR-only functions, resulting in no code
generation, so LLVM drops the `.text` directive.

Adding `#[no_mangle]` forces these tests back to their original intent -
assembly code is generated, and so a `.text` directive is generated as
well.
@rustbot
Copy link
Collaborator

rustbot commented Jan 3, 2025

Could not assign reviewer from: workingjubilee.
User(s) workingjubilee are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Jan 3, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot 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. llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) labels Jan 3, 2025
@maurer
Copy link
Contributor Author

maurer commented Jan 3, 2025

r? @durin42

@rustbot rustbot assigned durin42 and unassigned Mark-Simulacrum Jan 3, 2025
@workingjubilee
Copy link
Member

I wonder if this isn't also fixed by #![crate_type = "staticlib"] or something...?

@maurer
Copy link
Contributor Author

maurer commented Jan 3, 2025

I'm open to other fixes, this is just the lowest impact way I could think of to say "you need to actually put assembly in the assembly output".

A staticlib normally injects core/std, which this test intentionally seems to be avoiding. Additionally, I think this would be elided from a staticlib without a #[no_mangle], though I haven't tested. Would you like me to trial it?

@maurer
Copy link
Contributor Author

maurer commented Jan 3, 2025

Error before.

Fixed after.

cc @nikic given they just looked at the previous one of these.

@workingjubilee
Copy link
Member

A staticlib normally injects core/std, which this test intentionally seems to be avoiding

ah true, I was mostly just wondering.

@nikic
Copy link
Contributor

nikic commented Jan 4, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 4, 2025

📌 Commit cf679e4 has been approved by nikic

It is now in the queue for this repository.

@bors 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 Jan 4, 2025
@workingjubilee workingjubilee mentioned this pull request Jan 4, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2025
@bors
Auto merge of rust-lang#135101 - workingjubilee:rollup-owp3czl, r=wor…
1891c28 
…kingjubilee

Rollup of 6 pull requests

Successful merges:

 - rust-lang#135046 (turn rustc_box into an intrinsic)
 - rust-lang#135061 (crashes: add latest batch of tests)
 - rust-lang#135070 (std: sync to dep versions of backtrace)
 - rust-lang#135088 (Force code generation in assembly generation smoke-tests)
 - rust-lang#135091 (Bump backtrace to 0.3.75)
 - rust-lang#135094 (bootstrap: If dir_is_empty fails, show the non-existent directory path)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 030333a into rust-lang:master Jan 4, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 4, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2025
@rust-timer
Unrolled build for rust-lang#135088
fc5100a 
Rollup merge of rust-lang#135088 - maurer:force-asm, r=nikic

Force code generation in assembly generation smoke-tests

In llvm/llvm-project@7b23f41 , `.text` started being suppressed from LLVM assembly in cases where it wasn't strictly necessary. Currently, the sample functions in these two tests are frequently decided to be IR-only functions, resulting in no code generation, so LLVM drops the `.text` directive.

Adding `#[no_mangle]` forces these tests back to their original intent - assembly code is generated, and so a `.text` directive is generated as well.

`@rustbot` label: +llvm-main
r? `@workingjubilee`

I'll attach a buildbot link once it finishes consuming this PR
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@durin42 durin42

Labels
llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
7 participants
@maurer @rustbot @workingjubilee @nikic @bors @durin42 @Mark-Simulacrum