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

Sjoerd Meijer via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 13 02:37:50 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.
----------------
sjoerdmeijer wrote:

Thanks for that pointer. 

I agree with the following from that phabricator review, that's what I am saying too:

> At the LoopWG call we discussed that we still could call isLexicographicallyPositive before the interchange to check that the dependency is understood before risting breaking things. I don't think it's necessary, but I don't mind.

The impression that I get from that discussion and the mishandling of scalar dependencies, is that we are not really sure why we are doing things.

Anyway, I don't have too strong opinions on this, I still don't think it is necessary, but I will focus on #119345 first. 

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


More information about the llvm-commits mailing list