<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 25, 2017 at 5:49 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Jul 25, 2017 at 4:24 PM Peter Collingbourne <<a href="mailto:peter@pcc.me.uk" target="_blank">peter@pcc.me.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>On Tue, Jul 25, 2017 at 2:29 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hey Peter,<br><br>Bit of thread necromancy here - but I'm trying to change some things wrt alias+linkonce_odr (going to make it behave like the alias+external case, for simplicity, less duplication, and simplified debug info) & this test is failing:<br><br>My change causes none of the aliasees to be imported (not even those linkonce_odr ones).</div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Is that a good idea? I would have thought that we'd want to go in the opposite direction and try to import more functions that use aliases. I believe that is blocked on support for available_externally aliases though (this thread has the most recent discussion I can find on that topic: <a href="http://lists.llvm.org/pipermail/llvm-dev/2017-January/109227.html" target="_blank">http://lists.llvm.org/pipermail/llvm-dev/2017-January/109227.html</a>).</div></div></div></div></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>Right, ultimately that's the right direction - this functionality though, is a dead-end, it's not a stepping stone towards that outcome - and it gets in the way (specifically it gets in the way of debug info, but also means thinlto backend objects are larger than they need to be - by having linkonce functions when they could be relying on the linkonce-promoted-to-weak definition) without any specific reason to believe the benefit (optimizing across aliases of linkonce_odr functions) is significant.<br> </div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">This causes -thinlto-action=internalize to fail ("-internalize will not perform without -exportedsymbol").<br></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Is it really failing because of that?</div><div><a href="http://llvm-cs.pcc.me.uk/tools/llvm-lto/llvm-lto.cpp#618" target="_blank">http://llvm-cs.pcc.me.uk/tools/llvm-lto/llvm-lto.cpp#618</a></div><div>Looks like it just prints a warning.</div></div></div></div></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>Yep, it prints a warning & then doesn't internalize anything/is a no-op, as per the warning.<br> </div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">It looks like if I fix this by passing -exported-symbol=main, nothing is internalized (all the aliases are called from main) so it may defeat the purpose of this test?<br></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Right, if we export main then that would prevent us from internalizing anything across modules that is referenced by main, as we don't currently support cross-module internalization.</div></div></div></div></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>Not sure I follow that - it seemed to work (in the sense that saying main was exported, nothing was internalized - which seems like the correct answer to me, rather than "unsupported" (though unsupported could mean it does that but breaks elsewhere - fair enough))<br> </div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">(I don't really understand how this was working/what it was doing before, though: What kept the aliasees alive if not the calls from main? Maybe a record in the index saying "these are being used elsewhere" - but why isn't there a similar record for the aliases that would've stopped them from being internalized in the first place?)<br></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I don't recall for sure but I believe that the point of that part of the test was to show that if no symbols are exported, the linkage changes applied by promote are unimportant because they are wiped out by a later internalize pass (which would internalize pretty much everything because nothing is exported).<br></div></div></div></div></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>Except there are exported symbols... - if there are no exported symbols then internalize doesn't run so it wouldn't show/demonstrate anything?<br></div></div></div></blockquote><div><br>aside: It looked like the behavior was incorrect anyway: It internalized the aliases which were actually called cross-module, but it /didn't/ internalize the things that were cross-modular imported... - it looks like it did exactly the opposite of what would be desired?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div>So I think the right change to make in that part of the test (if we wanted to go with your proposed change) would be to test that the remaining declarations are internal.<br></div><div><br></div><div>Peter</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br>Preciate any thoughts you have here,<br><br>- Dave<div><div class="m_-6799572545276431315m_-8881266879760994289gmail-m_-7006284203021941902gmail-h5"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 6, 2016 at 4:00 PM Peter Collingbourne via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: pcc<br>
Date: Wed Jul 6 17:53:02 2016<br>
New Revision: 274699<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=274699&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=274699&view=rev</a><br>
Log:<br>
ThinLTO: Add test cases for promote+internalize.<br>
<br>
This tests the effect of both promotion and internalization on a module,<br>
and helps show that D21883 is NFC wrt promotion+internalization.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D21915" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21915</a><br>
<br>
Modified:<br>
llvm/trunk/test/ThinLTO/X86/alias_import.ll<br>
llvm/trunk/test/ThinLTO/X86/weak_resolution.ll<br>
<br>
Modified: llvm/trunk/test/ThinLTO/X86/alias_import.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/alias_import.ll?rev=274699&r1=274698&r2=274699&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/alias_import.ll?rev=274699&r1=274698&r2=274699&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/ThinLTO/X86/alias_import.ll (original)<br>
+++ llvm/trunk/test/ThinLTO/X86/alias_import.ll Wed Jul 6 17:53:02 2016<br>
@@ -2,6 +2,7 @@<br>
; RUN: opt -module-summary %p/Inputs/alias_import.ll -o %t2.bc<br>
; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc<br>
; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE<br>
+; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-lto -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE-INTERNALIZE<br>
; RUN: llvm-lto -thinlto-action=import -thinlto-index %t.index.bc %t1.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT<br>
<br>
;<br>
@@ -86,7 +87,45 @@<br>
; IMPORT-DAG: declare void @weakfuncWeakODRAlias()<br>
; IMPORT-DAG: declare void @weakfuncLinkonceODRAlias()<br>
<br>
-<br>
+; Promotion + internalization should internalize all of these, except for aliases of<br>
+; linkonce_odr functions, because alias to linkonce_odr are the only aliases we will<br>
+; import (see selectCallee() in FunctionImport.cpp).<br>
+; PROMOTE-INTERNALIZE-DAG: @globalfuncAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @internalfuncAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakODRAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakfuncAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)<br>
+; PROMOTE-INTERNALIZE-DAG: define internal void @globalfunc()<br>
+; PROMOTE-INTERNALIZE-DAG: define internal void @internalfunc.llvm.0()<br>
+; PROMOTE-INTERNALIZE-DAG: define internal void @linkonceODRfunc()<br>
+; PROMOTE-INTERNALIZE-DAG: define internal void @weakODRfunc()<br>
+; PROMOTE-INTERNALIZE-DAG: define internal void @linkoncefunc()<br>
+; PROMOTE-INTERNALIZE-DAG: define internal void @weakfunc()<br>
<br>
define i32 @main() #0 {<br>
entry:<br>
<br>
Modified: llvm/trunk/test/ThinLTO/X86/weak_resolution.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/weak_resolution.ll?rev=274699&r1=274698&r2=274699&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/weak_resolution.ll?rev=274699&r1=274698&r2=274699&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/ThinLTO/X86/weak_resolution.ll (original)<br>
+++ llvm/trunk/test/ThinLTO/X86/weak_resolution.ll Wed Jul 6 17:53:02 2016<br>
@@ -7,6 +7,7 @@<br>
; non-prevailing ODR are not kept when possible, but non-ODR non-prevailing<br>
; are not affected.<br>
; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD1<br>
+; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -exported-symbol=linkoncefunc -o - | llvm-lto -thinlto-action=internalize -thinlto-module-id=%t.bc - -thinlto-index=%t3.bc -exported-symbol=linkoncefunc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD1-INT<br>
; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD2<br>
; When exported, we always preserve a linkonce<br>
; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - --exported-symbol=linkonceodrfuncInSingleModule | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTED<br>
@@ -47,6 +48,7 @@ entry:<br>
ret void<br>
}<br>
; MOD1: define weak void @linkoncefunc()<br>
+; MOD1-INT: define weak void @linkoncefunc()<br>
; MOD2: define linkonce void @linkoncefunc()<br>
define linkonce void @linkoncefunc() #0 {<br>
entry:<br>
@@ -66,6 +68,7 @@ entry:<br>
}<br>
<br>
; MOD1: define linkonce_odr void @linkonceodrfuncInSingleModule()<br>
+; MOD1-INT: define internal void @linkonceodrfuncInSingleModule()<br>
; EXPORTED: define weak_odr void @linkonceodrfuncInSingleModule()<br>
define linkonce_odr void @linkonceodrfuncInSingleModule() #0 {<br>
entry:<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div></div>
</blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div class="m_-6799572545276431315m_-8881266879760994289gmail-m_-7006284203021941902gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</div></div></blockquote></div></div></blockquote></div></div>