[LoopInterchange] Skip legality check if surrounding loops already guarantee dependency (NFC)

[kasuga-fj]

The legality check in LoopInterchange allows two loops to be exchanged if all direction vectors are lexicographically positive (or zero) for both before and after the swap. The current implementation performs this routine naively. However, if a direction vector is lexicographically positive due to an element corresponding to a loop that is outside the given two loops (i.e., if there is an element < before the loops we are trying to interchange), then obviously it is also positive after exchanging them. For example, for a direction vector [< < >], swapping the last two elements doesn't make it lexicographically negative because the first element is <.
This patch adds a code to skip legality check if surrounding loops already guarantee that the direction vector is lexicographically positive. Note that this is only a small improvement on its own, but it's necessary to relax the legality check I'm working on.

Split off from #118267

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Ryotaro Kasuga (kasuga-fj)

Changes

The legality check in LoopInterchange allows two loops to be exchanged if all direction vectors are lexicographically positive (or zero) for both before and after the swap. The current implementation performs this routine naively. However, if a direction vector is lexicographically positive due to an element corresponding to a loop that is outside the given two loops (i.e., if there is an element < before the loops we are trying to interchange), then obviously it is also positive after exchanging them. For example, for a direction vector [< < >], swapping the last two elements doesn't make it lexicographically negative because the first element is <.
This patch adds an early exit logic for such cases. Note that this is only a small improvement on its own, but it's necessary to relax the legality check I'm working on.

Split off from #118267

---

Full diff: https://github.com/llvm/llvm-project/pull/139254.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/LoopInterchange.cpp (+19-6)

diff --git a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
index fb7bc7c4be6c8..f6d1860e66e1a 100644
--- a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
@@ -231,18 +231,22 @@ static void interChangeDependencies(CharMatrix &DepMatrix, unsigned FromIndx,
     std::swap(DepMatrix[I][ToIndx], DepMatrix[I][FromIndx]);
 }
 
-// After interchanging, check if the direction vector is valid.
+// Check if a direction vector is lexicographically positive. Return true if it
+// is positive, nullopt if it is "zero", othrewise false.
 // [Theorem] A permutation of the loops in a perfect nest is legal if and only
 // if the direction matrix, after the same permutation is applied to its
 // columns, has no ">" direction as the leftmost non-"=" direction in any row.
-static bool isLexicographicallyPositive(std::vector<char> &DV) {
-  for (unsigned char Direction : DV) {
+static std::optional<bool> isLexicographicallyPositive(std::vector<char> &DV,
+                                                       unsigned Begin,
+                                                       unsigned End) {
+  ArrayRef<char> DVRef(DV);
+  for (unsigned char Direction : DVRef.slice(Begin, End - Begin)) {
     if (Direction == '<')
       return true;
     if (Direction == '>' || Direction == '*')
       return false;
   }
-  return true;
+  return std::nullopt;
 }
 
 // Checks if it is legal to interchange 2 loops.
@@ -256,10 +260,19 @@ static bool isLegalToInterChangeLoops(CharMatrix &DepMatrix,
     // Create temporary DepVector check its lexicographical order
     // before and after swapping OuterLoop vs InnerLoop
     Cur = DepMatrix[Row];
-    if (!isLexicographicallyPositive(Cur))
+
+    // If the direction vector is lexicographically positive due to an element
+    // to the left of OuterLoopId, it is still positive after exchanging the two
+    // loops. In such a case we can skip the subsequent check.
+    if (isLexicographicallyPositive(Cur, 0, OuterLoopId) == true)
+      continue;
+
+    // Check if the direction vector is lexicographically positive (or zero)
+    // for both before/after exchanged.
+    if (isLexicographicallyPositive(Cur, 0, Cur.size()) == false)
       return false;
     std::swap(Cur[InnerLoopId], Cur[OuterLoopId]);
-    if (!isLexicographicallyPositive(Cur))
+    if (isLexicographicallyPositive(Cur, 0, Cur.size()) == false)
       return false;
   }
   return true;

[Meinersbur]

Can you make the title more concrete than "certain cases"? It's not an early exit either

Suggestion: Skip legality check if surronding loops already guarantee dependency

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[kasuga-fj]

Can you make the title more concrete than "certain cases"? It's not an early exit either

Suggestion: Skip legality check if surronding loops already guarantee dependency

Fixed, thanks!