[llvm] r274699 - ThinLTO: Add test cases for promote+internalize.

Tue Jul 25 20:51:50 PDT 2017

*nod* Removed those parts of the test in the change that's out for review
(D35875)

On Tue, Jul 25, 2017 at 8:19 PM Peter Collingbourne <peter at pcc.me.uk> wrote:

> I'd be in favour of that, especially if you are planning to remove the
> logic that makes linkonce aliases special.
>
> Peter
>
> On Tue, Jul 25, 2017 at 8:03 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> I'm not sure either of us are making much sense of what this test does,
>> or is intended to do - I think it's best to just remove it.
>>
>> On Tue, Jul 25, 2017 at 7:51 PM Peter Collingbourne <peter at pcc.me.uk>
>> wrote:
>>
>>> On Tue, Jul 25, 2017 at 5:52 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jul 25, 2017 at 5:49 PM David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>> On Tue, Jul 25, 2017 at 4:24 PM Peter Collingbourne <peter at pcc.me.uk>
>>>>> wrote:
>>>>>
>>>>>> On Tue, Jul 25, 2017 at 2:29 PM, David Blaikie <dblaikie at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hey Peter,
>>>>>>>
>>>>>>> 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:
>>>>>>>
>>>>>>> My change causes none of the aliasees to be imported (not even those
>>>>>>> linkonce_odr ones).
>>>>>>>
>>>>>>
>>>>>> 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:
>>>>>> http://lists.llvm.org/pipermail/llvm-dev/2017-January/109227.html).
>>>>>>
>>>>>
>>>>> 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.
>>>>>
>>>>>
>>>>>>
>>>>>> This causes -thinlto-action=internalize to fail ("-internalize will
>>>>>>> not perform without -exportedsymbol").
>>>>>>>
>>>>>>
>>>>>> Is it really failing because of that?
>>>>>> http://llvm-cs.pcc.me.uk/tools/llvm-lto/llvm-lto.cpp#618
>>>>>> Looks like it just prints a warning.
>>>>>>
>>>>>
>>>>> Yep, it prints a warning & then doesn't internalize anything/is a
>>>>> no-op, as per the warning.
>>>>>
>>>>
>>> That warning is misleading because the condition it tests for is not the
>>> same as the one that ThinLTOCodeGenerator uses.
>>> http://llvm-cs.pcc.me.uk/lib/LTO/ThinLTOCodeGenerator.cpp#763
>>> Right now we apply internalization despite the warning
>>> because ExportList.size() is equal to 3 at that point. I think what might
>>> be happening for you is that your change causes ExportList.size() to become
>>> 0, so we don't actually internalize anything. Presumably the 3 symbols are
>>> the linkonce aliases but I don't have time to confirm that.
>>>
>>>
>>>>>
>>>>>> 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?
>>>>>>>
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> 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))
>>>>>
>>>>>
>>>>>> (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?)
>>>>>>>
>>>>>>
>>>>>> 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).
>>>>>>
>>>>>
>>>>> Except there are exported symbols... - if there are no exported
>>>>> symbols then internalize doesn't run so it wouldn't show/demonstrate
>>>>> anything?
>>>>>
>>>>
>>> The symbols currently being exported are linkonce/weak so they
>>> presumably end up in ExportList regardless of what flags you pass in.
>>>
>>>>
>>>> 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?
>>>>
>>>
>>> That's certainly possible, although I suspect it is more likely an
>>> artifact of how llvm-lto works (which I find super confusing -- one of the
>>> reasons why I designed llvm-lto2 to use the same sequence of ThinLTO
>>> "passes" as production links).
>>>
>>> Peter
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>> 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.
>>>>>>
>>>>>> Peter
>>>>>>
>>>>>>
>>>>>>> Preciate any thoughts you have here,
>>>>>>>
>>>>>>> - Dave
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Jul 6, 2016 at 4:00 PM Peter Collingbourne via llvm-commits <
>>>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>> Author: pcc
>>>>>>>> Date: Wed Jul  6 17:53:02 2016
>>>>>>>> New Revision: 274699
>>>>>>>>
>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=274699&view=rev
>>>>>>>> Log:
>>>>>>>> ThinLTO: Add test cases for promote+internalize.
>>>>>>>>
>>>>>>>> This tests the effect of both promotion and internalization on a
>>>>>>>> module,
>>>>>>>> and helps show that D21883 is NFC wrt promotion+internalization.
>>>>>>>>
>>>>>>>> Differential Revision: http://reviews.llvm.org/D21915
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>>     llvm/trunk/test/ThinLTO/X86/alias_import.ll
>>>>>>>>     llvm/trunk/test/ThinLTO/X86/weak_resolution.ll
>>>>>>>>
>>>>>>>> Modified: llvm/trunk/test/ThinLTO/X86/alias_import.ll
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/alias_import.ll?rev=274699&r1=274698&r2=274699&view=diff
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- llvm/trunk/test/ThinLTO/X86/alias_import.ll (original)
>>>>>>>> +++ llvm/trunk/test/ThinLTO/X86/alias_import.ll Wed Jul  6 17:53:02
>>>>>>>> 2016
>>>>>>>> @@ -2,6 +2,7 @@
>>>>>>>>  ; RUN: opt -module-summary %p/Inputs/alias_import.ll -o %t2.bc
>>>>>>>>  ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc
>>>>>>>> %t2.bc
>>>>>>>>  ; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc
>>>>>>>> %t2.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE
>>>>>>>> +; 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
>>>>>>>>  ; RUN: llvm-lto -thinlto-action=import -thinlto-index %t.index.bc
>>>>>>>> %t1.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
>>>>>>>>
>>>>>>>>  ;
>>>>>>>> @@ -86,7 +87,45 @@
>>>>>>>>  ; IMPORT-DAG: declare void @weakfuncWeakODRAlias()
>>>>>>>>  ; IMPORT-DAG: declare void @weakfuncLinkonceODRAlias()
>>>>>>>>
>>>>>>>> -
>>>>>>>> +; Promotion + internalization should internalize all of these,
>>>>>>>> except for aliases of
>>>>>>>> +; linkonce_odr functions, because alias to linkonce_odr are the
>>>>>>>> only aliases we will
>>>>>>>> +; import (see selectCallee() in FunctionImport.cpp).
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @globalfuncAlias = internal alias void
>>>>>>>> (...), bitcast (void ()* @globalfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @globalfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @globalfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakODRAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @globalfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceODRAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @globalfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @internalfuncAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakODRAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceODRAlias =
>>>>>>>> internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void
>>>>>>>> (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncAlias = alias void
>>>>>>>> (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceAlias =
>>>>>>>> internal alias void (...), bitcast (void ()* @linkonceODRfunc to void
>>>>>>>> (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakODRAlias = weak_odr
>>>>>>>> alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceODRAlias =
>>>>>>>> weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void
>>>>>>>> (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakODRfuncAlias = internal alias void
>>>>>>>> (...), bitcast (void ()* @weakODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @weakODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakODRAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceODRAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkoncefuncAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @linkoncefunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @linkoncefunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakODRAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceODRAlias =
>>>>>>>> internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakfuncAlias = internal alias void
>>>>>>>> (...), bitcast (void ()* @weakfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @weakfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @weakfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakODRAlias = internal alias
>>>>>>>> void (...), bitcast (void ()* @weakfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceODRAlias = internal
>>>>>>>> alias void (...), bitcast (void ()* @weakfunc to void (...)*)
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: define internal void @globalfunc()
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: define internal void
>>>>>>>> @internalfunc.llvm.0()
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: define internal void @linkonceODRfunc()
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: define internal void @weakODRfunc()
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: define internal void @linkoncefunc()
>>>>>>>> +; PROMOTE-INTERNALIZE-DAG: define internal void @weakfunc()
>>>>>>>>
>>>>>>>>  define i32 @main() #0 {
>>>>>>>>  entry:
>>>>>>>>
>>>>>>>> Modified: llvm/trunk/test/ThinLTO/X86/weak_resolution.ll
>>>>>>>> URL:
>>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/weak_resolution.ll?rev=274699&r1=274698&r2=274699&view=diff
>>>>>>>>
>>>>>>>> ==============================================================================
>>>>>>>> --- llvm/trunk/test/ThinLTO/X86/weak_resolution.ll (original)
>>>>>>>> +++ llvm/trunk/test/ThinLTO/X86/weak_resolution.ll Wed Jul  6
>>>>>>>> 17:53:02 2016
>>>>>>>> @@ -7,6 +7,7 @@
>>>>>>>>  ; non-prevailing ODR are not kept when possible, but non-ODR
>>>>>>>> non-prevailing
>>>>>>>>  ; are not affected.
>>>>>>>>  ; RUN: llvm-lto -thinlto-action=promote %t.bc
>>>>>>>> -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s
>>>>>>>> --check-prefix=MOD1
>>>>>>>> +; 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
>>>>>>>>  ; RUN: llvm-lto -thinlto-action=promote %t2.bc
>>>>>>>> -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s
>>>>>>>> --check-prefix=MOD2
>>>>>>>>  ; When exported, we always preserve a linkonce
>>>>>>>>  ; RUN: llvm-lto -thinlto-action=promote %t.bc
>>>>>>>> -thinlto-index=%t3.bc -o - --exported-symbol=linkonceodrfuncInSingleModule
>>>>>>>> | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTED
>>>>>>>> @@ -47,6 +48,7 @@ entry:
>>>>>>>>    ret void
>>>>>>>>  }
>>>>>>>>  ; MOD1: define weak void @linkoncefunc()
>>>>>>>> +; MOD1-INT: define weak void @linkoncefunc()
>>>>>>>>  ; MOD2: define linkonce void @linkoncefunc()
>>>>>>>>  define linkonce void @linkoncefunc() #0 {
>>>>>>>>  entry:
>>>>>>>> @@ -66,6 +68,7 @@ entry:
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  ; MOD1: define linkonce_odr void @linkonceodrfuncInSingleModule()
>>>>>>>> +; MOD1-INT: define internal void @linkonceodrfuncInSingleModule()
>>>>>>>>  ; EXPORTED: define weak_odr void @linkonceodrfuncInSingleModule()
>>>>>>>>  define linkonce_odr void @linkonceodrfuncInSingleModule() #0 {
>>>>>>>>  entry:
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> llvm-commits mailing list
>>>>>>>> llvm-commits at lists.llvm.org
>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> --
>>>>>> Peter
>>>>>>
>>>>>
>>>
>>>
>>> --
>>> --
>>> Peter
>>>
>>
>
>
> --
> --
> Peter
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170726/5262f345/attachment.html>