[PATCH] D25781: AMDGPU: Emit runtime metadata as a note element in .note section

[Konstantin Zhuravlyov via llvm-commits]

kzhuravl added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:1130-1131
+
+  static const char Name[] = "AMD"; // Vendor name of node element
+  const unsigned NameSZ = sizeof(Name); // Size of Name including trailing null.
+
----------------
Once you move it to AMDGPUTargetStreamer classes as suggested by Tom, use AMDGPUTargetELFStreamer::NoteName instead of defining the name and size (see rL284760 for more info)


================
Comment at: lib/Target/AMDGPU/AMDGPUNoteType.h:20-32
+namespace AMDGPU {
+enum NoteType{
+    NT_AMDGPU_HSA_CODE_OBJECT_VERSION = 1,
+    NT_AMDGPU_HSA_HSAIL = 2,
+    NT_AMDGPU_HSA_ISA = 3,
+    NT_AMDGPU_HSA_PRODUCER = 4,
+    NT_AMDGPU_HSA_PRODUCER_OPTIONS = 5,
----------------
Note types are defined in AMDGPUTargetELFStreamer. Why redefine them?


================
Comment at: lib/Target/AMDGPU/AMDGPURuntimeMetadata.h:41
 #include <stdint.h>
+#include "AMDGPUNoteType.h"
 
----------------
I do not think we need to include this here.


================
Comment at: lib/Target/AMDGPU/AMDGPURuntimeMetadata.h:52
   // ELF section name containing runtime metadata
-  const char SectionName[] = ".AMDGPU.runtime_metadata";
+  const char SectionName[] = ".note";
 
----------------
This seems like a property of an elf, and not RuntimeMD. I would suggest making it `static const char *` in AMDGPUTargetELFStreamer (probably along with flags, etc.)


https://reviews.llvm.org/D25781