llvm dev - Jul 2016 - [ThinLTO] Using two different IRMovers for the same composite module? (related to PR28180)

Yes, I have the repro, though I can’t publish it externally. It would be great
if you can upstream the patch so I can try it. Thank you for your explanation as
well!

-- Taewook

From: <mehdi.amini at apple.com> on behalf of Mehdi Amini <mehdi.amini
at apple.com>
Date: Friday, July 22, 2016 at 2:16 PM
To: Taewook Oh <twoh at fb.com>
Cc: via llvm-dev <llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] [ThinLTO] Using two different IRMovers for the same
composite module? (related to PR28180)


On Jul 22, 2016, at 1:50 PM, Taewook Oh via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

Hello,

While trying ThinLTO, I ran into an assertion failure in IRMover: 
https://llvm.org/bugs/show_bug.cgi?id=28180<https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D28180&d=CwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=YPo1XVjbxI5dqJXOdMZRPzHnL3CBAEk6IR74Edlb3Jg&s=C4gDkGqx2eaddJPKkXGtaZqrAn7mTi2w675GAwvY0G8&e=>.

Great, we encountered this bug last month and I have an fix internally but
wasn’t sure how to reproduce (I didn’t have any source with internal bug
report), so I haven’t upstream the patch yet.
Do you have the repro?



I found that the assertion failure is happening because IRMover tries to map the
metadata that already mapped in the destination module, and it seems that this
happens because two different IRMovers are used for the same destination (or
composite) module.

It is not clear to me how using two different IRMovers is the issue: as you
mentioned, the assertions is encountered when the metadata is already in the
map, a new IRMovers would have a new fresh map.

My debugging of this issue lead me to the new "ODR type uniquing” feature
in the context as the culprit. In this mode, when multiple modules are loaded in
the context the composite type metadata are uniqued by id. It means that the
same composite type (same as same pointer in memory) can be reached from two
modules (here source and destination). So the mapper may reach a metadata in the
source module and try to map it to the destination module while it is already
there (but not in the map).

This happens only in ThinLTO and not in LTO because LTO starts with an empty
module, so when you move the first module into the "merged module”, the map
gets initialized. In ThinLTO the mover starts with the destination module not
empty.


—
Mehdi



