[PATCH] D18986: [ThinLTO] Prevent importing of "llvm.used" values

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In http://reviews.llvm.org/D18986#404576, @joker.eph wrote:

> > > If you import `bar()` it mean you are not considering my 1) but the 2) above (this is a function that *contains* inline asm and not a function that is *referenced* by inline asm).
>
> > 
>
> > 
>
> > I'm not following. When you said:
>
> > 
>
> > > 1. "llvm.used" symbols (that are referred *from* inline assembly)
>
> > 
>
> > 
>
> > I assumed you meant the variable like @myvar which is on the llvm.used, and is referenced in the inline asm:
>
> > 
>
> > > @llvm.used = appending global [1 x i8*] [i8* @myvar], section "llvm.metadata"
>
> > 
>
> > 
>
> > ...
>
> > 
>
> > >   call void asm sideeffect "movzbl     myvar(%rip), ...
>
> > 
>
> > 
>
> > (In the example there is no function referenced by inline asm, just a variable.)
>
> > 
>
> > It is the llvm.used symbol myvar which cannot be correctly handled with a private alias, unless I am missing something. Maybe you could show what you are proposing using this example (i.e. the example in the patch augmented with an import to the additional module containing bar() shown above).
>
>
> Let say `myvar` was instead `myfunc` and was referenced from the inline asm. And then let say you are trying to import `myfunc` into another module and you need to promote it. You should be able to rename `myfunc()` to `myfunc.1234()` and create a local alias `myfunc = alias myfunc.1234()`.
>  Makes sense?


I'm not sure local functions are fundamentally different. Note if we import a local function we don't really need to promote it (we can use the imported copy). But let's say a reference to local function myfunc is imported from module1.o, it will need to be promoted/renamed. We could have a different local function myfunc that we import a reference to from module2.o, also requiring a promotion/rename. You can't create a single myfunc alias to both of them.


================
Comment at: include/llvm/IR/Module.h:752
@@ -750,1 +751,3 @@
 
+/// \brief Given "llvm.used" or "llvm.compiler.used" as a global name, collect
+/// the initializer elements of that global in Set and return the global itself.
----------------
joker.eph wrote:
> (Remove brief).
Will do.

================
Comment at: lib/Transforms/Utils/FunctionImportUtils.cpp:219
@@ +218,3 @@
+    if (!V->hasLocalLinkage())
+      continue;
+    // We would have blocked importing from this module by suppressing index
----------------
joker.eph wrote:
> I feel we should check `GlobalsToImport` here, the client has the ability to ask for promotion for specific symbols right? 
But as I mentioned earlier, the problem comes into play with local symbols that are not in the GlobalsToImport set (but are rather referenced by a function in the GlobalsToImport set, requiring the promotion).


http://reviews.llvm.org/D18986