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

On Thu, Jun 4, 2015 at 5:02 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>
>> On 2015 Jun 4, at 16:51, Reid Kleckner <rnk at google.com> wrote:
>>
>> 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?
>
> SGTM.
Agreed. Although I assume you mean invoke the new pass under a
ThinLTO-only option so that avail extern are not dropped in the
compile pass before the LTO link?

Teresa

-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413

On Thu, Jun 4, 2015 at 5:17 PM, Teresa Johnson <tejohnson at google.com>
wrote:
>
> Agreed. Although I assume you mean invoke the new pass under a
> ThinLTO-only option so that avail extern are not dropped in the
> compile pass before the LTO link?

No, this pass would actually be an improvement to the standard -O2
pipeline. The special case is the regular LTO pass pipeline, which wants to
run GlobalDCE but doesn't want to drop available_externally function
definitions until after linker-stage inlining.

Consider this test case:

declare void @blah()
define i32 @main() {
  call void @foo()
  ret i32 0
}
define available_externally void @foo() noinline {
  call void @bar()
  ret void
}
define linkonce_odr void @bar() noinline {
  call void @blah()
  ret void
}

If I run opt -O2 on this and feed it to llc, it actually generates code for
bar, even though there are no actual references to bar in the final code:

main:                                   # @main
        pushq   %rax
        callq   foo
        xorl    %eax, %eax
        popq    %rdx
        retq

bar:                                    # @bar
        jmp     blah                    # TAILCALL

This corner case happens to come up organically with dllimported classes,
which is why I happened to think of it. :) I'm happy with a flag to
globaldce for LTO and the original patch, honestly.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150604/4a59b3f6/attachment.html>

On Thu, Jun 4, 2015 at 5:33 PM, Reid Kleckner <rnk at google.com>
wrote:
> On Thu, Jun 4, 2015 at 5:17 PM, Teresa Johnson <tejohnson at
google.com> wrote:
>>
>> Agreed. Although I assume you mean invoke the new pass under a
>> ThinLTO-only option so that avail extern are not dropped in the
>> compile pass before the LTO link?
>
>
> No, this pass would actually be an improvement to the standard -O2
pipeline.
> The special case is the regular LTO pass pipeline, which wants to run
> GlobalDCE but doesn't want to drop available_externally function
definitions
> until after linker-stage inlining.
Ok. It looks to me like the LTO compile step with -O2 -flto -c uses
this same -O2 optimization pipeline as without LTO. Clang communicates
the fact that we are doing an LTO compile to llvm via the
-emit-llvm-bc flag, which just tells it to emit bitcode and exit
before codegen. So I would either need to key off of that or setup a
new flag to indicate to llvm that we are doing an LTO -c compile. Or
is there some other way that I am missing?

Incidentally, we'll also want to skip this new pass and keep any
referenced avail extern functions in the ThinLTO -c compile step for
the same reasons (and there are no imported functions yet at that
point).

Teresa
>
> Consider this test case:
>
> declare void @blah()
> define i32 @main() {
>   call void @foo()
>   ret i32 0
> }
> define available_externally void @foo() noinline {
>   call void @bar()
>   ret void
> }
> define linkonce_odr void @bar() noinline {
>   call void @blah()
>   ret void
> }
>
> If I run opt -O2 on this and feed it to llc, it actually generates code for
> bar, even though there are no actual references to bar in the final code:
>
> main:                                   # @main
>         pushq   %rax
>         callq   foo
>         xorl    %eax, %eax
>         popq    %rdx
>         retq
>
> bar:                                    # @bar
>         jmp     blah                    # TAILCALL
>
> This corner case happens to come up organically with dllimported classes,
> which is why I happened to think of it. :) I'm happy with a flag to
> globaldce for LTO and the original patch, honestly.


-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413