During LTO, an IRMover is created in thinLTOBackendTask
function(tools/gold/gold-plugin.cpp). linkInModule function, which is called by
thinLTOBackendTask, calls the ‘move’ function of this IRMover. The other IRMover
is created when “TheLinker” is created in FunctionImporter::importFunctions
(lib/Transforms/IPO/FunctionImport.cpp). thinLTOBackendTask invokes
FunctionImporeter::importFunctions as well, with a call chain of
thinLTOBackendTask -->CodeGen::runAll (tools/gold/gold-plugin.cpp) -->
CodeGen::runLTOPasses ((tools/gold/gold-plugin.cpp)-->
FunctionImporter::importFunctions.

As these two IRMovers share the same destination module, when the second IRMover
tries to map the metadata already mapped by the first IRMover, it eventually
results the assertion failure. It seems that IRMover maintains SharedMDs to keep
the metadata mapping record across the multiple calls of its move function, but
that doesn’t help between two separate IRMovers.

What would be the right fix for this? Please let me know if I misunderstand
something.

Thanks,
Taewook


_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=CwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=YPo1XVjbxI5dqJXOdMZRPzHnL3CBAEk6IR74Edlb3Jg&s=-JVet3M9T-EiSCCak1-U757TIzbzbTT4zxkz4xbvlBs&e=>

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160722/19f64455/attachment.html>

On Fri, Jul 22, 2016 at 2:34 PM, Taewook Oh via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> Yes, I have the repro, though I can’t publish it externally. It would be
> great if you can upstream the patch so I can try it. Thank you for your
> explanation as well!
There's a reproducer attached (obtained via lld --reproduce option).
If that doesn't work, you can checkout mozjs and try to reduce from
there.
It happens while doing an LTO build with lld.
I have a fix for that (Mehdi has one as well, apparently) in my local
tree, but I don't have time to reduce. If you can take care of that,
chances are that an upstream fix will be committed shortly after.

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare

On Fri, Jul 22, 2016 at 2:44 PM, Davide Italiano <davide at freebsd.org>
wrote:
> On Fri, Jul 22, 2016 at 2:34 PM, Taewook Oh via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> Yes, I have the repro, though I can’t publish it externally. It would
be
>> great if you can upstream the patch so I can try it. Thank you for your
>> explanation as well!
>
> There's a reproducer attached (obtained via lld --reproduce option).
> If that doesn't work, you can checkout mozjs and try to reduce from
> there.
> It happens while doing an LTO build with lld.
> I have a fix for that (Mehdi has one as well, apparently) in my local
> tree, but I don't have time to reduce. If you can take care of that,
> chances are that an upstream fix will be committed shortly after.
>
Just to clarify, I'm talking about the issue in PR28180. I can't
comment about the ThinLTO one, sorry.

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare

Patch:



> On Jul 22, 2016, at 2:34 PM, Taewook Oh <twoh at fb.com> wrote:
> 
> Yes, I have the repro, though I can’t publish it externally. It would be
great if you can upstream the patch so I can try it. Thank you for your
explanation as well!
>  
> -- Taewook
>  
> From: <mehdi.amini at apple.com> on behalf of Mehdi Amini
<mehdi.amini at apple.com>
> Date: Friday, July 22, 2016 at 2:16 PM
> To: Taewook Oh <twoh at fb.com>
> Cc: via llvm-dev <llvm-dev at lists.llvm.org>
> Subject: Re: [llvm-dev] [ThinLTO] Using two different IRMovers for the same
composite module? (related to PR28180)
>  
>  
> On Jul 22, 2016, at 1:50 PM, Taewook Oh via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>  
> Hello,
>  
> While trying ThinLTO, I ran into an assertion failure in IRMover: 
https://llvm.org/bugs/show_bug.cgi?id=28180
<https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D28180&d=CwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=YPo1XVjbxI5dqJXOdMZRPzHnL3CBAEk6IR74Edlb3Jg&s=C4gDkGqx2eaddJPKkXGtaZqrAn7mTi2w675GAwvY0G8&e=>.
>  
> Great, we encountered this bug last month and I have an fix internally but
wasn’t sure how to reproduce (I didn’t have any source with internal bug
report), so I haven’t upstream the patch yet.
> Do you have the repro?
>  
> 
> 
> I found that the assertion failure is happening because IRMover tries to
map the metadata that already mapped in the destination module, and it seems
that this happens because two different IRMovers are used for the same
destination (or composite) module.
>  
> It is not clear to me how using two different IRMovers is the issue: as you
mentioned, the assertions is encountered when the metadata is already in the
map, a new IRMovers would have a new fresh map.
>  
> My debugging of this issue lead me to the new "ODR type uniquing”
feature in the context as the culprit. In this mode, when multiple modules are
loaded in the context the composite type metadata are uniqued by id. It means
that the same composite type (same as same pointer in memory) can be reached
from two modules (here source and destination). So the mapper may reach a
metadata in the source module and try to map it to the destination module while
it is already there (but not in the map).
>  
> This happens only in ThinLTO and not in LTO because LTO starts with an
empty module, so when you move the first module into the "merged module”,
the map gets initialized. In ThinLTO the mover starts with the destination
module not empty.
>  
>  
> — 
> Mehdi
>  
>  
>  
> During LTO, an IRMover is created in thinLTOBackendTask
function(tools/gold/gold-plugin.cpp). linkInModule function, which is called by
thinLTOBackendTask, calls the ‘move’ function of this IRMover. The other IRMover
is created when “TheLinker” is created in FunctionImporter::importFunctions
(lib/Transforms/IPO/FunctionImport.cpp). thinLTOBackendTask invokes
FunctionImporeter::importFunctions as well, with a call chain of
thinLTOBackendTask àCodeGen::runAll (tools/gold/gold-plugin.cpp) à
CodeGen::runLTOPasses ((tools/gold/gold-plugin.cpp)à
FunctionImporter::importFunctions.
>  
> As these two IRMovers share the same destination module, when the second
IRMover tries to map the metadata already mapped by the first IRMover, it
eventually results the assertion failure. It seems that IRMover maintains
SharedMDs to keep the metadata mapping record across the multiple calls of its
move function, but that doesn’t help between two separate IRMovers.
>  
> What would be the right fix for this? Please let me know if I misunderstand
something.
>  
> Thanks,
> Taewook
>  
>  
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=CwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=YPo1XVjbxI5dqJXOdMZRPzHnL3CBAEk6IR74Edlb3Jg&s=-JVet3M9T-EiSCCak1-U757TIzbzbTT4zxkz4xbvlBs&e=>
>  
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160722/f1bf412f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-ThinLTO-crash-with-debug-info.patch
Type: application/octet-stream
Size: 32790 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160722/f1bf412f/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160722/f1bf412f/attachment-0001.html>