[polly] r243677 - Move computations out of constructors

[Michael Kruse]

Author: meinersbur
Date: Thu Jul 30 14:27:04 2015
New Revision: 243677

URL: http://llvm.org/viewvc/llvm-project?rev=243677&view=rev
Log:
Move computations out of constructors

It is common practice to keep constructors lightweight. The reasons
include:

- The vtable during the constructor's execution is set to the static
type of the object, not to the vtable of the derived class. That is,
method calls behave differently in constructors and ordinary methods.
This way it is possible to call unimplemented methods of abstract
classes, which usually results in a segmentation fault.

- If an exception is thrown in the constructor, the destructor is not
called, potentially leaking memory.

- Code in constructors cannot be called in a regular way, e.g. from
non-constructor methods of derived classes.

- Because it is common practice, people may not expect the constructor
to do more than initializing data and skip them when looking for bugs.

Not all of these are applicable to LLVM (e.g. exceptions are disabled).

This patch refactors out the computational work in the constructors of
Scop and IslAst into regular init functions and introduces static
create-functions as replacement. 

Differential revision: http://reviews.llvm.org/D11491

Reviewers: grosser, jdoerfert

Modified:
    polly/trunk/include/polly/ScopInfo.h
    polly/trunk/lib/Analysis/ScopInfo.cpp
    polly/trunk/lib/CodeGen/IslAst.cpp

Modified: polly/trunk/include/polly/ScopInfo.h
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopInfo.h?rev=243677&r1=243676&r2=243677&view=diff
==============================================================================
--- polly/trunk/include/polly/ScopInfo.h (original)
+++ polly/trunk/include/polly/ScopInfo.h Thu Jul 30 14:27:04 2015
@@ -842,10 +842,17 @@ private:
   /// group to ensure the SCoP is executed in an alias free environment.
   MinMaxVectorPairVectorTy MinMaxAliasGroups;
 
+  /// @brief Scop constructor; used by static createFromTempScop
+  Scop(Region &R, ScalarEvolution &SE, isl_ctx *ctx, unsigned MaxLoopDepth);
+
+  /// @brief Initialize this ScopInfo using a TempScop object.
+  void initFromTempScop(TempScop &TempScop, LoopInfo &LI, ScopDetection &SD);
+
   /// Create the static control part with a region, max loop depth of this
   /// region and parameters used in this region.
-  Scop(TempScop &TempScop, LoopInfo &LI, ScalarEvolution &SE, ScopDetection &SD,
-       isl_ctx *ctx);
+  static Scop *createFromTempScop(TempScop &TempScop, LoopInfo &LI,
+                                  ScalarEvolution &SE, ScopDetection &SD,
+                                  isl_ctx *ctx);
 
   /// @brief Check if a basic block is trivial.
   ///

Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=243677&r1=243676&r2=243677&view=diff
==============================================================================
--- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
+++ polly/trunk/lib/Analysis/ScopInfo.cpp Thu Jul 30 14:27:04 2015
@@ -1664,19 +1664,20 @@ static unsigned getMaxLoopDepthInRegion(
   return MaxLD - MinLD + 1;
 }
 
