Page MenuHomePhabricator

AMDGPU/MC: Add .amdgpu_lds directive
ClosedPublic

Authored by nhaehnle on May 3 2019, 5:03 AM.

Details

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

Diff Detail

Event Timeline

nhaehnle created this revision.May 3 2019, 5:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2019, 5:03 AM
arsenm added inline comments.May 3 2019, 5:57 AM
lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp
210

Single quotes around '\n'

test/MC/AMDGPU/elf-lds.s
9–12

Weird indentation mix

18

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 updated this revision to Diff 198453.May 7 2019, 5:53 AM
nhaehnle marked 3 inline comments as done.
  • Use '\n' instead of "\n"
  • Cleanup and extend tests
nhaehnle updated this revision to Diff 204974.Jun 16 2019, 4:35 PM

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 added inline comments.Jun 18 2019, 5:18 PM
include/llvm/BinaryFormat/ELF.h
1429

Is there a particular reason for this value?

tools/llvm-objcopy/ELF/Object.cpp
601–602

No else after return

630

This seems like a problem?

nhaehnle updated this revision to Diff 206207.Jun 24 2019, 6:46 AM
nhaehnle marked 5 inline comments as done.

Address review

nhaehnle added inline comments.Jun 24 2019, 7:16 AM
include/llvm/BinaryFormat/ELF.h
1429

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

tools/llvm-objcopy/ELF/Object.cpp
601–602

Done.

630

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.

nhaehnle marked an inline comment as done.Jun 24 2019, 7:16 AM
arsenm accepted this revision.Jun 24 2019, 3:15 PM

LGTM

This revision is now accepted and ready to land.Jun 24 2019, 3:15 PM
This revision was automatically updated to reflect the committed changes.