[LoopInterchange] Require unordered load/store

[artagnon]

To match DependenceAnalysis' handling of unordered loads/stores, permit exactly this condition in LoopInterchange.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

To match DependenceAnalysis' handling of unordered loads/stores, permit exactly this condition in LoopInterchange.

---

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

1 Files Affected:

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

diff --git a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
index 9e3b4b82cc454..4d6efb051a773 100644
--- a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
@@ -141,11 +141,11 @@ static bool populateDependencyMatrix(CharMatrix &DepMatrix, unsigned Level,
       if (!isa<Instruction>(I))
         return false;
       if (auto *Ld = dyn_cast<LoadInst>(&I)) {
-        if (!Ld->isSimple())
+        if (!Ld->isUnordered())
           return false;
         MemInstr.push_back(&I);
       } else if (auto *St = dyn_cast<StoreInst>(&I)) {
-        if (!St->isSimple())
+        if (!St->isUnordered())
           return false;
         MemInstr.push_back(&I);
       }

[kasuga-fj]

Could you please add tests?

Also, IIUIC, isSimple imposes a strictly stronger condition than isUnordered. If the opposite were true (i.e., if DA required a stronger restriction than isSimple), this change would make perfectly sense to me. However, that is not the case here. So, it might be meaningful to call isSimple instead of isUnordered here (I feel like there's not a particularly strong reason, but I'm not entirely sure).

[sjoerdmeijer]

Also, IIUIC, isSimple imposes a strictly stronger condition than isUnordered. If the opposite were true (i.e., if DA required a stronger restriction than isSimple), this change would make perfectly sense to me. However, that is not the case here. So, it might be meaningful to call isSimple instead of isUnordered here (I feel like there's not a particularly strong reason, but I'm not entirely sure).

This is my understanding too. So, my question is what this patch is achieving?