[Openmp-commits] [PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

Thu Aug 24 10:15:24 PDT 2023

jhuber6 added inline comments.

================
Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h:49

-static_assert(sizeof(AMDGPUImplicitArgsTy) == 56,
-              "Unexpected size of implicit arguments");
+enum IMPLICITARGS : uint32_t {
+  COV4_SIZE = 56,
----------------
saiislam wrote:
> jhuber6 wrote:
> > We should probably be using `sizeof` now that it's back to being a struct and keep the old struct definition.
> AMDGPU plugin doesn't use any implicitarg for COV4, but it does so for COV5. So, we are not keeping two separate structures for implicitargs of COV4 and COV5.
> If we use sizeof then it will always return 256 corresponding to COV5 (even for cov4, which should be 56). That's why we need this function.
Yeah, I guess for COV4 the only thing that mattered was the size so that we could make sure it's all set to zero. We shouldn't use the enum value. It should be `sizeof(ImplicitArgsTy)` for `COV5` and either hard-code it in the function for V4 or make a dummy struct.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139730/new/

https://reviews.llvm.org/D139730