ToCString: emit stackalloc as 64-byte-aligned uint64_t array - #532
Open
spitters wants to merge 1 commit into
Open
ToCString: emit stackalloc as 64-byte-aligned uint64_t array#532spitters wants to merge 1 commit into
spitters wants to merge 1 commit into
Conversation
The previous emission used [uint8_t tmp[N] = {0};] then took its address
as a [br_word_t]. This produces unaligned access in downstream code that
treats the buffer as a word array (common in extracted bedrock2-WP proofs
where the allocation is the backing memory for a felem / scalar).
Switch to:
uint64_t tmp[ceil(N/8)] __attribute__((aligned(64))) = {0};
x = (br_word_t)tmp;
- 64-byte alignment matches typical cache-line + SIMD requirements.
- ceil(N/8) words rounds the allocation up so we keep ≥ N bytes available.
- Zero-init preserved (bedrock2's [stackalloc] specifies arbitrary contents
but downstream code may have observed zeros under the prior emission).
- Drops the [&] since [tmp] already decays to a pointer.
Verified clean compile of the bedrock2 + downstream Rocq tree after the
swap, and measured ~5-15% speedup on extracted-C ed25519_sign timing in
our downstream (curve25519-jasmin-rs) bench, attributable to aligned-load
fast paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The previous emission for
cmd.stackallocused a byte-granular array:```c
uint8_t _br_stackalloc_x[N] = {0};
x = (br_word_t)&_br_stackalloc_x;
```
Downstream code that treats the buffer as a word array (very common — most extracted-bedrock2 WP proofs allocate scratch for felem / scalar / pairing-tower values) then accesses through unaligned word loads/stores. On x86_64 this is fine for correctness but takes the unaligned-access slow path under some microarchitectures + SIMD; on ARM and RISC-V it can fault outright.
This patch switches to a word-sized, 64-byte-aligned allocation:
```c
uint64_t _br_stackalloc_x[ceil(N/8)] attribute((aligned(64))) = {0};
x = (br_word_t)_br_stackalloc_x;
```
ceil(N/8)words rounds up so we keep ≥ N bytes available.stackallocspecifies arbitrary contents, but downstream code may have observed zeros under the prior emission — keep that to avoid surprising downstream).&sincetmpalready decays to a pointer.Test plan
curve25519-jasmin-rsbench, attributable to aligned-load fast paths.__attribute__((aligned(64)))on the array declaration; clang likewise. MSVC syntax would be different but bedrock2's emitted C already uses GCC-style extensions.