[PATCH] D55851: Implement basic loop fusion pass

[Kit Barton via Phabricator via llvm-commits]

kbarton marked 2 inline comments as done.
kbarton added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:929
+          // defined in that loop, and we likely don't want to disturb it.
+          PHINode *DefPHI = dyn_cast<PHINode>(Def);
+          if (DefPHI)
----------------
dmgreen wrote:
> I think that you can just do something like
>   if (Instruction *Def = dyn_cast<Instruction>(Op))
>     if (FC0.L->contains(Def->getParent()))
>       return false;
> Providing there are no lcssa phis, this should rule out anything that depends on the first loop. 
I just finished up testing this change, and then realized I think this is overly restrictive.
I had originally restricted it to PHIs because I only wanted to include things that are (re)defined in the first loop. Basically, the first loop needs to complete in order to get the correct definition going into the second loop.

This alternate sequence will also include stores, not just PHIs. The store can be loop invariant, and if it is not hoisted out of the loop prior, then it would also prevent fusion.


================
Comment at: llvm/test/Transforms/LoopFusion/cannot_fuse.ll:283
+bb6:                                              ; preds = %bb9, %bb
+  %indvars.iv3 = phi i64 [ %indvars.iv.next4, %bb9 ], [ 0, %bb ]
+  %.01 = phi i32 [ 0, %bb ], [ %tmp11, %bb9 ]
----------------
dmgreen wrote:
> You can run something like -loop-rotate to make this into a more simply structured loop. -instcombine will remove any lcssa nodes, and -simplify-cfg can clean things up too.
> 
> I guess that will depend on what types of loops you want to test, ones that have been cleanup up or ones that are a little less structured. It's good to test both, but which is more important will depend on where in the pass pipeline this ends up.
I completely agree.
In fact we have a subsequent patch to post once this one lands that restricts loop fusion to only run on rotated loops. This seems to be a common theme among many loop optimizations, and restricting to rotated loops simplifies the implementation in several places. I'm planning on discussing this at my presentation at EuroLLVM in a couple weeks :)


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

https://reviews.llvm.org/D55851