Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
topperc opened this issue Apr 18, 2024 · 8 comments
Open

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

topperc opened this issue Apr 18, 2024 · 8 comments

Comments

@topperc
Copy link
Contributor

topperc commented Apr 18, 2024

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. llvm/llvm-project#88029

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

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

@appujee
Copy link

appujee commented May 14, 2024

Related llvm patch llvm/llvm-project#88954

@JeffreyALaw
Copy link

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
Copy link

appujee commented May 22, 2024

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

@nick-knight
Copy link

nick-knight commented May 22, 2024

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
Copy link
Contributor Author

topperc commented May 22, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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

riscv-non-isa/riscv-toolchain-conventions#49
riscv-non-isa/riscv-toolchain-conventions#50

@topperc
Copy link
Contributor Author

topperc commented Jun 10, 2024

PR to make the existing macros scalar only #80

topperc added a commit to topperc/llvm-project that referenced this issue Jun 10, 2024
…. Alias -m[no-]strict-align to scalar.

__riscv_misaligned_fast will be set based on -mno-scalar-strict-align
or -mno-strict-align.

This matches the direction gcc is proposing.

See
riscv-non-isa/riscv-c-api-doc#73
riscv-non-isa/riscv-toolchain-conventions#49
riscv-non-isa/riscv-toolchain-conventions#50
riscv-non-isa/riscv-c-api-doc#80
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

No branches or pull requests

5 participants