[PATCH] D30638: [SLP] Fixed non-determenistic behavior in Loop Vectorizer.

[Amjad Aboud via Phabricator via llvm-commits]

aaboud marked an inline comment as done.
aaboud added a comment.

Thanks, Michael for the comments.
Unfortunately, I do not have answers for all!



================
Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:97
+  typedef SmallVector<MemAccessInfo, 8> MemAccessInfoList;
   typedef SmallPtrSet<MemAccessInfo, 8> MemAccessInfoSet;
   /// \brief Set of potential dependent memory accesses.
----------------
mkuper wrote:
> I don't think there's a reason to keep the Set typedef here if the only use is for a local variable.
Agree, will fix.


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:825
           if ((IsWrite || IsReadOnlyPtr) && SetHasWrite) {
-            CheckDeps.insert(Access);
+            CheckDeps.push_back(Access);
             IsRTCheckAnalysisNeeded = true;
----------------
mkuper wrote:
> We never have duplicates here anyway, right? So it doesn't matter if it's a set or a list.
I do not know this code Michael, but even if we end up having duplication, we are going to process them once due to the "Visited" set.



================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1611
   }
+  CheckDeps.clear();
 
----------------
mkuper wrote:
> Is this necessary?
> I mean, it used to happen as a side-effect of the way the loop was structured, but is CheckDeps every actually used after this?
Again, I am not familiar with the code, from my check, this is why I preferred to keep same behavior as before.

I do not mind removing this line.


https://reviews.llvm.org/D30638