NFC. Use named constant to indicate all lanes, to handle 32 and 64 wide architectures
ABataev jdoerfert grokos ronlieb
- rOMP373793: Use named constant to indicate all lanes, to handle 32 and 64 wide architectures
rG4f75a73796ff: Use named constant to indicate all lanes, to handle 32 and 64 wide architectures
rL373793: Use named constant to indicate all lanes, to handle 32 and 64 wide architectures
|323 ↗||(On Diff #222957)|
Probably, but I'm not sure what. These files are a mixture of the LLVM convention and another one. At some point I guess we should batch convert all the symbols we control to the LLVM convention.
I think a strict reading of the guidelines would be __KmpcImplAllLanes, which I struggle to read. Or possibly __KMPCImplAllLanes. Overall I quite like -1.
I'm wondering whether it would be better to define __kmpc_impl_all_lanes conditionally, i.e. as 0xFFFF for 32-wide and 0xFFFFFFFF for 64-wide architectures. I'm not sure I like this implicit signed-to-unsigned conversion here... Also, __kmpc_impl_lanemask_t is currently typedef'ed as uint32_t, if we want to support 64-wide lanes, then __kmpc_impl_lanemask_t must also be conditionally defined accordingly.
More F's. But sure, I'm happy to define it as UINT32_C(0xffffffff) for nvptx instead.
The 64 lane architecture I'm thinking of has it's own target_impl.h where the corresponding typedef is indeed to uint64_t, and all the functions in this file have different implementations. See https://github.com/ROCm-Developer-Tools/llvm-project/blob/AOMP-190918/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h if you're curious.
A function has the advantage that it's easier to pick a consistent name for it.
Do we think this is a reasonable change as-is? I don't mind what the variable is called so please speak up if you have a preference.