[SYCL][ESIMD][NFC] Fix namespace of ESIMD implementation details. #3487
Conversation
- Move ESIMD API declarations that are implementaion details into
sycl::INTEL::gpu::detail and sycl::INTEL::gpu::emu::detail
namespaces.
- Make enum { BYTE = 1, WORD = 2, ..., GRF = 32 } local to a struct
to avoid global namespace pollution.
- Relocate AccessorPrivateProxy to memory intrinsics header(s) where
it is only supposed to be used.
Signed-off-by: Konstantin S Bobrovsky <[email protected]>
| /// Constant in number of bytes. | ||
| enum { BYTE = 1, WORD = 2, DWORD = 4, QWORD = 8, OWORD = 16, GRF = 32 }; | ||
| /// ESIMD intrinsic operand size in bytes. | ||
| struct OperandSize { |
There was a problem hiding this comment.
OperandSize::member can be conveniently used as integer w/o static_cast, hence I did not add class
|
|
||
| #include <assert.h> | ||
| #include <cstdint> | ||
|
|
||
| #define __SIGD sycl::INTEL::gpu::detail |
There was a problem hiding this comment.
I'm a little concerned about the name here. Maybe something like this would be better?
namespace sigd = sycl::INTEL::gpu::detail;
Similar to what we have for csd namespace.
There was a problem hiding this comment.
the problem with such short names in headers in the global namespace is that they can conflict with other code. {{__}} factors out user code.
There was a problem hiding this comment.
Yes, that's true. Do you know if we have project guidelines for such cases?
There was a problem hiding this comment.
The only guideline I know is the llvm coding style.
There was a problem hiding this comment.
The reason why I did not use namespace __SIGD = sycl::INTEL::gpu::detail; instead is because it is in the global scope and can't be undef'ed, unlike a macro. Which means all subsequent headers and user code will see that declaration. With macro, I just #undef it once no longer needed.
sycl/include/CL/sycl/INTEL/esimd/detail/esimd_memory_intrin.hpp
Outdated
Show resolved
Hide resolved
Two tests failed and need your attention. |
Signed-off-by: Konstantin S Bobrovsky <[email protected]>
Signed-off-by: Konstantin S Bobrovsky <[email protected]>
kbobrovs
changed the title
Signed-off-by: Konstantin S Bobrovsky <[email protected]>
Right, added |
yes, fixed |
Move ESIMD API declarations that are implementaion details into
sycl::INTEL::gpu::detail and sycl::INTEL::gpu::emu::detail
namespaces.
Make enum { BYTE = 1, WORD = 2, ..., GRF = 32 } local to a struct
to avoid global namespace pollution.
Relocate AccessorPrivateProxy to memory intrinsics header(s) where
it is only supposed to be used.
get rid of extra __esimd namespace
Signed-off-by: Konstantin S Bobrovsky [email protected]