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

[Yaxun Liu via llvm-commits]

yaxunl added inline comments.

================
Comment at: lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:927
@@ +926,3 @@
+    emitRuntimeMDIntValue(OutStreamer, RuntimeMD::KeyArgValueType,
+                          getRuntimeMDValueType(T, BaseTypeName), 2);
+
----------------
nhaustov wrote:
> DataType enum is defined as char, but here 2 bytes are used.
Data type is now defined as 2 bytes since 1 byte may not be enough for all runtimes.

================
Comment at: lib/Target/AMDGPU/AMDGPURuntimeMetadata.h:98
@@ +97,3 @@
+
+  enum LanguageVersion {
+    V100          = 100,
----------------
nhaustov wrote:
> Are these OpenCL versions? Enum here seems to be overly restricting. Can this field just be a short integer?
These version numbers follow Clang convention for OpenCL, but they could be used for other languages. We need to have an agreement between compiler and runtime what value is used for certain version. Otherwise, how runtime knows OpenCL version 1.0 is represented by value 100 instead of 10?

================
Comment at: lib/Target/AMDGPU/AMDGPURuntimeMetadata.h:130
@@ +129,3 @@
+
+    enum TypeQualifier : char {
+      Const    = 1,
----------------
nhaustov wrote:
> This can be now removed.
Will do.


https://reviews.llvm.org/D21849