[llvm] [AMDGPU] MCExpr-ify MC layer kernel descriptor (PR #80855)

Pierre van Houtryve via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 12 03:31:29 PST 2024


================
@@ -0,0 +1,68 @@
+//===--- AMDGPUMCKernelDescriptor.h ---------------------------*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file
+/// AMDHSA kernel descriptor MCExpr struct for use in MC layer. Uses
+/// AMDHSAKernelDescriptor.h for sizes and constants.
+///
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCKERNELDESCRIPTOR_H
+#define LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCKERNELDESCRIPTOR_H
+
+#include "llvm/Support/AMDHSAKernelDescriptor.h"
+
+namespace llvm {
+class MCExpr;
+class MCContext;
+namespace AMDGPU {
+
+struct MCKernelDescriptor {
+  const MCExpr *group_segment_fixed_size = nullptr;
+  const MCExpr *private_segment_fixed_size = nullptr;
+  const MCExpr *kernarg_size = nullptr;
+  const MCExpr *compute_pgm_rsrc3 = nullptr;
+  const MCExpr *compute_pgm_rsrc1 = nullptr;
+  const MCExpr *compute_pgm_rsrc2 = nullptr;
+  const MCExpr *kernel_code_properties = nullptr;
+  const MCExpr *kernarg_preload = nullptr;
+
+  static void bits_set(const MCExpr *&Dst, const MCExpr *Value, uint32_t Shift,
+                       uint32_t Mask, MCContext &Ctx);
+  static const MCExpr *bits_get(const MCExpr *Src, uint32_t Shift,
+                                uint32_t Mask, MCContext &Ctx);
+};
+
+enum : uint32_t {
+  SIZEOF_GROUP_SEGMENT_FIXED_SIZE =
----------------
Pierre-vh wrote:

I don't especially like those global names, they don't add meaningful info that can't be retrieved another way (just writing sizeof ourselves). If they were each used in multiple places (3 or more) I'd be ok with them, but they're all used in one or two places max. I'd try to remove them, but if you want to keep them:

maybe add those next to the _OFFSET ones instead of using a separate enum? You can also use a `_WIDTH` suffix instead of prefixing with `SIZEOF`.
You can also merge a few of those to reduce the number of entries:

- RSRC1 to 3 have the same size
- group_segment_fixed_size/private_segment_fixed_size could also be merged
- remove the ones with a single use ?
- Do we really need the _RESERVED ones? Those get confusing IMO. `SIZEOF_RESERVED0` is a very generic name for something that's not contained to a small namespace.

https://github.com/llvm/llvm-project/pull/80855


More information about the llvm-commits mailing list