[PATCH] D66355: [COFF] Add libcall symbols to the link when LTO is being used.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lld/COFF/Driver.cpp:1771
 
+  if (!BitcodeFile::instances.empty()) {
+    for (const char *s : libcallRoutineNames)
----------------
I think this should be inside the `do / while` loop above in case one of the bitcode objects that provides a libcall introduces a new undefined symbol reference.


================
Comment at: lld/COFF/SymbolTable.cpp:588
+void SymbolTable::addLibcall(StringRef name) {
+  Symbol *sym = find(name);
+  if (!sym)
----------------
pcc wrote:
> rnk wrote:
> > pcc wrote:
> > > The logic here should depend on the name. If it begins with `\01` use `find` on `name.substr(1)`, otherwise use `findUnderscore`.
> > So far as I can tell, none of the symbols from RuntimeLibcalls use the `\01` escape prefix. Is that necessary? Can this be simplified to just one call to `findUnderscore`?
> I vaguely recalled that we had some, but yes, apparently we don't. Simplifying to `findUnderscore` sounds good then.
I think you should be able to simplify this down to just findUnderscore, no need to check for startswith("_"). I think the reason you have it is that in your test case, the .s file would normally use a "mangled" symbol for i686-windows-msvc, so it would provide the symbol `__sync_val_compare_and_swap_8`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66355/new/

https://reviews.llvm.org/D66355