Skip to content

Add NEON support for ARMv7#792

Open
FooIbar wants to merge 2 commits into
libjxl:mainfrom
FooIbar:arm-neon
Open

Add NEON support for ARMv7#792
FooIbar wants to merge 2 commits into
libjxl:mainfrom
FooIbar:arm-neon

Conversation

@FooIbar

@FooIbar FooIbar commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

compare_incremental_tirr_photo, compare_pipelines_progressive and compare_pipelines_tirr_photo are skipped in CI due to OOM.

Local benchmark shows 2.7x speedup on bike.jxl (2.66 to 7.21 MP/s) and 2.2x speedup on green_queen_vardct_e3.jxl (3.79 to 8.47 MP/s).

- name: Clippy with all features
if: ${{ matrix.features == 'all' }}
run: cargo clippy --release --all-targets --all-features --tests --all -- -D warnings
run: cargo clippy --release --all-targets --all-features --all -- -D warnings

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is removed because --all-targets include tests.

@veluca93 veluca93 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some high-level thoughts: AFAIU NEON on ARMv7 is currently nightly because there are still some soundness issues with the backend. Thus, I'd prefer not to add the support unless we have some clear and important use cases.

If we do, then I imagine it should be fine if we rename the "nightly" feature to something a bit more scary ("enable-potentially-broken-targets"?) and leave it off by default. As a side note, why do we need the feature flag on jxl_transforms?

@FooIbar

FooIbar commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

AFAIU NEON on ARMv7 is currently nightly because there are still some soundness issues with the backend.

I'm aware of that, but not sure how it would affect us. Does jxl-rs rely on subnormals?

As a side note, why do we need the feature flag on jxl_transforms?

It's necessary because jxl_transforms calls simd_function!, which requires #![feature(arm_target_feature)].

@veluca93

Copy link
Copy Markdown
Member

AFAIU NEON on ARMv7 is currently nightly because there are still some soundness issues with the backend.

I'm aware of that, but not sure how it would affect us. Does jxl-rs rely on subnormals?

As a side note, why do we need the feature flag on jxl_transforms?

It's necessary because jxl_transforms calls simd_function!, which requires #![feature(arm_target_feature)].

I dislike soundness issues as a general rule :-)

Also, doesn't the jxl crate itself also call simd_function!? What is different?

@veluca93 veluca93 closed this Jun 10, 2026
@veluca93 veluca93 reopened this Jun 10, 2026
@FooIbar

FooIbar commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Also, doesn't the jxl crate itself also call simd_function!? What is different?

That's why the feature flag is added to the jxl crate as well

@veluca93

Copy link
Copy Markdown
Member

Also, doesn't the jxl crate itself also call simd_function!? What is different?

That's why the feature flag is added to the jxl crate as well

My bad, I missed that...

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.

2 participants