[Mesa-dev] NIR constant problem for GPU which doesn't have native integer support

[Jason Ekstrand]

On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin <imirkin at alum.mit.edu> wrote:

> Have a look at the first 4 patches in the series from Jonathan Marek
> to address some of these issues:
>
> https://patchwork.freedesktop.org/series/54295/
>
> Not sure exactly what state that work is in, but I've added Jonathan
> to CC, perhaps he can provide an update.
>
> Cheers,
>
>   -ilia
>
> On Wed, Jan 2, 2019 at 6:28 AM Qiang Yu <yuq825 at gmail.com> wrote:
> >
> > Hi guys,
> >
> > I found the problem with this test fragment shader when lima development:
> > uniform int color;
> > void main() {
> >     if (color > 1)
> >         gl_FragColor = vec4(1.0, 0.0, 0.0, 1);
> >     else
> >         gl_FragColor = vec4(0.0, 1.0, 0.0, 1);
> > }
> >
> > nir_print_shader output:
> > impl main {
> >         block block_0:
> >         /* preds: */
> >         vec1 32 ssa_0 = load_const (0x00000001 /* 0.000000 */)
> >         vec4 32 ssa_1 = load_const (0x3f800000 /* 1.000000 */,
> > 0x00000000 /* 0.000000 */, 0x00000000 /* 0.000000 */, 0x3f800000 /*
> > 1.000000 */)
> >         vec4 32 ssa_2 = load_const (0x00000000 /* 0.000000 */,
> > 0x3f800000 /* 1.000000 */, 0x00000000 /* 0.000000 */, 0x3f800000 /*
> > 1.000000 */)
> >         vec1 32 ssa_3 = load_const (0x00000000 /* 0.000000 */)
> >         vec1 32 ssa_4 = intrinsic load_uniform (ssa_3) (0, 1, 0) /*
> > base=0 */ /* range=1 */ /* component=0 */   /* color */
> >         vec1 32 ssa_5 = slt ssa_0, ssa_4
> >         vec1 32 ssa_6 = fnot ssa_5
> >         vec4 32 ssa_7 = bcsel ssa_6.xxxx, ssa_2, ssa_1
> >         intrinsic store_output (ssa_7, ssa_3) (0, 15, 0) /* base=0 */
> > /* wrmask=xyzw */ /* component=0 */       /* gl_FragColor */
> >         /* succs: block_1 */
> >         block block_1:
> > }
> >
> > ssa0 is not converted to float when glsl to nir. I see glsl_to_nir.cpp
> > will create flt/ilt/ult
> > based on source type for gpu support native integer, but for gpu not
> > support native
> > integer, just create slt for all source type. And in
> > nir_lower_constant_initializers,
> > there's also no type conversion for integer constant.
>

This is a generally sticky issue.  In NIR, we have no concept of types on
SSA values which has proven perfectly reasonable and actually very powerful
in a world where integers are supported natively.  Unfortunately, it causes
significant problems for float-only architectures.  There are two general
possible solutions:

 1. convert all integers to floats in glsl_to_nir and prog_to_nir and
adjust various lowering/optimization passes to handle
nir_compiler_options::supports_native_integers == false.

 2. Just allow integers all the way through until you get very close to the
end and then lower integers to floats at the last possible moment.

Both of these come with significant issues.  With the first approach, there
are potentially a lot of passes that will need to be adjusted and it's not
100% clear what to do with UBO offsets and indirect addressing;
fortunately, you should be able to disable most of those optimizations to
get going so it shouldn't be too bad.  The second would be less invasive to
NIR because it doesn't require modifying as many passes.  However, doing
such a lowering would be very tricky to get right primarily because of
constants.  With everything else, you can just sort of assume that inputs
are floats (you lowered, right?) and lower to produce a float output.  With
constants, however, you don't know whether or not it's an integer that
needs lowering.  We could, in theory, add an extra bit to load_const to
solve this problem but there are also significant problems with doing that
so it's not clear it's a good idea.

I think the patches from Marek (as indicated by ilia) attempt the first
approach.  If we can do it practically, my suspicion is that the first will
work better than the second.  However, it will take some experimentation in
order to actually determine that.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190102/69273d51/attachment.html>