Skip to content

core: switch calls to std::byteswap to bswap - #2498

Open
goodlyrottenapple wants to merge 1 commit into
sam/offset-trie-improvementsfrom
sam/core-bswap
Open

core: switch calls to std::byteswap to bswap#2498
goodlyrottenapple wants to merge 1 commit into
sam/offset-trie-improvementsfrom
sam/core-bswap

Conversation

@goodlyrottenapple

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Refactor is clean and correct. The monad::bit::byteswap indirection is a tidy way to substitute a libgcc-free implementation for the zkVM (riscv64ima) guest without perturbing host callers, and the mechanical rename of remaining call sites from std::byteswap to bswap keeps the layering (bit::byteswap for category/core/runtime/, bswap for everything else) consistent.

Verified in zkvm/category/core/runtime/bit.hpp:

  • The 8-byte SWAR sequence reduces 0x0102030405060708 to 0x0807060504030201 (traced by hand).
  • The 4-byte sequence is the standard 8/8/16 pattern.
  • The sizeof(T) != 4 && sizeof(T) != 8 fallback to std::byteswap covers uint16_t (which GCC inlines even without rev8) and 1-byte cases (identity).

No FFI / RLP semantics change on the host: bswap produces bit-identical output to std::byteswap for all migrated call sites, so consensus / wire encodings are unaffected.

The category/mpt/util.hpp refactor is functionally equivalent to the previous cascade of __builtin_bswap{16,32,64}; as a bonus, it silently fixes a latent uint8_t bug where the old __builtin_bswap16(n) assigned back into a uint8_t would zero the value (no current callers exercise sizeof==1, so no observable behavior change).

No new tests, but existing coverage in test/vm/unit/uint128_tests.cpp, test/vm/unit/uint256_tests.cpp, and test/vm/unit/runtime/transmute_tests.cpp exercises bswap / byteswap through the same code path on host builds. The zkVM open-coding is not directly covered by host tests, but the algorithm is a well-known idiom.

Verdict: CORRECT

🤖 Generated with Claude Code

Comment thread zkvm/category/core/runtime/bit.hpp
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/offset-trie-improvements branch 4 times, most recently from babfad7 to e5b92a8 Compare August 24, 2026 09:07
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/offset-trie-improvements branch from e5b92a8 to 073157e Compare August 24, 2026 11:07
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/offset-trie-improvements branch 2 times, most recently from 0d6d7ff to 57abbae Compare August 24, 2026 14:56
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/core-bswap branch 2 times, most recently from 0eb6ac7 to 4701ebf Compare August 24, 2026 15:10
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/offset-trie-improvements branch from 57abbae to f07ffab Compare August 25, 2026 12:21
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/offset-trie-improvements branch from f07ffab to 1593c6d Compare August 25, 2026 12:40
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/core-bswap branch 2 times, most recently from 2684331 to a52ebee Compare August 26, 2026 12:14
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/offset-trie-improvements branch from 1593c6d to ceb0ebc Compare August 26, 2026 12:14
@goodlyrottenapple
goodlyrottenapple force-pushed the sam/offset-trie-improvements branch from ceb0ebc to b7056a6 Compare August 28, 2026 14:37
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.

3 participants