Repository navigation
[CI/Build] Restore RISC-V support without README badge - #3816
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Restores native RISC-V build support by reintroducing CMake compatibility switches, handling 16-byte atomic runtime requirements, and documenting a validated RISC-V build configuration (without README badge changes).
Changes:
- Added
USE_RISCVoption to apply RISC-V-specific build settings (notably IPO/LTO behavior). - Added configure-time detection of 16-byte atomic support and conditional
libatomiclinkage. - Documented a working native
riscv64build path with Go-dependent components disabled.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| mooncake-store/src/CMakeLists.txt | Detects 16-byte atomic support under RISC-V and links libatomic when needed; minor message formatting change. |
| mooncake-common/common.cmake | Introduces USE_RISCV and disables IPO to avoid pybind11 full-LTO behavior on RISC-V hosts. |
| docs/source/getting_started/build.md | Adds a RISC-V build section and documents USE_RISCV in the CMake options table. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Define this before any pybind11 module is created. Otherwise pybind11 adds | ||
| # its default full-LTO target, which is prohibitively resource-intensive on | ||
| # RISC-V build hosts. | ||
| set(CMAKE_INTERPROCEDURAL_OPTIMIZATION OFF) | ||
| message(STATUS "RISC-V: IPO disabled for Mooncake Python extensions") |
| optimization for the Python extensions, avoiding the excessive memory use of | ||
| full GNU LTO on RISC-V build hosts. The build also detects whether 16-byte |
| check_cxx_source_compiles( | ||
| " | ||
| alignas(16) volatile unsigned __int128 value = 0; | ||
| int main(int argc, char**) { | ||
| __atomic_store_n(&value, static_cast<unsigned __int128>(argc), | ||
| __ATOMIC_SEQ_CST); | ||
| return static_cast<int>(__atomic_load_n(&value, __ATOMIC_SEQ_CST)); | ||
| } | ||
| " | ||
| MOONCAKE_HAS_NATIVE_16BYTE_ATOMICS) | ||
| if(NOT MOONCAKE_HAS_NATIVE_16BYTE_ATOMICS) | ||
| set(CMAKE_REQUIRED_LIBRARIES_SAVE ${CMAKE_REQUIRED_LIBRARIES}) | ||
| set(CMAKE_REQUIRED_LIBRARIES atomic) | ||
| check_cxx_source_compiles( | ||
| " | ||
| alignas(16) volatile unsigned __int128 value = 0; | ||
| int main(int argc, char**) { | ||
| __atomic_store_n(&value, static_cast<unsigned __int128>(argc), | ||
| __ATOMIC_SEQ_CST); | ||
| return static_cast<int>(__atomic_load_n(&value, __ATOMIC_SEQ_CST)); | ||
| } | ||
| " |
| list(APPEND EXTRA_LIBS atomic) | ||
| message(STATUS "16-byte atomics: using libatomic") | ||
| else() | ||
| message(FATAL_ERROR "16-byte atomic operations are not supported") |
github-actions
Bot
added
documentation
catyans
left a comment
catyans
left a comment
There was a problem hiding this comment.
LGTM. The RISC-V path is opt-in, the 16-byte atomic probe links libatomic only when native support is absent, and CI is green. suggestion (non-blocking): CMAKE_INTERPROCEDURAL_OPTIMIZATION=OFF is project-wide, so please either document that scope or limit it to the Python extension targets.
Description
Reintroduce the RISC-V build support from #3590 after it was reverted by #3759, while intentionally leaving
README.mdunchanged.USE_RISCVto control RISC-V compatibility settings.tentpymodule.libatomicwhen required.No overlapping open issue or PR was found for RISC-V support.
Module
mooncake-store)mooncake-integration)mooncake-common)Type of Change
How Has This Been Tested?
Test commands:
pre-commit run --files \ docs/source/getting_started/build.md \ mooncake-common/common.cmake \ mooncake-store/src/CMakeLists.txt cmake -G Ninja .. \ -DCMAKE_BUILD_TYPE=Release \ -DUSE_RISCV=ON \ -DWITH_STORE_GO=OFF \ -DWITH_P2P_STORE=OFF \ -DUSE_ETCD=OFF \ -DSTORE_USE_ETCD=OFF \ -DSTORE_USE_K8S_LEASE=OFF \ -DBUILD_UNIT_TESTS=OFF \ -DBUILD_EXAMPLES=OFF \ -DBUILD_BENCHMARK=OFF cmake --build . --parallel 4Test results:
Unit tests pass
Integration tests pass (if applicable)
Manual testing done
Native RISC-V Release build completed successfully on
riscv64Linux.mooncake_master,mooncake_client, and both Python extensions were validated as RISC-V ELF artifacts.The
engineandstorePython extensions imported successfully.A separate native
riscv64configuration withUSE_TENT=ONconfirmed thattentpyis generated without-flto.All changed-file pre-commit hooks pass.
The same CMake and documentation changes passed the full [CI/Build] Add RISC-V build support #3590 CI matrix before Revert "[CI/Build] Add RISC-V build support" #3759 reverted the PR.
Checklist
./scripts/code_format.sh(not applicable: no C/C++ source changed;cmake-formatpassed)AI Assistance Disclosure
OpenAI Codex assisted with diagnosing the RISC-V build failures, implementing and reviewing the CMake changes, writing documentation, and running validation. The human submitter is responsible for reviewing and defending every changed line.