[PATCH] D11554: Keep track of ScopArrayInfo objects that model PHI node storage

[Johannes Doerfert]

jdoerfert added a comment.

Some comments but overall LGTM.


================
Comment at: include/polly/ScopInfo.h:1053
@@ -1032,2 +1052,3 @@
   ///
-  /// @param ElementType The type of the elements stored in this array.
+  /// @param ElementType The type of the elements stored ino this array.
+  /// @param IsPHI       Is this ScopArrayInfo object modeling special
----------------
typo.

================
Comment at: lib/Analysis/ScopInfo.cpp:357
@@ +356,3 @@
+      DimensionSizes(DimensionSizes), IsPHI(IsPHI) {
+  std::string BasePtrName = getIslCompatibleName("MemRef_", BasePtr, "");
+  if (IsPHI)
----------------
I would have used the suffix (last argument) of this call to encode the __phi but you don't have to.

  std::string BasePtrName = getIslCompatibleName("MemRef_", BasePtr, IsPHI ? "__phi" : "");

================
Comment at: lib/Analysis/ScopInfo.cpp:1713
@@ -1710,1 +1712,3 @@
+                               bool IsPHI) {
+  auto &SAI = ScopArrayInfoMap[std::make_pair(BasePtr, IsPHI ? 1 : 0)];
   if (!SAI)
----------------
Why not use a bool in the pair but introduce this 1/0 integer?

================
Comment at: lib/Analysis/ScopInfo.cpp:1721
@@ -1717,1 +1720,3 @@
+const ScopArrayInfo *Scop::getScopArrayInfo(Value *BasePtr, bool IsPHI) {
+  auto SAI = ScopArrayInfoMap[std::make_pair(BasePtr, IsPHI ? 1 : 0)].get();
   assert(SAI && "No ScopArrayInfo available for this base pointer");
----------------
auto *


http://reviews.llvm.org/D11554