[PATCH] D87293: [GVN] Fix undef incoming value for phi node when new loop exit block created.
weiwei via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 8 07:58:52 PDT 2020
wwei added inline comments.
================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:2751
}
+ for (BasicBlock *C : Candidates) {
----------------
asbirlea wrote:
> wwei wrote:
> > asbirlea wrote:
> > > Could you please explain in more details the mechanics here?
> > > When added into Candidates, the llvm::any_of condition is true. What makes the condition become false below?
> > > What's the purpose of the intersection of non-dead succs and preds?
> > I have updated the code, and some comments to descibe the details for it.
> >
> > ```
> > // - - - - - - - -
> > // | ... ... |
> > // |[D1] | => A simplified loop<L>
> > // | | \ |
> > // | | [D2] [B1] |
> > // -|- - |- - |- -
> > // | | |
> > // \ | /
> > // \ | /
> > // [ DF1 ] => exit block for loop<L>
> > //
> > // Consider the above scenario, there're two dead blocks [D1] + [D2], and
> > // a live block[B1]([D1][D2][B1] in one loop). [DF1] will a common dominance
> > // frontier block for [D1] and [D2], so Candidates will contain two elements
> > // [D1, D2]. [D1] has two succs: [D2] and [DF1], for [D1],first llvm::any_of
> > // will return true, so splitCriticalEdges will be called.
> > //
> > // - - - - - - - -
> > // | ... ... |
> > // |[D1] | => A simplified loop<L>
> > // | | \ |
> > // | | [D2] [B1] |
> > // -|- - |- - |- -
> > // | | |
> > // | \ /
> > // [D3] [DF2] => New Exit block[DF2] for loop<L>
> > // \ |
> > // [ DF1 ]
> > //
> > // After splitCriticalEdges, a new crit_edge block[D3] will be created, and
> > // it is added into dead blocks. Also, a new loop exit block[DF2] is created
> > // to preserve LoopSimplify. After that, llvm::any_of will return false for
> > // [D2], and we need to add [DF2] as a new dominance frontier block for [D2]
> > // since it has a pred edge to non-dead block[B1], and then, we need to
> > // update the undef incoming value for phi in [DF2].
> > ```
> >
> > About the purpose of the intersection of non-dead succs and preds, we should consider another case like below:
> >
> > ```
> > // - - - - - - - -
> > // | ... ... |
> > // | ... ... | => A simplified loop<L>
> > // | [ D1 ] [B1] |
> > // -|- |- |- -|- -
> > // | | | |
> > // | | | |
> > // \ | | |
> > // \ | / /
> > // [ DF1 ] => exit block for loop<L>
> > //
> > // After split edeges,
> > // - - - - - - - -
> > // | ... ... |
> > // | ... ... | => A simplified loop<L>
> > // | [ D1 ] [B1]|
> > // -|- |- |- -|--
> > // | | | |
> > // | | \ /
> > // [D2][D3][DF2] => New Exit block[DF2] for loop<L>
> > // \ | /
> > // [ DF1 ]
> > //
> > // switch-case: [ D1 ] will have multi edges to [ DF1 ], after split, [D2][D3] are new created,
> > // both will be dead blocks, so we need to use the intersection of non-dead succs and preds to get [DF2]
> > ```
> Thank you for adding the example in code, that's very helpful!
>
> I may be missing some details in the last example, but it seems like the non-dead predecessors for B (the IDF block) should cover the new blocks? (`[D2]`and `[D3]` should be in DeadBlocks following the split).
yeah, the non-dead predecessors for B should cover the new blocks, there're some mistakes for the diagram after split edeges, the right one should be:
```
// After split edeges,
// - - - - - - - -
// | ... ... |
// | ... ... | => A simplified loop<L>
// | [ D1 ] [B1]|
// -|- |- |- -|--
// | | | |
// | \ | /
// [D2] [DF2] => New Exit block[DF2] for loop<L>
// \ |
// [ DF1 ]
```
[D2] and [DF2] are both new created blocks after split edeges,[D2] will be a dead block while [DF2] will be a new IDF block
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87293/new/
https://reviews.llvm.org/D87293
More information about the llvm-commits
mailing list