[PATCH] D65464: [LoopFusion] Add ability to fuse guarded loops

[Kit Barton via Phabricator via llvm-commits]

kbarton marked an inline comment as done.
kbarton added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:1057
+        if (FC0CmpInst->isIdenticalTo(FC1CmpInst))
+          return true;
+
----------------
Whitney wrote:
> kbarton wrote:
> > Whitney wrote:
> > > what if FC0 guard first successor is FC0 Loop, but FC1 guard second successor is FC1 Loop? 
> > Then this check will fail.
> > This isn't meant to be exhaustive, but a first pass. If we run into situations where the guards are logically equivalent, and not identical, then we can extend this at that point. 
> I am more worry about correctness. When the two compare instructions are the same, but FC0 guard first successor is FC0 Loop, but FC1 guard second successor is FC1 Loop, then we may incorrectly fuse the two loops?
Oh, I see what you mean. The guards need to be identical, and the position of the in-loop successor and out-of-loop successors also needs to be the same. That's a good catch. I'll update the patch to test this now. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65464