Closed BeMg closed 1 year ago
For further context, I understand this intends to match what Clang currently provides (which is gated on enabling experimental extension support right now - awaiting the intrinsics being standardised).
For more context: this also match what @cmuellner proposed https://github.com/riscv-non-isa/riscv-c-api-doc/issues/30#issuecomment-1200199130 before
c.c. @aswaterman
This is LGTM, and although this is not implemented on GCC, but I think this is implementable and we have enough time frame to implement that before next release (which expected released at 2024/04).
LGTM. The only comment I'd make is that we might consider adding overloaded versions that omit the domain
argument, implying ALL
. But I don't know offhand whether overloaded intrinsics are valid in C (as opposed to C++), and maybe this "easy-mode" will prove controversial for other reasons--so if there's disagreement, I don't want to block forward progress.
But I don't know offhand whether overloaded intrinsics are valid in C (as opposed to C++),
Both GCC's and Clang's C-language dialects support a limited form of C++ function overloading. We use this already in the vector intrinsics ("overloaded API"). Since these are the only compilers we require to prove concepts, this seems actionable here as well.
LGTM. The only comment I'd make is that we might consider adding overloaded versions that omit the
domain
argument, implyingALL
. But I don't know offhand whether overloaded intrinsics are valid in C (as opposed to C++), and maybe this "easy-mode" will prove controversial for other reasons--so if there's disagreement, I don't want to block forward progress.
The version that omit the domain
argument I think the clang exists the similar built-in function __builtin_nontemporal_load/store
[1].
[1] https://clang.llvm.org/docs/LanguageExtensions.html#non-temporal-load-store-builtins
@BeMg OK. Can we document that API here so that GCC and LLVM will match? Or is the problem that we can't make it part of this API because it does not include the __riscv
prefix?
If the answer is that we can't include __builtin_nontemporal_load
/store
in this API, then I'll again recommend we add the overloaded variants that imply domain=ALL.
I'm not sure whether to add the __builtin_nontemporal_load/store
to proposal. Besides it doesn't not including the __riscv
prefix, GCC seem doesn't has these built-in functions now.
cc @kito-cheng
The overloaded version look like more consistent, so I add it into proposal.
It could be implemented by some macros.
#define __riscv_ntl_load_with_domain(PTR, DOMAIN) __builtin_riscv_ntl_load((PTR), (DOMAIN))
#define __riscv_ntl_load_without_domain(PTR) __builtin_riscv_ntl_load((PTR), __RISCV_NTLH_ALL)
#define SELECT_NTL_LOAD(_1,_2,NAME,...) NAME
#define __riscv_ntl_load(...) SELECT_NTL_LOAD(__VA_ARGS__, __riscv_ntl_load_with_domain, __riscv_ntl_load_without_domain)(__VA_ARGS__)
LGTM, seems like the overloading trick is work both on GCC and clang/LLVM, and I will try to find some resource to implement GCC part.
I would like to get explicitly ack from @aswaterman and @asb before merge.
LGTM.
OK with me, too.
I will implement the overloaded version in LLVM.
Thanks for everyone, I gonna merge this :)
This is a proposal for zihintntl intrinsic function.