Proposal: Add RISC-V Optimization for XXH_mult32to64_add64 with Inline Assembly #1018

New issue

Closed

opened 2025-03-13 01:43:47 -05:00 by Jay-Lokhande · 23 comments

Jay-Lokhande commented 2025-03-13 01:43:47 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi @Cyan4973,

I’ve been working on compiling xxHash as part of CouchDB on a RISC-V machine (64-bit, riscv64-unknown-linux-gnu target). While reviewing xxhash.h, I noticed there’s no specific configuration or optimization for RISC-V, unlike the AArch64 case which uses umaddl for XXH_mult32to64_add64. Given RISC-V’s increasing relevance, I thought it might benefit from a tailored optimization.

To address this, I added a RISC-V-specific version of XXH_mult32to64_add64 using inline assembly with slli and srli to mask inputs to 32 bits, followed by mul and add. Here’s the implementation I’ve tested:

#elif defined(__riscv) && (__riscv_xlen == 64)
XXH_FORCE_INLINE xxh_u64
XXH_mult32to64_add64(xxh_u64 lhs, xxh_u64 rhs, xxh_u64 acc)
{
    xxh_u64 ret;
    __asm__ (
        "slli t0, %1, 32 \n\t"
        "srli t0, t0, 32 \n\t"
        "slli t1, %2, 32 \n\t"
        "srli t1, t1, 32 \n\t"
        "mul %0, t0, t1 \n\t"
        "add %0, %0, %3"
        : "=r" (ret)
        : "r" (lhs), "r" (rhs), "r" (acc)
        : "t0", "t1"
    );
    return ret;
}

I built this within CouchDB on a RISC-V QEMU environment, disassembled the resulting exxhash.so, and confirmed the assembly matches my intent (e.g., slli t0, a0, 32; srli t0, t0, 32; ... appearing in XXH3_hashLong_64b_default).

Before I commit this change and create a pull request, I wanted to get your thoughts:

Do you think this RISC-V optimization aligns with xxHash’s goals?
Are there specific tests or benchmarks you’d recommend I run to validate it further?
Any suggestions for refining the implementation (e.g., alternative instructions or constraints)?
I’d be happy to provide more details or adjust based on your feedback. Thanks for your time and for maintaining this great library!

Best regards,
Jay

Hi @Cyan4973, I’ve been working on compiling xxHash as part of CouchDB on a RISC-V machine (64-bit, riscv64-unknown-linux-gnu target). While reviewing `xxhash.h`, I noticed there’s no specific configuration or optimization for RISC-V, unlike the AArch64 case which uses `umaddl` for `XXH_mult32to64_add64`. Given RISC-V’s increasing relevance, I thought it might benefit from a tailored optimization. To address this, I added a RISC-V-specific version of `XXH_mult32to64_add64` using inline assembly with `slli` and `srli` to mask inputs to 32 bits, followed by `mul` and `add`. Here’s the implementation I’ve tested: ```c #elif defined(__riscv) && (__riscv_xlen == 64) XXH_FORCE_INLINE xxh_u64 XXH_mult32to64_add64(xxh_u64 lhs, xxh_u64 rhs, xxh_u64 acc) { xxh_u64 ret; __asm__ ( "slli t0, %1, 32 \n\t" "srli t0, t0, 32 \n\t" "slli t1, %2, 32 \n\t" "srli t1, t1, 32 \n\t" "mul %0, t0, t1 \n\t" "add %0, %0, %3" : "=r" (ret) : "r" (lhs), "r" (rhs), "r" (acc) : "t0", "t1" ); return ret; } ``` I built this within CouchDB on a RISC-V QEMU environment, disassembled the resulting `exxhash.so`, and confirmed the assembly matches my intent (e.g., `slli t0, a0, 32; srli t0, t0, 32; ...` appearing in `XXH3_hashLong_64b_default`). Before I commit this change and create a pull request, I wanted to get your thoughts: Do you think this RISC-V optimization aligns with xxHash’s goals? Are there specific tests or benchmarks you’d recommend I run to validate it further? Any suggestions for refining the implementation (e.g., alternative instructions or constraints)? I’d be happy to provide more details or adjust based on your feedback. Thanks for your time and for maintaining this great library! Best regards, Jay

👍 1 👀 1

Jay-Lokhande commented 2025-04-03 06:58:59 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@Cyan4973 what do you think should we go ahead with proposed changes?

@Cyan4973 what do you think should we go ahead with proposed changes?

t-mat commented 2025-04-03 07:39:31 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi, @Jay-Lokhande

Since it seems Yann is busy for now, I'm asking basic question.

Do you observe actual benefit from your inline assembly implementation?

I mean that every version of RISC-V gcc (8.2.0 .. trunk) generates good code. All of them are similar to your inline assmebly version. Order of instructions are different. But total amount of latency seems same.

EDIT : Sorry, it's my mistake. gcc [8.2.0 .. 13.3.0] generates different code. gcc [14.x] generates similar to your code. So it may have benefit for older version of gcc. But I'm not sure about actual performance of them.

EDIT2 : I think I understand the point of the optimization. Since it uses less temporary register (li .. srli .. and sequence needs one more register for and mask), it may reduces register pressure and latency (for initialization of the mask value).

Experiment: https://godbolt.org/z/YqzosTfTM

Output of gcc 14.x

#include <stdint.h>

typedef uint32_t XXH32_hash_t;
typedef uint64_t XXH64_hash_t;
typedef XXH32_hash_t xxh_u32;
typedef XXH64_hash_t xxh_u64;

xxh_u64 XXH_mult32to64(xxh_u64 x, xxh_u64 y) {
   return (x & 0xFFFFFFFF) * (y & 0xFFFFFFFF);
}

xxh_u64 XXH_mult32to64_add64(xxh_u64 lhs, xxh_u64 rhs, xxh_u64 acc) {
    return XXH_mult32to64((xxh_u32)lhs, (xxh_u32)rhs) + acc;
}

XXH_mult32to64(unsigned long, unsigned long):
        slli    a0,a0,32
        slli    a1,a1,32
        srli    a1,a1,32
        srli    a0,a0,32
        mul     a0,a0,a1
        ret
XXH_mult32to64_add64(unsigned long, unsigned long, unsigned long):
        slli    a0,a0,32
        slli    a1,a1,32
        srli    a1,a1,32
        srli    a0,a0,32
        mul     a0,a1,a0
        add     a0,a0,a2
        ret

Output of gcc [8.2.0 .. 13.3.0]

XXH_mult32to64(unsigned long, unsigned long):
        li      a5,-1
        srli    a5,a5,32
        and     a0,a0,a5
        and     a1,a1,a5
        mul     a0,a0,a1
        ret
