[AMDGPU] Don't mark the .note section as ALLOC

[Flakebi]

Summary

Marking a section as ALLOC tells the ELF loader to load the section into memory.
As we do not want to load the notes into VRAM, the flag should not be there.

On AMDHSA, .note is still marked as ALLOC, apparently this is currently
needed for OpenCL (see https://reviews.llvm.org/D74995).

[Flakebi]

Fix formatting issue

[Flakebi]

Anyone willing to review this?

[scott.linder]

LGTM, although I don't understand the issue with OpenCL. Reading the comments on the linked review leave me more confused. If anyone from that discussion can weigh in I am happy to approve this.

[t-tye]

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp

441–443

Why does the AMDHSA ABI need this note section to be allocated?

@kzhuravl do you have any information about this?

[Flakebi]

ping

[Flakebi]

ping

[nhaehnle]

LGTM. My suggestion would be to leave a TODO comment for the OpenCL thing and go ahead.

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp

441–443

Since we seem to have been unable to make progress on this OpenCL question, I would suggest to add a comment here saying that part of the OpenCL runtime relies on that, and add a link to that review (even though I agree that it's not particularly enlightening).

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.h

void EmitAmdhsaKernelDescriptor(
       const MCSubtargetInfo &STI, StringRef KernelName,
       const amdhsa::kernel_descriptor_t &KernelDescriptor, uint64_t NextVGPR,
       uint64_t NextSGPR, bool ReserveVCC, bool ReserveFlatScr,
       bool ReserveXNACK) override;
 };
 
 class AMDGPUTargetELFStreamer final : public AMDGPUTargetStreamer {
   MCStreamer &Streamer;
+  Triple::OSType Os;
 
   void EmitNote(StringRef Name, const MCExpr *DescSize, unsigned NoteType,
                 function_ref<void(MCELFStreamer &)> EmitDesc);
 
 public:
   AMDGPUTargetELFStreamer(MCStreamer &S, const MCSubtargetInfo &STI);
 
   MCELFStreamer &getStreamer();

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp

#undef PRINT_FIELD
 
   OS << "\t.end_amdhsa_kernel\n";
 }
 
 //===----------------------------------------------------------------------===//
 // AMDGPUTargetELFStreamer
 //===----------------------------------------------------------------------===//
 
-AMDGPUTargetELFStreamer::AMDGPUTargetELFStreamer(
-    MCStreamer &S, const MCSubtargetInfo &STI)
-    : AMDGPUTargetStreamer(S), Streamer(S) {
+AMDGPUTargetELFStreamer::AMDGPUTargetELFStreamer(MCStreamer &S,
+                                                 const MCSubtargetInfo &STI)
+    : AMDGPUTargetStreamer(S), Streamer(S), Os(STI.getTargetTriple().getOS()) {
   MCAssembler &MCA = getStreamer().getAssembler();
   unsigned EFlags = MCA.getELFHeaderEFlags();
 
   EFlags &= ~ELF::EF_AMDGPU_MACH;
   EFlags |= getElfMach(STI.getCPU());
 
   EFlags &= ~ELF::EF_AMDGPU_XNACK;
   if (AMDGPU::hasXNACK(STI))

 void AMDGPUTargetELFStreamer::EmitNote(
     StringRef Name, const MCExpr *DescSZ, unsigned NoteType,
     function_ref<void(MCELFStreamer &)> EmitDesc) {
   auto &S = getStreamer();
   auto &Context = S.getContext();
 
   auto NameSZ = Name.size() + 1;
 
+  unsigned NoteFlags = 0;
+  // TODO Apparently, this is currently needed for OpenCL as mentioned in
+  // https://reviews.llvm.org/D74995
+  if (Os == Triple::AMDHSA)
+    NoteFlags = ELF::SHF_ALLOC;
+
   S.PushSection();
-  S.SwitchSection(Context.getELFSection(
-    ElfNote::SectionName, ELF::SHT_NOTE, ELF::SHF_ALLOC));
+  S.SwitchSection(
+      Context.getELFSection(ElfNote::SectionName, ELF::SHT_NOTE, NoteFlags));
   S.emitInt32(NameSZ);                                        // namesz
   S.emitValue(DescSZ, 4);                                     // descz
   S.emitInt32(NoteType);                                      // type
   S.emitBytes(Name);                                          // name
   S.emitValueToAlignment(4, 0, 1, 0);                         // padding 0
   EmitDesc(S);                                                // desc
   S.emitValueToAlignment(4, 0, 1, 0);                         // padding 0
   S.PopSection();

llvm/test/CodeGen/AMDGPU/amdpal-elf.ll

+; RUN: llc < %s -mtriple=amdgcn--amdpal -mcpu=kaveri -filetype=obj -mattr=-code-object-v3 | llvm-readobj -symbols -s -sd | FileCheck --check-prefix=ELF %s
+; RUN: llc < %s -mtriple=amdgcn--amdpal -mcpu=kaveri -mattr=-code-object-v3 | llvm-mc -filetype=obj -triple amdgcn--amdpal -mcpu=kaveri -mattr=-code-object-v3 | llvm-readobj -symbols -s -sd | FileCheck %s --check-prefix=ELF
+; RUN: llc < %s -mtriple=amdgcn--amdpal -mcpu=gfx1010 -mattr=+WavefrontSize32,-WavefrontSize64,-code-object-v3 | FileCheck --check-prefix=GFX10-W32 %s
+; RUN: llc < %s -mtriple=amdgcn--amdpal -mcpu=gfx1010 -mattr=-WavefrontSize32,+WavefrontSize64,-code-object-v3 | FileCheck --check-prefix=GFX10-W64 %s
+
+; ELF: Section {
+; ELF: Name: .text
+; ELF: Type: SHT_PROGBITS (0x1)
+; ELF: Flags [ (0x6)
+; ELF: SHF_ALLOC (0x2)
+; ELF: SHF_EXECINSTR (0x4)
+; ELF: }
+
+; ELF: SHT_NOTE
+; ELF: Flags [ (0x0)
+; ELF: ]
+
+; ELF: Symbol {
+; ELF: Name: simple
+; ELF: Size: 36
+; ELF: Section: .text (0x2)
+; ELF: }
+
+; GFX10-W32: NumSGPRsForWavesPerEU: 4
+; GFX10-W32: NumVGPRsForWavesPerEU: 3
+; GFX10-W64: NumSGPRsForWavesPerEU: 2
+; GFX10-W64: NumVGPRsForWavesPerEU: 3
+
+define amdgpu_kernel void @simple(i32 addrspace(1)* %out) {
+entry:
+  store i32 0, i32 addrspace(1)* %out
+  ret void
+}