llvm dev - Dec 2016 - ThinLTO promotion is ending up with "invalid" IR before IR-Linking

Hey,

As I’m playing with Metadata lazy-loading, I added a verifier right before
running the IRLinker in FunctionImport.cpp, and it does not pass (on current
trunk) in a few cases. One that I looked at ended up with aliases pointing to
available_externally functions for instance.

I’m hesitant about breaking the IR verifier like that before calling the
IRLinker. The alternative I can see now would be:
- to perform any handling that requires breaking the verifier as part of the
IRLinker process,
- or to perform the verifier-breaking changes that we do as a pre-process step
to the IRLinker as a post-process step on the linked module.

Any thoughts on this? 

— 
Mehdi

On Thu, Dec 22, 2016 at 8:55 PM, Mehdi Amini <mehdi.amini at apple.com>
wrote:
> Hey,
>
> As I’m playing with Metadata lazy-loading, I added a verifier right before
> running the IRLinker in FunctionImport.cpp, and it does not pass (on
> current trunk) in a few cases. One that I looked at ended up with aliases
> pointing to available_externally functions for instance.
>
How do these look after IRLinking (in the dest module)? I looked at the
logic in renameModuleForThinLTO and all the conversions to
available_externally are predicated on it being in the provided
GlobalsToImport set. But in FunctionImport.cpp selectCallee() we
specifically prevent importing of aliases that would result in the aliasee
becoming available_externally. Presumably the resulting IRLinked dest
module looks legit, otherwise we would have later verifier failures.

> I’m hesitant about breaking the IR verifier like that before calling the
> IRLinker. The alternative I can see now would be:
> - to perform any handling that requires breaking the verifier as part of
> the IRLinker process,
> - or to perform the verifier-breaking changes that we do as a pre-process
> step to the IRLinker as a post-process step on the linked module.
>
Wouldn't this second option result in verification errors on the resulting
dest module?

Teresa

>
> Any thoughts on this?
>
> —
> Mehdi
>
>

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161223/13c7d9f0/attachment.html>

> On Dec 23, 2016, at 9:34 AM, Teresa Johnson <tejohnson at google.com>
wrote:
> 
> 
> 
> On Thu, Dec 22, 2016 at 8:55 PM, Mehdi Amini <mehdi.amini at apple.com
<mailto:mehdi.amini at apple.com>> wrote:
> Hey,
> 
> As I’m playing with Metadata lazy-loading, I added a verifier right before
running the IRLinker in FunctionImport.cpp, and it does not pass (on current
trunk) in a few cases. One that I looked at ended up with aliases pointing to
available_externally functions for instance.
> 
> How do these look after IRLinking (in the dest module)? I looked at the
logic in renameModuleForThinLTO and all the conversions to available_externally
are predicated on it being in the provided GlobalsToImport set. But in
FunctionImport.cpp selectCallee() we specifically prevent importing of aliases
that would result in the aliasee becoming available_externally. Presumably the
resulting IRLinked dest module looks legit, otherwise we would have later
verifier failures.
So the source module is:

@weakalias = weak alias void (...), bitcast (void ()* @globalfunc1 to void
(...)*)
define void @globalfunc1() #0 {
entry:
  ret void
}

But we turn globalfunc1 into available_externally in renameModuleForThinLTO(),
which make the alias invalid.

We don’t import the IR so the destination module is OK.
36

> 
> 
> I’m hesitant about breaking the IR verifier like that before calling the
IRLinker. The alternative I can see now would be:
> - to perform any handling that requires breaking the verifier as part of
the IRLinker process,
> - or to perform the verifier-breaking changes that we do as a pre-process
step to the IRLinker as a post-process step on the linked module.
> 
> Wouldn't this second option result in verification errors on the
resulting dest module?
Not sure why? Since currently the destination module is valid, the post-process
should be OK as well?

— 
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161223/87abc32b/attachment.html>