[PATCH] [Refactor] Replace RegionPasses by FunctionPasses

zino zinob at codeaurora.org
Wed Mar 4 10:17:59 PST 2015

Johannes and Tobias, 

We just discovered an issues with compiler time in the region pass manager. In particular in Release build where Name are not preserved

Lib/Analysis/RegionPass.cpp (86):
dumpPassInfo(P, EXECUTION_MSG, ON_REGION_MSG,	CurrentRegion->getNameStr());

getNameStr() is very expensive because it LLVM construct a new Name. The compile-time issue you guys are seeing might be due to this issue. 

Our fix right now is to pass empty string in release build. Toby, if you think this acceptable short term solution. Sanjin can submit a patch? 


-----Original Message-----
From: Johannes Doerfert [mailto:doerfert at cs.uni-saarland.de] 
Sent: Wednesday, March 04, 2015 2:13 AM
To: reviews+D7986+public+8f09c2ba4f202324 at reviews.llvm.org
Cc: sebpop at gmail.com; simbuerg at fim.uni-passau.de; zinob at codeaurora.org; llvm-commits at cs.uiuc.edu; dpeixott at codeaurora.org
Subject: Re: [PATCH] [Refactor] Replace RegionPasses by FunctionPasses

Hey Tobias,

I attached 2 lnt reports to this mail. The first (report_region.json) was created using the current polly/master. The second
(report_function.json) was created with the region->function pass patch.
Note that this patch is not yet at its full capacity, however over the 3 runs I measured I got:
  Performance Regressions  16
  Performance Improvements  159
  Unchanged Tests   817
  Total Tests   992
Where as the execution time changes (only 8 in total) should be jitter.

The point I try to make here is, 3 runs with the region pass manager take
  2:35:27 (3 runs region passes)
but 3 runs with the function pass manager only
  2:07:48 (3 runs function passes)
hence we save ~18% of the lnt time (if I didn't do a stupid math mistake).

To be precise, we save compile time "only" for larger functions with small SCoP coverage but we do not pay for it in any other case.

Does this convince you that region passes have a bad impact on compile time?

Best regards,

On 03/03, Tobias Grosser wrote:
> Hi Johannes,
> what is the motivation and the impact of this change? The only motivation of this change you give in the commit message is compile time. Did you measure any significant improvements here? I personally think there are very good reasons to change/improve our pass infrastructure, but am surprised compile-time is the one and only that motivates your change.
> I see two main reasons to work on the pass infrastructure:
> 1. Make it work with Chandler's new pass manager
> Chandler's new pass manager uses caching to keep multiple analysis results. I believe when we perform changes to our pass infrastructure, we should try to make sure it will work with Chandler's new pass manager. 
> Besides his last PassManager talk, commits such as https://llvm.org/svn/llvm-project/llvm/trunk@226560 show the idea of using caching results.
> 2. Fix the LoopInfo/RegionInfo misconception
> We currently assume ScopPasses on different regions do not affect each 
> other. However, they indeed affect each other, which means code-generating one scop may invalidate the next scop. Hence, we have some hacks in place to detect this invalidated scops. The (implicit) pass order change your patch brings does not seem to improve this in any way.
> I am not saying your patch should solve those two issues, but maybe it is a good time to reason about this and at least understand if they will not negatively affect these changes (or possibly even help to solve some).
> I added a couple of first comments to your patch, but before going further I would like to understand your intentions.
> Cheers,
> Tobias
> ================
> Comment at: include/polly/DependenceInfo.h:1 @@ -1,2 +1,2 @@
>  //===------ polly/Dependences.h - Polyhedral dependency analysis *- 
> C++ -*-===//  //
> ----------------
> Needs update, if renamed.
> ================
> Comment at: include/polly/DependenceInfo.h:24 @@ -23,3 +23,3 @@  
> ----------------
> Needs update, if renamed.
> ================
> Comment at: include/polly/DependenceInfo.h:140
> @@ +139,3 @@
> +  /// @brief Struct type to remember all kinds of dependences for a SCoP.
> +  struct Dependences {
> +    /// @brief The different kinds of dependences we calculate.
> ----------------
> The idea of introducing an analysis result for each individual Scop is 
> very much in line with Chandler's new analysis infrastructure. However, to me it seem you only went half wayby  leaving most functions on the pass itself. Looking at this, introducing per-scop-dependence objects seems to be an almost independent change.
> ================
> Comment at: include/polly/LinkAllPasses.h:31 @@ -30,3 +30,3 @@  
> llvm::Pass *createDeadCodeElimPass(); -llvm::Pass 
> *createDependencesPass();
> +llvm::Pass *createDependenceInfoPass();
>  llvm::Pass *createDOTOnlyPrinterPass();
> ----------------
> I would personally not perform such renaming as part this patch, as it causes noise all over the place.
> ================
> Comment at: lib/Analysis/DependenceInfo.cpp:1 @@ -1,2 +1,2 @@
>  //===- Dependences.cpp - Calculate dependency information for a Scop. 
> -----===//  //
> ----------------
> Needs updated if you want to perform renaming.
> ================
> Comment at: lib/Analysis/DependenceInfo.cpp:351
> @@ -351,1 +350,3 @@
> +    D.RAW = isl_union_map_subtract(D.RAW, isl_union_map_copy(D.RED));
> +    D.WAW = isl_union_map_subtract(D.WAW, isl_union_map_copy(D.RED));
> ----------------
> These are a lot of D.*? Would it make sense to make all this functionality part of the Dependences object/class.
> ================
> Comment at: lib/Analysis/DependenceInfo.cpp:453
> @@ -453,1 +452,3 @@
> +  isl_union_map *Dependences =
> +      getDependences(S, TYPE_RAW | TYPE_WAW | TYPE_WAR);
>    isl_space *Space = S.getParamSpace();
> ----------------
> I have the feeling passing the scop to each of this functions unnecessarily complicates the interface. Could we not just once ask for the dependences object of this scop and then work with it?
> http://reviews.llvm.org/D7986
>   http://reviews.llvm.org/settings/panel/emailpreferences/


Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.31

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert

More information about the llvm-commits mailing list