[PATCH] D21849: [AMDGPU] Add metadata for runtime

[Yaxun Liu via llvm-commits]

yaxunl marked 25 inline comments as done.

================
Comment at: lib/Target/AMDGPU/AMDGPURuntimeMetadata.h:50
@@ +49,3 @@
+  // ELF section name containing runtime metadata
+  const char SectionName[] = ".AMDGPU.runtime_metadata";
+
----------------
arsenm wrote:
> global should not defined in a header
const global variable in C++ has internal linkage, so should be fine in header.

================
Comment at: lib/Target/AMDGPU/AMDGPURuntimeMetadata.h:88-93
@@ +87,8 @@
+
+  enum Language : char {
+    OpenCL_C      = 0,
+    HCC           = 1,
+    OpenMP        = 2,
+    OpenCL_CPP    = 3,
+  };
+
----------------
arsenm wrote:
> arsenm wrote:
> > should we re-use dwarf language IDs + extensions or something instead of inventing a new enum? Maybe an unknown value for 0 or -1?
> Specify the signedness, uint8_t
This key is intended to indicate the required language spec supported by the runtime, not necessarily the original source language. For example, we can use an LLVM assembly to create a binary which can be executed on OpenCL 1.2 runtime, then we can add a key OpenCL_C to this binary, so that the runtime knows whether it can be executed.

Maybe these keys should be renamed as RuntimeSpec and RuntimeSpecVersion?


================
Comment at: test/CodeGen/AMDGPU/runtime-metadata.ll:718
@@ +717,3 @@
+!50 = !{i32 1, i32 2, i32 3}
+!51 = !{!"int *", !"int *", !"int *"}
+!60 = !{i32 1, i32 1, i32 1}
----------------
arsenm wrote:
> Are these supposed to be missing the address space?
No. address space is represented separately in kernel_arg_addr_space metaata.


https://reviews.llvm.org/D21849