Add tests for slice bounds check optimization #139129

reez12g · 2025-03-30T09:02:44Z

rustbot · 2025-03-30T09:02:48Z

rustbot has assigned @fee1-dead.
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

fee1-dead · 2025-03-30T14:58:58Z

tests/ui/optimizations/slice-last-elements-bounds-check.rs

I don't think this file is needed because it doesn't test anything new. Could you remove this?

Thanks. I removed it.

fee1-dead · 2025-03-30T15:02:00Z

tests/codegen/slice-last-elements-optimization.rs

+    // In a suboptimal implementation this would contain a call to
+    // slice_start_index_len_fail that is unreachable.
+
+    // The fix exists in LLVM 20: We use this implementation with
+    // bounds check outside of slice operation.


Suggested change

// In a suboptimal implementation this would contain a call to

// slice_start_index_len_fail that is unreachable.

// The fix exists in LLVM 20: We use this implementation with

// bounds check outside of slice operation.

// Previously this would generate a branch to slice_start_index_len_fail

// that is unreachable. The LLVM 20 fix should eliminate this branch.

fee1-dead · 2025-03-30T15:03:10Z

tests/assembly/x86_64-slice-last-elements-optimization.rs

similarly, I think it is okay to only keep the codegen test. assembly is one step further than we need since we just have to check if the LLVM IR contains the call

+1 to this -- assembly is annoyingly platform specific in a bunch of ways, so we should test it as little as possible in rustc. Specific assembly is almost always LLVM's problem, not our problem.

Makes sense — I’ve already removed the assembly test accordingly.
Only the codegen test remains now.

fee1-dead · 2025-03-30T15:03:56Z

tests/codegen/slice-last-elements-optimization.rs

+    // This implementation avoids the issue by moving the slice operation
+    // inside the conditional.


Suggested change

// This implementation avoids the issue by moving the slice operation

// inside the conditional.

// This version was already correctly optimized before the fix in LLVM 20.

Thanks for the suggestion — I’ve applied it.

tests/codegen/slice-last-elements-optimization.rs

reez12g · 2025-04-01T02:05:23Z

@rustbot ready

fee1-dead · 2025-04-01T03:59:31Z

@bors r+ rollup

bors · 2025-04-01T03:59:34Z

📌 Commit dea9472 has been approved by fee1-dead

It is now in the queue for this repository.

…-check-optimization, r=fee1-dead Add tests for slice bounds check optimization Closes rust-lang#134466

Rollup of 12 pull requests Successful merges: - rust-lang#110406 (rustdoc-json: Add test for #[automatically_derived] attribute) - rust-lang#137738 (Make slice iterator constructors unstably const) - rust-lang#138492 (remove `feature(inline_const_pat)`) - rust-lang#138928 (Fix UWP reparse point check) - rust-lang#138950 (replace extra_filename with strict version hash in metrics file names) - rust-lang#139002 (Add release notes for 1.86.0) - rust-lang#139022 (increment depth of nested obligations) - rust-lang#139060 (replace commit placeholder in vendor status with actual commit) - rust-lang#139102 (coverage: Avoid splitting spans during span extraction/refinement) - rust-lang#139129 (Add tests for slice bounds check optimization) - rust-lang#139188 (PassWrapper: adapt for llvm/llvm-project@94122d58fc77079a291a3d008914…) - rust-lang#139193 (Feed HIR for by-move coroutine body def, since the inliner tries to read its attrs) r? `@ghost` `@rustbot` modify labels: rollup

…-check-optimization, r=fee1-dead Add tests for slice bounds check optimization Closes rust-lang#134466

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#110406 (rustdoc-json: Add test for #[automatically_derived] attribute) - rust-lang#138790 (Note potential but private items in show_candidates) - rust-lang#138950 (replace extra_filename with strict version hash in metrics file names) - rust-lang#139002 (Add release notes for 1.86.0) - rust-lang#139022 (increment depth of nested obligations) - rust-lang#139129 (Add tests for slice bounds check optimization) - rust-lang#139188 (PassWrapper: adapt for llvm/llvm-project@94122d58fc77079a291a3d008914…) - rust-lang#139193 (Feed HIR for by-move coroutine body def, since the inliner tries to read its attrs) - rust-lang#139202 (Improve docs of ValTreeKind) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#110406 (rustdoc-json: Add test for #[automatically_derived] attribute) - rust-lang#138790 (Note potential but private items in show_candidates) - rust-lang#139002 (Add release notes for 1.86.0) - rust-lang#139022 (increment depth of nested obligations) - rust-lang#139129 (Add tests for slice bounds check optimization) - rust-lang#139188 (PassWrapper: adapt for llvm/llvm-project@94122d58fc77079a291a3d008914…) - rust-lang#139193 (Feed HIR for by-move coroutine body def, since the inliner tries to read its attrs) - rust-lang#139202 (Improve docs of ValTreeKind) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139129 - reez12g:add-tests-for-slice-bounds-check-optimization, r=fee1-dead Add tests for slice bounds check optimization Closes rust-lang#134466

