Skip to content

RISC-V ASM unaligned read/writes: alternative assembly#10530

Open
SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:riscv_unaligned_fix
Open

RISC-V ASM unaligned read/writes: alternative assembly#10530
SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:riscv_unaligned_fix

Conversation

@SparkiDev
Copy link
Copy Markdown
Contributor

Description

Not all RISC-V chips allow unaligned reads and writes with basic assembly instructions like lw/sw.
Add alternative assembly that is turned on with:
WOLFSSL_RISCV_ASM_NO_UNALIGNED.

Fixes #10525

Testing

./configure --disable-shared LDFLAGS=--static --host=riscv64 CC=riscv64-linux-gnu-gcc --enable-riscv-asm CFLAGS=-DWOLFSSL_RISCV_ASM_NO_UNALIGNED

@SparkiDev
Copy link
Copy Markdown
Contributor Author

Jenkins: retest this please

Not all RISC-V chips allow unaligned reads and writes with basic
assembly instructions like lw/sw.
Add alternative assembly that is turned on with:
WOLFSSL_RISCV_ASM_NO_UNALIGNED.
@SparkiDev SparkiDev force-pushed the riscv_unaligned_fix branch from 3b8e981 to b4611fa Compare May 26, 2026 04:19
@EAlexJ
Copy link
Copy Markdown

EAlexJ commented May 26, 2026

I would like to push back on this implementation a bit.

Currently, all instructions that could be unaligned get emulated.

I think it makes sense to check if the pointers are actually unaligned and only then use the more costly emulation.
I suggest something like this (with the check guarded behind WOLFSSL_RISCV_ASM_NO_UNALIGNED):

Sha512Transform(wc_Sha512* sha512, const byte* data,
    word32 blocks)
{
if(!(uintptr_t) data % 8)
    <normal execution>
else
    <emulate using the UNALIGNED_* macro>
}

There should only be a negligible impact on performance in case the data is aligned. This also only introduces a small size overhead, the check should be a few instructions at most.

@EAlexJ
Copy link
Copy Markdown

EAlexJ commented May 26, 2026

On a separate issue: In my opinion readability would be increased if you just used the UNALIGNED_* Macro unconditionally and only check once in the actual definition of the macro if WOLFSSL_RISCV_ASM_NO_UNALIGNED is defined and then emit the emulation.
This is then similar to how REV8 and others are handled. It would then also make sense to name the macro differently.

EDIT: This is probably obsolete in case the suggestion above gets adopted

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.

[Bug]: Several Issues with respect to alignment for RISC-V assembly routines

2 participants