riscv-non-isa / riscv-c-api-doc

Documentation of the RISC-V C API
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
75 stars 41 forks source link

Should we have a separate _riscv_misaligned* for scalar/vector? #73

Closed topperc closed 2 months ago

topperc commented 7 months ago

The LLVM project was recently informed that the CPU used in the Kendryte K230 board supports unaligned scalar accesses in hardware, but unaligned vector accesses will trap. https://github.com/llvm/llvm-project/issues/88029

We should consider splitting the preprocessor define to handle this case.

CC: @nick-knight @kito-cheng @vineetgarc @asb @aswaterman

appujee commented 6 months ago

Related llvm patch https://github.com/llvm/llvm-project/pull/88954

JeffreyALaw commented 6 months ago

I suspect the spacemit K1 (aka x60) core in the banana pi f3 has the same behavior. It's certainly faulting for byte aligned vector loads. I haven't checked its scalar behavior though.

appujee commented 6 months ago

Related discussions in llvm-project and google/android-riscv

nick-knight commented 6 months ago

The task group that maintains this document has a fairly clear policy for adding new __riscv* macros to detect architectural features. We do not have a clear policy for adding macros to detect implementation-defined behavior or microarchitectural properties. When we previously introduced the __riscv_misaligned* macros, it was at the prompting of a particular vendor, who argued that their implementation-defined behavior was likely to become commonplace, thus their macros would be useful more broadly. But, by accepting these macros, we opened a Pandora's box. This proposal is just another thing to come out of that box.

My main concern is software portability and fragmentation, and the toolchain burden of supporting a potential proliferation of these macros. I think we should set a high bar on their utility and generality. I think this decision should be made by consensus within a meeting of the task group (or parent committee).

topperc commented 6 months ago

There is a pending patch to the hwprobe documentation to clarify that it only detects unaligned scalar accesses. https://lore.kernel.org/lkml/tencent_9D721BDDF88C04DBB5151D57711D62524209@qq.com/T/

As far as I understand, hwprobe does its detection by doing an unaligned scalar access and measuring how long it takes. Without proper emulation of vector in SBI, I guess the kernel can't measure unaligned vector time this way. Hopefully, the missing unaligned vector emulation in SBI will come soon. This is needed to support the Zicclsm requirements of RVA23 on CPUs without unaligned vector support.

kito-cheng commented 5 months ago

GCC is trying to add -mvector-strict-align and -mscalar-strict-align, -mstrict-align will become alias of -mscalar-strict-align

https://patchwork.sourceware.org/project/gcc/patch/aadc64da-0b0a-4035-89a8-660010da58bd@gmail.com/

kito-cheng commented 5 months ago

I created two PR for document, one for -mstrict-align and -mno-strict-align and another one is -m[no-][scalar|vector]-strict-align.

https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/49 https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/50

topperc commented 5 months ago

PR to make the existing macros scalar only https://github.com/riscv-non-isa/riscv-c-api-doc/pull/80