XXH_mult32to64_add64(unsigned long, unsigned long, unsigned long):
        li      a5,-1
        srli    a5,a5,32
        and     a1,a1,a5
        and     a0,a0,a5
        mul     a0,a1,a0
        add     a0,a0,a2
        ret

Hi, @Jay-Lokhande Since it seems Yann is busy for now, I'm asking basic question. Do you observe actual benefit from your inline assembly implementation? <strike>I mean that every version of RISC-V gcc (8.2.0 .. trunk) generates good code. All of them are similar to your inline assmebly version. Order of instructions are different. But total amount of latency seems same. </strike> EDIT : Sorry, it's my mistake. gcc [8.2.0 .. 13.3.0] generates different code. gcc [14.x] generates similar to your code. So it may have benefit for older version of gcc. But I'm not sure about actual performance of them. EDIT2 : I think I understand the point of the optimization. Since it uses less temporary register (`li .. srli .. and` sequence needs one more register for `and` mask), it may reduces register pressure and latency (for initialization of the mask value). Experiment: https://godbolt.org/z/YqzosTfTM <details> <summary>Output of gcc 14.x</summary> ``` #include <stdint.h> typedef uint32_t XXH32_hash_t; typedef uint64_t XXH64_hash_t; typedef XXH32_hash_t xxh_u32; typedef XXH64_hash_t xxh_u64; xxh_u64 XXH_mult32to64(xxh_u64 x, xxh_u64 y) { return (x & 0xFFFFFFFF) * (y & 0xFFFFFFFF); } xxh_u64 XXH_mult32to64_add64(xxh_u64 lhs, xxh_u64 rhs, xxh_u64 acc) { return XXH_mult32to64((xxh_u32)lhs, (xxh_u32)rhs) + acc; } ``` ``` XXH_mult32to64(unsigned long, unsigned long): slli a0,a0,32 slli a1,a1,32 srli a1,a1,32 srli a0,a0,32 mul a0,a0,a1 ret XXH_mult32to64_add64(unsigned long, unsigned long, unsigned long): slli a0,a0,32 slli a1,a1,32 srli a1,a1,32 srli a0,a0,32 mul a0,a1,a0 add a0,a0,a2 ret ``` </details> <details> <summary>Output of gcc [8.2.0 .. 13.3.0]</summary> ``` XXH_mult32to64(unsigned long, unsigned long): li a5,-1 srli a5,a5,32 and a0,a0,a5 and a1,a1,a5 mul a0,a0,a1 ret XXH_mult32to64_add64(unsigned long, unsigned long, unsigned long): li a5,-1 srli a5,a5,32 and a1,a1,a5 and a0,a0,a5 mul a0,a1,a0 add a0,a0,a2 ret ``` </details>

Jay-Lokhande commented 2025-04-04 11:11:36 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi, @t-mat Thank you for your suggestion. What do you think adding RISC-V support in xxHash will be beneficial would you like to give me any suggestions over it should I go ahead with the changes or would you like to suggest any benchmarking/tests over it?

Hi, @t-mat Thank you for your suggestion. What do you think adding RISC-V support in xxHash will be beneficial would you like to give me any suggestions over it should I go ahead with the changes or would you like to suggest any benchmarking/tests over it?

t-mat commented 2025-04-16 06:23:14 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

What do you think adding RISC-V support in xxHash will be beneficial would you like to give me any suggestions over it should I go ahead with the changes or would you like to suggest any benchmarking/tests over it?

First of all, since RISC-V has many variants, basically we should avoid inline-assmbly code which tend to prevent compiler's further optimization.
Actually, it seems recent compilers improved their code generator/optimizer. So, even if this patch will be merged, eventually it will be removed or applied to specifc versions of older compilers.

But this patch has some merit for older compilers. Therefore, I think we need benchmarks. It would be nice to have the result of xxhsum's built-in benchmark. And we can compare between

(1) Original (patch-less) code with older compiler
(2) This patch with older compiler
(3) Original (patch-less) code with the latest compiler
(4) This patch with the latest compiler

I suppose (2) (your patch) may be better than (1) (the original). But (2), (3) and (4) will be similar result thanks to the latest compilers.

> What do you think adding RISC-V support in xxHash will be beneficial would you like to give me any suggestions over it should I go ahead with the changes or would you like to suggest any benchmarking/tests over it? First of all, since RISC-V has many variants, basically we should avoid inline-assmbly code which tend to prevent compiler's further optimization. Actually, it seems recent compilers improved their code generator/optimizer. So, even if this patch will be merged, eventually it will be removed or applied to specifc versions of older compilers. But this patch has some merit for older compilers. Therefore, I think we need benchmarks. It would be nice to have the result of `xxhsum`'s built-in benchmark. And we can compare between (1) Original (patch-less) code with older compiler (2) This patch with older compiler (3) Original (patch-less) code with the latest compiler (4) This patch with the latest compiler I suppose (2) (your patch) may be better than (1) (the original). But (2), (3) and (4) will be similar result thanks to the latest compilers.

👍 1 👀 1

Jay-Lokhande commented 2025-04-25 08:05:55 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

But this patch has some merit for older compilers. Therefore, I think we need benchmarks. It would be nice to have the result of xxhsum's built-in benchmark. And we can compare between

Hi @t-mat , thanks for the feedback!

I agree that real numbers will help decide whether, and for which toolchains, we should ship this. Here’s what I think I have to do next:

1. Run xxhsumbenchmarks under QEMU/RISC-V for both small and large inputs (1 KB, 100 KB, 1 MB, 100 MB), using:

   • GCC 8–13 (original vs. patched)
   • GCC 14+ (original vs. patched)

2. Guard the inline-asm behind a compiler-version check so that it only applies for older GCCs:

#if defined(__riscv) && (__riscv_xlen == 64) && (__GNUC__ < 14)
// …inline-asm version…
#endif

3. Post a mini-table of results here and propose merging only when it shows a measurable win on older compilers.

Does that plan sound good?
Are there any additional input sizes or flags you’d recommend including in the benchmark?

Thanks again for helping refine this!
Jay

> But this patch has some merit for older compilers. Therefore, I think we need benchmarks. It would be nice to have the result of `xxhsum`'s built-in benchmark. And we can compare between Hi @t-mat , thanks for the feedback! I agree that real numbers will help decide whether, and for which toolchains, we should ship this. Here’s what I think I have to do next: 1. Run xxhsumbenchmarks under QEMU/RISC-V for both small and large inputs (1 KB, 100 KB, 1 MB, 100 MB), using: - GCC 8–13 (original vs. patched) - GCC 14+ (original vs. patched) 2. Guard the inline-asm behind a compiler-version check so that it only applies for older GCCs: ```c #if defined(__riscv) && (__riscv_xlen == 64) && (__GNUC__ < 14) // …inline-asm version… #endif ``` 3. Post a mini-table of results here and propose merging only when it shows a measurable win on older compilers. Does that plan sound good? Are there any additional input sizes or flags you’d recommend including in the benchmark? Thanks again for helping refine this! Jay

