[PATCH] D64353: [AMDGPU] Run '' after isel to simplify PHIs.

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 17 12:17:14 PDT 2019


arsenm added a comment.

No matter what, running UnreachableMachineBlockElim is not a fix for this.

In D64353#1574085 <https://reviews.llvm.org/D64353#1574085>, @hliao wrote:

> In D64353#1574073 <https://reviews.llvm.org/D64353#1574073>, @arsenm wrote:
>
> > In D64353#1574005 <https://reviews.llvm.org/D64353#1574005>, @hliao wrote:
> >
> > > In D64353#1574004 <https://reviews.llvm.org/D64353#1574004>, @arsenm wrote:
> > >
> > > > In D64353#1573974 <https://reviews.llvm.org/D64353#1573974>, @hliao wrote:
> > > >
> > > > > In D64353#1573922 <https://reviews.llvm.org/D64353#1573922>, @arsenm wrote:
> > > > >
> > > > > > I think you should probably fix SILowerControlFlow
> > > > >
> > > > >
> > > > > the misplacement of def/use is not the fault of SILowerControlFlow but PHIEliminate. However, even PHIEliminate should not be blamed as cf.end is part of MBB prologue. cf.end should not take any input from PHI.
> > > > >
> > > > > In O2 <https://reviews.llvm.org/owners/package/2/> compilation, there won't be such issue as, in optimized RA, livevariable is used to eliminate PHI better. LiveVariables depends on unreachable-mbb-elimin, which remove the PHI to cf.end.
> > > >
> > > >
> > > > This isn't a property the pass should rely on. SILowerControlFlow should defend against unexpected iteration order and unreachable blocks
> > >
> > >
> > > any use before def is invalid MIR, right?
> >
> >
> > So the MIR is valid before phi elimination, and then becomes invalid after. Removing unreachable blocks earlier to avoid this happening is a workaround. It seems to me like we need to fix PHI elimination or avoid relying on the prolog instructions
>
>
> my understanding is that MBB prologue instructions are designed NOT to take PHI as input. Like these pseudo instructions for structurized CFG, by design, they don't take any PHI as input. As an alternative approach (not workaround), we could ensure these instructions not taking PHI as input before PHIElim instead of teaching PHIElim to under them, more or less very target-specific stuff.


The verifier should probably check this. We definitely should not hack around this by deleting unreachable blocks that happens to run into this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64353





More information about the llvm-commits mailing list