[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