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

[Erik Faye-Lund]

On Fri, 2019-01-04 at 09:40 -0600, Jason Ekstrand wrote:
> On Fri, Jan 4, 2019 at 4:07 AM Erik Faye-Lund <
> erik.faye-l...@collabora.com> wrote:
> > On Thu, 2019-01-03 at 11:58 -0600, Jason Ekstrand wrote:
> > > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund <
> > > erik.faye-l...@collabora.com> wrote:
> > > > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> > > > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin <
> > imir...@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 
> > > > 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 (0x0001 /*
> > 0.00
> > > > */)
> > > > > > > vec4 32 ssa_1 = load_const (0x3f80 /*
> > 1.00
> > > > */,
> > > > > > > 0x /* 0.00 */, 0x /* 0.00 */,
> > > > 0x3f80
> > > > > > /*
> > > > > > > 1.00 */)
> > > > > > > vec4 32 ssa_2 = load_const (0x /*
> > 0.00
> > > > */,
> > > > > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */,
> > > > 0x3f80
> > > > > > /*
> > > > > > > 1.00 */)
> > > > > > > vec1 32 ssa_3 = load_const (0x /*
> > 0.00
> > > > */)
> > > > > > > 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., 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.
> > > > 
> > > > I would like to take this chance to say that this untyped SSA-
> > value
> > > > choice has lead to issues in both radeon_si (because LLVM
> > values
> > > > are
> > > > typed) and zink (similarly, because SPIR-V values are typed),
> > where
> > > > we
> > > > need to to bitcasts on every access because there's just not
> > enough
> > > > information available to emit variables with the right type.
> > > 
> > > I'm not sure if I agree that the two problems are the same or
> > not... 
> > > More on that in a bit.
> > >  
> > > > It took us a lot of time to realize that the meta-data from the
> > > > opcodes
> > > > doesn't *really* provide this, because the rest of nir doesn't
> > > > treat
> > > > values consistently. In fact, this feels arguably more like
> > buggy
> > > > behavior; why do we even have fmov when all of the time the
> > > > compiler
> > > > will emit imovs for floating-point values...? Or why do we have
> > > > bitcast
> > > 
> > > Why do we have different mov opcodes?  Because they have
> > different
> > > behavior in the presence of source/destination modifiers.
> > 
> > Is this general NIR-behavior (i.e will this be honored by constant
> > folding etc), or is it Intel specific? If it's NIR-behavior, is it
> > documented somewhere?
> 
> No, constant folding doesn't do modifiers.  I had completely
> forgotten about this fact until 

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

[Jason Ekstrand]

Qiang,

Sorry things got so side-tracked

Going back to the original conversation, I mentioned that there are two
general methods to solve this: Making glsl_to_nir generate int-free NIR and
doing some sort of lowering.  I believe Jonathan is attempting the first in
which case the patch you linked to below is what you want.  You likely want
more of the patches in his series as well.

If, on the other hand, someone wanted to attempt the lowering approach, I
just sent out something which is my idea of what an after-the-fact
type-assignment pass would look like.  Be warned that I haven't even
compiled it much less tested it but it should give some idea of what needs
to be done to figure out the types of SSA values.

--Jason

On Fri, Jan 4, 2019 at 9:03 PM Qiang Yu  wrote:

> Thanks for your guys' information, although it's much deeper than my
> question and also my understanding of the NIR and compiler knowledge.
>
> This patch fix my problem:
> https://patchwork.freedesktop.org/patch/268946/
> But seems it's not the final solution, right? And I'm not sure if we will
> have other integer problem besides constant for non-integer-gpu, do we?
>
> To be honest, as a backend developer, NIR non-integer-gpu support is
> out of my ability. So maybe I have to take this patch temporarily and work
> on other topic for sometime to see if any progress of this support in NIR
> in the future.
>
> Thanks,
> Qiang
>
> On Sat, Jan 5, 2019 at 12:40 AM Ilia Mirkin  wrote:
> >
> > On Fri, Jan 4, 2019 at 10:46 AM Jason Ekstrand 
> wrote:
> > >
> > > On Fri, Jan 4, 2019 at 4:07 AM Erik Faye-Lund <
> erik.faye-l...@collabora.com> wrote:
> > >>
> > >> On Thu, 2019-01-03 at 11:58 -0600, Jason Ekstrand wrote:
> > >> > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund <
> > >> > erik.faye-l...@collabora.com> wrote:
> > >> > > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> > >> > > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin <
> imir...@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 
> > >> > > 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 (0x0001 /* 0.00
> > >> > > */)
> > >> > > > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00
> > >> > > */,
> > >> > > > > > 0x /* 0.00 */, 0x /* 0.00 */,
> > >> > > 0x3f80
> > >> > > > > /*
> > >> > > > > > 1.00 */)
> > >> > > > > > vec4 32 ssa_2 = load_const (0x /* 0.00
> > >> > > */,
> > >> > > > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */,
> > >> > > 0x3f80
> > >> > > > > /*
> > >> > > > > > 1.00 */)
> > >> > > > > > vec1 32 ssa_3 = load_const (0x /* 0.00
> > >> > > */)
> > >> > > > > > 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., 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 

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

