Locality for prefetch built-in proposal #46
Conversation
|
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 |
GCC implementation also require I guess this worth to put to the next meeting agenda? |
riscv-c-api.md
Outdated
| | `__builtin_prefetch(ptr, 0, 2 /* locality */);` | `ntl.p1 + prefetch.r (ptr)` | | ||
| | `__builtin_prefetch(ptr, 0, 3 /* locality */);` | `prefetch.r (ptr)` | | ||
|
|
||
| Compiler only emits the ntlh hints if the Zihintntl extension is enabled. |
There was a problem hiding this comment.
I think this might be over-specifying what compilers should do (plus it doesn't mention Zicbop).
How about "The following table presents the mapping from the __builtin_prefetch function to the corresponding assembly instructions assuming the presence of the Zihintntl and Zicbop extensions."?
Based on the last discussion we're probably erring in favour of being conservative and not emitting those hints eagerly (due to concerns about the opcodes being used for something else), but I think such a description wouldn't restrict compilers from emitting those hints even if the support for zicbop/zihintntl is unknown.
riscv-c-api.md
Outdated
| @@ -235,6 +235,25 @@ long __riscv_clmul (long a, long b); // clmul rd, rs1, rs2 | |||
| vint8m1_t __riscv_vadd_vv_i8m1(vint8m1_t vs2, vint8m1_t vs1, size_t vl); // vadd.vv vd, vs2, vs1 | |||
| ``` | |||
|
|
|||
| ### Prefetch Intrinsics | |||
|
|
|||
| The Zicbop extension provide the prefetch instruction to allow users to optimize data access patterns by providing hints to the hardware regarding future data accesses. It is supported through a compiler-defined built-in function with three arguments that specify its behavior. | |||
riscv-c-api.md
Outdated
| void __builtin_prefetch(const void *addr, int rw, int locality) | ||
| ``` | ||
|
|
||
| The locality for the built-in `__builtin_prefetch` function in RISC-V can be achieved using the Non-Temporal Locality Hints (Zihintntl) extension. According to Non-Temporal Locality Hints extension, it indicates that a cache line should be prefetched into a cache that is outer from the level specified by the NTL when a NTL instruction is applied to prefetch instruction. |
There was a problem hiding this comment.
nit:
According to Non-Temporal Locality Hints extension, it indicates that a cache line should be prefetched into a cache that is outer from the level specified by the NTL when a NTL instruction is applied to prefetch instruction.
When a Non-Temporal Locality (NTL) Hints instruction is applied to prefetch instruction, a cache line should be prefetched into a cache level that is higher than the level specified by the NTL.
BeMg
force-pushed
the
prefetch-builtin
branch
from
34a8ab9
to
ecf9315
Compare
|
Thanks for addressing the changes i requested. |
BeMg
force-pushed
the
prefetch-builtin
branch
from
ecf9315
to
d37ece8
Compare
|
Resolve merge conflict. |
|
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. |
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