[PATCH] D73815: AMDGPU: Fix divergence analysis of control flow intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

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

> In D73815#1857100 <https://reviews.llvm.org/D73815#1857100>, @arsenm wrote:
>
> > In D73815#1857088 <https://reviews.llvm.org/D73815#1857088>, @alex-t wrote:
> >
> > > I agree that current expensive walk on IR should be removed. 
> > >  I initially consider it as a temporary solution that allows to switch to the new concept 
> > >  of assignment correct register classes keeping in mind to eventually find another way to workaround CF results exceptions.
> > >
> > > What I conceptually don't like here is mixing the "divergence" and register class notions.
> > >  The mask produced by the CF intrinsics is always scalar - not same as always uniform.
> >
> >
> > Scalar is the only modeled concept of uniform we have. We don't currently try to model workgroup uniform, so these should be the same
>
>
> It should work if we can guarantee that the values produced by the CF intrinsics are only used by the another CF intrinsics.
>  I mean something like AND/OR the uniform mask with some divergent value and then feeding result to another CF...
>  but normally this should not happen.


In no context should this ever happen. The control flow intrinsics are not general use, and are just a hack to get some control flow representation through the DAG. We should not be trying to worry about handling some bug case where somehow a divergent operation was introduced between a control flow intrinsic and the user. That is a bug and it should break. We shouldn't need to bend over backwards to support this case.

> Once again: I like getting rid of requiresUniformRegister but insist on the extended testing.
>  Also, the trouble with LCSSA use of the uniform mask out of the divergent loop need to be solved anyway.

I've split that into an independent problem. The control flow intrinsics can still have correct divergence information without yet ripping out requiresUniformRegister


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

https://reviews.llvm.org/D73815