[PATCH] D27722: [GVNHoist] Move GVNHoist to function simplification part of pipeline.
Hal Finkel via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 13 13:37:07 PST 2016
----- 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
> other than vectorization and some related loop transformations are
> by the CallGraphSCC pass manager which (ignoring recursive loops in
> 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
> 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
> callees before they are inlined into calleers.
Okay, that's what I thought happened before I read Sebastian's comment. Thanks!
> > -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.
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits