llvm dev - Jul 2016 - [ThinLTO] assert(GS != DefinedGlobals.end()) failed in FunctionImport.cpp

Encountered “assert(GS != DefinedGlobals.end())” failure while running ThinLTO.
The assertion statement is in MustPreserveGV lambda function in
llvm::thinLTOInternalizeModule (lib/Transforms/IPO/FunctionImport.cpp).

It seems that the assertion fails because it fails to recover the "original
name" of the global value. ModuleSummaryIndex::getOriginalNameBeforePromote
attempts to get the original name by stripping .llvm.{HASH}, but what I observe
is that ".1" is still appended to the expected original name.

Then where this extra ".1" comes from? It is appended when the global
value is materialized. IRLinker::materialize function calls
IRLinker::linkGlobalValueProto function, and inside that function if DGV is
nullptr or ShouldLink is true then IRLinker::copyGlobalValueProto function is
called to create a global variable in the destination module that corresponds to
SGV. I found that newly created global variable has the extra ".1"
added to the name of the SGV.

When this happens? I don't have a complete understanding but I observed that
the same global value is materialized twice during the single invocation of
IRLinker::run, once with GlobalValueMaterializer and once with
LocalValueMaterializer. First, the global value

; Materializable
; Function Attrs: nounwind uwtable
define weak_odr void @foo(%1*) unnamed_addr #7 comdat($comdat1) align 2
personality i32 (...)* @__gxx_personality_v0 {}
(I renamed the function and comdat)

is materialized with GlobalValueMaterializer, (so the IRLinker::materialize
function is called with ForAlias == false), and the materialized value is

; Function Attrs: nounwind uwtable
declare void @foo(%"type1"*) unnamed_addr #2 align 2

Then, later, the same value is materialized again with LocalValueMaterializer
(so ForAlias == true for IRLinker::materialize), and the materialized value is

; Function Attrs: nounwind uwtable
define internal void @foo.1(%"type 0x7efb6ee89d80"*) unnamed_addr #2
comdat($comdat1) align 2 personality i32 (...)* @__gxx_personality_v0 !dbg
!12345 {
  // function definition
}
(here, “type 0x7efb6ee89d80” is not “type1” )

So for me it seems that we need a mechanism to retrieve the original name of the
global value even when it is renamed during the materialization. I submitted the
bug report as well (https://llvm.org/bugs/show_bug.cgi?id=28760).

Thanks,
Taewook
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160728/296d3c53/attachment.html>

Hi Taewook,



On Thu, Jul 28, 2016 at 4:38 PM, Taewook Oh via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Encountered “assert(GS != DefinedGlobals.end())” failure while running
> ThinLTO. The assertion statement is in MustPreserveGV lambda function in
> llvm::thinLTOInternalizeModule (lib/Transforms/IPO/FunctionImport.cpp).
>
>
>
> It seems that the assertion fails because it fails to recover the
> "original name" of the global value.
> ModuleSummaryIndex::getOriginalNameBeforePromote attempts to get the
> original name by stripping .llvm.{HASH}, but what I observe is that
".1" is
> still appended to the expected original name.
>
>
>
> Then where this extra ".1" comes from? It is appended when the
global
> value is materialized. IRLinker::materialize function calls
> IRLinker::linkGlobalValueProto function, and inside that function if DGV is
> nullptr or ShouldLink is true then IRLinker::copyGlobalValueProto function
> is called to create a global variable in the destination module that
> corresponds to SGV. I found that newly created global variable has the
> extra ".1" added to the name of the SGV.
>
>
>
> When this happens? I don't have a complete understanding but I observed
> that the same global value is materialized twice during the single
> invocation of IRLinker::run, once with GlobalValueMaterializer and once
> with LocalValueMaterializer. First, the global value
>
Yes, the IRLinker will append an integer if it encounters a naming
conflict. Normally this would happen in full LTO, but I guess is happening
here since we are linking twice due to the alias.

>
>
> ; Materializable
>
> ; Function Attrs: nounwind uwtable
>
> define weak_odr void @foo(%1*) unnamed_addr #7 comdat($comdat1) align 2
> personality i32 (...)* @__gxx_personality_v0 {}
>
> (I renamed the function and comdat)
>
>
>
> is materialized with GlobalValueMaterializer, (so the
> IRLinker::materialize function is called with ForAlias == false), and the
> materialized value is
>
>
>
> ; Function Attrs: nounwind uwtable
>
> declare void @foo(%"type1"*) unnamed_addr #2 align 2
>
>
>
> Then, later, the same value is materialized again with
> LocalValueMaterializer (so ForAlias == true for IRLinker::materialize), and
> the materialized value is
>
>
>
> ; Function Attrs: nounwind uwtable
>
> define internal void @foo.1(%"type 0x7efb6ee89d80"*) unnamed_addr
#2
> comdat($comdat1) align 2 personality i32 (...)* @__gxx_personality_v0 !dbg
> !12345 {
>
>   // function definition
>
> }
>
> (here, “type 0x7efb6ee89d80” is not “type1” )
>
>
>
> So for me it seems that we need a mechanism to retrieve the original name
> of the global value even when it is renamed during the materialization. I
> submitted the bug report as well (
> https://llvm.org/bugs/show_bug.cgi?id=28760).
>
Interestingly, we are already trying to handle a similar case in that
lambda (see the comment about weak values linked in as a local copy due to
an alias). However, we weren't anticipating the original weak value being
linked in as well, which is causing the rename.

Do you have a small reproducer? Is this with gold? I need to think about
the best way to handle this case. One way of course is to conservatively
return true if we can't find it in the map, but I don't love that since
we
may miss other cases that need to be handled.

Thanks,
Teresa
>
>
> Thanks,
>
> Taewook
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>

-- 
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/20160728/280e2b63/attachment-0001.html>

Hello Teresa,

Thank you for your reply. I’m trying to create a small repro but find it hard to
nail down because originally it is a big build. This happens with gold linker.

Thanks,
Taewook

From: Teresa Johnson <tejohnson at google.com>
Date: Thursday, July 28, 2016 at 5:08 PM
To: Taewook Oh <twoh at fb.com>
Cc: via llvm-dev <llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] [ThinLTO] assert(GS != DefinedGlobals.end()) failed in
FunctionImport.cpp

Hi Taewook,



On Thu, Jul 28, 2016 at 4:38 PM, Taewook Oh via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

Encountered “assert(GS != DefinedGlobals.end())” failure while running ThinLTO.
The assertion statement is in MustPreserveGV lambda function in
llvm::thinLTOInternalizeModule (lib/Transforms/IPO/FunctionImport.cpp).



It seems that the assertion fails because it fails to recover the "original
name" of the global value. ModuleSummaryIndex::getOriginalNameBeforePromote
attempts to get the original name by stripping .llvm.{HASH}, but what I observe
is that ".1" is still appended to the expected original name.



Then where this extra ".1" comes from? It is appended when the global
value is materialized. IRLinker::materialize function calls
IRLinker::linkGlobalValueProto function, and inside that function if DGV is
nullptr or ShouldLink is true then IRLinker::copyGlobalValueProto function is
called to create a global variable in the destination module that corresponds to
SGV. I found that newly created global variable has the extra ".1"
added to the name of the SGV.



When this happens? I don't have a complete understanding but I observed that
the same global value is materialized twice during the single invocation of
IRLinker::run, once with GlobalValueMaterializer and once with
LocalValueMaterializer. First, the global value

Yes, the IRLinker will append an integer if it encounters a naming conflict.
Normally this would happen in full LTO, but I guess is happening here since we
are linking twice due to the alias.




; Materializable

; Function Attrs: nounwind uwtable

define weak_odr void @foo(%1*) unnamed_addr #7 comdat($comdat1) align 2
personality i32 (...)* @__gxx_personality_v0 {}

(I renamed the function and comdat)



is materialized with GlobalValueMaterializer, (so the IRLinker::materialize
function is called with ForAlias == false), and the materialized value is



; Function Attrs: nounwind uwtable

declare void @foo(%"type1"*) unnamed_addr #2 align 2



Then, later, the same value is materialized again with LocalValueMaterializer
(so ForAlias == true for IRLinker::materialize), and the materialized value is



; Function Attrs: nounwind uwtable

define internal void @foo.1(%"type 0x7efb6ee89d80"*) unnamed_addr #2
comdat($comdat1) align 2 personality i32 (...)* @__gxx_personality_v0 !dbg
!12345 {

  // function definition

}

(here, “type 0x7efb6ee89d80” is not “type1” )



So for me it seems that we need a mechanism to retrieve the original name of the
global value even when it is renamed during the materialization. I submitted the
bug report as well
(https://llvm.org/bugs/show_bug.cgi?id=28760)<https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D28760-29&d=CwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=XtlNH01OW0mwOi0no2wur-HO6RY5szj-dgWaIcCki-k&s=9pGLGnxeOI3J3lvx9-ayiZJUImAefSzcGGJN5xo9_Kc&e=>.

Interestingly, we are already trying to handle a similar case in that lambda
(see the comment about weak values linked in as a local copy due to an alias).
However, we weren't anticipating the original weak value being linked in as
well, which is causing the rename.

Do you have a small reproducer? Is this with gold? I need to think about the
best way to handle this case. One way of course is to conservatively return true
if we can't find it in the map, but I don't love that since we may miss
other cases that need to be handled.

Thanks,
Teresa



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=XtlNH01OW0mwOi0no2wur-HO6RY5szj-dgWaIcCki-k&s=eA2ODuBrwjiWfuW-_sPfCVr1H774iHS5j89ydb7KY2E&e=>



--
Teresa Johnson |

 Software Engineer |

 tejohnson at google.com<mailto:tejohnson at google.com> |

 408-460-2413


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