Skip to content
New issue

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

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

Already on GitHub? # to your account

Add fxvector-copy! #922

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add fxvector-copy! #922

wants to merge 1 commit into from

Conversation

williewillus
Copy link

@williewillus williewillus commented Mar 9, 2025

This PR adds fxvector-copy! to provide bulk blitting operations on fxvectors.
Useful for all the same reason bytevector-copy! is useful, to avoid the overhead
of a manual loop and assignments for every single element. I have an array I need to shift and the lack of copying operations that don't copy the entire vector means I have to do it manually.

Implementation and tests are mostly stolen from bytevector-copy! and adjusted to match.
One thing I was unsure of is noted inline.

Once I have indication that I'm not going completely in the wrong direction, I'll update the docs and changelog, and will send a flvector version as well (which is probably even more useful because it guarantees no float boxing/unboxing for each element).

@@ -1429,6 +1429,7 @@
(fxvector [sig [(fixnum ...) -> (fxvector)]] [flags alloc cp02 safeongoodargs])
(fxvector->list [sig [(fxvector) -> (list)]] [flags alloc safeongoodargs])
(fxvector-copy [sig [(fxvector) -> (fxvector)]] [flags alloc safeongoodargs])
(fxvector-copy! [sig [(fxvector sub-length fxvector sub-length sub-length) -> (void)]] [flags true])
Copy link
Author

Choose a reason for hiding this comment

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

is there documentation on what all these flags mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

The flags are defined in https://github.com/cisco/ChezScheme/blob/main/s/cmacros.ss#L1871 but not all of them are explained.

true means that the result is always true, so

(if (fxvector-copy! ...) x y)

is reduced in cp0 to

(begin (fxvector-copy! ...) x)

Now it's an automatic flag, so the (void) result in the signature implies this flag, so true is not necessary but not harmful.

($ptr-copy! fxv1 (fx+ (constant fxvector-data-disp)
(fx* (constant ptr-bytes) i1))
fxv2 (fx+ (constant fxvector-data-disp)
(fx* (constant ptr-bytes) i2))
Copy link
Author

Choose a reason for hiding this comment

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

the problem here is this can overflow. How should I handle that, add explicit checks? But it seems like an arbitrary limit on the user-facing side.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just use + and *? The calculations can overflow the fixnum range, but not the range allowed by $ptr-copy! due to the bounds checks. The overflow checks in + and * (which will not fail in practice) seem like a negligible cost, especially relative to the bounds guards.

@williewillus williewillus marked this pull request as ready for review March 9, 2025 09:05
@mflatt
Copy link
Contributor

mflatt commented Mar 19, 2025

This seems ok to me, so I encourage you to go ahead with flvector-copy!, docs, and release notes.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants