[PATCH] D139601: [LoopDistribute] Clear cache of `LoopAccessInfoManager`

[Florian Hahn via Phabricator via llvm-commits]

fhahn requested changes to this revision.
fhahn added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/Scalar/LoopDistribute.cpp:995
       Changed |= LDL.processLoop();
+    LAIs.clear();
   }
----------------
Saldivarcher wrote:
> fhahn wrote:
> > We only need to clear if changes were made AFAICT
> The assertion is still hit if we only clear when changes are made, I think it's because we insert the loop [[ https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Scalar/LoopDistribute.cpp#L681 | here ]], then fail right after. So, there's still cached data when there are no changes made. Keep in mind that the LoopDistribute pass fails with all the loops, and all fail [[ https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Scalar/LoopDistribute.cpp#L686 | here. ]]
Right, this indicates that `LoopDistribute` isn't the right place to fix it. We should only need invalidation *after* the IR changes. If the IR stays the same, there should be no issue with the cached loop-info objects.

I think the right place to invalidate here is in `LoopVectorize's` `runImpl`.


================
Comment at: llvm/test/Transforms/LoopDistribute/pr59319.ll:2
+; RUN: opt -passes=loop-distribute,loop-vectorize -enable-loop-distribute -S \
+; RUN: < %s | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
----------------
no need to use `<`


================
Comment at: llvm/test/Transforms/LoopDistribute/pr59319.ll:5
+target triple = "x86_64-unknown-linux-gnu"
+;REQUIRES: x86-registered-target
+
----------------
nit: usually there's a space after `;` both here and below.


================
Comment at: llvm/test/Transforms/LoopDistribute/pr59319.ll:8
+define void @reduced(i32* %0, i32* %1, i64 %iv, i32* %2, i64 %iv76, i64 %iv93) {
+;CHECK-LABEL: @reduced(
+entry:
----------------
check the full IR?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139601/new/

https://reviews.llvm.org/D139601