Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

add @memmove builtin #22605

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

add @memmove builtin #22605

wants to merge 5 commits into from

Conversation

dweiller
Copy link
Contributor

@dweiller dweiller commented Jan 25, 2025

This PR adds a new builtin @memmove(dest: T, src: T) void. It is similar to @memcpy except it can be used for overlapping memory regions. The intended semantics are:

  • src and dest can each be a slice, pointer to array, or many-item pointer
  • at least one of src and dest must provide a length, and if they both do the lengths must be the same
  • unlike (the intended runtime, and current comptime) behaviour of@memcpy, @memmove requires the element type of src and dest to be coercible to each other in memory
  • the result of @memmove(dest, src) is that dest[0..len] contains bytes as if a element-by-element loop copied src[0..len] to an auxiliary buffer, and then a second loop copied from that buffer to dest[0..len].

This builtin is currently using up the last ZIR tag.

Related: #767

@dweiller dweiller force-pushed the memmove branch 3 times, most recently from 4e8a431 to c976221 Compare January 25, 2025 15:35
@nektro
Copy link
Contributor

nektro commented Jan 25, 2025

  • should it be @memmove(dest, src) to match order of @memcpy ?
  • does @memmove perform the same length equality assertion as @memcpy ?

@dweiller
Copy link
Contributor Author

dweiller commented Jan 25, 2025

  • should it be @memmove(dest, src) to match order of @memcpy ?

    • does @memmove perform the same length equality assertion as @memcpy ?

Whoops the order is the same, I just wrote it wrong. It has the same length checks too - will update the OP.

@alexrp
Copy link
Member

alexrp commented Jan 27, 2025

unlike (the intended runtime, and current comptime) behaviour of@memcpy, @memmove requires the element type of src and dest to be coercible to each other in memory

Just noting that we concluded in the discussion that @memcpy will be changed to only allow in-memory coercions, i.e. coerceInMemoryAllowed. So the builtins will be consistent on this point.

@dweiller dweiller force-pushed the memmove branch 2 times, most recently from 3602635 to d67f0b0 Compare January 28, 2025 05:09
@alexrp alexrp mentioned this pull request Jan 28, 2025
@alexrp
Copy link
Member

alexrp commented Jan 28, 2025

Additionally, per the @memcpy discussion, the addition of @memmove should include deprecation of std.mem.copy(Backwards,Forwards) (and later removal in 0.15.0).

@dweiller
Copy link
Contributor Author

Additionally, per the @memcpy discussion, the addition of @memmove should include deprecation of std.mem.copy(Backwards,Forwards) (and later removal in 0.15.0).

Does deprecation mean replace with @compileError, or say so in a doc comment? I see both being done in lib/std.

@alexrp
Copy link
Member

alexrp commented Jan 28, 2025

Does deprecation mean replace with @compileError, or say so in a doc comment? I see both being done in lib/std.

Doc comment.

I think it's removal that either means outright deletion, or @compileError for an additional release cycle. Not sure if we have any guidelines for when to do which. cc @andrewrk

@dweiller dweiller marked this pull request as ready for review January 29, 2025 05:49
@dweiller dweiller force-pushed the memmove branch 2 times, most recently from f8d1141 to 7f0001a Compare January 30, 2025 09:57
@alexrp alexrp added this to the 0.14.0 milestone Jan 30, 2025
@dweiller dweiller force-pushed the memmove branch 3 times, most recently from 21b518e to 99eff11 Compare February 4, 2025 04:01
@dweiller dweiller force-pushed the memmove branch 3 times, most recently from 7a4c39c to fd577a8 Compare February 25, 2025 04:28
@dweiller
Copy link
Contributor Author

I don't know what the aarch64-macos-debug CI issue is - it failed before and after a rebase for @disableIntrinsics(), but I can't really believe this PR is causing the problem.

@dweiller
Copy link
Contributor Author

Looks like the aarch64-macos-debug CI issue is unrelated, there are other PRs that are having the same failure.

@andrewrk
Copy link
Member

macho linker bug filed at #23010

@andrewrk andrewrk removed this from the 0.14.0 milestone Mar 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants