Skip to content

Test for armv7 get_unchecked(...) inlining#152847

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Jamesbarford:chore/get_unchecked-inline-test
Mar 9, 2026
Merged

Test for armv7 get_unchecked(...) inlining#152847
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Jamesbarford:chore/get_unchecked-inline-test

Conversation

@Jamesbarford
Copy link
Contributor

@Jamesbarford Jamesbarford commented Feb 19, 2026

Added test for get_unchecked(...) to be inlined on armv7.

closes #131745

@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. labels Feb 19, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 19, 2026

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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Fallback group: @Mark-Simulacrum, @jieyouxu
  • @Mark-Simulacrum, @jieyouxu expanded to Mark-Simulacrum, jieyouxu
  • Random selection from Mark-Simulacrum, jieyouxu

@rust-log-analyzer

This comment has been minimized.

Sign in to view
lqd
lqd reviewed Feb 19, 2026
View reviewed changes
@Jamesbarford Jamesbarford force-pushed the chore/get_unchecked-inline-test branch 2 times, most recently from 8a39e77 to 41fa18d Compare February 19, 2026 14:02
Mark-Simulacrum
Mark-Simulacrum reviewed Feb 23, 2026
View reviewed changes
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Feb 23, 2026

Why does this need an assembly test? It seems like we should be able to check that inlining behaves as expected across target_feature differences (specifically subsets, it sounds like, from the issue)?

That would also probably let us check this without relying on the standard library having a particular set of target features enabled, since we can write all N functions that we're checking for inlining in the test itself?

@Mark-Simulacrum Mark-Simulacrum 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 Feb 23, 2026
@Jamesbarford Jamesbarford force-pushed the chore/get_unchecked-inline-test branch 2 times, most recently from a19b39a to 9a23bbb Compare February 23, 2026 11:51
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Jamesbarford Jamesbarford force-pushed the chore/get_unchecked-inline-test branch from 9a23bbb to 8cfc347 Compare February 24, 2026 09:04
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 5, 2026
Mark-Simulacrum
Mark-Simulacrum approved these changes Mar 8, 2026
View reviewed changes
tests/codegen-llvm/inlining-target-feature-differences.rs
#![crate_type = "lib"]
#![no_std]

#[inline]
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum Mar 8, 2026

Choose a reason for hiding this comment

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

I was a little worried this would get MIR inlined, but it seems like that doesn't happen right now at least... https://rust.godbolt.org/z/jvs7K3W9f

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Mar 8, 2026

r=me with the lang_items feature removed

@Mark-Simulacrum Mark-Simulacrum 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 Mar 8, 2026
@Jamesbarford Jamesbarford force-pushed the chore/get_unchecked-inline-test branch from 8cfc347 to 00980f4 Compare March 9, 2026 09:18
@Jamesbarford
Copy link
Contributor Author

Jamesbarford commented Mar 9, 2026

@bors r=Mark-Simulacrum

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 9, 2026

@Jamesbarford: 🔑 Insufficient privileges: not in review users

@Jamesbarford
Copy link
Contributor Author

Jamesbarford commented Mar 9, 2026

@Mark-Simulacrum I don't have permissions to do @bors r=<user_name>

@lqd
Copy link
Member

lqd commented Mar 9, 2026

@bors delegate+

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 9, 2026

✌️ @Jamesbarford, you can now approve this pull request!

If @lqd told you to "r=me" after making some further change, then please make that change and post @bors r=lqd.

@Jamesbarford
Copy link
Contributor Author

Jamesbarford commented Mar 9, 2026

@bors r=Mark-Simulacrum

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 9, 2026

📌 Commit 00980f4 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@rust-bors rust-bors bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 9, 2026
@JonathanBrouwer JonathanBrouwer mentioned this pull request Mar 9, 2026
Merged
rust-bors bot pushed a commit that referenced this pull request Mar 9, 2026
@bors
Auto merge of #153605 - JonathanBrouwer:rollup-p3xMXMQ, r=JonathanBro…
3945997 
…uwer

Rollup of 11 pull requests

Successful merges:

 - #152847 (Test for armv7 `get_unchecked(...)` inlining)
 - #153290 (tests: codegen-llvm: iter-repeat-n-trivial-drop: Allow non-zero lower bound to __rust_alloc size)
 - #153413 (std: organise `sys::pal::os`)
 - #139692 (Rustfmt now support use closures)
 - #152800 (Adds recursion limit into FindParamInClause)
 - #153244 (tools: remote-test-server: Add UEFI run support)
 - #153305 (Adapt codegen test to accept operand bundles)
 - #153340 (tweak r-a default settings)
 - #153509 (Cleanup unused diagnostic emission methods - part 2)
 - #153527 (Fix LegacyKeyValueFormat report from docker build: ohos)
 - #153594 (Move some ui tests)
@rust-bors rust-bors bot merged commit a5bd6be into rust-lang:main Mar 9, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 9, 2026
rust-timer added a commit that referenced this pull request Mar 9, 2026
@rust-timer
Unrolled build for #152847
1dc1199 
Rollup merge of #152847 - Jamesbarford:chore/get_unchecked-inline-test, r=Mark-Simulacrum

Test for armv7 `get_unchecked(...)` inlining

Added test for `get_unchecked(...)` to be inlined on armv7.

closes #131745
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lqd lqd lqd left review comments

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum approved these changes

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels

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.96.0

Development

Successfully merging this pull request may close these issues.

get_unchecked() is never inlined on armv7-unknown-linux-gnueabihf in functions with #[target_feature(enable = "neon")]

5 participants

@Jamesbarford @rustbot @rust-log-analyzer @Mark-Simulacrum @lqd