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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In https://reviews.llvm.org/D40849#952514, @compnerd wrote:

> @jakehehrlich


I think you're getting mixed up between me and Jake :-)

> LLVM currently does 4-byte alignment and this code is following suit.  My reading of the gABI was that it was 4-byte on 32-bit, 8-byte on 64-bit.

But this code doesn't do what the gABI says? I would expect 8-byte note sections on 64-bit ELF, such as x86_64. Anyway, as noted, there's quite a bit of ongoing discussion on the gABI mailing list, so maybe it's best to wait on a final decision on format until that concludes?

@ruiu's proposal works for me. There is one new pragma defined "linker_command", which takes two options, one being the switch ("-l"), and the other the values for that switch ("iberty"), with an empty string being used ("") for switches with no argument, if we ever decide they are supported in the future, and similarly and empty string being used for the switch for new positional arguments (again I'm not saying we should necessarily support this, but the option will be there). Each linker will then be able to accept/reject/ignore the switch, without having to go through complicated parsing routines. I would hope it should be possible to pass it to the existing command line parsing library and find out which switch it corresponds to, somehow, but I don't know that area so well. The compiler does not have to change at all after the pragma is put in, even if new options are desired.



================
Comment at: include/llvm/BinaryFormat/ELF.h:1384
+  NT_LLVM_LINKER_OPTIONS = 0x2302,
+};
+
----------------
This number seems a bit arbitrary. Any particular reason for this value specifically?


================
Comment at: test/Feature/elf-linker-options.ll:19
+; CHECK: .byte 0
+; CHECK: .p2align 3
----------------
I'm confused. Why is this .p2align 3 and not .p2align 2?


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:3464-3469
+    uint32_t ID;
+    const char *Name;
+  } Notes[] = {
+    {ELF::NT_LLVM_LINKER_OPTIONS, "NT_LLVM_LINKER_OPTIONS (linker options)"},
+  };
+
----------------
Why not make this a proper map of some kind (e.g. DenseMap), so that you can use a straightforward lookup?


Repository:
  rL LLVM

https://reviews.llvm.org/D40849