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

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: include/llvm/BinaryFormat/ELF.h:1384
+  NT_LLVM_LINKER_OPTIONS = 0x2302,
+};
+
----------------
jhenderson wrote:
> 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.
Ah, in that case, Ill go ahead and change to 1.  There's no need to not use an arbitrary value then.  That was the one that showed up on my screen when I was looking for the values for various notes.  I had looked at the values and thought that the values were all in a single pool and that there was no pattern to any of them.


Repository:
  rL LLVM

https://reviews.llvm.org/D40849