[PATCH] D73641: [LoopFusion] Move instructions from FC1.GuardBlock to FC0.GuardBlock and from FC0.ExitBlock to FC1.ExitBlock when proven safe.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM with the caveat that the last change didn't do what I hoped for, see below.



================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:776
             continue;
           }
 
----------------
jdoerfert wrote:
> Whitney wrote:
> > jdoerfert wrote:
> > > What happens now if there is a FC1 guard branch with non-movable instructions but no fc0 guard branch. I think we now miss that case and do sth? Could you add a test if we don't have one?
> > I think that's not allowed in https://reviews.llvm.org/D71569.
> Fair, then you didn't need to change `FC1->GuardBranch &&` into `FC0->GuardBranch && FC1->GuardBranch &&`, right? At least that is what confused me here.
> Maybe we do an outer `if (FC0->GuardBranch) {...}` and an assertion that both are set or none is? That makes the two conditionals also simpler as they only check `isSafeToMoveBefore`.
Now you check FC1->GuardBranch twice. What I meant was:

```
 if (FC0->GuardBranch) {
            assert(FC1->GuardBranch && "Expecting valid FC1 guard branch");
             if (!isSafeToMoveBefore(*FC0->ExitBlock,
                                  *FC1->ExitBlock->getFirstNonPHIOrDbg(), DT,
                                  PDT, DI)) {
            LLVM_DEBUG(dbgs() << "Fusion candidate contains unsafe "
                                 "instructions in exit block. Not fusing.\n");


            reportLoopFusion<OptimizationRemarkMissed>(*FC0, *FC1,
                                                       NonEmptyExitBlock);
            continue;
          }

            if (!isSafeToMoveBefore(
                    *FC1->GuardBranch->getParent(),
                    *FC0->GuardBranch->getParent()->getTerminator(), DT, PDT,
                    DI)) {
              LLVM_DEBUG(dbgs()
                         << "Fusion candidate contains unsafe "
                            "instructions in guard block. Not fusing.\n");
              reportLoopFusion<OptimizationRemarkMissed>(*FC0, *FC1,
                                                         NonEmptyGuardBlock);
              continue;
            }
     }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73641