[PATCH] D106317: [SimplifyCFG] performBranchToCommonDestFolding(): form sudo-LCSSA before cloning instructions (PR51125)

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D106317#2888900 <https://reviews.llvm.org/D106317#2888900>, @lebedev.ri wrote:

> In D106317#2888819 <https://reviews.llvm.org/D106317#2888819>, @jdoerfert wrote:
>
>> At first glance, only looking at pr51125, it seems we treat uses in PHIs as uses of the phi block and not the predecessor (edge). Is that the problem?
>
> "as uses *in* the phi block" you mean? I'm afraid i'm not really seeing it?

yes, uses *in* the phi block. In `pr51125` we have 4 uses of `%ld` and right now we treat one different than the remaining 3. The reason, as I imagine, is
that we do not consider the use of `%ld` in the phi `%r` to be inside of %L` but inside of `%exit` instead. Conceptually it's on the edge but it's certainly
closer to the end of `%L` than the beginning of `%exit`.

I was playing around with this a bit and while the follow code seems to work on that example it is by no means thought through very well:

       SSAUpdate.AddAvailableValue(PredBlock, NewBonusInst);
       for (Use &U : make_early_inc_range(BonusInst.uses())) {
         auto *UI = cast<Instruction>(U.getUser());
  +      if (auto *PHI = dyn_cast<PHINode>(UI))
  +        if (PHI->getIncomingBlock(U) == PredBlock) {
  +          Value *V = SSAUpdate.GetValueInMiddleOfBlock(PredBlock);
  +          if (!isa<UndefValue>(V)) {
  +            U.set(V);
  +            continue;
  +          }
  +        }
         if (UI->getParent() != PredBlock)
           SSAUpdate.RewriteUseAfterInsertions(U);
         else // Use is in the same block as, and comes before, NewBonusInst.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106317