[PATCH] D51199: CodeGen: Add two more conditions for adding symbols to the address-significance table.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1546
+      if (!GV.use_empty() && !GV.isThreadLocal() &&
+          !GV.hasDLLImportStorageClass() && !GV.getName().startswith("llvm.") &&
           !GV.hasAtLeastLocalUnnamedAddr())
----------------
rnk wrote:
> Dllimport things aren't actually always imported, sometimes they can be resolved to symbols in the same image. MSVC will warn for this by default, but in my experience developers disable the warning, especially if they don't use Windows for development.
> 
> Can we end up with an incomplete table if we apply dllimport, resolve the symbol locally, and take the symbols address in that TU?
To answer my own question, maybe we just don't worry about doing extra ICF on symbols like that. Basically, if you have code like this, you see this warning, and then disable it, and *then* you rely on the symbol's address being unique, you are out of luck.

These symbols tend to be the result of template instantiation anyway, which often have non-unique addresses with -fvisibility-inlines-hidden is set, which is pretty common.


Repository:
  rL LLVM

https://reviews.llvm.org/D51199