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

[Yaxun Liu via llvm-commits]

yaxunl added inline comments.


================
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,
----------------
kzhuravl wrote:
> Note types are defined in AMDGPUTargetELFStreamer. Why redefine them?
Moved to AMDGPUPTNote.h, which is shared between compiler and runtime.

The runtime needs to know the enum value for note type for runtime. To avoid maintaining two copies of enum definitions, I moved these enum definitions to the shared header file.


================
Comment at: lib/Target/AMDGPU/AMDGPURuntimeMetadata.h:52
   // ELF section name containing runtime metadata
-  const char SectionName[] = ".AMDGPU.runtime_metadata";
+  const char SectionName[] = ".note";
 
----------------
kzhuravl wrote:
> 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.)
This needs to be defined in the shared header file between runtime and compiler.

As you said, it is not runtime metadata specific, so I created AMDPTNote.h and moved it there.


================
Comment at: lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp:14
 
+#include "AMDGPUNoteType.h"
 #include "AMDGPUTargetStreamer.h"
----------------
tstellarAMD wrote:
> Are the changes in this file necessary?
We need to share the enums for PT_NOTE sections with runtime.


https://reviews.llvm.org/D25781