AMDGPU/MC: Add .amdgpu_lds directive

[nhaehnle]

Summary

The directive defines a symbol as an group/local memory (LDS) symbol.
LDS symbols are of the processor-specific STT_AMDGPU_LDS type and encode
their minimum alignment requirement in the st_other field.

It is the linker and/or runtime loader's job to "instantiate" LDS symbols
and resolve relocations that reference them.

It is not possible to initialize LDS memory (not even zero-initialize
as for .bss).

We want to be able to link together objects -- starting with relocatable
objects, but possible expanding to shared objects in the future -- that
access LDS memory in a flexible way.

LDS memory is in an address space that is entirely separate from the
address space that contains the program image (code and normal data),
so having program segments for it doesn't really make sense.

Furthermore, we want to be able to compile multiple kernels in a
compilation unit which have disjoint use of LDS memory. In that case,
we may want to place LDS symbols differently for different kernels
to save memory (LDS memory is very limited and physically private to
each kernel invocation), so we can't simply place LDS symbols in a
.lds section.

Hence this solution where LDS symbols always stay undefined.

Change-Id: I08cbc37a7c0c32f53f7b6123aa0afc91dbc1748f

[arsenm]

lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp

206 ↗

(On Diff #197961)

Single quotes around '\n'

test/MC/AMDGPU/elf-lds.s

8–11 ↗

(On Diff #197961)

Weird indentation mix

17 ↗

(On Diff #197961)

Should add some more cases to stress the parser with no and extra whitespace. Also for the error cases with missing/invalid alignment values

[nhaehnle]

• Use '\n' instead of "\n"
• Cleanup and extend tests

[nhaehnle]

Significant update to this patch to make .amdgpu_lds work more
like common symbols:

• the assembler directive syntax is the same as .comm
• LDS symbols have a normal type but reside in the processor-specific SHN_AMDGPU_LDS section
• alignment is encoded like for common symbols

[arsenm]

include/llvm/BinaryFormat/ELF.h

1429 ↗

(On Diff #204974)

Is there a particular reason for this value?

tools/llvm-objcopy/ELF/Object.cpp

601–602 ↗

(On Diff #204974)

No else after return

630 ↗

(On Diff #204974)

This seems like a problem?

[nhaehnle]

Address review

[nhaehnle]

include/llvm/BinaryFormat/ELF.h

1429 ↗

(On Diff #204974)

Yes, this is the first processor-specific section index. SHN_HEXAGON_SCOMMON* are defined similarly.

tools/llvm-objcopy/ELF/Object.cpp

601–602 ↗

(On Diff #204974)

Done.

630 ↗

(On Diff #204974)

After some more meditation on this, I think the correct approach is to treat all OS- or processor-specific reserved sections as acceptable.

Otherwise, the code would have to be refactored with knowledge about what the current OS and processor are at this point, which seems a major and unnecessary change.

[arsenm]

LGTM

llvm/docs/AMDGPUUsage.rst

 Symbols
 -------
 
 Symbols include the following:
 
   .. table:: AMDGPU ELF Symbols
      :name: amdgpu-elf-symbols-table
 
-     ===================== ============== ============= ==================
-     Name                  Type           Section       Description
-     ===================== ============== ============= ==================
-     *link-name*           ``STT_OBJECT`` - ``.data``   Global variable
+     ===================== ================== ================ ==================
+     Name                  Type               Section          Description
+     ===================== ================== ================ ==================
+     *link-name*           ``STT_OBJECT``     - ``.data``      Global variable
                                               - ``.rodata``
-                                          - ``.bss``
-     *link-name*\ ``.kd``  ``STT_OBJECT`` - ``.rodata`` Kernel descriptor
-     *link-name*           ``STT_FUNC``   - ``.text``   Kernel entry point
-     ===================== ============== ============= ==================
+                - ``.bss``
+     *link-name*\ ``.kd``  ``STT_OBJECT``     - ``.rodata``    Kernel descriptor
+     *link-name*           ``STT_FUNC``       - ``.text``      Kernel entry point
+     *link-name*           ``STT_OBJECT``     - SHN_AMDGPU_LDS Global variable in LDS
+     ===================== ================== ================ ==================
 
 Global variable
   Global variables both used and defined by the compilation unit.
 
   If the symbol is defined in the compilation unit then it is allocated in the
   appropriate section according to if it has initialized data or is readonly.
 
   If the symbol is external then its section is ``STN_UNDEF`` and the loader
   will resolve relocations using the definition provided by another code object
   or explicitly defined by the runtime.
 
-  All global symbols, whether defined in the compilation unit or external, are
-  accessed by the machine code indirectly through a GOT table entry. This
-  allows them to be preemptable. The GOT table is only supported when the target
-  triple OS is ``amdhsa`` (see :ref:`amdgpu-target-triples`).
+  If the symbol resides in local/group memory (LDS) then its section is the
+  special processor-specific section name ``SHN_AMDGPU_LDS``, and the
+  ``st_value`` field describes alignment requirements as it does for common
+  symbols.
 
   .. TODO
      Add description of linked shared object symbols. Seems undefined symbols
      are marked as STT_NOTYPE.
 
 Kernel descriptor
   Every HSA kernel has an associated kernel descriptor. It is the address of the
   kernel descriptor that is used in the AQL dispatch packet used to invoke the

llvm/include/llvm/BinaryFormat/ELF.h

enum : unsigned {
   GNU_PROPERTY_X86_FEATURE_2_YMM = 1 << 4,
   GNU_PROPERTY_X86_FEATURE_2_ZMM = 1 << 5,
   GNU_PROPERTY_X86_FEATURE_2_FXSR = 1 << 6,
   GNU_PROPERTY_X86_FEATURE_2_XSAVE = 1 << 7,
   GNU_PROPERTY_X86_FEATURE_2_XSAVEOPT = 1 << 8,
   GNU_PROPERTY_X86_FEATURE_2_XSAVEC = 1 << 9,
 };
 
+// AMDGPU-specific section indices.
+enum {
+  SHN_AMDGPU_LDS = 0xff00, // Variable in LDS; symbol encoded like SHN_COMMON
+};
+
 // AMD specific notes. (Code Object V2)
 enum {
   // Note types with values between 0 and 9 (inclusive) are reserved.
   NT_AMD_AMDGPU_HSA_METADATA = 10,
   NT_AMD_AMDGPU_ISA = 11,
   NT_AMD_AMDGPU_PAL_METADATA = 12
 };
 

llvm/include/llvm/MC/MCSymbol.h

protected:
 
   /// A symbol can contain an Offset, or Value, or be Common, but never more
   /// than one of these.
   enum Contents : uint8_t {
     SymContentsUnset,
     SymContentsOffset,
     SymContentsVariable,
     SymContentsCommon,
+    SymContentsTargetCommon, // Index stores the section index
   };
 
   // Special sentinal value for the absolute pseudo fragment.
   static MCFragment *AbsolutePseudoFragment;
 
   /// If a symbol has a Fragment, the section is implied, so we only need
   /// one pointer.
   /// The special AbsolutePseudoFragment value is for absolute symbols.
protected:
   /// unsigned to avoid sign extension and achieve better bitpacking with MSVC.
   unsigned Kind : 3;
 
   /// True if we have created a relocation that uses this symbol.
   mutable unsigned IsUsedInReloc : 1;
 
   /// This is actually a Contents enumerator, but is unsigned to avoid sign
   /// extension and achieve better bitpacking with MSVC.
-  unsigned SymbolContents : 2;
+  unsigned SymbolContents : 3;
 
   /// The alignment of the symbol, if it is 'common', or -1.
   ///
   /// The alignment is stored as log2(align) + 1.  This allows all values from
   /// 0 to 2^31 to be stored which is every power of 2 representable by an
   /// unsigned.
   enum : unsigned { NumCommonAlignmentBits = 5 };
   unsigned CommonAlignLog2 : NumCommonAlignmentBits;
uint64_t getCommonSize() const {
     assert(isCommon() && "Not a 'common' symbol!");
     return CommonSize;
   }
 
   /// Mark this symbol as being 'common'.
   ///
   /// \param Size - The size of the symbol.
   /// \param Align - The alignment of the symbol.
-  void setCommon(uint64_t Size, unsigned Align) {
+  /// \param Target - Is the symbol a target-specific common-like symbol.
+  void setCommon(uint64_t Size, unsigned Align, bool Target = false) {
     assert(getOffset() == 0);
     CommonSize = Size;
-    SymbolContents = SymContentsCommon;
+    SymbolContents = Target ? SymContentsTargetCommon : SymContentsCommon;
 
     assert((!Align || isPowerOf2_32(Align)) &&
            "Alignment must be a power of 2");
     unsigned Log2Align = Log2_32(Align) + 1;
     assert(Log2Align < (1U << NumCommonAlignmentBits) &&
            "Out of range alignment");
     CommonAlignLog2 = Log2Align;
   }
 
   ///  Return the alignment of a 'common' symbol.
   unsigned getCommonAlignment() const {
     assert(isCommon() && "Not a 'common' symbol!");
     return CommonAlignLog2 ? (1U << (CommonAlignLog2 - 1)) : 0;
   }
 
   /// Declare this symbol as being 'common'.
   ///
   /// \param Size - The size of the symbol.
   /// \param Align - The alignment of the symbol.
+  /// \param Target - Is the symbol a target-specific common-like symbol.
   /// \return True if symbol was already declared as a different type
-  bool declareCommon(uint64_t Size, unsigned Align) {
+  bool declareCommon(uint64_t Size, unsigned Align, bool Target = false) {
     assert(isCommon() || getOffset() == 0);
     if(isCommon()) {
-      if(CommonSize != Size || getCommonAlignment() != Align)
+      if (CommonSize != Size || getCommonAlignment() != Align ||
+          isTargetCommon() != Target)
         return true;
     } else
-      setCommon(Size, Align);
+      setCommon(Size, Align, Target);
     return false;
   }
 
   /// Is this a 'common' symbol.
   bool isCommon() const {
-    return SymbolContents == SymContentsCommon;
+    return SymbolContents == SymContentsCommon ||
+           SymbolContents == SymContentsTargetCommon;
+  }
+
+  /// Is this a target-specific common-like symbol.
+  bool isTargetCommon() const {
+    return SymbolContents == SymContentsTargetCommon;
   }
 
   MCFragment *getFragment(bool SetUsed = true) const {
     MCFragment *Fragment = FragmentAndHasName.getPointer();
     if (Fragment || !isVariable())
       return Fragment;
     Fragment = getVariableValue(SetUsed)->findAssociatedFragment();
     FragmentAndHasName.setPointer(Fragment);

llvm/lib/MC/ELFObjectWriter.cpp

void ELFWriter::writeHeader(const MCAssembler &Asm) {
 
   // e_shstrndx  = Section # of '.shstrtab'
   assert(StringTableIndex < ELF::SHN_LORESERVE);
   W.write<uint16_t>(StringTableIndex);
 }
 
 uint64_t ELFWriter::SymbolValue(const MCSymbol &Sym,
                                 const MCAsmLayout &Layout) {
-  if (Sym.isCommon() && Sym.isExternal())
+  if (Sym.isCommon() && (Sym.isTargetCommon() || Sym.isExternal()))
     return Sym.getCommonAlignment();
 
   uint64_t Res;
   if (!Layout.getSymbolOffset(Sym, Res))
     return 0;
 
   if (Layout.getAssembler().isThumbFunc(&Sym))
     Res |= 1;
for (const MCSymbol &S : Asm.symbols()) {
     MSD.Symbol = cast<MCSymbolELF>(&Symbol);
 
     bool Local = Symbol.getBinding() == ELF::STB_LOCAL;
     assert(Local || !Symbol.isTemporary());
 
     if (Symbol.isAbsolute()) {
       MSD.SectionIndex = ELF::SHN_ABS;
     } else if (Symbol.isCommon()) {
+      if (Symbol.isTargetCommon()) {
+        MSD.SectionIndex = Symbol.getIndex();
+      } else {
         assert(!Local);
         MSD.SectionIndex = ELF::SHN_COMMON;
+      }
     } else if (Symbol.isUndefined()) {
       if (isSignature && !Used) {
         MSD.SectionIndex = RevGroupMap.lookup(&Symbol);
         if (MSD.SectionIndex >= ELF::SHN_LORESERVE)
           HasLargeSectionIndex = true;
       } else {
         MSD.SectionIndex = ELF::SHN_UNDEF;
       }

llvm/lib/ObjectYAML/ELFYAML.cpp

#define ECase(X) IO.enumCase(Value, #X, ELF::X)
   ECase(SHN_LOPROC);
   ECase(SHN_HIPROC);
   ECase(SHN_LOOS);
   ECase(SHN_HIOS);
   ECase(SHN_ABS);
   ECase(SHN_COMMON);
   ECase(SHN_XINDEX);
   ECase(SHN_HIRESERVE);
+  ECase(SHN_AMDGPU_LDS);
   ECase(SHN_HEXAGON_SCOMMON);
   ECase(SHN_HEXAGON_SCOMMON_1);
   ECase(SHN_HEXAGON_SCOMMON_2);
   ECase(SHN_HEXAGON_SCOMMON_4);
   ECase(SHN_HEXAGON_SCOMMON_8);
 #undef ECase
   IO.enumFallback<Hex32>(Value);
 }

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

private:
   bool ParseDirectiveAMDKernelCodeT();
   bool subtargetHasRegister(const MCRegisterInfo &MRI, unsigned RegNo) const;
   bool ParseDirectiveAMDGPUHsaKernel();
 
   bool ParseDirectiveISAVersion();
   bool ParseDirectiveHSAMetadata();
   bool ParseDirectivePALMetadataBegin();
   bool ParseDirectivePALMetadata();
+  bool ParseDirectiveAMDGPULDS();
 
   /// Common code to parse out a block of text (typically YAML) between start and
   /// end directives.
   bool ParseToEndDirective(const char *AssemblerDirectiveBegin,
                            const char *AssemblerDirectiveEnd,
                            std::string &CollectString);
 
   bool AddNextRegisterToList(unsigned& Reg, unsigned& RegWidth,
for (;;) {
     PALMetadata->setRegister(Key, Value);
     if (getLexer().isNot(AsmToken::Comma))
       break;
     Lex();
   }
   return false;
 }
 
+/// ParseDirectiveAMDGPULDS
+///  ::= .amdgpu_lds identifier ',' size_expression [',' align_expression]
+bool AMDGPUAsmParser::ParseDirectiveAMDGPULDS() {
+  if (getParser().checkForValidSection())
+    return true;
+
+  StringRef Name;
+  SMLoc NameLoc = getLexer().getLoc();
+  if (getParser().parseIdentifier(Name))
+    return TokError("expected identifier in directive");
+
+  MCSymbol *Symbol = getContext().getOrCreateSymbol(Name);
+  if (parseToken(AsmToken::Comma, "expected ','"))
+    return true;
+
+  unsigned LocalMemorySize = AMDGPU::IsaInfo::getLocalMemorySize(&getSTI());
+
+  int64_t Size;
+  SMLoc SizeLoc = getLexer().getLoc();
+  if (getParser().parseAbsoluteExpression(Size))
+    return true;
+  if (Size < 0)
+    return Error(SizeLoc, "size must be non-negative");
+  if (Size > LocalMemorySize)
+    return Error(SizeLoc, "size is too large");
+
+  int64_t Align = 4;
+  if (getLexer().is(AsmToken::Comma)) {
+    Lex();
+    SMLoc AlignLoc = getLexer().getLoc();
+    if (getParser().parseAbsoluteExpression(Align))
+      return true;
+    if (Align < 0 || !isPowerOf2_64(Align))
+      return Error(AlignLoc, "alignment must be a power of two");
+
+    // Alignment larger than the size of LDS is possible in theory, as long
+    // as the linker manages to place to symbol at address 0, but we do want
+    // to make sure the alignment fits nicely into a 32-bit integer.
+    if (Align >= 1u << 31)
+      return Error(AlignLoc, "alignment is too large");
+  }
+
+  if (parseToken(AsmToken::EndOfStatement,
+                 "unexpected token in '.amdgpu_lds' directive"))
+    return true;
+
+  Symbol->redefineIfPossible();
+  if (!Symbol->isUndefined())
+    return Error(NameLoc, "invalid symbol redefinition");
+
+  getTargetStreamer().emitAMDGPULDS(Symbol, Size, Align);
+  return false;
+}
+
 bool AMDGPUAsmParser::ParseDirective(AsmToken DirectiveID) {
   StringRef IDVal = DirectiveID.getString();
 
   if (AMDGPU::IsaInfo::hasCodeObjectV3(&getSTI())) {
     if (IDVal == ".amdgcn_target")
       return ParseDirectiveAMDGCNTarget();
 
     if (IDVal == ".amdhsa_kernel")
if (AMDGPU::IsaInfo::hasCodeObjectV3(&getSTI())) {
 
     if (IDVal == ".amd_amdgpu_isa")
       return ParseDirectiveISAVersion();
 
     if (IDVal == AMDGPU::HSAMD::AssemblerDirectiveBegin)
       return ParseDirectiveHSAMetadata();
   }
 
+  if (IDVal == ".amdgpu_lds")
+    return ParseDirectiveAMDGPULDS();
+
   if (IDVal == PALMD::AssemblerDirectiveBegin)
     return ParseDirectivePALMetadataBegin();
 
   if (IDVal == PALMD::AssemblerDirective)
     return ParseDirectivePALMetadata();
 
   return true;
 }

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

virtual void EmitDirectiveHSACodeObjectISA(uint32_t Major, uint32_t Minor,
                                              uint32_t Stepping,
                                              StringRef VendorName,
                                              StringRef ArchName) = 0;
 
   virtual void EmitAMDKernelCodeT(const amd_kernel_code_t &Header) = 0;
 
   virtual void EmitAMDGPUSymbolType(StringRef SymbolName, unsigned Type) = 0;
 
+  virtual void emitAMDGPULDS(MCSymbol *Symbol, unsigned Size,
+                             unsigned Align) = 0;
+
   /// \returns True on success, false on failure.
   virtual bool EmitISAVersion(StringRef IsaVersionString) = 0;
 
   /// \returns True on success, false on failure.
   virtual bool EmitHSAMetadataV2(StringRef HSAMetadataString);
 
   /// \returns True on success, false on failure.
   virtual bool EmitHSAMetadataV3(StringRef HSAMetadataString);
public:
   void EmitDirectiveHSACodeObjectISA(uint32_t Major, uint32_t Minor,
                                      uint32_t Stepping, StringRef VendorName,
                                      StringRef ArchName) override;
 
   void EmitAMDKernelCodeT(const amd_kernel_code_t &Header) override;
 
   void EmitAMDGPUSymbolType(StringRef SymbolName, unsigned Type) override;
 
+  void emitAMDGPULDS(MCSymbol *Sym, unsigned Size, unsigned Align) override;
+
   /// \returns True on success, false on failure.
   bool EmitISAVersion(StringRef IsaVersionString) override;
 
   /// \returns True on success, false on failure.
   bool EmitHSAMetadata(msgpack::Document &HSAMetadata, bool Strict) override;
 
   /// \returns True on success, false on failure.
   bool EmitHSAMetadata(const AMDGPU::HSAMD::Metadata &HSAMetadata) override;
public:
   void EmitDirectiveHSACodeObjectISA(uint32_t Major, uint32_t Minor,
                                      uint32_t Stepping, StringRef VendorName,
                                      StringRef ArchName) override;
 
   void EmitAMDKernelCodeT(const amd_kernel_code_t &Header) override;
 
   void EmitAMDGPUSymbolType(StringRef SymbolName, unsigned Type) override;
 
+  void emitAMDGPULDS(MCSymbol *Sym, unsigned Size, unsigned Align) override;
+
   /// \returns True on success, false on failure.
   bool EmitISAVersion(StringRef IsaVersionString) override;
 
   /// \returns True on success, false on failure.
   bool EmitHSAMetadata(msgpack::Document &HSAMetadata, bool Strict) override;
 
   /// \returns True on success, false on failure.
   bool EmitHSAMetadata(const AMDGPU::HSAMD::Metadata &HSAMetadata) override;

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

void AMDGPUTargetAsmStreamer::EmitAMDGPUSymbolType(StringRef SymbolName,
   switch (Type) {
     default: llvm_unreachable("Invalid AMDGPU symbol type");
     case ELF::STT_AMDGPU_HSA_KERNEL:
       OS << "\t.amdgpu_hsa_kernel " << SymbolName << '\n' ;
       break;
   }
 }
 
+void AMDGPUTargetAsmStreamer::emitAMDGPULDS(MCSymbol *Symbol, unsigned Size,
+                                            unsigned Align) {
+  OS << "\t.amdgpu_lds " << Symbol->getName() << ", " << Size << ", " << Align
+     << '\n';
+}
+
 bool AMDGPUTargetAsmStreamer::EmitISAVersion(StringRef IsaVersionString) {
   OS << "\t.amd_amdgpu_isa \"" << IsaVersionString << "\"\n";
   return true;
 }
 
 bool AMDGPUTargetAsmStreamer::EmitHSAMetadata(
     const AMDGPU::HSAMD::Metadata &HSAMetadata) {
   std::string HSAMetadataString;

 
 void AMDGPUTargetELFStreamer::EmitAMDGPUSymbolType(StringRef SymbolName,
                                                    unsigned Type) {
   MCSymbolELF *Symbol = cast<MCSymbolELF>(
       getStreamer().getContext().getOrCreateSymbol(SymbolName));
   Symbol->setType(Type);
 }
 
+void AMDGPUTargetELFStreamer::emitAMDGPULDS(MCSymbol *Symbol, unsigned Size,
+                                            unsigned Align) {
+  assert(isPowerOf2_32(Align));
+
+  MCSymbolELF *SymbolELF = cast<MCSymbolELF>(Symbol);
+  SymbolELF->setType(ELF::STT_OBJECT);
+
+  if (!SymbolELF->isBindingSet()) {
+    SymbolELF->setBinding(ELF::STB_GLOBAL);
+    SymbolELF->setExternal(true);
+  }
+
+  if (SymbolELF->declareCommon(Size, Align, true)) {
+    report_fatal_error("Symbol: " + Symbol->getName() +
+                       " redeclared as different type");
+  }
+
+  SymbolELF->setIndex(ELF::SHN_AMDGPU_LDS);
+  SymbolELF->setSize(MCConstantExpr::create(Size, getContext()));
+}
+
 bool AMDGPUTargetELFStreamer::EmitISAVersion(StringRef IsaVersionString) {
   // Create two labels to mark the beginning and end of the desc field
   // and a MCExpr to calculate the size of the desc field.
   auto &Context = getContext();
   auto *DescBegin = Context.createTempSymbol();
   auto *DescEnd = Context.createTempSymbol();
   auto *DescSZ = MCBinaryExpr::createSub(
     MCSymbolRefExpr::create(DescEnd, Context),

llvm/test/MC/AMDGPU/elf-lds-error.s

+// RUN: not llvm-mc -triple amdgcn-- -mcpu gfx900 %s -o - 2>&1 | FileCheck %s
+
+// CHECK: :[[@LINE+1]]:27: error: size is too large
+        .amdgpu_lds huge, 200000
+
+// CHECK: :[[@LINE+1]]:30: error: size must be non-negative
+        .amdgpu_lds negsize, -4
+
+// CHECK: :[[@LINE+1]]:36: error: alignment must be a power of two
+        .amdgpu_lds zero_align, 5, 0
+
+// CHECK: :[[@LINE+1]]:39: error: alignment must be a power of two
+        .amdgpu_lds non_pot_align, 0, 12
+
+// CHECK: :[[@LINE+1]]:36: error: alignment is too large
+        .amdgpu_lds huge_align, 0, 1099511627776
+
+// CHECK: :[[@LINE+1]]:9: error: unknown directive
+        .amdgpu_ldsnowhitespace, 8

llvm/test/MC/AMDGPU/elf-lds.s

+// RUN: llvm-mc -filetype=obj -triple amdgcn-- -mcpu gfx900 %s -o - | llvm-readobj -t -r | FileCheck %s
+
+  .text
+  .globl test_kernel
+  .p2align 8
+  .type test_kernel,@function
+test_kernel:
+  s_mov_b32 s0, lds0@abs32@lo
+  v_lshl_add_u32 v3, v0, 2, s0
+  ds_read2_b32 v[1:2], v3 offset1:1
+
+  s_mov_b32 s0, lds4@abs32@lo
+  v_lshl_add_u32 v3, v0, 2, s0
+  ds_write_b32 v3, v1
+  s_endpgm
+.Lfunc_end:
+  .size test_kernel, .Lfunc_end-test_kernel
+
+  .globl lds0
+  .amdgpu_lds lds0, 192, 16
+
+  .globl lds1
+  .amdgpu_lds lds1,387,8
+
+  ; Weird whitespace cases
+  .globl lds2
+  .amdgpu_lds lds2, 12
+
+  ; No alignment or .globl directive, not mentioned anywhere
+  .amdgpu_lds lds3, 16
+
+  ; No alignment or .globl directive, size 0, but mentioned in .text
+  .amdgpu_lds lds4, 0
+
+// CHECK:      Relocations [
+// CHECK:        Section (3) .rel.text {
+// CHECK-NEXT:     0x4 R_AMDGPU_ABS32 lds0 0x0
+// CHECK-NEXT:     0x1C R_AMDGPU_ABS32 lds4 0x0
+// CHECK-NEXT:   }
+// CHECK:      ]
+
+// CHECK:      Symbol {
+// CHECK:        Name: lds0 (54)
+// CHECK-NEXT:   Value: 0x10
+// CHECK-NEXT:   Size: 192
+// CHECK-NEXT:   Binding: Global (0x1)
+// CHECK-NEXT:   Type: Object (0x1)
+// CHECK-NEXT:   Other: 0
+// CHECK-NEXT:   Section: Processor Specific (0xFF00)
+// CHECK-NEXT: }
+
+// CHECK:      Symbol {
+// CHECK:        Name: lds1 (49)
+// CHECK-NEXT:   Value: 0x8
+// CHECK-NEXT:   Size: 387
+// CHECK-NEXT:   Binding: Global (0x1)
+// CHECK-NEXT:   Type: Object (0x1)
+// CHECK-NEXT:   Other: 0
+// CHECK-NEXT:   Section: Processor Specific (0xFF00)
+// CHECK-NEXT: }
+
+// CHECK:      Symbol {
+// CHECK:        Name: lds2 (44)
+// CHECK-NEXT:   Value: 0x4
+// CHECK-NEXT:   Size: 12
+// CHECK-NEXT:   Binding: Global (0x1)
+// CHECK-NEXT:   Type: Object (0x1)
+// CHECK-NEXT:   Other: 0
+// CHECK-NEXT:   Section: Processor Specific (0xFF00)
+// CHECK-NEXT: }
+
+// CHECK-NOT:    Name: lds3
+
+// CHECK:      Symbol {
+// CHECK:        Name: lds4 (39)
+// CHECK-NEXT:   Value: 0x4
+// CHECK-NEXT:   Size: 0
+// CHECK-NEXT:   Binding: Global (0x1)
+// CHECK-NEXT:   Type: Object (0x1)
+// CHECK-NEXT:   Other: 0
+// CHECK-NEXT:   Section: Processor Specific (0xFF00)
+// CHECK-NEXT: }

llvm/tools/llvm-objcopy/ELF/Object.h

 // Symbols have a st_shndx field that normally stores an index but occasionally
 // stores a different special value. This enum keeps track of what the st_shndx
 // field means. Most of the values are just copies of the special SHN_* values.
 // SYMBOL_SIMPLE_INDEX means that the st_shndx is just an index of a section.
 enum SymbolShndxType {
   SYMBOL_SIMPLE_INDEX = 0,
   SYMBOL_ABS = ELF::SHN_ABS,
   SYMBOL_COMMON = ELF::SHN_COMMON,
+  SYMBOL_AMDGPU_LDS = ELF::SHN_AMDGPU_LDS,
   SYMBOL_HEXAGON_SCOMMON = ELF::SHN_HEXAGON_SCOMMON,
   SYMBOL_HEXAGON_SCOMMON_2 = ELF::SHN_HEXAGON_SCOMMON_2,
   SYMBOL_HEXAGON_SCOMMON_4 = ELF::SHN_HEXAGON_SCOMMON_4,
   SYMBOL_HEXAGON_SCOMMON_8 = ELF::SHN_HEXAGON_SCOMMON_8,
   SYMBOL_XINDEX = ELF::SHN_XINDEX,
 };
 
 struct Symbol {

llvm/tools/llvm-objcopy/ELF/Object.cpp

 }
 
 static bool isValidReservedSectionIndex(uint16_t Index, uint16_t Machine) {
   switch (Index) {
   case SHN_ABS:
   case SHN_COMMON:
     return true;
   }
+
+  if (Machine == EM_AMDGPU) {
+    return Index == SHN_AMDGPU_LDS;
+  }
+
   if (Machine == EM_HEXAGON) {
     switch (Index) {
     case SHN_HEXAGON_SCOMMON:
     case SHN_HEXAGON_SCOMMON_2:
     case SHN_HEXAGON_SCOMMON_4:
     case SHN_HEXAGON_SCOMMON_8:
       return true;
     }
   }
   return false;
 }
 
 // Large indexes force us to clarify exactly what this function should do. This
 // function should return the value that will appear in st_shndx when written
 // out.
 uint16_t Symbol::getShndx() const {
   if (DefinedIn != nullptr) {
     if (DefinedIn->Index >= SHN_LORESERVE)
       return SHN_XINDEX;
     return DefinedIn->Index;
   }
-  switch (ShndxType) {
+
+  if (ShndxType == SYMBOL_SIMPLE_INDEX) {
     // This means that we don't have a defined section but we do need to
     // output a legitimate section index.
-  case SYMBOL_SIMPLE_INDEX:
     return SHN_UNDEF;
-  case SYMBOL_ABS:
-  case SYMBOL_COMMON:
-  case SYMBOL_HEXAGON_SCOMMON:
-  case SYMBOL_HEXAGON_SCOMMON_2:
-  case SYMBOL_HEXAGON_SCOMMON_4:
-  case SYMBOL_HEXAGON_SCOMMON_8:
-  case SYMBOL_XINDEX:
-    return static_cast<uint16_t>(ShndxType);
   }
-  llvm_unreachable("Symbol with invalid ShndxType encountered");
+
+  assert(ShndxType == SYMBOL_ABS || ShndxType == SYMBOL_COMMON ||
+         (ShndxType >= ELF::SHN_LOPROC && ShndxType <= ELF::SHN_HIPROC) ||
+         (ShndxType >= ELF::SHN_LOOS && ShndxType <= ELF::SHN_HIOS));
+  return static_cast<uint16_t>(ShndxType);
 }
 
 bool Symbol::isCommon() const { return getShndx() == SHN_COMMON; }
 
 void SymbolTableSection::assignIndices() {
   uint32_t Index = 0;
   for (auto &Sym : Symbols)
     Sym->Index = Index++;