[PATCH] D27722: [GVNHoist] Move GVNHoist to function simplification part of pipeline.

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Geoff Berry" <gberry at codeaurora.org>
> To: "Hal Finkel" <hfinkel at anl.gov>, reviews+D27722+public+2c6cdd0c6150e26d at reviews.llvm.org
> Cc: "amara emerson" <amara.emerson at arm.com>, "mehdi amini" <mehdi.amini at apple.com>, mcrosier at codeaurora.org,
> llvm-commits at lists.llvm.org, dberlin at dberlin.org, hiraditya at msn.com, sebpop at gmail.com
> Sent: Tuesday, December 13, 2016 3:28:59 PM
> Subject: Re: [PATCH] D27722: [GVNHoist] Move GVNHoist to function simplification part of pipeline.
> 
> 
> 
> On 12/13/2016 2:59 PM, Hal Finkel wrote:
> > ----- Original Message -----
> >> From: "Sebastian Pop via Phabricator" <reviews at reviews.llvm.org>
> >> To: gberry at codeaurora.org, dberlin at dberlin.org, hiraditya at msn.com,
> >> hfinkel at anl.gov, sebpop at gmail.com
> >> Cc: "amara emerson" <amara.emerson at arm.com>, "mehdi amini"
> >> <mehdi.amini at apple.com>, mcrosier at codeaurora.org,
> >> llvm-commits at lists.llvm.org
> >> Sent: Tuesday, December 13, 2016 1:45:55 PM
> >> Subject: [PATCH] D27722: [GVNHoist] Move GVNHoist to function
> >> simplification part of pipeline.
> >>
> >> sebpop accepted this revision.
> >> sebpop added a comment.
> >>
> >> Overall LGTM.
> >> In the past I have seen some improvements to function inlining due
> >> to
> >> hoisting happening before inlining.
> >> Also there may be some sinking (in cfg-simplify) happening before
> >> we
> >> have a chance to hoist expressions.
> >> Let's commit this, and I will report if I see perf degradations,
> >> in
> >> which case we may as well run hoisting before and after inlining.
> > Broader topic, but shouldn't we run a full function-simplification
> > pipeline before inlining? That seems necessary in order to
> > generate good inline cost estimates. Otherwise, for example,
> > you'll inline something that looks small only to fully unroll a
> > loop, etc. and have it become larger.
> 
> My understanding is that is what we do now.  All of the IR
> optimizations
> other than vectorization and some related loop transformations are
> run
> by the CallGraphSCC pass manager which (ignoring recursive loops in
> the
> call graph), runs all of the simplification passes on each of the
> functions in bottom up order. The cutoff for what gets run on callees
> before inlining into callers is what passes get added after the
> barrier
> noop pass added in PassManagerBuilder::populateModulePassManager().
> Specifically, we do run loop unrolling (the first time) on callees
> before running inlining on the callers.  Also to be explicit, with my
> change to GVNHoist's placement in the pipeline, it is still being run
> on
> callees before they are inlined into calleers.

Okay, that's what I thought happened before I read Sebastian's comment. Thanks!

 -Hal

> 
> -Geoff
> 
> >   -Hal
> >
> >> Thanks for your patch!
> >>
> >>
> >> https://reviews.llvm.org/D27722
> >>
> >>
> >>
> >>
> 
> --
> Geoff Berry
> Employee of Qualcomm Datacenter Technologies, Inc.
>   Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
>   Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
>   Code Aurora Forum, a Linux Foundation Collaborative Project.
> 
> 

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory