[llvm] [LoopInterchange] Redundant isLexicographicallyPositive check (PR #117343)

via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 12 13:19:37 PST 2024


================
@@ -204,11 +204,12 @@ static bool isLegalToInterChangeLoops(CharMatrix &DepMatrix,
   std::vector<char> Cur;
   // For each row check if it is valid to interchange.
   for (unsigned Row = 0; Row < NumRows; ++Row) {
-    // Create temporary DepVector check its lexicographical order
-    // before and after swapping OuterLoop vs InnerLoop
+    // A dependence vector has to be lexigraphically positive. We could assert
+    // that here, as a sanity check for the dependency analysis, but it can
+    // contain "don't know" elements ('*'), which will be rightfully
+    // interpreted as not lexicographical positive. So, skip that sanity check,
+    // it suffices just to check the interchanged direction vector.
----------------
CongzheUalberta wrote:

AFAIK, there was a reason for `isLexicographicallyPositive(Cur)` to be called before the swap. I did not recall explicitly but it likely has something to do with `'S'` or `'*'` dependency. We had encountered a number of bugs due to the fact that `'S'` and `'*'` not very easily dealt with in loop interchange. 

If you remove the `isLexicographicallyPositive()` call here, likely bugs would appear. I probably do not have enough time digging into it at the moment, but you may refer to this patch (`https://reviews.llvm.org/D137461`) and also earlier patches and issues in loop interchange.

https://github.com/llvm/llvm-project/pull/117343


More information about the llvm-commits mailing list