[PATCH] D20770: [GSoC 2016][Polly][Refactor] Decouple SCoP building logic from pass

[Utpal Bora via llvm-commits]

cs14mtech11017 added a comment.

I have done the changes you requested. Have arc for the new patch.


================
Comment at: include/polly/ScopInfo.h:2476-2478
@@ +2475,5 @@
+
+  /// @brief Build and initialize Polly IR of static control part of the current
+  /// SESE-Region.
+  void createScopInfo(Region *R, AssumptionCache &AC);
+
----------------
Ok, removed this function and inlined code into the constructor.

================
Comment at: include/polly/ScopInfo.h:2482
@@ -2474,1 +2481,3 @@
+
+  virtual void print(raw_ostream &O, const Module *M = nullptr) const;
 
----------------
jdoerfert wrote:
> Remove the virtual please.
Removed. thanks for pointing that out.

================
Comment at: include/polly/ScopInfo.h:2498
@@ +2497,3 @@
+  /// @brief The ScopInfo pointer which is used to construct a Scop.
+  std::unique_ptr<ScopInfo> SI;
+
----------------
jdoerfert wrote:
> I don't know why you need a unique_ptr here but I don't care to much about it.
As this is an analysis pass, I think it is better for memory management. Don't have to track all the pointer uses.

================
Comment at: include/polly/ScopInfo.h:2510
@@ +2509,3 @@
+  /// @return Return ScopInfo object for the current Region.
+  ScopInfo &getScopInfo() { return *SI; }
+  const ScopInfo &getScopInfo() const { return *SI; }
----------------
jdoerfert wrote:
> I would not return a reference here because there is no way to know that SI is actually not null. Please return the SI pointer.
Ok. After making the change at 4896, this makes sense.

================
Comment at: lib/Analysis/ScopInfo.cpp:4869
@@ +4868,3 @@
+
+  if (!SD.isMaxRegionInScop(*R))
+    return;
----------------
jdoerfert wrote:
> can you move this check to the runOnRegion method below?
Done. Also removed this function and inlined code into constructor.


http://reviews.llvm.org/D20770