[Qiang Yu]

Thanks for your guys' information, although it's much deeper than my
question and also my understanding of the NIR and compiler knowledge.

This patch fix my problem:
https://patchwork.freedesktop.org/patch/268946/
But seems it's not the final solution, right? And I'm not sure if we will
have other integer problem besides constant for non-integer-gpu, do we?

To be honest, as a backend developer, NIR non-integer-gpu support is
out of my ability. So maybe I have to take this patch temporarily and work
on other topic for sometime to see if any progress of this support in NIR
in the future.

Thanks,
Qiang

On Sat, Jan 5, 2019 at 12:40 AM Ilia Mirkin  wrote:
>
> On Fri, Jan 4, 2019 at 10:46 AM Jason Ekstrand  wrote:
> >
> > On Fri, Jan 4, 2019 at 4:07 AM Erik Faye-Lund 
> >  wrote:
> >>
> >> On Thu, 2019-01-03 at 11:58 -0600, Jason Ekstrand wrote:
> >> > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund <
> >> > erik.faye-l...@collabora.com> wrote:
> >> > > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> >> > > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
> >> > > > 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 
> >> > > 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 (0x0001 /* 0.00
> >> > > */)
> >> > > > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00
> >> > > */,
> >> > > > > > 0x /* 0.00 */, 0x /* 0.00 */,
> >> > > 0x3f80
> >> > > > > /*
> >> > > > > > 1.00 */)
> >> > > > > > vec4 32 ssa_2 = load_const (0x /* 0.00
> >> > > */,
> >> > > > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */,
> >> > > 0x3f80
> >> > > > > /*
> >> > > > > > 1.00 */)
> >> > > > > > vec1 32 ssa_3 = load_const (0x /* 0.00
> >> > > */)
> >> > > > > > 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., 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.
> >> > >
> >> > > I would like to take this chance to say that this untyped SSA-value
> >> > > choice has lead to issues in both radeon_si (because LLVM values
> >> > > are
> >> > > typed) and zink (similarly, because SPIR-V values are typed), where
> >> > > we
> >> > > need to to bitcasts on every access because there's just not enough
> >> > > information available to emit variables with the right type.
> >> >
> >> > I'm not sure if I agree that the two problems are the same or not...
> >> > More on that in a bit.
> >> >
> >> > > It took us a lot of time to realize that the meta-data from the
> >> > > opcodes
> >> > > doesn't *really* provide this, because the rest of nir doesn't
> >> > > treat
> >> > > values consistently. 

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

[Ilia Mirkin]

On Fri, Jan 4, 2019 at 10:46 AM Jason Ekstrand  wrote:
>
> On Fri, Jan 4, 2019 at 4:07 AM Erik Faye-Lund  
> wrote:
>>
>> On Thu, 2019-01-03 at 11:58 -0600, Jason Ekstrand wrote:
>> > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund <
>> > erik.faye-l...@collabora.com> wrote:
>> > > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
>> > > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
>> > > > 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 
>> > > 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 (0x0001 /* 0.00
>> > > */)
>> > > > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00
>> > > */,
>> > > > > > 0x /* 0.00 */, 0x /* 0.00 */,
>> > > 0x3f80
>> > > > > /*
>> > > > > > 1.00 */)
>> > > > > > vec4 32 ssa_2 = load_const (0x /* 0.00
>> > > */,
>> > > > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */,
>> > > 0x3f80
>> > > > > /*
>> > > > > > 1.00 */)
>> > > > > > vec1 32 ssa_3 = load_const (0x /* 0.00
>> > > */)
>> > > > > > 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., 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.
>> > >
>> > > I would like to take this chance to say that this untyped SSA-value
>> > > choice has lead to issues in both radeon_si (because LLVM values
>> > > are
>> > > typed) and zink (similarly, because SPIR-V values are typed), where
>> > > we
>> > > need to to bitcasts on every access because there's just not enough
>> > > information available to emit variables with the right type.
>> >
>> > I'm not sure if I agree that the two problems are the same or not...
>> > More on that in a bit.
>> >
>> > > It took us a lot of time to realize that the meta-data from the
>> > > opcodes
>> > > doesn't *really* provide this, because the rest of nir doesn't
>> > > treat
>> > > values consistently. In fact, this feels arguably more like buggy
>> > > behavior; why do we even have fmov when all of the time the
>> > > compiler
>> > > will emit imovs for floating-point values...? Or why do we have
>> > > bitcast
>> >
>> > Why do we have different mov opcodes?  Because they have different
>> > behavior in the presence of source/destination modifiers.
>>
>> Is this general NIR-behavior (i.e will this be honored by constant
>> folding etc), or is it Intel specific? If it's NIR-behavior, is it
>> documented somewhere?
>
>
> No, constant folding doesn't do modifiers.  I had completely forgotten about 
> this fact until we started this discussion.
>
>>
>> In either case, I have a feeling that this is a mis-design; the
>> modifiers apply to the operands, not the opcodes. It sounds a bit like
>> imov shouldn't have been an ALU instruction, but perhaps some other,
>> new 

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

[Jason Ekstrand]

On Fri, Jan 4, 2019 at 4:07 AM Erik Faye-Lund 
wrote:

> On Thu, 2019-01-03 at 11:58 -0600, Jason Ekstrand wrote:
> > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund <
> > erik.faye-l...@collabora.com> wrote:
> > > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> > > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
> > > > 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 
> > > 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 (0x0001 /* 0.00
> > > */)
> > > > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00
> > > */,
> > > > > > 0x /* 0.00 */, 0x /* 0.00 */,
> > > 0x3f80
> > > > > /*
> > > > > > 1.00 */)
> > > > > > vec4 32 ssa_2 = load_const (0x /* 0.00
> > > */,
> > > > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */,
> > > 0x3f80
> > > > > /*
> > > > > > 1.00 */)
> > > > > > vec1 32 ssa_3 = load_const (0x /* 0.00
> > > */)
> > > > > > 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., 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.
> > >
> > > I would like to take this chance to say that this untyped SSA-value
> > > choice has lead to issues in both radeon_si (because LLVM values
> > > are
> > > typed) and zink (similarly, because SPIR-V values are typed), where
> > > we
> > > need to to bitcasts on every access because there's just not enough
> > > information available to emit variables with the right type.
> >
> > I'm not sure if I agree that the two problems are the same or not...
> > More on that in a bit.
> >
> > > It took us a lot of time to realize that the meta-data from the
> > > opcodes
> > > doesn't *really* provide this, because the rest of nir doesn't
> > > treat
> > > values consistently. In fact, this feels arguably more like buggy
> > > behavior; why do we even have fmov when all of the time the
> > > compiler
> > > will emit imovs for floating-point values...? Or why do we have
> > > bitcast
> >
> > Why do we have different mov opcodes?  Because they have different
> > behavior in the presence of source/destination modifiers.
>
> Is this general NIR-behavior (i.e will this be honored by constant
> folding etc), or is it Intel specific? If it's NIR-behavior, is it
> documented somewhere?
>

No, constant folding doesn't do modifiers.  I had completely forgotten
about this fact until we started this discussion.


> In either case, I have a feeling that this is a mis-design; the
> modifiers apply to the operands, not the opcodes. It sounds a bit like
> imov shouldn't have been an ALU instruction, but perhaps some other,
> new type. Or perhaps the concept we have should have been a bit finer
> granularity. It's a bit hard to tell without more details, and I can't
> seem to find any on my own.
>

I'm very unclear about 

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

[Erik Faye-Lund]

