[PATCH] D20962: [Polly][GSoC 2016]New function pass ScopInfoWrapperPass

[Hongbin Zheng via llvm-commits]

etherzhhb added inline comments.

================
Comment at: include/polly/ScopInfo.h:2539
@@ +2538,3 @@
+
+  /// @brief Get the ScopInfo object for the given Region
+  Scop *getScop(Region *R) {
----------------
Could we document what happened if a children region of a SCoP region is passed to this function?

Supposed we built Scop S0 for Region R_0, and R_0 has a child (contains) R_1.

What I am getting if I call getScop(R_1)?

================
Comment at: include/polly/ScopInfo.h:2542-2545
@@ +2541,6 @@
+    auto it = regionToScopMap.find(R);
+    if (it != regionToScopMap.end())
+      return it->second.get();
+    else
+      return nullptr;
+  }
----------------
These if-else could written as the following to reduce indent:

```
    if (it != regionToScopMap.end())
      return it->second.get();

    return nullptr;
```

================
Comment at: lib/Analysis/ScopInfo.cpp:4968
@@ +4967,3 @@
+    ScopBuilder SB(R, AC, AA, DL, DT, LI, SD, SE);
+    regionToScopMap.insert(std::make_pair(R, SB.getScop()));
+  }
----------------
jdoerfert wrote:
> ```
> RegionToScopMap[R] = SB.getScop();
> ```
Or

```
bool Inserted = RegionToScopMap.insert(std::make_pair(R, SB.getScop())).second;
assert(Inserted && "Building Scop for the same region twice!");
(void) Inserted;
```

================
Comment at: lib/Analysis/ScopInfo.cpp:4974
@@ +4973,3 @@
+void ScopInfoWrapperPass::print(raw_ostream &OS, const Module *) const {
+  for (auto &it : regionToScopMap) {
+    if (it.second)
----------------
jdoerfert wrote:
> ```
> It
> ```
> 
> Please take another look at the naming convention we use.
> 
http://llvm.org/docs/CodingStandards.html


http://reviews.llvm.org/D20962