Skip to content

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Dec 23, 2024

Ensure that the pointer gets swapped correctly even if it is not stored at an aligned offset. This rules out implementations that copy things in a usize loop -- so our implementation needs to be adjusted to avoid such a loop when running in const context.

Part of #133668

@rustbot
Copy link
Collaborator

rustbot commented Dec 23, 2024

r? @workingjubilee

rustbot has assigned @workingjubilee.
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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 23, 2024

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @rust-lang/wg-const-eval

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

r=me with output blessed

@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Dec 23, 2024

📌 Commit af1c8da has been approved by oli-obk

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 Dec 23, 2024
@oli-obk oli-obk assigned oli-obk and unassigned workingjubilee Dec 23, 2024
@RalfJung
Copy link
Member Author

Somehow this does not show up in the queue?
@bors retry

@RalfJung RalfJung closed this Dec 23, 2024
@RalfJung RalfJung reopened this Dec 23, 2024
@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Dec 23, 2024

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Dec 23, 2024

📌 Commit af1c8da has been approved by oli-obk

It is now in the queue for this repository.

@RalfJung
Copy link
Member Author

@bors rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 24, 2024
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134638 (Fix effect predicates from item bounds in old solver)
 - rust-lang#134662 (Fix safety docs for `dyn Any + Send {+ Sync}`)
 - rust-lang#134689 (core: fix const ptr::swap_nonoverlapping when there are pointers at odd offsets)
 - rust-lang#134699 (Belay new reviews for workingjubilee)
 - rust-lang#134701 (Correctly note item kind in `NonConstFunctionCall` error message)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bbd30b5 into rust-lang:master Dec 24, 2024
12 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 24, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 24, 2024
Rollup merge of rust-lang#134689 - RalfJung:ptr-swap-test, r=oli-obk

core: fix const ptr::swap_nonoverlapping when there are pointers at odd offsets

Ensure that the pointer gets swapped correctly even if it is not stored at an aligned offset. This rules out implementations that copy things in a `usize` loop -- so our implementation needs to be adjusted to avoid such a loop when running in const context.

Part of rust-lang#133668
@RalfJung RalfJung deleted the ptr-swap-test branch December 24, 2024 07:18
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 31, 2024
Redo the swap code for better tail & padding handling

A couple of parts here

## Fixes rust-lang#134713

Actually using the type passed to `ptr::swap_nonoverlapping` for anything other than its size + align turns out to not work, so this redo goes back to *always* swapping via not-`!noundef` integers.

(Except in `const`, which keeps doing the same thing as before to preserve `@RalfJung's` fix from rust-lang#134689)

## Fixes rust-lang#134946

I'd previously moved the swapping to use auto-vectorization, but someone pointed out on Discord that the tail loop handling from that left a whole bunch of byte-by-byte swapping around.  This PR goes back to a manual chunk, with at most logarithmic more instructions for the tail.

(There are other ways that could potentially handle the tail even better, but this seems to be pretty good, since it's how LLVM ends up lowering operations on types like `i56`.)

## Polymorphization

Since it's silly to have separate copies of swapping -- especially *untyped* swapping! -- for `u32`, `i32`, `f32`, `[u16; 2]`, etc, this sends everything to byte versions, but still mono'd by alignment.  That should make it more ok that the code is somewhat more complex, since we only get 7 monomorphizations of the complicated bit.

(One day we'll be able to remove some of the hacks by being able to just call `foo::<{align_of::<T>()}>`, but since alignments are only powers of two, the manual dispatch out isn't a big deal.)
tautschnig added a commit to tautschnig/verify-rust-std that referenced this pull request Jan 17, 2025