On Thu, 2019-01-03 at 11:58 -0600, Jason Ekstrand wrote:
> On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund <
> erik.faye-l...@collabora.com> wrote:
> > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
> > > 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 
> > 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 (0x0001 /* 0.00
> > */)
> > > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00
> > */,
> > > > > 0x /* 0.00 */, 0x /* 0.00 */,
> > 0x3f80
> > > > /*
> > > > > 1.00 */)
> > > > > vec4 32 ssa_2 = load_const (0x /* 0.00
> > */,
> > > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */,
> > 0x3f80
> > > > /*
> > > > > 1.00 */)
> > > > > vec1 32 ssa_3 = load_const (0x /* 0.00
> > */)
> > > > > 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., 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.
> > 
> > I would like to take this chance to say that this untyped SSA-value
> > choice has lead to issues in both radeon_si (because LLVM values
> > are
> > typed) and zink (similarly, because SPIR-V values are typed), where
> > we
> > need to to bitcasts on every access because there's just not enough
> > information available to emit variables with the right type.
> 
> I'm not sure if I agree that the two problems are the same or not... 
> More on that in a bit.
>  
> > It took us a lot of time to realize that the meta-data from the
> > opcodes
> > doesn't *really* provide this, because the rest of nir doesn't
> > treat
> > values consistently. In fact, this feels arguably more like buggy
> > behavior; why do we even have fmov when all of the time the
> > compiler
> > will emit imovs for floating-point values...? Or why do we have
> > bitcast
> 
> Why do we have different mov opcodes?  Because they have different
> behavior in the presence of source/destination modifiers.

Is this general NIR-behavior (i.e will this be honored by constant
folding etc), or is it Intel specific? If it's NIR-behavior, is it
documented somewhere?

In either case, I have a feeling that this is a mis-design; the
modifiers apply to the operands, not the opcodes. It sounds a bit like
imov shouldn't have been an ALU instruction, but perhaps some other,
new type. Or perhaps the concept we have should have been a bit finer
granularity. It's a bit hard to tell without more details, and I can't
seem to find any on my own.

Generally, I wish we had some more of the details here written down.
Implementing support for NIR for a new architecture is currently quite
tricky, mostly because chasing down the details are hard.

> You likely don't use those in radeon or Zink but we do use them on
> Intel.  That being said, I've very seriously considered adding a
> generic nir_op_mov which is entirely typeless and doesn't support
> modifiers at all and make 

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

[Roland Scheidegger]

