[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