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

[Mehdi AMINI via llvm-commits]

joker.eph accepted this revision.
joker.eph added a comment.
This revision is now accepted and ready to land.

> I looked at this functionality and it doesn't handle this type of inline assembly. Confirmed by walking through the similar handling in IRObjectFile on which CollectAsmUndefinedRefs was modeled. It only handles "Module-level assembly" as described here: http://llvm.org/docs/LangRef.html#module-level-inline-assembly.


OK

> So if we instead want to use a finer grained approach here, I think we need to conservatively reject importing for any function containing inline assembly if there is any llvm.used.


If this is what I am after.

> [...]




> > > The big hammer seems reasonable to me on the very short term.

> 

> > 

> 

> 

> This is my preference given what I think needs to be done to get this to work on a finer grained level vs how uncommon I think the inline assembly is in practice. It can be refined if we find important cases from modules with inline assembly.


Sure.
I just noticed that I didn't approve the rev, I thought I did yesterday *before* all this discussion. To be clear I was not saying we need the fine grain now, I was just clarifying that it is "big hammer" and what can be done later.

> > > Preventing the import of these function can be done using a "flag" in the summary (part of the flags we already talked about, i.e. optnone, optsize, etc.).

> 

> > 

> 

> 

> To get the finer-grained solution to work, we need to add this indication to the summary, and prevent importing of those functions. Then on the exporting side we need to add an alias to the old name for anything in the llvm.used set (even if we don't export any functions with inline assembly, a reference in another function could be exported, requiring the promotion).


Yes, this is what I had in mind.

What needs to be added also is that there is an issue to import *into* a Module with llvm-used in some cases because of name collision:

Module A:  internal foo()    (<- referenced in llvm.used
Module B: void foo()

And now you want to import foo() from B into A?
Another case to forbid...


http://reviews.llvm.org/D18986