Am 03.01.19 um 20:50 schrieb Jason Ekstrand:
> > The problem you're describing is in converting from NIR to another IR,
> > not to hardware.  In LLVM they made a choice to put types on SSA
> values
> > but then to have the actual semantics be based on the instruction
> > itself.  This leads to lots of redundancy in the IR but also lots of
> > things you can validate which is kind-of nice.  Is that redundancy
> > really useful?  In our experience with NIR, we haven't found it to be
> > other than booleans (now sorted), this one constant issue, and
> > translating to IRs that have that redundancy.  Then why did they add
> > it?  I'm really not sure but it may, at least somewhat, be related to
> > the fact that they allow arrays and structs in their SSA values and so
> > need full types.  This is another design decision in LLVM which I find
> > highly questionable.  What you're is more-or-less that NIR should
> carry,
> > maintain, and validate extra useless information just so we can
> pass it
> > on to LLVM which is going to use it for what, exactly?  Sorry if I'm
> > extremely reluctant to make fairly fundamental changes to NIR with no
> > better reason than "LLVM did it that way".
> >
> > There's a second reason why I don't like the idea of putting types on
> > SSA values: It's extremely deceptive.  In SPIR-V you're allowed to
> do an
> > OpSConvert with a source that is an unsigned 16-bit integer and a
> > destination that is an unsigned 32-bit integer.  What happens?  Your
> > uint -> uint cast sign extends!  Yup That's what happens.  No
> joke. 
> > The same is true of signed vs. unsigned division or modulus.  The end
> > result is that the types in SPIR-V are useless and you can't actually
> > trust them for anything except bit-size and sometimes labelling
> > something as a float vs. int.
> This is really a decision of spir-v only though, llvm doesn't have that
> nonsense (maybe for making it easier to translate to other languages?) -
> there's just int and float types there, with no distinction between
> signed and unsigned.
> 
> 
> I think with SPIR-V you could probably just pick one and make everything
> either signed or unsigned.  I'm not immediately aware of any opcodes
> that require one signedness or the other; most just require an integer
> or require a float.  That said, this is SPIR-V so I'm not going to bet
> money on that...
>  
> 
> You are quite right though that float vs. int is somewhat redundant too
> due to the instructions indicating the type. I suppose there might be
> reasons why there's different types - hw may use different registers for
> instance (whereas of course noone in their right mind would use
> different registers for signed vs. unsigned ints), or there might be
> some real cost of bitcasts (at least bypass delays are common for cpus
> when transitioning values between int and float execution units). For
> instance, it makes a difference with x86 sse if you use float or int
> loads, which otherwise you couldn't indicate directly (llvm can optimize
> this into the right kind of load nowadays even if you use the wrong kind
> of variable for the load, e.g. int load then bitcast to float and do
> some float op will change it into a float load, but this is IIRC
> actually a pretty new ability, and possibly doesn't happen if you
> disable enough optimization passes).
> 
> 
> Having it for the purpose of register allocation makes sense in the CPU
> world.  In the GPU world, everything is typically designed float-first
> and I'm not aware of any hardware has separate int and float registers
> like x86 does.  That said, hardware changes over time and it's entirely
> possible that someone will decide that a heterogeneous register file is
> a good idea on a GPU.  (Technically, most GPUs do have flag regs or
> accumulators or something but it's not as bad as x86.)  Our of
> curiosity, do you (or anyone else) know if LLVM actually uses them for
> that purpose?  I could see that information getting lost in the back-end
> and them using something else to choose registers.
> 
> --Jason

For llvm with x86 sse, I don't think a variable being characterized as
float or int via bitcast actually makes a difference whatsoever (at
least not with optimization). llvm would determine if it's float or int
on its own (based on preceeding / succeeding instructions). For example,
as you might know, sse has both float and int logic ops, whereas llvm of
course does not - but it would still use float logic ops appropriately
(when the value is coming out of / going into a "true" float op),
despite that you have to cast it to int in llvm to do the logic op. (A
more interesting examples are actually shuffles, since again due to sse
being quite horrid there some are only directly possible with ints, some

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

[Jason Ekstrand]

On Thu, Jan 3, 2019 at 2:03 PM Bas Nieuwenhuizen 
wrote:

> On Thu, Jan 3, 2019 at 6:59 PM Jason Ekstrand 
> wrote:
> >
> > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund <
> erik.faye-l...@collabora.com> wrote:
> >>
> >> On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> >> > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
> >> > 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  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 (0x0001 /* 0.00 */)
> >> > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
> >> > > > 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80
> >> > > /*
> >> > > > 1.00 */)
> >> > > > vec4 32 ssa_2 = load_const (0x /* 0.00 */,
> >> > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80
> >> > > /*
> >> > > > 1.00 */)
> >> > > > vec1 32 ssa_3 = load_const (0x /* 0.00 */)
> >> > > > 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., 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.
> >>
> >> I would like to take this chance to say that this untyped SSA-value
> >> choice has lead to issues in both radeon_si (because LLVM values are
> >> typed) and zink (similarly, because SPIR-V values are typed), where we
> >> need to to bitcasts on every access because there's just not enough
> >> information available to emit variables with the right type.
> >
> >
> > I'm not sure if I agree that the two problems are the same or not...
> More on that in a bit.
> >
> >>
> >> It took us a lot of time to realize that the meta-data from the opcodes
> >> doesn't *really* provide this, because the rest of nir doesn't treat
> >> values consistently. In fact, this feels arguably more like buggy
> >> behavior; why do we even have fmov when all of the time the compiler
> >> will emit imovs for floating-point values...? Or why do we have bitcast
> >
> >
> > Why do we have different mov opcodes?  Because they have different
> behavior in the presence of source/destination modifiers.  You likely don't
> use those in radeon or Zink but we do use them on Intel.  That being said,
> I've very seriously considered adding a generic nir_op_mov which is
> entirely typeless and doesn't support modifiers at all and make most of
> core NIR use that.  For that matter, there's no real reason why we need
> fmov with modifiers at all when we could we could easily replace "ssa1 =
> fmov.sat |x|" with "ssa1 = fsat |x|" or "ssa1 = fabs.sat x".  If we had a
> generic nir_op_mov to deal with swizzling etc., the only thing having
> i/fmov would really accomplish is lowering the number of different ways you
> can write "fmov.sat |x|".  The only reason I haven't added this nir_op_mov
> opcode is because it's a pile of work and anyone who can't do integers can
> just implement imov as fmov and it's not a big deal.
>
> Is there anything blocking just 

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

[Bas Nieuwenhuizen]

On Thu, Jan 3, 2019 at 6:59 PM Jason Ekstrand  wrote:
>
> On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund  
> wrote:
>>
>> On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
>> > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
>> > 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  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 (0x0001 /* 0.00 */)
>> > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
>> > > > 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80
>> > > /*
>> > > > 1.00 */)
>> > > > vec4 32 ssa_2 = load_const (0x /* 0.00 */,
>> > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80
>> > > /*
>> > > > 1.00 */)
>> > > > vec1 32 ssa_3 = load_const (0x /* 0.00 */)
>> > > > 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., 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.
>>
>> I would like to take this chance to say that this untyped SSA-value
>> choice has lead to issues in both radeon_si (because LLVM values are
>> typed) and zink (similarly, because SPIR-V values are typed), where we
>> need to to bitcasts on every access because there's just not enough
>> information available to emit variables with the right type.
>
>
> I'm not sure if I agree that the two problems are the same or not...  More on 
> that in a bit.
>
>>
>> It took us a lot of time to realize that the meta-data from the opcodes
>> doesn't *really* provide this, because the rest of nir doesn't treat
>> values consistently. In fact, this feels arguably more like buggy
>> behavior; why do we even have fmov when all of the time the compiler
>> will emit imovs for floating-point values...? Or why do we have bitcast
>
>
> Why do we have different mov opcodes?  Because they have different behavior 
> in the presence of source/destination modifiers.  You likely don't use those 
> in radeon or Zink but we do use them on Intel.  That being said, I've very 
> seriously considered adding a generic nir_op_mov which is entirely typeless 
> and doesn't support modifiers at all and make most of core NIR use that.  For 
> that matter, there's no real reason why we need fmov with modifiers at all 
> when we could we could easily replace "ssa1 = fmov.sat |x|" with "ssa1 = fsat 
> |x|" or "ssa1 = fabs.sat x".  If we had a generic nir_op_mov to deal with 
> swizzling etc., the only thing having i/fmov would really accomplish is 
> lowering the number of different ways you can write "fmov.sat |x|".  The only 
> reason I haven't added this nir_op_mov opcode is because it's a pile of work 
> and anyone who can't do integers can just implement imov as fmov and it's not 
> a big deal.

