[PATCH] D40849: CodeGen: support an extension to pass linker options on ELF

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: include/llvm/BinaryFormat/ELF.h:1384
+  NT_LLVM_LINKER_OPTIONS = 0x2302,
+};
+
----------------
compnerd wrote:
> jhenderson wrote:
> > compnerd wrote:
> > > jhenderson wrote:
> > > > This number seems a bit arbitrary. Any particular reason for this value specifically?
> > > No, there is no logical reasoning for this value.  Would another value be preferable?
> > Assuming this is the first LLVM-vendor note, maybe 1? If not, what are the other note type values?
> I used `1` originally as well.  However, the note values themselves are not valued by owner, but in a global pool.  We have values from various places like `0x53494749` which is if the `siginfo_t` is present in the core file that the ELF file represents.  Or did I manage to confuse myself when I looked at the values?
According to the ELF gABI, the note type values are vendor-defined, so it doesn't matter that they're in a global pool (looking at ELF.h, we already support some duplicate values like NT_FREEBSD_PROCSTAT_VMMAP/NT_AMD_AMDGPU_HSAMETADATA, both with value 10). I'm not seeing a problem with sharing this value still? Consumers should always test the vendor type first.

I'm not sure where you're getting the 0x53494749 from, or its relevance here.


Repository:
  rL LLVM

https://reviews.llvm.org/D40849