t-mat commented 2025-04-28 04:32:52 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Does that plan sound good?

Yes, it sounds good as a first step.

Are there any additional input sizes or flags you’d recommend including in the benchmark?

Having benchmark for small data would be nice.

For example, 16 bytes:

cd /path/to/xxHash
make clean default

# -b5=XXH3_64b, 11=XXH128, 21=XXH3_stream, 25=XXH128_stream
./xxhsum -b5,11,21,25 -B16     # 16 bytes

If you need to know the special lengthes in XXH3, please refer the following codes:

• XXH3_128bits_internal()
• XXH3_len_0to16_128b()
• XXH3_MIDSIZE_MAX

> Does that plan sound good? Yes, it sounds good as a first step. > Are there any additional input sizes or flags you’d recommend including in the benchmark? Having benchmark for [small data](https://github.com/Cyan4973/xxHash/wiki/Performance-comparison#benchmarks-concentrating-on-small-data-) would be nice. For example, 16 bytes: ``` cd /path/to/xxHash make clean default # -b5=XXH3_64b, 11=XXH128, 21=XXH3_stream, 25=XXH128_stream ./xxhsum -b5,11,21,25 -B16 # 16 bytes ``` If you need to know the special lengthes in XXH3, please refer the following codes: - [`XXH3_128bits_internal()`](https://github.com/Cyan4973/xxHash/blob/v0.8.3/xxhash.h#L6970-L6989) - [`XXH3_len_0to16_128b()`](https://github.com/Cyan4973/xxHash/blob/v0.8.3/xxhash.h#L6747-L6761) - [`XXH3_MIDSIZE_MAX`](https://github.com/Cyan4973/xxHash/blob/v0.8.3/xxhash.h#L1945)

Jay-Lokhande commented 2025-04-29 08:56:51 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi @t-mat,

Thanks again for the detailed suggestions and support throughout!

I’ve set up multiple GCC versions (9 through 14) inside the RISC-V QEMU environment.
IMAGE

I’m using a benchmarking script that iterates over these versions and runs xxhsum with the flags you recommended (-b5,11,21,25 -B16). Here's a quick look at the setup script I'm using:

#!/bin/bash

GCC_VERSIONS=("9" "10" "11" "12" "13" "14")
BENCHMARK_FLAGS="-b5,11,21,25 -B16"

for ver in "${GCC_VERSIONS[@]}"; do
    echo "============================================"
    echo "Benchmarking with GCC-$ver"
    echo "============================================"

    export CC=/usr/bin/gcc-$ver

    make clean > /dev/null
    make default > /dev/null

    if [ -f ./xxhsum ]; then
        echo "Running benchmark with gcc-$ver:"
        ./xxhsum $BENCHMARK_FLAGS
    else
        echo "Build failed for gcc-$ver"
    fi

    echo -e "\n\n"
done

I’m currently facing some issues with installing GCC-8, but I’ll try to resolve that soon for a more complete comparison. Due to ongoing college exams, I’ll need 2–3 days to complete all benchmarks and post the results table here.

Thanks again for your patience and valuable feedback!

Best regards,
Jay

Hi @t-mat, Thanks again for the detailed suggestions and support throughout! I’ve set up multiple GCC versions (9 through 14) inside the RISC-V QEMU environment. [IMAGE](https://github.com/user-attachments/assets/879795a1-5a41-4015-9ebd-e61cd361edae) I’m using a benchmarking script that iterates over these versions and runs xxhsum with the flags you recommended (-b5,11,21,25 -B16). Here's a quick look at the setup script I'm using: ```bash #!/bin/bash GCC_VERSIONS=("9" "10" "11" "12" "13" "14") BENCHMARK_FLAGS="-b5,11,21,25 -B16" for ver in "${GCC_VERSIONS[@]}"; do echo "============================================" echo "Benchmarking with GCC-$ver" echo "============================================" export CC=/usr/bin/gcc-$ver make clean > /dev/null make default > /dev/null if [ -f ./xxhsum ]; then echo "Running benchmark with gcc-$ver:" ./xxhsum $BENCHMARK_FLAGS else echo "Build failed for gcc-$ver" fi echo -e "\n\n" done ``` I’m currently facing some issues with installing GCC-8, but I’ll try to resolve that soon for a more complete comparison. Due to ongoing college exams, I’ll need 2–3 days to complete all benchmarks and post the results table here. Thanks again for your patience and valuable feedback! Best regards, Jay

👍 1

Jay-Lokhande commented 2025-05-04 00:21:49 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hello @t-mat,

📊 xxHash Benchmark Results (Without Patch) CLI-img

GCC Version	5#XXH3_64b (MB/s)	11#XXH128 (MB/s)	21#XXH3_stream (MB/s)	25#XXH128_stream (MB/s)
GCC-9	66.6	53.4	11.0	10.9
GCC-10	66.2	64.5	11.0	10.9
GCC-11	65.8	63.8	12.4	12.6
GCC-12	65.8	63.8	12.4	11.5
GCC-13	70.6	68.8	10.1	11.3
GCC-14	71.9	64.0	12.5	12.5

📌 Based on xxhsum 0.8.3 sample of 16 bytes benchmark run across GCC 9–14

I dont have exciting results with patch which i shown in the first comment. CLI-img CLI-img2

🛠️ xxHash Benchmark Results (With Patch - Sanity Check Failed)

GCC Version	Status	Failed Test	Got Value (truncated)	Expected Value
GCC-10	❌ Sanity check failed	164	0x55F2F290F6F4B191	0xCDEB804D65C6DEA4
GCC-11	❌ Sanity check failed	166	0xCE6F2AA671ACD460	0xCDEB804D65C6DEA4
GCC-12	❌ Sanity check failed	166	0x68151A0F13EAEC97	0xCDEB804D65C6DEA4
GCC-13	❌ Sanity check failed	167	0x3D208805571CAB500	0xCDEB804D65C6DEA4
GCC-14	❌ Sanity check failed	164	0x5B4357B5295B4218	0xCDEB804D65C6DEA4

same for GCC-9 also

⚠️ Note: All versions fail the internal sanity check with the patch. If you modified the hash functions, make sure to either update the values or temporarily recompile with XSUM_NO_TESTS=1.

Hello @t-mat, ### 📊 xxHash Benchmark Results (Without Patch) [CLI-img](https://github.com/user-attachments/assets/2f0ef251-7201-4ef8-ba0a-6fa2e198c872) | GCC Version | 5#XXH3_64b (MB/s) | 11#XXH128 (MB/s) | 21#XXH3_stream (MB/s) | 25#XXH128_stream (MB/s) | |-------------|------------------|------------------|------------------------|--------------------------| | GCC-9 | 66.6 | 53.4 | 11.0 | 10.9 | | GCC-10 | 66.2 | 64.5 | 11.0 | 10.9 | | GCC-11 | 65.8 | 63.8 | 12.4 | 12.6 | | GCC-12 | 65.8 | 63.8 | 12.4 | 11.5 | | GCC-13 | 70.6 | 68.8 | 10.1 | 11.3 | | GCC-14 | 71.9 | 64.0 | 12.5 | 12.5 | > 📌 Based on `xxhsum 0.8.3` sample of 16 bytes benchmark run across GCC 9–14 I dont have exciting results with patch which i shown in the [first comment](https://github.com/Cyan4973/xxHash/issues/1018#issue-2916032572). [CLI-img](https://github.com/user-attachments/assets/139f0afe-7853-45d0-8716-328cbf76441d) [CLI-img2](https://github.com/user-attachments/assets/4bf0ca00-175e-4758-b814-d61eb4515be6) ### 🛠️ xxHash Benchmark Results (With Patch - Sanity Check Failed) | GCC Version | Status | Failed Test | Got Value (truncated) | Expected Value | |-------------|-------------------------------|-------------|------------------------|------------------------| | GCC-10 | ❌ Sanity check failed | 164 | `0x55F2F290F6F4B191` | `0xCDEB804D65C6DEA4` | | GCC-11 | ❌ Sanity check failed | 166 | `0xCE6F2AA671ACD460` | `0xCDEB804D65C6DEA4` | | GCC-12 | ❌ Sanity check failed | 166 | `0x68151A0F13EAEC97` | `0xCDEB804D65C6DEA4` | | GCC-13 | ❌ Sanity check failed | 167 | `0x3D208805571CAB500` | `0xCDEB804D65C6DEA4` | | GCC-14 | ❌ Sanity check failed | 164 | `0x5B4357B5295B4218` | `0xCDEB804D65C6DEA4` | same for GCC-9 also > ⚠️ Note: All versions fail the internal sanity check with the patch. If you modified the hash functions, make sure to either update the values or temporarily recompile with `XSUM_NO_TESTS=1`.

Jay-Lokhande commented 2025-05-09 14:10:44 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hello @t-mat , any thoughts in it.

Hello @t-mat , any thoughts in it.

t-mat commented 2025-05-09 22:47:04 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Before further discussion, all tests including sanity check must be passed.

Since failure of sanity check means broken calculation, please debug your patch.

Before further discussion, all tests including sanity check must be passed. Since failure of sanity check means broken calculation, please debug your patch.

👍 1

Jay-Lokhande commented 2025-05-11 08:33:44 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Update on RISC-V implementation of XXH_mult32to64_add64:

#elif defined(__riscv) && (__riscv_xlen == 64)
XXH_FORCE_INLINE xxh_u64
XXH_mult32to64_add64(xxh_u64 lhs, xxh_u64 rhs, xxh_u64 acc)
{
    xxh_u64 ret;
    __asm__ (
        "slli  t0, %1, 32    \n\t"  // extract low 32 bits of lhs
        "srli  t0, t0, 32    \n\t"
        "slli  t1, %2, 32    \n\t"  // extract low 32 bits of rhs
        "srli  t1, t1, 32    \n\t"
        "mul   t0, t0, t1    \n\t"  // multiply
        "add   %0, t0, %3    \n\t"  // add acc
        : "=&r" (ret)              // <-- early-clobber (&) prevents aliasing!
        : "r" (lhs), "r" (rhs), "r" (acc)
        : "t0", "t1"
    );
    return ret;
}

Uses =&r to safely mark ret as early-clobber (non-overlapping with inputs),
Performs all computation in temporary registers (t0, t1), ensuring inputs are untouched,
It has passed all test cases and is compatible with GCC

I will put the comparison table soon....

Update on RISC-V implementation of XXH_mult32to64_add64: ```c #elif defined(__riscv) && (__riscv_xlen == 64) XXH_FORCE_INLINE xxh_u64 XXH_mult32to64_add64(xxh_u64 lhs, xxh_u64 rhs, xxh_u64 acc) { xxh_u64 ret; __asm__ ( "slli t0, %1, 32 \n\t" // extract low 32 bits of lhs "srli t0, t0, 32 \n\t" "slli t1, %2, 32 \n\t" // extract low 32 bits of rhs "srli t1, t1, 32 \n\t" "mul t0, t0, t1 \n\t" // multiply "add %0, t0, %3 \n\t" // add acc : "=&r" (ret) // <-- early-clobber (&) prevents aliasing! : "r" (lhs), "r" (rhs), "r" (acc) : "t0", "t1" ); return ret; } ``` Uses =&r to safely mark ret as early-clobber (non-overlapping with inputs), Performs all computation in temporary registers (t0, t1), ensuring inputs are untouched, It has passed all test cases and is compatible with GCC I will put the comparison table soon....

Jay-Lokhande commented 2025-05-11 09:45:10 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hello @t-mat,

🛠️ xxHash Benchmark Results (With Patch)

GCC Version	5#XXH3_64b (MB/s)	11#XXH128 (MB/s)	21#XXH3_stream (MB/s)	25#XXH128_stream (MB/s)
GCC-9	124.2	117.5	17.2	18.0
GCC-10	124.8	122.3	15.8	17.3
GCC-11	130.4	131.5	23.6	19.8
GCC-12	130.3	127.6	23.0	23.3
GCC-13	143.4	134.6	19.0	23.6
GCC-14	126.7	106.3	19.8	18.5

Hello @t-mat, ### 🛠️ xxHash Benchmark Results (With Patch) | GCC Version | 5#XXH3_64b (MB/s) | 11#XXH128 (MB/s) | 21#XXH3_stream (MB/s) | 25#XXH128_stream (MB/s) | |-------------|------------------|------------------|------------------------|--------------------------| | GCC-9 | 124.2 | 117.5 | 17.2 | 18.0 | | GCC-10 | 124.8 | 122.3 | 15.8 | 17.3 | | GCC-11 | 130.4 | 131.5 | 23.6 | 19.8 | | GCC-12 | 130.3 | 127.6 | 23.0 | 23.3 | | GCC-13 | 143.4 | 134.6 | 19.0 | 23.6 | | GCC-14 | 126.7 | 106.3 | 19.8 | 18.5 |  

t-mat commented 2025-05-14 07:46:23 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@Jay-Lokhande, thanks for your improvement.

It seems good. Could you provide more complete result for further evaluation? I mean

(1) We need the benchmark result of both of the original (without patch) version and patched version.
(2) We need different sizes. e.g. 16bytes, 1KiBytes, 1MiBytes, etc.

We need more information. Because, on my (cheap, low-end) physical 64-bit RISC-V environment, your patch makes not good result with old gcc-10.

compiler options are:

-mcpu=c906fdv -march=rv64imafdcv0p7xthead -mcmodel=medany -mabi=lp64d -O3 -DNDEBUG

Summary of machine environment is:

[root@milkv-duo]~# uname -a
Linux milkv-duo 5.10.4-tag- #1 PREEMPT Fri Nov 22 11:31:04 CST 2024 riscv64 GNU/Linux

[root@milkv-duo]~# lscpu
Architecture:        riscv64
Byte Order:          Little Endian
CPU(s):              1
On-line CPU(s) list: 0
Thread(s) per core:  1
Core(s) per socket:  1
Socket(s):           1

[root@milkv-duo]~# cat /proc/cpuinfo
processor       : 0
hart            : 0
isa             : rv64imafdvcsu
mmu             : sv39

Original (without patch) version of xxhash

[root@milkv-duo]~# ./xxhash -b5,11,21,25 -B16 && ./xxhash -b5,11,21,25 -B1K && ./xxhash -b5,11,21,25 -B1M
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 16 bytes...
 5#XXH3_64b                      :         16 -> 12478840 it/s (  190.4 MB/s)
11#XXH128                        :         16 ->  8803429 it/s (  134.3 MB/s)
21#XXH3_stream                   :         16 ->  2168652 it/s (   33.1 MB/s)
25#XXH128_stream                 :         16 ->  2017362 it/s (   30.8 MB/s)
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 1024 bytes...
 5#XXH3_64b                      :       1024 ->   655246 it/s (  639.9 MB/s)
11#XXH128                        :       1024 ->   613372 it/s (  599.0 MB/s)
21#XXH3_stream                   :       1024 ->   446996 it/s (  436.5 MB/s)
25#XXH128_stream                 :       1024 ->   427232 it/s (  417.2 MB/s)
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 1024 KB...
 5#XXH3_64b                      :    1048576 ->      566 it/s (  566.5 MB/s)
11#XXH128                        :    1048576 ->      565 it/s (  565.2 MB/s)
21#XXH3_stream                   :    1048576 ->      570 it/s (  569.7 MB/s)
25#XXH128_stream                 :    1048576 ->      570 it/s (  570.0 MB/s)

[root@milkv-duo]~# ./xxhash -b
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 100 KB...
 1#XXH32                         :     102400 ->     5393 it/s (  526.7 MB/s)
 3#XXH64                         :     102400 ->     4889 it/s (  477.5 MB/s)
 5#XXH3_64b                      :     102400 ->     5763 it/s (  562.8 MB/s)
11#XXH128                        :     102400 ->     5752 it/s (  561.7 MB/s)

With your patch

[root@milkv-duo]~# ./xxhash -b5,11,21,25 -B16 && ./xxhash -b5,11,21,25 -B1K && ./xxhash -b5,11,21,25 -B1M
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 16 bytes...
 5#XXH3_64b                      :         16 -> 12497303 it/s (  190.7 MB/s)
11#XXH128                        :         16 ->  8808957 it/s (  134.4 MB/s)
21#XXH3_stream                   :         16 ->  2169839 it/s (   33.1 MB/s)
25#XXH128_stream                 :         16 ->  2020052 it/s (   30.8 MB/s)
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 1024 bytes...
 5#XXH3_64b                      :       1024 ->   419025 it/s (  409.2 MB/s)  <--
11#XXH128                        :       1024 ->   392692 it/s (  383.5 MB/s)  <--
21#XXH3_stream                   :       1024 ->   310142 it/s (  302.9 MB/s)  <--
25#XXH128_stream                 :       1024 ->   299162 it/s (  292.2 MB/s)  <--
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 1024 KB...
 5#XXH3_64b                      :    1048576 ->      357 it/s (  357.5 MB/s)  <--
11#XXH128                        :    1048576 ->      357 it/s (  357.0 MB/s)  <--
21#XXH3_stream                   :    1048576 ->      360 it/s (  359.5 MB/s)  <--
25#XXH128_stream                 :    1048576 ->      360 it/s (  359.7 MB/s)  <--

[root@milkv-duo]~# ./xxhash -b
xxhash 0.8.3 by Yann Collet
compiled as 64-bit unknown little endian with GCC 10.2.0
Sample of 100 KB...
 1#XXH32                         :     102400 ->     5390 it/s (  526.3 MB/s)
 3#XXH64                         :     102400 ->     4888 it/s (  477.4 MB/s)
 5#XXH3_64b                      :     102400 ->     3651 it/s (  356.6 MB/s)  <--
11#XXH128                        :     102400 ->     3643 it/s (  355.8 MB/s)  <--

@Jay-Lokhande, thanks for your improvement. It seems good. Could you provide more complete result for further evaluation? I mean (1) We need the benchmark result of both of the original (without patch) version and patched version. (2) We need different sizes. e.g. 16bytes, 1KiBytes, 1MiBytes, etc. We need more information. Because, on my ([cheap, low-end](https://milkv.io/docs/duo/getting-started/duo)) physical 64-bit RISC-V environment, your patch makes not good result with old `gcc-10`. compiler options are: ``` -mcpu=c906fdv -march=rv64imafdcv0p7xthead -mcmodel=medany -mabi=lp64d -O3 -DNDEBUG ``` Summary of machine environment is: ``` [root@milkv-duo]~# uname -a Linux milkv-duo 5.10.4-tag- #1 PREEMPT Fri Nov 22 11:31:04 CST 2024 riscv64 GNU/Linux [root@milkv-duo]~# lscpu Architecture: riscv64 Byte Order: Little Endian CPU(s): 1 On-line CPU(s) list: 0 Thread(s) per core: 1 Core(s) per socket: 1 Socket(s): 1 [root@milkv-duo]~# cat /proc/cpuinfo processor : 0 hart : 0 isa : rv64imafdvcsu mmu : sv39 ``` ## Original (without patch) version of xxhash ``` [root@milkv-duo]~# ./xxhash -b5,11,21,25 -B16 && ./xxhash -b5,11,21,25 -B1K && ./xxhash -b5,11,21,25 -B1M xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 16 bytes... 5#XXH3_64b : 16 -> 12478840 it/s ( 190.4 MB/s) 11#XXH128 : 16 -> 8803429 it/s ( 134.3 MB/s) 21#XXH3_stream : 16 -> 2168652 it/s ( 33.1 MB/s) 25#XXH128_stream : 16 -> 2017362 it/s ( 30.8 MB/s) xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 1024 bytes... 5#XXH3_64b : 1024 -> 655246 it/s ( 639.9 MB/s) 11#XXH128 : 1024 -> 613372 it/s ( 599.0 MB/s) 21#XXH3_stream : 1024 -> 446996 it/s ( 436.5 MB/s) 25#XXH128_stream : 1024 -> 427232 it/s ( 417.2 MB/s) xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 1024 KB... 5#XXH3_64b : 1048576 -> 566 it/s ( 566.5 MB/s) 11#XXH128 : 1048576 -> 565 it/s ( 565.2 MB/s) 21#XXH3_stream : 1048576 -> 570 it/s ( 569.7 MB/s) 25#XXH128_stream : 1048576 -> 570 it/s ( 570.0 MB/s) [root@milkv-duo]~# ./xxhash -b xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 100 KB... 1#XXH32 : 102400 -> 5393 it/s ( 526.7 MB/s) 3#XXH64 : 102400 -> 4889 it/s ( 477.5 MB/s) 5#XXH3_64b : 102400 -> 5763 it/s ( 562.8 MB/s) 11#XXH128 : 102400 -> 5752 it/s ( 561.7 MB/s) ``` ## With your patch ``` [root@milkv-duo]~# ./xxhash -b5,11,21,25 -B16 && ./xxhash -b5,11,21,25 -B1K && ./xxhash -b5,11,21,25 -B1M xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 16 bytes... 5#XXH3_64b : 16 -> 12497303 it/s ( 190.7 MB/s) 11#XXH128 : 16 -> 8808957 it/s ( 134.4 MB/s) 21#XXH3_stream : 16 -> 2169839 it/s ( 33.1 MB/s) 25#XXH128_stream : 16 -> 2020052 it/s ( 30.8 MB/s) xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 1024 bytes... 5#XXH3_64b : 1024 -> 419025 it/s ( 409.2 MB/s) <-- 11#XXH128 : 1024 -> 392692 it/s ( 383.5 MB/s) <-- 21#XXH3_stream : 1024 -> 310142 it/s ( 302.9 MB/s) <-- 25#XXH128_stream : 1024 -> 299162 it/s ( 292.2 MB/s) <-- xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 1024 KB... 5#XXH3_64b : 1048576 -> 357 it/s ( 357.5 MB/s) <-- 11#XXH128 : 1048576 -> 357 it/s ( 357.0 MB/s) <-- 21#XXH3_stream : 1048576 -> 360 it/s ( 359.5 MB/s) <-- 25#XXH128_stream : 1048576 -> 360 it/s ( 359.7 MB/s) <-- [root@milkv-duo]~# ./xxhash -b xxhash 0.8.3 by Yann Collet compiled as 64-bit unknown little endian with GCC 10.2.0 Sample of 100 KB... 1#XXH32 : 102400 -> 5390 it/s ( 526.3 MB/s) 3#XXH64 : 102400 -> 4888 it/s ( 477.4 MB/s) 5#XXH3_64b : 102400 -> 3651 it/s ( 356.6 MB/s) <-- 11#XXH128 : 102400 -> 3643 it/s ( 355.8 MB/s) <-- ```

Jay-Lokhande commented 2025-05-14 09:37:26 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@t-mat ,
Thank you for your feedback and detailed benchmarks.

To clarify: my benchmarks were run on a RISC-V environment using QEMU (software emulation), not physical hardware. Is it fine?

I will put comparison table soon with and without parch with all gcc versions. also I have used this ./xxhsum -b5,11,21,25 -B16 ./xxhsum -b5,11,21,25 -B1K ./xxhsun -b5,11,21,25 -B1M as you suggested earlier While benchmarking

@t-mat , Thank you for your feedback and detailed benchmarks. To clarify: my benchmarks were run on a RISC-V environment using QEMU (software emulation), not physical hardware. Is it fine? I will put comparison table soon with and without parch with all gcc versions. also I have used this ```./xxhsum -b5,11,21,25 -B16 ./xxhsum -b5,11,21,25 -B1K ./xxhsun -b5,11,21,25 -B1M ``` as you suggested earlier While benchmarking

t-mat commented 2025-05-14 10:36:07 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

To clarify: my benchmarks were run on a RISC-V environment using QEMU (software emulation), not physical hardware. Is it fine?

It's fine for initial testing. But it may not be fine for final evaluation.

If we won't have more benchmark of physical machines, I should oppose this patch. Because unfortunately my benchmark doesn't show any gain of it in the real world processor.

> To clarify: my benchmarks were run on a RISC-V environment using QEMU (software emulation), not physical hardware. Is it fine? It's fine for initial testing. But it may not be fine for final evaluation. If we won't have more benchmark of physical machines, I should oppose this patch. Because unfortunately my benchmark doesn't show any gain of it in the real world processor.

👍 1

Cyan4973 commented 2025-05-14 12:48:21 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

To clarify: my benchmarks were run on a RISC-V environment using QEMU (software emulation), not physical hardware. Is it fine?

It's fine for initial testing. But it may not be fine for final evaluation.

If we won't have more benchmark of physical machines, I should oppose this patch. Because unfortunately my benchmark doesn't show any gain of it in the real world processor.

I have to agree on this with @t-mat .
Performance on qemu is not representative of real target.

> > To clarify: my benchmarks were run on a RISC-V environment using QEMU (software emulation), not physical hardware. Is it fine? > > It's fine for initial testing. But it may not be fine for final evaluation. > > If we won't have more benchmark of physical machines, I should oppose this patch. Because unfortunately my benchmark doesn't show any gain of it in the real world processor. I have to agree on this with @t-mat . Performance on `qemu` is not representative of real target.

Cyan4973 commented 2025-06-19 16:45:19 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

FYI, there is now an RVV implementation of XXH3 (#1043).

The vector code path is triggered for long inputs.
This is different from the proposed optimization here, focusing on XXH_mult32to64_add64(), which is more important for short inputs.

My understanding is that this investigation stopped due to lack of proper RISC-V hardware to measure the speed impact.

It is planned to resume in the future, or is it permanently closed ?

FYI, there is now an `RVV` implementation of `XXH3` (#1043). The vector code path is triggered for long inputs. This is different from the proposed optimization here, focusing on `XXH_mult32to64_add64()`, which is more important for short inputs. My understanding is that this investigation stopped due to lack of proper RISC-V hardware to measure the speed impact. It is planned to resume in the future, or is it permanently closed ?

camel-cdr commented 2025-09-06 03:03:07 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Why was this even consider reasonable?

The comments for the specialized XXH_mult32to64_add64 implementation clearly states that the only reason aarch64 has a specialized implementation is that for low performance implementations that don't have a full 64-bit hardware multiplier. In which case it uses the 32-bit -> 64-bit widening multiply add instruction. Such an instruction doesn't exist in RISC-V, there is just regular multiplication, which is fine for all relevant hardware today.

Any inline assembly implementation will be worse than what the compiler generates.

Why was this even consider reasonable? The comments for the specialized `XXH_mult32to64_add64` implementation clearly states that the only reason aarch64 has a specialized implementation is that for low performance implementations that don't have a full 64-bit hardware multiplier. In which case it uses the 32-bit -> 64-bit widening multiply add instruction. **Such an instruction doesn't exist in RISC-V**, there is just regular multiplication, which is fine for all relevant hardware today. Any inline assembly implementation will be worse than what the compiler generates.

t-mat commented 2025-09-08 01:31:33 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Why was this even consider reasonable?

I'm not sure, but AFAIK this PR has tried to bring the same optimization from newer (gcc RISC-V) compilers to older ones.
It uses fewer temporary registers, and newer compiler already emit the same instructions as proposed here.

Implementations	mult32to64_add64() (non-inlined) in RISC-V
this PR inline asm	6 insts, 3 regs (3 args)
newer gcc (>=14)	6 insts, 3 regs (3 args)
older gcc (<=13)	6 insts, 4 regs (3 args + 1 temp)

// newer gcc (>=14) and this PR
uint64_t mult32to64_add64(uint64_t a0, uint64_t a1, uint64_t a2) {
slli a0, a0, 32    # shift left logical: a0 <<= 32
slli a1, a1, 32    # shift left logical: a1 <<= 32
srli a1, a1, 32    # shift right logical: a1 >>= 32  → keep lower 32 bits of a1
srli a0, a0, 32    # shift right logical: a0 >>= 32  → keep lower 32 bits of a0
mul  a0, a1, a0
add  a0, a0, a2
ret                # return a0
}

// older gcc (<=13)
uint64_t mult32to64_add64(uint64_t a0, uint64_t a1, uint64_t a2) {
li   a5, -1        # load immediate: a5 = -1 (all bits set)
srli a5, a5, 32    # shift right logical: a5 >>= 32  → a5 = 0xFFFFFFFF (mask for lower 32 bits)
and  a1, a1, a5    # a1 = a1 & 0xFFFFFFFF  → keep lower 32 bits of a1
and  a0, a0, a5    # a0 = a0 & 0xFFFFFFFF  → keep lower 32 bits of a0
mul  a0, a1, a0
add  a0, a0, a2
ret                # return a0

But also from my test results, I'm leading negative on it as you mentioned.

> Why was this even consider reasonable? I'm not sure, but AFAIK this PR has tried to bring the same optimization from newer (gcc RISC-V) compilers to older ones. It uses fewer temporary registers, and newer compiler already emit the same instructions as proposed here. | Implementations |mult32to64_add64() (non-inlined) in RISC-V | | -- |- | |this PR inline asm |6 insts, 3 regs (3 args) | |newer gcc (>=14) |6 insts, 3 regs (3 args) | |older gcc (<=13) |6 insts, 4 regs (3 args + 1 temp) | ``` // newer gcc (>=14) and this PR uint64_t mult32to64_add64(uint64_t a0, uint64_t a1, uint64_t a2) { slli a0, a0, 32 # shift left logical: a0 <<= 32 slli a1, a1, 32 # shift left logical: a1 <<= 32 srli a1, a1, 32 # shift right logical: a1 >>= 32 → keep lower 32 bits of a1 srli a0, a0, 32 # shift right logical: a0 >>= 32 → keep lower 32 bits of a0 mul a0, a1, a0 add a0, a0, a2 ret # return a0 } ``` ``` // older gcc (<=13) uint64_t mult32to64_add64(uint64_t a0, uint64_t a1, uint64_t a2) { li a5, -1 # load immediate: a5 = -1 (all bits set) srli a5, a5, 32 # shift right logical: a5 >>= 32 → a5 = 0xFFFFFFFF (mask for lower 32 bits) and a1, a1, a5 # a1 = a1 & 0xFFFFFFFF → keep lower 32 bits of a1 and a0, a0, a5 # a0 = a0 & 0xFFFFFFFF → keep lower 32 bits of a0 mul a0, a1, a0 add a0, a0, a2 ret # return a0 ``` But also from my test results, I'm leading negative on it as you mentioned.

camel-cdr commented 2025-09-08 08:26:05 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@t-mat the codegen for the function it self isn't that important. Important is how it's integrated into the calling function, which the compiler is a lot better at if it can rearrange the instructions on it's own: https://godbolt.org/z/Pz1dW8oG5

The regular C code allows the compiler to use zext instead of slli+srli if the bitmanip extension is supported.

Also notice how I needed to add -mno-strict-align, because RISC-V doesn't allow misaligned loads by default.
If RISC-V without RVV is a target worth optimizing for (or others without unaligned load/store), then a specialized XXH3_accumulate_512_scalar implementation using N+1 64-bit aligned loads and combining the adjacent two loads the unaligned value would give a huge speedup.

Something like (untested sketch, I probably messed up endianness):

typedef uint64_t __attribute__((__may_alias__)) u64alias;

void
XXH3_accumulate_512_scalar_aligned(
        void* restrict acc,
        const void* restrict input,
        const void* restrict secret)
{
    uint64_t* xacc = (uint64_t*)acc;
    const u64alias *xinput  = (const u64alias*)((uintptr_t)input  & ~(uintptr_t)7);
    const u64alias *xsecret = (const u64alias*)((uintptr_t)secret & ~(uintptr_t)7);
    const uint64_t rinput  = ((uintptr_t)input  & 7)*8;
    const uint64_t rsecret = ((uintptr_t)secret & 7)*8;
    xinput  = (const u64alias*)(((uintptr_t)input + 7) & ~(uintptr_t)7);
    xsecret = (const u64alias*)(((uintptr_t)secret + 7) & ~(uintptr_t)7);
    uint64_t curInput  = *xinput;
    uint64_t curSecret = *xsecret;
    size_t i;
    XXH_ASSERT(((size_t)acc & (64-1)) == 0);
    for (i = 0; i < 64/8; ++i) {
        uint64_t prevInput = rinput == 0 ? 0 : curInput;
        uint64_t prevSecret = rsecret == 0 ? 0 : curSecret;
        curInput = xinput[i];
        curSecret = xsecret[i];
        uint64_t val = prevInput  >> rinput  | curInput  << ((64 - rinput) & 63); // & 63 is implicit
        uint64_t key = prevSecret >> rsecret | curSecret << ((64 - rsecret) & 63;
        key ^= val;
        xacc[i ^ 1] += val;
        xacc[i] += (key & 0xFFFFFFFF) * (key >> 32);
    }

}

Evaluating this using llvm-mca vs the original code compiled without the -mno-strict-align flag shows a 3x speedup: https://godbolt.org/z/8999K98Y4
It's now only 2x slower than if the compiler is allowed to generated misaligned loads and the hardware supports them properly.

We can test for the __riscv_misaligned_fast macro (see RISC-V c-api docs) to check if the compiler would generates misaligned loads.

@t-mat the codegen for the function it self isn't that important. Important is how it's integrated into the calling function, which the compiler is a lot better at if it can rearrange the instructions on it's own: https://godbolt.org/z/Pz1dW8oG5 The regular C code allows the compiler to use zext instead of slli+srli if the bitmanip extension is supported. Also notice how I needed to add `-mno-strict-align`, because RISC-V doesn't allow misaligned loads by default. If RISC-V without RVV is a target worth optimizing for (or others without unaligned load/store), then a specialized `XXH3_accumulate_512_scalar` implementation using N+1 64-bit aligned loads and combining the adjacent two loads the unaligned value would give a huge speedup. Something like (untested sketch, I probably messed up endianness): ```c typedef uint64_t __attribute__((__may_alias__)) u64alias; void XXH3_accumulate_512_scalar_aligned( void* restrict acc, const void* restrict input, const void* restrict secret) { uint64_t* xacc = (uint64_t*)acc; const u64alias *xinput = (const u64alias*)((uintptr_t)input & ~(uintptr_t)7); const u64alias *xsecret = (const u64alias*)((uintptr_t)secret & ~(uintptr_t)7); const uint64_t rinput = ((uintptr_t)input & 7)*8; const uint64_t rsecret = ((uintptr_t)secret & 7)*8; xinput = (const u64alias*)(((uintptr_t)input + 7) & ~(uintptr_t)7); xsecret = (const u64alias*)(((uintptr_t)secret + 7) & ~(uintptr_t)7); uint64_t curInput = *xinput; uint64_t curSecret = *xsecret; size_t i; XXH_ASSERT(((size_t)acc & (64-1)) == 0); for (i = 0; i < 64/8; ++i) { uint64_t prevInput = rinput == 0 ? 0 : curInput; uint64_t prevSecret = rsecret == 0 ? 0 : curSecret; curInput = xinput[i]; curSecret = xsecret[i]; uint64_t val = prevInput >> rinput | curInput << ((64 - rinput) & 63); // & 63 is implicit uint64_t key = prevSecret >> rsecret | curSecret << ((64 - rsecret) & 63; key ^= val; xacc[i ^ 1] += val; xacc[i] += (key & 0xFFFFFFFF) * (key >> 32); } } ``` Evaluating this using llvm-mca vs the original code compiled without the `-mno-strict-align` flag shows a 3x speedup: https://godbolt.org/z/8999K98Y4 It's now only 2x slower than if the compiler is allowed to generated misaligned loads and the hardware supports them properly. We can test for the `__riscv_misaligned_fast` macro (see [RISC-V c-api docs](https://github.com/riscv-non-isa/riscv-c-api-doc/blob/main/src/c-api.adoc)) to check if the compiler would generates misaligned loads.

👍 1

t-mat commented 2025-09-09 03:51:34 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@camel-cdr ,
Basically, our status of this issue is waiting for OP's investigation. If they come up with a better inline-asm solution for older compilers, that would be a great finding. If not, we will unfortunately close this issue.

If needed, please open a new issue for your evaluation so that we can avoid confusion.

@camel-cdr , Basically, our status of this issue is waiting for OP's investigation. If they come up with a better inline-asm solution for older compilers, that would be a great finding. If not, we will unfortunately close this issue. If needed, please open a new issue for your evaluation so that we can avoid confusion.

Jay-Lokhande commented 2025-12-17 22:49:16 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi @t-mat, @Cyan4973, and @camel-cdr,
Thank you for the detailed feedback and testing. I've added a compiler version guard to address the concern about interfering with modern compiler optimizations.
Changes Made
The RISC-V inline assembly optimization is now only enabled for GCC < 14:

#elif defined(__riscv) && (__riscv_xlen == 64) && defined(__GNUC__) && (__GNUC__ < 14)

This ensures GCC 14+ can use their own optimizations without interference.
I'm unsure whether to proceed. Would you prefer to:
Close this issue , Investigate further, Keep it for specific cases (Only enable for very old GCC versions where it might help)
I understand the concerns raised about Physical hardware performance, Compiler optimization interference

Hi @t-mat, @Cyan4973, and @camel-cdr, Thank you for the detailed feedback and testing. I've added a compiler version guard to address the concern about interfering with modern compiler optimizations. Changes Made The RISC-V inline assembly optimization is now only enabled for GCC < 14: ```c #elif defined(__riscv) && (__riscv_xlen == 64) && defined(__GNUC__) && (__GNUC__ < 14) ``` This ensures GCC 14+ can use their own optimizations without interference. I'm unsure whether to proceed. Would you prefer to: Close this issue , Investigate further, Keep it for specific cases (Only enable for very old GCC versions where it might help) I understand the concerns raised about Physical hardware performance, Compiler optimization interference

Cyan4973 commented 2026-09-16 23:40:28 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

dropping,
I think there is simply no equivalent to UMADDL in risc-v instruction set
which makes the proposed assembly replacement face an uphill battle,
effectively detrimental to performance once compared to modern compiler optimizations.

dropping, I think there is simply no equivalent to `UMADDL` in risc-v instruction set which makes the proposed assembly replacement face an uphill battle, effectively detrimental to performance once compared to modern compiler optimizations.

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

dev

dependabot/github_actions/actions/upload-artifact-7.0.2

dependabot/github_actions/actions/download-artifact-8.0.2

dependabot/github_actions/actions/setup-node-7.1.0

archive/20261011T062114Z-rescue/fhgffy/fix/cmake-dispatch-off

dependabot/github_actions/msys2/setup-msys2-2.33.0

archive/20261011T062114Z-rescue/filtede98/fix-null-guards

ci/release-windows

fix/mingw-exe-symlink

gh-pages

release

ci/pin-actions

docs/refresh-readme

archive/20261011T062114Z-recovered/docs/refresh-readme

v0.8.4

v0.8.3

v0.8.2

v0.8.1

v0.8.0

v0.7.4

v0.7.3

v0.7.2

v0.7.1

v0.7.0

graphs

v0.6.5

v0.6.4

v0.6.3

v0.6.2

v0.6.1

v0.6.0

v0.5.1

v0.5.0

r42

r41

r40

r39

r38

r37

r36

r35

Labels

Clear labels   

announcement

  

bug

  

build

  

CI

Continuous Integration tests

  

dependencies

Pull requests that update a dependency file

  

documentation

  

duplicate

  

enhancement

  

experience feedback

  

feature request

  

github_actions

Pull requests that update GitHub Actions code

  

help wanted

  

install

  

invalid

  

Long-term

  

optimization

  

portability

  

question

  

target-specific

issue triggered in a specific environment (harware, compiler, OS)

  

test issue

  

wontfix

No labels

announcement

bug

build

CI

dependencies

documentation

duplicate

enhancement

experience feedback

feature request

github_actions

help wanted

install

invalid

Long-term

optimization

portability

question

target-specific

test issue

wontfix

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Cyan4973/xxHash#1018

No description provided.