-Scop::Scop(TempScop &tempScop, LoopInfo &LI, ScalarEvolution &ScalarEvolution,
-           ScopDetection &SD, isl_ctx *Context)
-    : SE(&ScalarEvolution), R(tempScop.getMaxRegion()), IsOptimized(false),
-      MaxLoopDepth(getMaxLoopDepthInRegion(tempScop.getMaxRegion(), LI, SD)) {
-  IslCtx = Context;
+Scop::Scop(Region &R, ScalarEvolution &ScalarEvolution, isl_ctx *Context,
+           unsigned MaxLoopDepth)
+    : SE(&ScalarEvolution), R(R), IsOptimized(false),
+      MaxLoopDepth(MaxLoopDepth), IslCtx(Context) {}
 
+void Scop::initFromTempScop(TempScop &TempScop, LoopInfo &LI,
+                            ScopDetection &SD) {
   buildContext();
 
   SmallVector<Loop *, 8> NestLoops;
 
   // Build the iteration domain, access functions and schedule functions
   // traversing the region tree.
-  Schedule = buildScop(tempScop, getRegion(), NestLoops, LI, SD);
+  Schedule = buildScop(TempScop, getRegion(), NestLoops, LI, SD);
   if (!Schedule)
     Schedule = isl_schedule_empty(getParamSpace());
 
@@ -1687,6 +1688,16 @@ Scop::Scop(TempScop &tempScop, LoopInfo
   assert(NestLoops.empty() && "NestLoops not empty at top level!");
 }
 
+Scop *Scop::createFromTempScop(TempScop &TempScop, LoopInfo &LI,
+                               ScalarEvolution &SE, ScopDetection &SD,
+                               isl_ctx *ctx) {
+  auto &R = TempScop.getMaxRegion();
+  auto MaxLoopDepth = getMaxLoopDepthInRegion(R, LI, SD);
+  auto S = new Scop(R, SE, ctx, MaxLoopDepth);
+  S->initFromTempScop(TempScop, LI, SD);
+  return S;
+}
+
 Scop::~Scop() {
   isl_set_free(Context);
   isl_set_free(AssumedContext);
@@ -2165,7 +2176,7 @@ bool ScopInfo::runOnRegion(Region *R, RG
     return false;
   }
 
-  scop = new Scop(*tempScop, LI, SE, SD, ctx);
+  scop = Scop::createFromTempScop(*tempScop, LI, SE, SD, ctx);
 
   DEBUG(scop->print(dbgs()));
 

Modified: polly/trunk/lib/CodeGen/IslAst.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/IslAst.cpp?rev=243677&r1=243676&r2=243677&view=diff
==============================================================================
--- polly/trunk/lib/CodeGen/IslAst.cpp (original)
+++ polly/trunk/lib/CodeGen/IslAst.cpp Thu Jul 30 14:27:04 2015
@@ -71,8 +71,7 @@ static cl::opt<bool> NoEarlyExit(
 namespace polly {
 class IslAst {
 public:
-  IslAst(Scop *Scop, const Dependences &D);
-
+  static IslAst *create(Scop *Scop, const Dependences &D);
   ~IslAst();
 
   /// Print a source code representation of the program.
@@ -88,6 +87,9 @@ private:
   isl_ast_node *Root;
   isl_ast_expr *RunCondition;
 
+  IslAst(Scop *Scop);
+  void init(const Dependences &D);
+
   void buildRunCondition(__isl_keep isl_ast_build *Build);
 };
 } // End namespace polly.
@@ -372,14 +374,14 @@ static bool benefitsFromPolly(Scop *Scop
   return true;
 }
 
-IslAst::IslAst(Scop *Scop, const Dependences &D)
-    : S(Scop), Root(nullptr), RunCondition(nullptr) {
+IslAst::IslAst(Scop *Scop) : S(Scop), Root(nullptr), RunCondition(nullptr) {}
 
+void IslAst::init(const Dependences &D) {
   bool PerformParallelTest = PollyParallel || DetectParallel ||
                              PollyVectorizerChoice != VECTORIZER_NONE;
 
   // Skip AST and code generation if there was no benefit achieved.
-  if (!benefitsFromPolly(Scop, PerformParallelTest))
+  if (!benefitsFromPolly(S, PerformParallelTest))
     return;
 
   isl_ctx *Ctx = S->getIslCtx();
@@ -411,6 +413,12 @@ IslAst::IslAst(Scop *Scop, const Depende
   isl_ast_build_free(Build);
 }
 
+IslAst *IslAst::create(Scop *Scop, const Dependences &D) {
+  auto Ast = new IslAst(Scop);
+  Ast->init(D);
+  return Ast;
+}
+
 IslAst::~IslAst() {
   isl_ast_node_free(Root);
   isl_ast_expr_free(RunCondition);
@@ -436,7 +444,7 @@ bool IslAstInfo::runOnScop(Scop &Scop) {
 
   const Dependences &D = getAnalysis<DependenceInfo>().getDependences();
 
-  Ast = new IslAst(&Scop, D);
+  Ast = IslAst::create(&Scop, D);
 
   DEBUG(printScop(dbgs(), Scop));
   return false;