rustbot assigned fee1-dead Mar 30, 2025

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 Mar 30, 2025

This comment has been minimized.

Sign in to view

reez12g force-pushed the add-tests-for-slice-bounds-check-optimization branch from f8d5a87 to 599a94d Compare March 30, 2025 10:04

This comment has been minimized.

Sign in to view

reez12g force-pushed the add-tests-for-slice-bounds-check-optimization branch from 599a94d to f98f095 Compare March 30, 2025 10:50

reez12g marked this pull request as ready for review March 30, 2025 12:00

fee1-dead requested changes Mar 30, 2025

View reviewed changes

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 Mar 30, 2025

clubby789 reviewed Mar 30, 2025

View reviewed changes

tests/codegen/slice-last-elements-optimization.rs Show resolved Hide resolved

Add tests for LLVM 20 slice bounds check optimization

dea9472

reez12g force-pushed the add-tests-for-slice-bounds-check-optimization branch from f98f095 to dea9472 Compare March 31, 2025 13:38

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 Apr 1, 2025

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 Apr 1, 2025

Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 1, 2025

Rollup merge of rust-lang#139129 - reez12g:add-tests-for-slice-bounds…

fdd80b4

…-check-optimization, r=fee1-dead Add tests for slice bounds check optimization Closes rust-lang#134466

Zalathar mentioned this pull request Apr 1, 2025

Rollup of 16 pull requests #139203

Closed

Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 1, 2025

Rollup merge of rust-lang#139129 - reez12g:add-tests-for-slice-bounds…

b22a776

…-check-optimization, r=fee1-dead Add tests for slice bounds check optimization Closes rust-lang#134466

Zalathar mentioned this pull request Apr 1, 2025

Rollup of 12 pull requests #139204

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 1, 2025

Rollup merge of rust-lang#139129 - reez12g:add-tests-for-slice-bounds…

d2c7cc8

…-check-optimization, r=fee1-dead Add tests for slice bounds check optimization Closes rust-lang#134466

matthiaskrgr mentioned this pull request Apr 1, 2025

Rollup of 9 pull requests #139216

Closed

matthiaskrgr mentioned this pull request Apr 1, 2025

Rollup of 8 pull requests #139220

Merged

bors merged commit a04b0e3 into rust-lang:master Apr 2, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for slice bounds check optimization #139129

Add tests for slice bounds check optimization #139129

reez12g commented Mar 30, 2025 •

edited by clubby789

Loading

rustbot commented Mar 30, 2025

This comment has been minimized.

This comment has been minimized.

fee1-dead Mar 30, 2025

reez12g Mar 31, 2025

fee1-dead Mar 30, 2025

fee1-dead Mar 30, 2025

scottmcm Mar 31, 2025

reez12g Mar 31, 2025

fee1-dead Mar 30, 2025

reez12g Mar 31, 2025

reez12g commented Apr 1, 2025

fee1-dead commented Apr 1, 2025

bors commented Apr 1, 2025

		// This implementation avoids the issue by moving the slice operation
		// inside the conditional.

	// This implementation avoids the issue by moving the slice operation
	// inside the conditional.
	// This version was already correctly optimized before the fix in LLVM 20.

Add tests for slice bounds check optimization #139129

Add tests for slice bounds check optimization #139129

Conversation

reez12g commented Mar 30, 2025 • edited by clubby789 Loading

rustbot commented Mar 30, 2025

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reez12g commented Apr 1, 2025

fee1-dead commented Apr 1, 2025

bors commented Apr 1, 2025

reez12g commented Mar 30, 2025 •

edited by clubby789

Loading