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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 8 01:58:43 PST 2022


fhahn added a reviewer: fhahn.
fhahn added a comment.

Thanks for the patch! Some comments inline.

Could you add `Fixes #59319` to the description, to make sure the issue is closed once this lands?



================
Comment at: llvm/lib/Transforms/Scalar/LoopDistribute.cpp:995
       Changed |= LDL.processLoop();
+    LAIs.clear();
   }
----------------
We only need to clear if changes were made AFAICT


================
Comment at: llvm/test/Transforms/LoopDistribute/pr59319.ll:3
+; RUN: < %s | FileCheck %s
+; ModuleID = '<bc file>'
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
----------------
nit: remove


================
Comment at: llvm/test/Transforms/LoopDistribute/pr59319.ll:5
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
----------------
Is this needed? If so, the test requires something like `; REQUIRES: x86-registered-target` to make sure it is only run when the X86 backend is built.


================
Comment at: llvm/test/Transforms/LoopDistribute/pr59319.ll:13
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv761 = phi i64 [ 0, %entry ], [ %indvars.iv.next77, %for.body ]
+  %indvars.iv4 = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
----------------
nit: strip the `indvars.` prefix, it doesn't add anything and makes the names unnecessarily long.


================
Comment at: llvm/test/Transforms/LoopDistribute/pr59319.ll:24
+
+for.body26.lr.ph:                                 ; preds = %for.body13
+  %idxprom.i.i61 = and i64 %indvars.iv761, 1
----------------
Could you improve the naming of the blocks, e.g. `loop.1`, `loop.2`, something like that to make things easier to parse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139601



More information about the llvm-commits mailing list