Is there anything blocking just always using fmov and removing imov
from nir? Currently anyone that want to add a modifier to an imov can
just change the opcode to an fmov and add the modifier, is it not?
>
>>
>> I would really love it if we could at least consider making this "we
>> can just treat floats 

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

[Jason Ekstrand]

On Thu, Jan 3, 2019 at 1:37 PM Roland Scheidegger 
wrote:

> Am 03.01.19 um 18:58 schrieb Jason Ekstrand:
> > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund
> > mailto:erik.faye-l...@collabora.com>>
> wrote:
> >
> > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin  > >
> > > 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/
> > <
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F54295%2F=02%7C01%7Csroland%40vmware.com%7Cb5a1b18f854a4533274508d671a52647%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636821351481601457=RPh1ZhwqvjBP8eSmE9D%2BErOVgCcmb4kobJVy%2BpJeyIc%3D=0
> >
> > > >
> > > > 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  > > 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 (0x0001 /* 0.00 */)
> > > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
> > > > > 0x /* 0.00 */, 0x /* 0.00 */,
> 0x3f80
> > > > /*
> > > > > 1.00 */)
> > > > > vec4 32 ssa_2 = load_const (0x /* 0.00 */,
> > > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */,
> 0x3f80
> > > > /*
> > > > > 1.00 */)
> > > > > vec1 32 ssa_3 = load_const (0x /* 0.00 */)
> > > > > 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., 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.
> >
> > I would like to take this chance to say that this untyped SSA-value
> > choice has lead to issues in both radeon_si (because LLVM values are
> > typed) and zink (similarly, because SPIR-V values are typed), where
> we
> > need to to bitcasts on every access because there's just not enough
> > information available to emit variables with the right type.
> >
> >
> > I'm not sure if I agree that the two problems are the same or not...
> > More on that in a bit.
> >
> >
> > It took us a lot of time to realize that the meta-data from the
> opcodes
> > doesn't *really* provide this, because the rest of nir doesn't treat
> > values consistently. In fact, this feels arguably more like buggy
> > behavior; why do we even have fmov when all of the time the compiler
> > will emit imovs for floating-point values...? Or why do we have
> bitcast
> >
> >
> > Why do we have different mov opcodes?  Because they have different
> > behavior in the presence of source/destination modifiers.  You likely
> > don't use those in radeon or Zink but we do use them on Intel.  That
> > being said, I've very seriously considered adding a generic nir_op_mov
> > which is entirely typeless and doesn't support modifiers at 

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

