Skip to content

add alignment on JPEG [i16; 64] blocks#28

Merged
vstroebel merged 9 commits into
vstroebel:mainfrom
mcroomp:alignblock
Feb 10, 2026
Merged

add alignment on JPEG [i16; 64] blocks#28
vstroebel merged 9 commits into
vstroebel:mainfrom
mcroomp:alignblock

Conversation

@mcroomp

@mcroomp mcroomp commented Nov 28, 2025

Copy link
Copy Markdown
Contributor

Ensure that we access coefficient blocks 32 byte aligned so that we can autovectorize and optimize some codepaths via SIMD

@vstroebel

Copy link
Copy Markdown
Owner

Just some random thoughts, but wouldn't it be better to expose the 8x8 nature of the blocks to the compiler by introducing a Block structure that is backed by an [8; [i16;8]]?
In addition it might be possible to make the exact datatype of a "row" an implementation detail. This could make it possible to switch from a [i16;8] to a i16x8 if wide or std:simd is enabled.
I'm not sure sure whether this really works, but maybe this helps to unify some code of different fdct implementations.

@Shnatsel

Copy link
Copy Markdown
Contributor

i16x8 should be a zero-cost conversion as long as the data is guaranteed to be aligned in memory, so I don't think it will be of any benefit compared to AlignedBlock.

We can even keep using unaligned loads for safety and the compiler will automatically transform them into aligned ones where possible.

@mcroomp

mcroomp commented Nov 29, 2025

Copy link
Copy Markdown
Contributor Author

The best row size depends on how wide the SIMD registers are, so it probably is better not to hardcode. The main advantage of this change is that even if you transmute to a SIMD in-place, the compile knows that it's properly aligned.

If you use the bytemuck crate you can cast directly from [i16;64] to the appropriate SIMD type safely (it statically asserts on the alignment).

@mcroomp

mcroomp commented Nov 29, 2025

Copy link
Copy Markdown
Contributor Author

Also there's some changes in here that came from cargo fmt, maybe as part of the checkin test you can add

cargo fmt --check

to the github action so that changes are always checked for formatting before checkin

@mcroomp

mcroomp commented Nov 29, 2025

Copy link
Copy Markdown
Contributor Author

I checked the generated code and it's true the compiler changes the loads to aligned with this change.

@vstroebel

Copy link
Copy Markdown
Owner

Also there's some changes in here that came from cargo fmt, maybe as part of the checkin test you can add

cargo fmt --check

to the github action so that changes are always checked for formatting before checkin

I've add the check and did a cargo fmt in #30.

Otherwise, this PR looks good. There are a few merge conflicts that need to be resolved before it can be merged.

@mcroomp

mcroomp commented Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

I've fixed the merge issues and cleaned up any unnecessary changes. It needs workflow approval though to run the tests. Thanks!

@vstroebel vstroebel merged commit d854970 into vstroebel:main Feb 10, 2026
7 checks passed
@mcroomp mcroomp deleted the alignblock branch February 10, 2026 13:16
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.

3 participants