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

Kit Barton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 8 12:01:02 PDT 2019


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

In D65464#1617929 <https://reviews.llvm.org/D65464#1617929>, @Meinersbur wrote:

> Is there a long-term plan to avoid the code duplication? E.g. refactoring/keeping only the fuseGuardedLoops variant when loop guards become a normal form? One could also add some preprocessing that makes the two loop guards guard both loops, then use the standard fuse code.


Yes, the intention is to clean this up in subsequent patches. 
The next patch will restrict fusion to only fuse rotated loops, at which point the previous performFusion method can be removed and we can consolidate on fuseGuardedLoops (maybe rename fuseGuardedLoops to performFusion, but still consolidate on a single method and remove the duplicate code).



================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:1427
+    // trip-counts are equal after all.
+    // KB: Would this sequence be simpler to just just make FC0.ExitingBlock go
+    // to FC1.Header? I think this is basically what the three sequences are
----------------
Meinersbur wrote:
> [typo] "just just"
> 
> These internal remarks don't seem to be confident in what the code is doing.
> 
> Sometimes some ASCII-art might clear up which blocks are connected to where.
These comments were old and I forgot to remove them. I've updated them now to reflect what is actually being done. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65464





More information about the llvm-commits mailing list