[Roland Scheidegger]

Am 03.01.19 um 18:58 schrieb Jason Ekstrand:
> On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund
> mailto:erik.faye-l...@collabora.com>> wrote:
> 
> On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin  >
> > 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  > 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 (0x0001 /* 0.00 */)
> > > >         vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
> > > > 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80
> > > /*
> > > > 1.00 */)
> > > >         vec4 32 ssa_2 = load_const (0x /* 0.00 */,
> > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80
> > > /*
> > > > 1.00 */)
> > > >         vec1 32 ssa_3 = load_const (0x /* 0.00 */)
> > > >         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., 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.
> 
> I would like to take this chance to say that this untyped SSA-value
> choice has lead to issues in both radeon_si (because LLVM values are
> typed) and zink (similarly, because SPIR-V values are typed), where we
> need to to bitcasts on every access because there's just not enough
> information available to emit variables with the right type.
> 
> 
> I'm not sure if I agree that the two problems are the same or not... 
> More on that in a bit.
>  
> 
> It took us a lot of time to realize that the meta-data from the opcodes
> doesn't *really* provide this, because the rest of nir doesn't treat
> values consistently. In fact, this feels arguably more like buggy
> behavior; why do we even have fmov when all of the time the compiler
> will emit imovs for floating-point values...? Or why do we have bitcast
> 
> 
> Why do we have different mov opcodes?  Because they have different
> behavior in the presence of source/destination modifiers.  You likely
> don't use those in radeon or Zink but we do use them on Intel.  That
> being said, I've very seriously considered adding a generic nir_op_mov
> which is entirely typeless and doesn't support modifiers at all and make
> most of core NIR use that.  For that matter, there's no real reason why
> we need fmov with modifiers at all when we could we could easily replace
> "ssa1 = fmov.sat |x|" with "ssa1 = fsat |x|" or "ssa1 = fabs.sat x".  If
> we had a generic nir_op_mov to deal 

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

[Jason Ekstrand]

On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund 
wrote:

> On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
> > 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  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 (0x0001 /* 0.00 */)
> > > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
> > > > 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80
> > > /*
> > > > 1.00 */)
> > > > vec4 32 ssa_2 = load_const (0x /* 0.00 */,
> > > > 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80
> > > /*
> > > > 1.00 */)
> > > > vec1 32 ssa_3 = load_const (0x /* 0.00 */)
> > > > 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., 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.
>
> I would like to take this chance to say that this untyped SSA-value
> choice has lead to issues in both radeon_si (because LLVM values are
> typed) and zink (similarly, because SPIR-V values are typed), where we
> need to to bitcasts on every access because there's just not enough
> information available to emit variables with the right type.
>

I'm not sure if I agree that the two problems are the same or not...  More
on that in a bit.


> It took us a lot of time to realize that the meta-data from the opcodes
> doesn't *really* provide this, because the rest of nir doesn't treat
> values consistently. In fact, this feels arguably more like buggy
> behavior; why do we even have fmov when all of the time the compiler
> will emit imovs for floating-point values...? Or why do we have bitcast
>

Why do we have different mov opcodes?  Because they have different behavior
in the presence of source/destination modifiers.  You likely don't use
those in radeon or Zink but we do use them on Intel.  That being said, I've
very seriously considered adding a generic nir_op_mov which is entirely
typeless and doesn't support modifiers at all and make most of core NIR use
that.  For that matter, there's no real reason why we need fmov with
modifiers at all when we could we could easily replace "ssa1 = fmov.sat
|x|" with "ssa1 = fsat |x|" or "ssa1 = fabs.sat x".  If we had a generic
nir_op_mov to deal with swizzling etc., the only thing having i/fmov would
really accomplish is lowering the number of different ways you can write
"fmov.sat |x|".  The only reason I haven't added this nir_op_mov opcode is
because it's a pile of work and anyone who can't do integers can just
implement imov as fmov and it's not a big deal.


