[PATCH] D73129: [LoopUnrollAndJam] Correctly update LoopInfo when unroll and jam more than 2-levels loop nests.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/test/Transforms/LoopUnrollAndJam/loopnest.ll:6
+; CHECK-LABEL: foo
+; CHECK: %i = phi i64 [ 0, %entry ], [ %inc.i.3, %for.i.latch ]
+
----------------
Whitney wrote:
> jdoerfert wrote:
> > dmgreen wrote:
> > > Whitney wrote:
> > > > jdoerfert wrote:
> > > > > I would generate the check lines with the update_checks script. This doesn't test much.
> > > > I tried to follow the same pattern as the other unroll and jam lit tests.  The main purpose of this test is to check if loop info verified correctly. I will update the test with check lines generated by use update_checks script.
> > > Maybe just show control flow, like the test that was changed below?
> > > Maybe just show control flow, like the test that was changed below?
> > 
> > The problem there is that the conditions are missing and you don't know where the loop bodies end up.
> > 
> > It's debatable what is best but this representation has some advantages when it comes to changes, it makes them automatically applicable and explicitly shows everything.
> > 
> > That said, I'm fine with manually cutting it down if ppl prefer that.
> > 
> How about I only keep the control flow, conditions, and phi nodes?
If this test is intended to check that we generate the correct control flow, why not keep the inner loop body to a bare minimum (e.g.  just the induction variables and maybe a function call that is passed the induction variables? We force unrolling, so the loop body, so the body is not really important, right?

That would reduce the clutter to a bare minimum, even if we use the script to auto-generate the CHECK lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73129