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
68 stars 38 forks source link

Locality for prefetch built-in proposal #46

Closed BeMg closed 9 months ago

BeMg commented 1 year ago

ARM[1] provide the its own semantic on prefetch built-in locality.

It seem we could do the same thing in RISC-V.

[1] https://developer.arm.com/documentation/101458/2010/Coding-best-practice/Prefetching-with---builtin-prefetch

kito-cheng commented 1 year ago

cc @asb @topperc @palmer-dabbelt @JeffreyALaw

asb commented 1 year ago

Thanks for documenting this. I think it would be good to explicitly mention the zicbop extension here.

This also exposes an issue with our treatment of the 'zihintntl' extension on the LLVM side at least (I'm not sure if the same issue is there in GCC). The proposed prefetch locality patch for LLVM only emits the ntl hints if the zihintntl extension is known to be present. We should probably emit the add x0, x0, x? based mnemonics directly even if zihintntl isn't known to be present.

kito-cheng commented 1 year ago

This also exposes an issue with our treatment of the 'zihintntl' extension on the LLVM side at least (I'm not sure if the same issue is there in GCC). The proposed prefetch locality patch for LLVM only emits the ntl hints if the zihintntl extension is known to be present. We should probably emit the add x0, x0, x? based mnemonics directly even if zihintntl isn't known to be present.

GCC implementation also require zihintntl too, that also remind another issue I discussed with @topperc here is: should we allow those hint mnemonics still available even corresponding is not enabled? And actually prefetch is also a hint instruction too, so we might did the same treatment for Zicbop too.

I guess this worth to put to the next meeting agenda?

hiraditya commented 10 months ago

Thanks for addressing the changes i requested.

BeMg commented 9 months ago

Resolve merge conflict.

kito-cheng commented 9 months ago

Although @asb is not response yet, but he has aware this PR and patch for LLVM[1], also it merged into LLVM trunk, so I treat it as no objection from LLVM community and moving this PR forward.

[1] https://reviews.llvm.org/D154691