llvm dev - Jun 2015 - [LLVMdev] Removing AvailableExternal values in GlobalDCE (was Re: RFC: ThinLTO Impementation Plan)

On Thu, Jun 4, 2015 at 11:27 AM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
> Since the compiler is always free to delete available_externally
> functions, I think you could just add a pass to the -flto=thin pipeline
> that deletes all of them (referenced or not) -- it's just a single loop
> through all the functions deleting the bodies of those with the right
> linkage.  I imagine there are other pass pipelines that might want to do
> something similar.  I don't really like having GlobalDCE delete them
> (even behind a flag) because they aren't actually dead, and I think a
> separate pass makes it easier to test and all that.  (I haven't
actually
> worked much with pass pipelines, though, so there's a chance I'm on
my
> own here?)
>
It's possible to get into situations where available_externally functions
or globals (dllimported vftable) reference linkonce_odr functions (virtual
destructor thunks), so a single pass to drop available_externally function
bodies isn't as strong as adding a flag to GlobalDCE.

Personally, I think the right approach is to add a bool to
createGlobalDCEPass defaulting to true named something like
IsAfterInlining. In most standard pass pipelines, GlobalDCE runs after
inlining for obvious reasons, so the default makes sense. The special case
is the LTO pass pipeline, where the code will be reloaded and reoptimized
later. IMO it's natural to put the customization there.

You make an interesting point about applying different thresholds
to
> imported functions, but is there any reason not to change all
> available_externally functions in the some way?  Moreover, it feels like
> thin-LTO's imported functions are a new source of functions with
> available_externally linkage, but otherwise they aren't interestingly
> different.

Right, ThinLTO functions and C99 inline functions aren't interestingly
different. Having GlobalDCE drop bodies of available_externally functions
in the standard -O2 pipeline avoids wasting time running IR passes like
CodeGenPrepare on them.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150604/36d4c5f2/attachment.html>

> On 2015 Jun 4, at 14:37, Reid Kleckner <rnk at google.com> wrote:
> 
> On Thu, Jun 4, 2015 at 11:27 AM, Duncan P. N. Exon Smith <dexonsmith at
apple.com> wrote:
> Since the compiler is always free to delete available_externally
> functions, I think you could just add a pass to the -flto=thin pipeline
> that deletes all of them (referenced or not) -- it's just a single loop
> through all the functions deleting the bodies of those with the right
> linkage.  I imagine there are other pass pipelines that might want to do
> something similar.  I don't really like having GlobalDCE delete them
> (even behind a flag) because they aren't actually dead, and I think a
> separate pass makes it easier to test and all that.  (I haven't
actually
> worked much with pass pipelines, though, so there's a chance I'm on
my
> own here?)
> 
> It's possible to get into situations where available_externally
functions or globals (dllimported vftable) reference linkonce_odr functions
(virtual destructor thunks), so a single pass to drop available_externally
function bodies isn't as strong as adding a flag to GlobalDCE.
Unless I'm missing something, running the pass to drop
available_externally function bodies just before running GlobalDCE
is equivalent to incorporating it into GlobalDCE.
> 
> Personally, I think the right approach is to add a bool to
createGlobalDCEPass defaulting to true named something like IsAfterInlining. In
most standard pass pipelines, GlobalDCE runs after inlining for obvious reasons,
so the default makes sense. The special case is the LTO pass pipeline, where the
code will be reloaded and reoptimized later. IMO it's natural to put the
customization there.
That's a good point.  But I still like these as separate passes.

On Thu, Jun 4, 2015 at 2:37 PM, Reid Kleckner <rnk at google.com> wrote:
> On Thu, Jun 4, 2015 at 11:27 AM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>> Since the compiler is always free to delete available_externally
>> functions, I think you could just add a pass to the -flto=thin pipeline
>> that deletes all of them (referenced or not) -- it's just a single
loop
>> through all the functions deleting the bodies of those with the right
>> linkage.  I imagine there are other pass pipelines that might want to
do
>> something similar.  I don't really like having GlobalDCE delete
them
>> (even behind a flag) because they aren't actually dead, and I think
a
>> separate pass makes it easier to test and all that.  (I haven't
actually
>> worked much with pass pipelines, though, so there's a chance
I'm on my
>> own here?)
>>
>
> It's possible to get into situations where available_externally
functions
> or globals (dllimported vftable) reference linkonce_odr functions (virtual
> destructor thunks), so a single pass to drop available_externally function
> bodies isn't as strong as adding a flag to GlobalDCE.
>
> Personally, I think the right approach is to add a bool to
> createGlobalDCEPass defaulting to true named something like
> IsAfterInlining. In most standard pass pipelines, GlobalDCE runs after
> inlining for obvious reasons, so the default makes sense. The special case
> is the LTO pass pipeline, where the code will be reloaded and reoptimized
> later. IMO it's natural to put the customization there.
>
>
+1. That is what I was going to suggest too. The flag can be more general
to indicate bodies are needed for referenced functions. For instance if
there are cloning phase after inlining, then DCE before the cloning still
needs to keep the bodies.



You make an interesting point about applying different thresholds
to
>> imported functions, but is there any reason not to change all
>> available_externally functions in the some way?  Moreover, it feels
like
>> thin-LTO's imported functions are a new source of functions with
>> available_externally linkage, but otherwise they aren't
interestingly
>> different.
>
>
> Right, ThinLTO functions and C99 inline functions aren't interestingly
> different
>
I think they are more like GNU extern inline functions.

> . Having GlobalDCE drop bodies of available_externally functions in the
> standard -O2 pipeline avoids wasting time running IR passes like
> CodeGenPrepare on them.
>
yes.

thanks,

David
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150604/fbdcf52f/attachment.html>

On Thu, Jun 4, 2015 at 3:58 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > Personally, I think the right approach is to add a bool to
> createGlobalDCEPass defaulting to true named something like
> IsAfterInlining. In most standard pass pipelines, GlobalDCE runs after
> inlining for obvious reasons, so the default makes sense. The special case
> is the LTO pass pipeline, where the code will be reloaded and reoptimized
> later. IMO it's natural to put the customization there.
>
> That's a good point.  But I still like these as separate passes.

Hm, yeah, that's pretty sane. So, add a pass to downgrade
available_externally global definitions to declarations with external
linkage, add it to the standard -O2 pass pipeline, ensure that it's not
part of LTO, and then call it done?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150604/282219a4/attachment.html>