[PATCH] D63489: [InstSimplify] LCSSA PHIs should not be simplified away

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 18 07:58:56 PDT 2019


lebedev.ri added a comment.

In D63489#1548451 <https://reviews.llvm.org/D63489#1548451>, @alex-t wrote:

> In D63489#1548288 <https://reviews.llvm.org/D63489#1548288>, @lebedev.ri wrote:
>
> > > This causes generation of incorrect code in AMDGPU backend.
> >
> > This sounds like some other check is missing elsewhere?
> >  What happens if you feed it such an ir as-if after this transform, but manually written?
> >  ("that will result in broken asm/crashes" is hopefully not the answer)
> >
> > That being said, why is `LCSSAPass` not sufficient?
> >  It's already supposed to undo transforms like this.
>
>
> It will result in syntactically correct asm and no crashes. In runtime we'll get incorrect result though :)
>  Adding LCSSA pass again later on is difficult in the sense of the pass dependencies.
>  So, it's better to fix the explicit bug in SimplifyPHI....


Aha, so it's not `-instsimplify` pass itself, but how it's used during transition into backend.

1. You certainly don't want to make this blacklist unconditional, it should still run when the `-instsimplify` pass itself is run. (+instsimplify test)
2. How does this affect other targets (backends)? Does this need some TLI hook?


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

https://reviews.llvm.org/D63489





More information about the llvm-commits mailing list