Skip to content

Keep RISC-V targets on the scalar path - #165

Merged
kimwalisch merged 3 commits into
ridiculousfish:masterfrom
carlosqwqqwq:riscv-libdivide
Jun 19, 2026
Merged

Keep RISC-V targets on the scalar path#165
kimwalisch merged 3 commits into
ridiculousfish:masterfrom
carlosqwqqwq:riscv-libdivide

Conversation

@carlosqwqqwq

@carlosqwqqwq carlosqwqqwq commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Why

libdivide already has a portable scalar implementation, but its current CMake auto-detection logic still probes x86 and ARM vector backends even when the configured target is riscv64.

On a RISC-V target, that is not target-correct:

  • it can try x86 or ARM SIMD checks that do not belong to the declared target;
  • in cross-compiling mode, those probes can fail before the scalar path is even configured.

This change keeps riscv64 on libdivide's existing scalar path unless a vector option is enabled manually.

What changed

  • Updated CMakeLists.txt only.
  • When CMAKE_SYSTEM_PROCESSOR matches riscv*, any SIMD option that is still AUTO is set to OFF before the existing autodetection probes run.
  • This keeps the existing behavior for explicit user choices (ON or OFF) and for non-RISC-V targets.

Verification

  • Reproduced the original problem on upstream master with a riscv64 cross-compiling configure:
    • the configure step still runs ARM/x86 SIMD probes and fails.
  • Verified the updated branch with the same simulated riscv64 configure:
    • cmake -S . -B build-riscv-sim-review -G Ninja -DCMAKE_BUILD_TYPE=Release -DLIBDIVIDE_BUILD_TESTS=ON -DCMAKE_SYSTEM_NAME=Linux -DCMAKE_SYSTEM_PROCESSOR=riscv64 -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY
    • configuration succeeds;
    • the configure output prints RISC-V target detected; libdivide will use the scalar path unless vector macros are enabled manually.
  • Verified the updated simulated riscv64 build files:
    • build-riscv-sim-review/build.ninja contains no LIBDIVIDE_SSE2, LIBDIVIDE_AVX2, LIBDIVIDE_AVX512, LIBDIVIDE_NEON, or LIBDIVIDE_SVE;
    • it also contains no -march=native, -mavx, -msse, arm_neon, or arm_sve.
  • Re-ran native non-benchmark regression checks on the updated branch:
    • tester passed;
    • test_c99 passed;
    • test_divlu passed.

Notes

  • This is a conservative CMake-only portability fix.
  • It does not add RVV or any dedicated RISC-V vector backend.
  • My validation is from cross-compilation plus qemu-based checking, not from a physical RISC-V machine.

@kimwalisch

Copy link
Copy Markdown
Collaborator

Can you explain personnally (no AI generated reply) what error your pull request fixes and provide the logs of the error you encountered on your RISC-V system?

@carlosqwqqwq

Copy link
Copy Markdown
Contributor Author

Thanks for asking, and sorry for the unclear words.

I should correct that: this was not tested on a physical RISC-V machine. My validation was done with a riscv64 cross-compilation setup plus qemu.

The issue this PR fixes is that a riscv64-targeted CMake configuration still runs ARM/x86 SIMD auto-detection probes on upstream master, and that breaks configuration in my setup. The relevant errors were:

E:/.../src.cxx:2:14: fatal error: arm_neon.h: No such file or directory
E:/.../src.cxx:2:14: fatal error: arm_sve.h: No such file or directory
.../avx512fintrin.h:343:1: error: inlining failed in call to 'always_inline' '__m512i _mm512_setzero_si512()': target specific option mismatch
.../avx2intrin.h:724:1: error: inlining failed in call to 'always_inline' '__m256i _mm256_srai_epi32(__m256i, int)': target specific option mismatch
CMake Error: Recorded try_compile output location doesn't exist: .../TryCompile-n5xwpx/cmTC_43222
-- Configuring incomplete, errors occurred!

@kimwalisch

kimwalisch commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

OK. Ideally I would like you to fix this by only modifying CMakeLists.txt. I think your libdivide.h (and the other files you modified) changes are not needed. Please try to keep your changes minimal.

@carlosqwqqwq

Copy link
Copy Markdown
Contributor Author

Thanks, I reduced the patch and pushed an update in 4349780.

The PR now only modifies CMakeLists.txt.
For riscv* targets, it only turns the AUTO SIMD options off before the existing probes run, so the configure step stays on the scalar path instead of trying the x86/ARM checks.

I re-tested it on my side with the same cross-compilation setup as before:

  • upstream master still fails during the SIMD autodetection step for a riscv64 configure;
  • the updated branch configures successfully for riscv64;
  • I also re-ran the native tester, test_c99, and test_divlu tests to make sure the reduced patch did not break those targets.

Thanks for pointing me to the smaller fix.

@kimwalisch

Copy link
Copy Markdown
Collaborator

Your new fix is already much better than your first version. However it is not optimal since you have now basically hardcoded a workaround for RISC-V. The issue is still present for other CPU architectures. I think the right fix is to use proper cross compilation detection for SSE2, AVX2, AVX512 similar to how we handle LIBDIVIDE_SVE2 in CMakeLists.txt.

@carlosqwqqwq

Copy link
Copy Markdown
Contributor Author

Ok, please give me some time to think about what to do. This might take a while.

@kimwalisch

Copy link
Copy Markdown
Collaborator

I think this code can be used to fix the issue:

    if (CMAKE_CROSSCOMPILING)
        check_cxx_source_compiles("${NEON_TEST}" LIBDIVIDE_NEON_ENABLED)
    else()
        check_cxx_source_runs("${NEON_TEST}" LIBDIVIDE_NEON_ENABLED)
    endif()

@carlosqwqqwq

Copy link
Copy Markdown
Contributor Author

Thanks, your suggestion was right.

I removed the RISC-V-specific workaround and updated the SSE2, AVX2, and AVX512 autodetection to follow the same cross-compiling pattern as NEON/SVE/SVE2: check_cxx_source_compiles(...) when cross-compiling, and check_cxx_source_runs(...) otherwise.

I also re-validated this with a real riscv64-linux-gnu-g++ cross-compilation setup in Docker, not just by setting CMAKE_SYSTEM_PROCESSOR. With that setup, the RISC-V configure succeeds and the x86/ARM SIMD probes are no longer incorrectly enabled.

Please take another look when you have time. thank you very much.

@kimwalisch
kimwalisch merged commit 9d11a8b into ridiculousfish:master Jun 19, 2026
13 of 16 checks passed
@kimwalisch

Copy link
Copy Markdown
Collaborator

Thanks!

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