[PATCH] D53495: Add OUTPUT_FORMAT linker script directive support.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/ScriptParser.cpp:97
   SymbolAssignment *readAssignment(StringRef Tok);
+  std::pair<ELFKind, uint16_t> readBfdName();
   void readSort();
----------------
It was confusing naming for me. Was not clear what is "bfd name".
Maybe `readBfdArch` or `readBfdArchName`?


================
Comment at: lld/ELF/ScriptParser.cpp:394
+    return {ELF32LEKind, EM_X86_64};
+  if (S == "elf64-littleaarch64")
+    return {ELF64LEKind, EM_AARCH64};
----------------
An observation: bfd code contains both `elf64-littleaarch64-cloudabi` and `elf64-littleaarch64` for example.
Should `S ==` in this function be changed to `S.startsWith` maybe?


================
Comment at: lld/ELF/ScriptParser.cpp:410
+  if (Config->EKind == ELFNoneKind)
+    std::tie(Config->EKind, Config->EMachine) = BfdName;
+
----------------
So I think this means LLD will error out when seeing the object file with incompatible architecture after parsing the script containing `OUTPUT_FORMAT`. Behavior is fine, but I think there is no test provided for that case?


================
Comment at: lld/test/ELF/linkerscript/output-format.s:5
-# RUN: ld.lld -shared -o %t2 %t1 %t.script
-# RUN: llvm-readobj %t2 > /dev/null
-
----------------
This tested we are ignoring the big and little parameters. I think the test was useful. Should it be moved to emulation.s?


https://reviews.llvm.org/D53495