Skip to content

Avoid leaking x86_64 SSE2 paths into forced RISC-V probes#127

Open
carlosqwqqwq wants to merge 1 commit into
xiph:mainfrom
carlosqwqqwq:riscv-vorbis
Open

Avoid leaking x86_64 SSE2 paths into forced RISC-V probes#127
carlosqwqqwq wants to merge 1 commit into
xiph:mainfrom
carlosqwqqwq:riscv-vorbis

Conversation

@carlosqwqqwq

Copy link
Copy Markdown

Why

vorbis already has a portable scalar fallback in lib/os.h, but the x86_64 SSE2 fast path was still too loosely gated for portability verification.

The concrete problem is that a forced riscv64 smoke build on an x86_64 host could still inherit the host-only SSE2 path:

  • the x86_64 helper uses emmintrin.h and _mm_cvtsd_si32;
  • prior to this patch, the GCC branch only needed the host-side SSE2 condition to become active;
  • that made forced non-x86 target probes weaker than they should be, because they could validate the wrong backend.

This patch keeps the codec logic unchanged and tightens the architecture gate so the existing generic fallback is what gets exercised for RISC-V portability checks.

What changed

  • Updated lib/os.h:
    • keep the SSE2 helper restricted to x86_64 builds;
    • explicitly step aside when a forced non-x86 probe defines __riscv, so portability smoke tests do not inherit the host x86_64 SSE2 path.
  • Updated README.md:
    • document that non-x86 targets such as riscv64 use the generic scalar fallback.

Verification

  • Verified the forced RISC-V probe on Linux in Docker (ubuntu:24.04):
    • compiled a minimal probe including lib/os.h with forced __riscv=1 and __riscv_xlen=64;
    • confirmed the preprocessed output now falls back to return (int)floor(f+.5);;
    • confirmed the output no longer includes emmintrin.h or _mm_cvtsd_si32;
    • object compilation succeeded.
  • Verified native Linux build and tests in Docker:
    • cmake -S /work -B /work/build-native -G Ninja -DCMAKE_BUILD_TYPE=Release
    • cmake --build /work/build-native --parallel 4
    • ctest --test-dir /work/build-native --output-on-failure
    • result: 2/2 tests passed.
  • Verified real riscv64 Linux cross-compilation and test execution in Docker:
    • cross-built xiph/ogg for riscv64 and installed it to /opt/riscv-ogg;
    • configured vorbis with riscv64-linux-gnu-gcc, CMAKE_SYSTEM_PROCESSOR=riscv64, and OGG_ROOT=/opt/riscv-ogg;
    • built the full test targets successfully;
    • ran ctest --test-dir /work/build-riscv --output-on-failure;
    • result: 2/2 tests passed.
  • Verified the produced test binaries are real RISC-V executables:
    • riscv64-linux-gnu-readelf -h /work/build-riscv/test/vorbis_test
    • riscv64-linux-gnu-readelf -h /work/build-riscv/test/test_codebook
    • both report Machine: RISC-V.

Notes

  • This patch does not add a RISC-V-specific optimized backend.
  • The goal is to ensure vorbis uses and validates its existing portable scalar path correctly for RISC-V-oriented builds, not to change codec behavior or claim new architecture-specific acceleration.

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.

1 participant