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

[Geoff Berry via llvm-commits]

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.

-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.