Hint that memchr returns an in-bounds index - #159784
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @petrochenkov (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? libs |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready |
…result-bounds, r=Darksonn Hint that memchr returns an in-bounds index `memchr_aligned` always returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant with `assert_unchecked` so LLVM can simplify callers such as `str::find(char)` In x86_64 codegen, `find(char)` shrank from 463 to 377 bytes.
LLVM 21 preserves the bounds assumption but does not eliminate the aggregate phi that LLVM 22 removes. Check each version's supported optimization and restore the shared postcondition so direct callers can eliminate bounds checks.
|
@bors try jobs=x86_64-gnu-llvm-21-3 |
This comment has been minimized.
This comment has been minimized.
…ds, r=<try> Hint that memchr returns an in-bounds index try-job: x86_64-gnu-llvm-21-3
|
@bors r+ rollup=iffy |
…result-bounds, r=Darksonn Hint that memchr returns an in-bounds index `memchr_aligned` always returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant with `assert_unchecked` so LLVM can simplify callers such as `str::find(char)` In x86_64 codegen, `find(char)` shrank from 463 to 377 bytes.
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #160615 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#160615), which was unapproved. |
There was a problem hiding this comment.
I think we should take this test out. It's too fragile.
…result-bounds, r=Darksonn Hint that memchr returns an in-bounds index `memchr_aligned` always returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant with `assert_unchecked` so LLVM can simplify callers such as `str::find(char)` In x86_64 codegen, `find(char)` shrank from 463 to 377 bytes.
…uwer Rollup of 21 pull requests Successful merges: - #159784 (Hint that memchr returns an in-bounds index) - #150885 (Revive L4Re target) - #159643 (Add support for splatted function pointers) - #160433 (delegation: add support for wrapping of the return value with `From::from`) - #160530 (refactor handling of target features in Session) - #160606 (bootstrap: Store and use an explicit CheckKind in `check::Rustc`) - #160628 (fix ICE in `suggest_add_reference_to_arg` for non-callable items) - #160634 (miri subtree update) - #157641 (Do not promote extern statics) - #158904 (Fix FutureDropPoll shim for by-move async closures) - #160103 (Add regression test for GAT bound mismatched type error) - #160335 (dlopen offload) - #160445 (codegen: classify localized MSVC linker progress as linker_info) - #160499 (rustc_resolve: move diagnostic attribute linting to attr parsing) - #160504 (cleanup borrowck, improve c-variadic handling) - #160577 (expand: Feature gate AST-based attribute macros on expressions and statements) - #160587 (Add regression test for associated type outlives bound at call site) - #160625 (platform-support/netbsd.md: No longer mention 8.x, due to EoL.) - #160636 (derive(Diagnostic): link to proper docs) - #160644 (Clean up some manual debug impls) - #160649 (move naked function ui tests)
…result-bounds, r=Darksonn Hint that memchr returns an in-bounds index `memchr_aligned` always returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant with `assert_unchecked` so LLVM can simplify callers such as `str::find(char)` In x86_64 codegen, `find(char)` shrank from 463 to 377 bytes.
…uwer Rollup of 20 pull requests Successful merges: - #159784 (Hint that memchr returns an in-bounds index) - #150885 (Revive L4Re target) - #159643 (Add support for splatted function pointers) - #160433 (delegation: add support for wrapping of the return value with `From::from`) - #160530 (refactor handling of target features in Session) - #160606 (bootstrap: Store and use an explicit CheckKind in `check::Rustc`) - #160628 (fix ICE in `suggest_add_reference_to_arg` for non-callable items) - #160634 (miri subtree update) - #157641 (Do not promote extern statics) - #158904 (Fix FutureDropPoll shim for by-move async closures) - #160103 (Add regression test for GAT bound mismatched type error) - #160335 (dlopen offload) - #160445 (codegen: classify localized MSVC linker progress as linker_info) - #160499 (rustc_resolve: move diagnostic attribute linting to attr parsing) - #160504 (cleanup borrowck, improve c-variadic handling) - #160577 (expand: Feature gate AST-based attribute macros on expressions and statements) - #160587 (Add regression test for associated type outlives bound at call site) - #160625 (platform-support/netbsd.md: No longer mention 8.x, due to EoL.) - #160636 (derive(Diagnostic): link to proper docs) - #160644 (Clean up some manual debug impls)
View all comments
memchr_alignedalways returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant withassert_uncheckedso LLVM can simplify callers such asstr::find(char)In x86_64 codegen,
find(char)shrank from 463 to 377 bytes.