[PATCH] D21105: [Polly][GSoC 2016][WIP]New function pass DependenceInfoWrapperPass

[Johannes Doerfert via llvm-commits]

jdoerfert added a comment.

Please look at the comments __and__ add some test cases as you did with the scopinfo function pass.


================
Comment at: include/polly/DependenceInfo.h:243
@@ +242,3 @@
+
+  using ScopToDepsMapTy = DenseMap<Scop *, std::unique_ptr<Dependences>>;
+
----------------
Move this to the private section of the class please.

================
Comment at: include/polly/DependenceInfo.h:250
@@ +249,3 @@
+  ///
+  /// @param Level The granularity of dependence analysis result.
+  ///
----------------
@param S

================
Comment at: lib/Analysis/DependenceInfo.cpp:808
@@ +807,3 @@
+
+//===----------------------------------------------------------------------===//
+const Dependences &
----------------
I am still not a fan of these lines and you introduce 2 of them in this patch. Could you remove them or do you think they really help?

================
Comment at: lib/Analysis/DependenceInfo.cpp:825
@@ +824,3 @@
+  auto Inserted = ScopToDepsMap.insert(std::make_pair(S, std::move(D)));
+  assert(Inserted.second && "Recomputing Scop for the same scop twice!");
+  return *Inserted.first->second;
----------------
assertion message seems odd.

And why cant we recompute dependences for the same SCoP? Isn't that what this function is all about?

================
Comment at: lib/Analysis/DependenceInfo.cpp:837
@@ +836,3 @@
+/// @brief Print the dependences for the given SCoP to @p OS.
+
+void DependenceInfoWrapperPass::print(raw_ostream &OS, const Module *M) const {
----------------
Empty line + @brief comment in the cpp file.


http://reviews.llvm.org/D21105