[PATCH] [LAA] Merge memchecks for accesses separated by a constant offset

[silviu.baranga at arm.com]

Hi Adam,

Thanks for the comments! I've uploaded a new version, which should (hopefully) deal with all the issues raised.

Thanks,
Silviu


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:196-199
@@ +195,6 @@
+  SmallVector<unsigned, 2> Mins(Pointers.size());
+  // For each pointer, holds the index to the first element
+  // that was added to the pointers equivalence class. We do this
+  // because the leader of an equivalence class can change, and
+  // would like to always compare against the same element.
+  SmallVector<unsigned, 2> FirstInSetIndex(Pointers.size());
----------------
anemet wrote:
> Why do we need to compare against the same element?
Sorry, the last sentence of that comment was stale. Removed in the new version.

================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:202-213
@@ +201,14 @@
+
+  // Go through all equivalence classes, get the the "pointer check groups"
+  // and add them to the overall solution.
+  while (DI != DE) {
+    auto MI = DepCands.member_begin(DI);
+    auto ME = DepCands.member_end();
+
+    // Each pointer is part of an equivalence class. If two
+    // pointers are part of the same equivalence class, they will
+    // be part of the same group.
+    EquivalenceClasses<unsigned> CheckClass;
+
+    while (MI != ME) {
+      unsigned Pointer = PositionMap[MI->getPointer()];
----------------
anemet wrote:
> Shouldn't we only look through the members if DI is a leader?
We should! Nice catch!

================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:208-211
@@ +207,6 @@
+
+    // Each pointer is part of an equivalence class. If two
+    // pointers are part of the same equivalence class, they will
+    // be part of the same group.
+    EquivalenceClasses<unsigned> CheckClass;
+
----------------
anemet wrote:
> anemet wrote:
> > I don't understand what you gain by using an EC for this.
> > 
> > It seems to me that you just want a vector of CheckGroups for each DepCands set.
> > 
> > For each pointer then you go through this vector and try to merge to an existing group.
> > 
> > If nothing found you add a new group at the end of the vector.
> > 
> > As another general comment, it would be good to push some of the low-level mechanics into the CheckingGroup class so that the outline of algorithm is separated from the details.
> I just want to clarify one more thing about the above algorithm above using a local vector of CheckingGroup instead of an EC.
> 
> When we're done merging for a DepCands set, then you'd merge the "local" vector of CheckingGroups to the main one.
I've updated the algorithm. By using a SmallVector of CheckingPtrGroup we got rid of the not only the EquivalenceClass, but also the other vectors (Mins, Maxs, etc).

http://reviews.llvm.org/D10386

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/