Skip to content

Build: Fix RISC-V ISA detection dead code and sanitize -march against modern kernel/toolchain mismatch#14530

Open
fengpengboa wants to merge 1 commit intofacebook:mainfrom
fengpengboa:detect_platform
Open

Build: Fix RISC-V ISA detection dead code and sanitize -march against modern kernel/toolchain mismatch#14530
fengpengboa wants to merge 1 commit intofacebook:mainfrom
fengpengboa:detect_platform

Conversation

@fengpengboa
Copy link
Copy Markdown

@fengpengboa fengpengboa commented Mar 30, 2026

Background:

In build_tools/build_detect_platform, there is a section intended to optimize RocksDB for RISC-V natively by parsing /proc/cpuinfo to populate the -march flag.
However, it contained a typo: it assigned the result to RISC_ISA but checked if [ -n "${RISCV_ISA}" ]. Due to this typo, the RISC-V specific hardware optimization was effectively dead code and never executed.

If one simply fixes the typo (RISC_ISA -> RISCV_ISA), the build immediately breaks on modern RISC-V hardware due to a mismatch between bleeding-edge Linux kernels and the GNU toolchain.

Crash 1: GCC rejects privileged extensions
Modern RISC-V Linux kernels report Supervisor-level (_s*), Hypervisor-level (_h*), and Custom (_x*) extensions in /proc/cpuinfo (e.g., sscofpmf_sstc_svinval).
Passing this raw string to GCC when compiling user-space code like RocksDB causes a fatal error:

$DEBUG_LEVEL is 0, $LIB_MODE is static
g++: error: ‘-march=rv64imafdcv_zicbom_zicboz_zicntr_zicond_zicsr_zifencei_zihintntl_zihintpause_zihpm_zawrs_zfa_zfh_zfhmin_zca_zcb_zcd_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvfh_zvfhmin_sscofpmf_sstc_svinval_svnapot_svpbmt’: unexpected ISA string at end: ‘sscofpmf_sstc_svinval_svnapot_svpbmt’

Crash 2: Binutils (Assembler) truncates the ISA string and loses zbb/zbs
Even if we strip the privileged _s* extensions, the kernel also exposes newly ratified user-space extensions (like zicntr or zihpm). Older versions of Binutils (as) do not recognize these new zi* extensions.
When the assembler encounters an unknown extension like zicntr, it throws:

Assembler messages:
Error: rv64imafdcv_zicbom _zicboz_zicntr_zicond_zicsr_zifencei_zihintntl_zihintpause_zihpm_zawrs_zfa_zfh_zfhmin_zca_zcb_zcd_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvfh_zvfhmin_zvl128b_zvl32b_zvl64b: unknown prefixed ISA extension `zicntr'
/tmp/ccZR3Vg7.s:3: Error: rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicbom_zicboz_zicntr_zicond_zicsr2p0_zifencei2p0_zihintntl_zihintpause_zihpm_zawrs_zfa_zfh_zfhmin_zca_zcb_zcd_zba1p0_zbb1p0_zbc1p0_zbs1p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvfh_zvfhmin_zvl128b1p0_zvl32b1p0_zvl64b1p0: unknown prefixed ISA extension `zicntr'
/tmp/ccZR3Vg7.s:1702: Error: unrecognized opcode `bseti a5,zero,63', extension `zbs' required
/tmp/ccZR3Vg7.s:1854: Error: unrecognized opcode `bseti a5,zero,63', extension `zbs' required

The Solution: Whitelisting & Compiler Probing

To make the build script robust against future kernel updates and lagging toolchains, this PR introduces:

  1. Typo Fix: Corrected variable names to activate the logic.
  2. Whitelist Filtering: Instead of passing everything, we extract the Base ISA (e.g., rv64imafdcv) and strictly append only safe, performance-enhancing extensions that RocksDB actually benefits from: _zb* (Bitmanip), _zv* (Vector), and _zk* (Crypto).
  3. Feature Probing: Added a dummy C++ compilation probe (echo "int main(){}" | $CXX ...). Before trusting the synthesized -march string, we test if the local host compiler can actually parse it. If it fails (e.g., severely outdated GCC), it gracefully falls back to the compiler's default.

Test Plan

Tested natively on a modern RISC-V (rv64) SG2044 environment

@meta-cla
Copy link
Copy Markdown

meta-cla bot commented Mar 30, 2026

Hi @fengpengboa!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@fengpengboa
Copy link
Copy Markdown
Author

Hi @archang19 , I have a CLA-related question for this PR.Our company has previously signed the Meta Corporate CLA for other open source projects. When I tried to sign again for RocksDB, the system indicated that our company is already registered and prevented me from signing a new one.However, I do not know who the original signer was, and I am unable to locate that person internally to be added to the authorized contributor list.
I have already sent an email to cla@meta.com regarding this, but haven't received a response yet. Could someone please help check the status or assist in associating my GitHub account with our company's existing CLA record?Thank you for your help!

@fengpengboa fengpengboa closed this Apr 1, 2026
@fengpengboa fengpengboa changed the title Fix variable name mismatch for RISC-V ISA detection in build_detect_platform Build: Fix RISC-V ISA detection dead code and sanitize -march against modern kernel/toolchain mismatch Apr 1, 2026
@fengpengboa fengpengboa reopened this Apr 1, 2026
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