[PATCH] D97407: [LoopUnrollAndJam] Avoid repeated instructions for UAJ analysis

[Danilo Carvalho Grael via Phabricator via llvm-commits]

dancgr added a comment.

@dmgreen I think so. The code is indeed exponential. Adding this check will avoid the exponential case, but we will be visiting each instruction both ways, we just avoid visiting repeated ones. IMO it won't affect the analysis result.



================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp:161
+          if (!VisitedInstr.count(II))
+              Worklist.push_back(II);
   }
----------------
dmgreen wrote:
> Formatting is a bit off here.
Will fix that!


================
Comment at: llvm/test/Transforms/LoopUnrollAndJam/unroll-and-jam-many-instr.ll:1
+; ModuleID = 'test.c'
+source_filename = "test.c"
----------------
dmgreen wrote:
> This needs RUN: lines and some basic CHECKS, maybe with a comment explaining the test for bonus points.
> 
> If it requires the aarch64 backend (which it might not) then it would need to be in llvm/test/Transforms/LoopUnrollAndJam/AArch64 directory, so it only runs when the compiler is built with aarch64 as a registered target. It can likely remove the aarch64 though, and rely on the datalayout and command line args.
> 
> It might be possible to clean this up quite a bit. My understanding is that for.cond13.preheader (the aft block) needs to contain a lot of instructions to show the timeout. The main() and attributes can often be removed.
Yes, I will update this code. I doesn't necessarily needs to be for AArch64. I will also add a RUN line with some basic checks.

Good point, I will cleanup all the code that is not necessary for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97407