> I would really love it if we could at least consider making this "we
> can just treat floats as ints without bitcasts if we feel like it"-
> design optional for the backend somehow.
>
> I guess the assumption is that bitcasts are for free? They aren't once
> you have to emit them and have a back-end remove a bunch of redundant
> ones... We should already have all the information to trivially place
> casts for backends that care, yet we currently make it hard (unless
> your HW/backend happens to 

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

[Erik Faye-Lund]

On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote:
> On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin 
> 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  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 (0x0001 /* 0.00 */)
> > > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
> > > 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80
> > /*
> > > 1.00 */)
> > > vec4 32 ssa_2 = load_const (0x /* 0.00 */,
> > > 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80
> > /*
> > > 1.00 */)
> > > vec1 32 ssa_3 = load_const (0x /* 0.00 */)
> > > 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., 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.

I would like to take this chance to say that this untyped SSA-value
choice has lead to issues in both radeon_si (because LLVM values are
typed) and zink (similarly, because SPIR-V values are typed), where we
need to to bitcasts on every access because there's just not enough
information available to emit variables with the right type.

It took us a lot of time to realize that the meta-data from the opcodes
doesn't *really* provide this, because the rest of nir doesn't treat
values consistently. In fact, this feels arguably more like buggy
behavior; why do we even have fmov when all of the time the compiler
will emit imovs for floating-point values...? Or why do we have bitcast

I would really love it if we could at least consider making this "we
can just treat floats as ints without bitcasts if we feel like it"-
design optional for the backend somehow.

I guess the assumption is that bitcasts are for free? They aren't once
you have to emit them and have a back-end remove a bunch of redundant
ones... We should already have all the information to trivially place
casts for backends that care, yet we currently make it hard (unless
your HW/backend happens to be untyped)...

Is there some way we can perhaps improve this for backends that care?

>   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, 

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

[Ilia Mirkin]

On Wed, Jan 2, 2019 at 11:17 AM Jason Ekstrand  wrote:
>
> On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin  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  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 (0x0001 /* 0.00 */)
>> > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
>> > 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80 /*
>> > 1.00 */)
>> > vec4 32 ssa_2 = load_const (0x /* 0.00 */,
>> > 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80 /*
>> > 1.00 */)
>> > vec1 32 ssa_3 = load_const (0x /* 0.00 */)
>> > 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., 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.

Just a handful of thoughts, which I suspect Jason is already well
aware of, but I'll point them out anyways:

 - On nv30, uniform indirect addressing consumes a float value
directly. Other architectures have a dedicated "address register" used
for such things, which needs to be loaded with a float (but is
internally stored in an opaque manner). However direct addressing
takes an integer, and indirect addressing may be able to take a "bias"
integer offset.
 - All uniform values should be coming in as float, even if they say
"int" or "bool" on them -- this is what mesa core already does.
 - Bools (consumed by branches, etc) are awkward everywhere. I think
the recent rework to make them 1-bit generics enables the awkwardness
to be passed on to the backend where it belongs, which means everyone
can just handle it however they want.

The main point I'm trying to make is that there are only a handful of

Re: [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  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  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 (0x0001 /* 0.00 */)
> > vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
> > 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80 /*
> > 1.00 */)
> > vec4 32 ssa_2 = load_const (0x /* 0.00 */,
> > 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80 /*
> > 1.00 */)
> > vec1 32 ssa_3 = load_const (0x /* 0.00 */)
> > 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., 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Ilia Mirkin]

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  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 (0x0001 /* 0.00 */)
> vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
> 0x /* 0.00 */, 0x /* 0.00 */, 0x3f80 /*
> 1.00 */)
> vec4 32 ssa_2 = load_const (0x /* 0.00 */,
> 0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80 /*
> 1.00 */)
> vec1 32 ssa_3 = load_const (0x /* 0.00 */)
> 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., 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.
>
> Do you know how to fix this problem?
>
> Thanks,
> Qiang
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Qiang Yu]

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 (0x0001 /* 0.00 */)
vec4 32 ssa_1 = load_const (0x3f80 /* 1.00 */,
0x /* 0.00 */, 0x /* 0.00 */, 0x3f80 /*
1.00 */)
vec4 32 ssa_2 = load_const (0x /* 0.00 */,
0x3f80 /* 1.00 */, 0x /* 0.00 */, 0x3f80 /*
1.00 */)
vec1 32 ssa_3 = load_const (0x /* 0.00 */)
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., 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.

Do you know how to fix this problem?

Thanks,
Qiang
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev