Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-09 Thread Jason Ekstrand
On Mon, Apr 9, 2018 at 4:41 PM, Bas Nieuwenhuizen 
wrote:

> On Tue, Apr 10, 2018 at 12:37 AM, Rob Clark  wrote:
> > On Mon, Apr 9, 2018 at 1:35 AM, Jason Ekstrand 
> wrote:
> >> Rather lively discussion we've got going here...
> >>
> >> On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:
> >>>
> >>> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
> >>>  wrote:
> >>> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark 
> wrote:
> >>> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
> >>> >>  wrote:
> >>> >>>
> >>> >>> You mean insert it into the fatptr every time deref_cast is called?
> >>> >>>
> >>> >>> Wouldn't that blow up the IR size significantly for very little
> >>> >>> benefit?
> >>> >>
> >>> >> in an easy to clean up way, so meh?
> >>> >
> >>> > We can't clean it up if we want to keep the information. Also nir is
> >>> > pretty slow to compile already, so I'd like not to add a significant
> >>> > number of instruction for very little benefit.
> >>
> >>
> >> Really?  I mean, I can believe it, but do you have any actual numbers to
> >> back this up?  It's considerably faster than some other IRs we have in
> mesa
> >> though they are known to be pretty big pigs if we're honest.  I'm very
> open
> >> to any suggestions on how to improve compile times.  If NIR is
> fundamentally
> >> a pig, we should fix that.
> >>
> >
> > just a side-note, I guess mostly obvious but just pointing it out
> > because it has caught others by surprise.  Debug mesa builds by
> > default run nir_validate after every pass (unless you NIR_VALIDATE=0).
> > And this adds a *lot* of overhead (for a *lot* of debugging benefit)..
> >
> > But if nir seems slow when running shader-db/etc, if you are using a
> > debug build at least make sure to NIR_VALIDATE=0 (or better yet use a
> > release build) when measuring performance
>
> Yeah I was aware of that. Given this discussions I've actually run
> some numbers for radv with a cold shader cache:
>
> time total: 76.507337 sec
> spirv 1.625971 sec
> nir_to_llvm 10.146195 sec
> llvm 46.058714 sec
>

Ok, that's more-or-less what I would have expected.  I'm a bit surprised
that spirv_to_nir is so expensive but there's some crazy juggling we have
to do in there.  We could probably improve it.


> hence total - spirv - nir_to_llvm - llvm = ~18.7 sec which is mostly due
> to nir.
>

Which is only 40% of the time you spend in LLVM. :-)  If you're letting NIR
optimize, I expect it to take some real time but it doesn't look too bad.

I'm a bit surprised how long nir_to_llvm takes though...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-09 Thread Bas Nieuwenhuizen
On Tue, Apr 10, 2018 at 12:37 AM, Rob Clark  wrote:
> On Mon, Apr 9, 2018 at 1:35 AM, Jason Ekstrand  wrote:
>> Rather lively discussion we've got going here...
>>
>> On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:
>>>
>>> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
>>>  wrote:
>>> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark  wrote:
>>> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
>>> >>  wrote:
>>> >>>
>>> >>> You mean insert it into the fatptr every time deref_cast is called?
>>> >>>
>>> >>> Wouldn't that blow up the IR size significantly for very little
>>> >>> benefit?
>>> >>
>>> >> in an easy to clean up way, so meh?
>>> >
>>> > We can't clean it up if we want to keep the information. Also nir is
>>> > pretty slow to compile already, so I'd like not to add a significant
>>> > number of instruction for very little benefit.
>>
>>
>> Really?  I mean, I can believe it, but do you have any actual numbers to
>> back this up?  It's considerably faster than some other IRs we have in mesa
>> though they are known to be pretty big pigs if we're honest.  I'm very open
>> to any suggestions on how to improve compile times.  If NIR is fundamentally
>> a pig, we should fix that.
>>
>
> just a side-note, I guess mostly obvious but just pointing it out
> because it has caught others by surprise.  Debug mesa builds by
> default run nir_validate after every pass (unless you NIR_VALIDATE=0).
> And this adds a *lot* of overhead (for a *lot* of debugging benefit)..
>
> But if nir seems slow when running shader-db/etc, if you are using a
> debug build at least make sure to NIR_VALIDATE=0 (or better yet use a
> release build) when measuring performance

Yeah I was aware of that. Given this discussions I've actually run
some numbers for radv with a cold shader cache:

time total: 76.507337 sec
spirv 1.625971 sec
nir_to_llvm 10.146195 sec
llvm 46.058714 sec

hence total - spirv - nir_to_llvm - llvm = ~18.7 sec which is mostly due to nir.

Honestly a lot less than I expected, looks like the LLVM stuff is
spread over more functions and hence the nir stuff is higher in my
profiles.
>
> BR,
> -R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-09 Thread Rob Clark
On Mon, Apr 9, 2018 at 1:35 AM, Jason Ekstrand  wrote:
> Rather lively discussion we've got going here...
>
> On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:
>>
>> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
>>  wrote:
>> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark  wrote:
>> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
>> >>  wrote:
>> >>>
>> >>> You mean insert it into the fatptr every time deref_cast is called?
>> >>>
>> >>> Wouldn't that blow up the IR size significantly for very little
>> >>> benefit?
>> >>
>> >> in an easy to clean up way, so meh?
>> >
>> > We can't clean it up if we want to keep the information. Also nir is
>> > pretty slow to compile already, so I'd like not to add a significant
>> > number of instruction for very little benefit.
>
>
> Really?  I mean, I can believe it, but do you have any actual numbers to
> back this up?  It's considerably faster than some other IRs we have in mesa
> though they are known to be pretty big pigs if we're honest.  I'm very open
> to any suggestions on how to improve compile times.  If NIR is fundamentally
> a pig, we should fix that.
>

just a side-note, I guess mostly obvious but just pointing it out
because it has caught others by surprise.  Debug mesa builds by
default run nir_validate after every pass (unless you NIR_VALIDATE=0).
And this adds a *lot* of overhead (for a *lot* of debugging benefit)..

But if nir seems slow when running shader-db/etc, if you are using a
debug build at least make sure to NIR_VALIDATE=0 (or better yet use a
release build) when measuring performance

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


Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-09 Thread Rob Clark
On Mon, Apr 9, 2018 at 10:25 AM, Jason Ekstrand  wrote:
> On Mon, Apr 9, 2018 at 5:35 AM, Rob Clark  wrote:
>>
>> On Mon, Apr 9, 2018 at 1:35 AM, Jason Ekstrand 
>> wrote:
>> > Rather lively discussion we've got going here...
>> >
>> > On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:
>> >>
>> >> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
>> >>  wrote:
>> >> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark 
>> >> > wrote:
>> >> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
>> >> >>  wrote:
>> >> >>> +
>> >> >>> +   /** The mode of the underlying variable */
>> >> >>> +   nir_variable_mode mode;
>> >> >>
>> >> >> In fact, it seems like deref->mode is unused outside of
>> >> >> nir_print and
>> >> >> nir_validate.. for logical addressing we can get the mode
>> >> >> from
>> >> >> the
>> >> >> deref_var->var at the start of the chain, and deref->mode
>> >> >> has
>> >> >> no
>> >> >> meaning for physical addressing (where the mode comes from
>> >> >> the
>> >> >> pointer).
>> >> >>
>> >> >> So maybe just drop deref->mode?
>> >> >
>> >> > Isn't it still useful with logical addressing in case a var
>> >> > is
>> >> > not
>> >> > immediately available? (think VK_KHR_variable_pointers)
>> >
>> >
>> > Yes, the idea here is to basically be the NIR equivalent of storage
>> > classes.
>> > For cases where you can chase it all the way back to the variable, this
>> > is
>> > pointless.  For cases where you can't, this can be very useful.
>> >
>> >>
>> >>  not sure, maybe this should just also use fat-pointers like
>> >>  physical
>> >>  addressing does??
>> >
>> >
>> > I *think* (without knowing the future with perfect accuracy) that I'd
>> > like
>> > us to start moving away from fat pointers in SPIR-V -> NIR.  They've
>> > been
>> > working ok thus far but we already have had complaints from the radv
>> > guys
>> > that they don't want fat pointers for SLM and I'm not sure there that
>> > great
>> > of a fit for other things.  The big reason is that it's actually fairly
>> > painful to change from one fat pointer scheme to another if whatever the
>> > spirv_to_nir authors picked doesn't really fit your hardware.
>> >
>> > I'm not 100% sure what to do but the idea I've got at the moment is
>> > something like this:
>> >
>> >  1) For every mode (or storage class), the driver provides to
>> > spirv_to_nir a
>> > glsl_type which is the type it wants the pointer represented as.
>> >  2) Add a deref_to_pointer intrinsic to convert the deref to that type
>> > and
>> > use casts to convert the fat pointer to a deref again
>> >
>> > Why the deref_to_pointer intrinsic?  Because right now some annoying
>> > details
>> > about nir_intrinsic_info force us to have all drefs have a single
>> > component.
>> > If we didn't, then nir_intrinsic_store_var would have two variable-size
>> > sources which aren't the same size.  We could modify the rules and have
>> > a
>> > concept of a "deref source" and to nir_intrinsic_info and say that a
>> > deref
>> > source can be any size.  That would also work and may actually be a
>> > better
>> > plan.  I'm open to other suggestions as well.
>> >
>> > The key point, however, is (1) where we just let the driver choose the
>> > storage type of pointers and then they can have whatever lowering pass
>> > they
>> > want.  If that's a "standard" fat-pointers pass in nir_lower_io, so be
>> > it.
>> > If they want to translate directly into LLVM derefs, they can do that
>> > too, I
>> > suppose.
>> >
>> >> > Also I could see this being useful in physical addressing too
>> >> > to
>> >> > avoid
>> >> > all passes working with derefs needing to do the constant
>> >> > folding?
>> >
>> >
>> > Constant folding is cheap, so I'm not too worried about that.  The
>> > bigger
>> > thing that actual derefs gain us is the ability of the compiler to
>> > reason
>> > about derefs.  Suppose, for instance, that you had the following:
>> >
>> > struct S {
>> >float a;
>> >float b;
>> > };
>> >
>> > location(set = 0, binding = 0) Block {
>> >S arr[10];
>> > } foo;
>> >
>> > void
>> > my_func(int i)
>> > {
>> >foo.arr[i].a = make_a_value();
>> >foo.arr[i].b = make_another_value();
>> >use_value(foo.arr[i].a);
>> > }
>> >
>> > If everything is lowered to fat pointers, the compiler can't very easily
>> > tell that the second line of my_func() didn't stomp foo.arr[i].a and so
>> > it
>> > has to re-load the value in the third line.  With derefs, we can easily
>> > see
>> > that the second line didn't stomp it and just re-use the re-use the
>> > result
>> > of the make_a_value() call.  Yes, you may be able to tell the difference
>> > between foo.arr[i].a and foo.arr[i].b with some fancy analysis and
>> > arithmetic tracking but it'd be very painful to do.

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-09 Thread Jason Ekstrand
On Mon, Apr 9, 2018 at 5:35 AM, Rob Clark  wrote:

> On Mon, Apr 9, 2018 at 1:35 AM, Jason Ekstrand 
> wrote:
> > Rather lively discussion we've got going here...
> >
> > On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:
> >>
> >> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
> >>  wrote:
> >> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark 
> wrote:
> >> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
> >> >>  wrote:
> >> >>> +
> >> >>> +   /** The mode of the underlying variable */
> >> >>> +   nir_variable_mode mode;
> >> >>
> >> >> In fact, it seems like deref->mode is unused outside of
> >> >> nir_print and
> >> >> nir_validate.. for logical addressing we can get the mode
> from
> >> >> the
> >> >> deref_var->var at the start of the chain, and deref->mode has
> >> >> no
> >> >> meaning for physical addressing (where the mode comes from
> the
> >> >> pointer).
> >> >>
> >> >> So maybe just drop deref->mode?
> >> >
> >> > Isn't it still useful with logical addressing in case a var is
> >> > not
> >> > immediately available? (think VK_KHR_variable_pointers)
> >
> >
> > Yes, the idea here is to basically be the NIR equivalent of storage
> classes.
> > For cases where you can chase it all the way back to the variable, this
> is
> > pointless.  For cases where you can't, this can be very useful.
> >
> >>
> >>  not sure, maybe this should just also use fat-pointers like
> >>  physical
> >>  addressing does??
> >
> >
> > I *think* (without knowing the future with perfect accuracy) that I'd
> like
> > us to start moving away from fat pointers in SPIR-V -> NIR.  They've been
> > working ok thus far but we already have had complaints from the radv guys
> > that they don't want fat pointers for SLM and I'm not sure there that
> great
> > of a fit for other things.  The big reason is that it's actually fairly
> > painful to change from one fat pointer scheme to another if whatever the
> > spirv_to_nir authors picked doesn't really fit your hardware.
> >
> > I'm not 100% sure what to do but the idea I've got at the moment is
> > something like this:
> >
> >  1) For every mode (or storage class), the driver provides to
> spirv_to_nir a
> > glsl_type which is the type it wants the pointer represented as.
> >  2) Add a deref_to_pointer intrinsic to convert the deref to that type
> and
> > use casts to convert the fat pointer to a deref again
> >
> > Why the deref_to_pointer intrinsic?  Because right now some annoying
> details
> > about nir_intrinsic_info force us to have all drefs have a single
> component.
> > If we didn't, then nir_intrinsic_store_var would have two variable-size
> > sources which aren't the same size.  We could modify the rules and have a
> > concept of a "deref source" and to nir_intrinsic_info and say that a
> deref
> > source can be any size.  That would also work and may actually be a
> better
> > plan.  I'm open to other suggestions as well.
> >
> > The key point, however, is (1) where we just let the driver choose the
> > storage type of pointers and then they can have whatever lowering pass
> they
> > want.  If that's a "standard" fat-pointers pass in nir_lower_io, so be
> it.
> > If they want to translate directly into LLVM derefs, they can do that
> too, I
> > suppose.
> >
> >> > Also I could see this being useful in physical addressing too
> to
> >> > avoid
> >> > all passes working with derefs needing to do the constant
> >> > folding?
> >
> >
> > Constant folding is cheap, so I'm not too worried about that.  The bigger
> > thing that actual derefs gain us is the ability of the compiler to reason
> > about derefs.  Suppose, for instance, that you had the following:
> >
> > struct S {
> >float a;
> >float b;
> > };
> >
> > location(set = 0, binding = 0) Block {
> >S arr[10];
> > } foo;
> >
> > void
> > my_func(int i)
> > {
> >foo.arr[i].a = make_a_value();
> >foo.arr[i].b = make_another_value();
> >use_value(foo.arr[i].a);
> > }
> >
> > If everything is lowered to fat pointers, the compiler can't very easily
> > tell that the second line of my_func() didn't stomp foo.arr[i].a and so
> it
> > has to re-load the value in the third line.  With derefs, we can easily
> see
> > that the second line didn't stomp it and just re-use the re-use the
> result
> > of the make_a_value() call.  Yes, you may be able to tell the difference
> > between foo.arr[i].a and foo.arr[i].b with some fancy analysis and
> > arithmetic tracking but it'd be very painful to do.  I've given this
> > substantial thought for almost two years now and haven't actually come up
> > with a good way to do it without the information provided by derefs.
> >
> > The biggest thing that I think we will gain from deref instructions isn't
> > fancy syntax sugar and better nir_printability of pointers.  It al

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-09 Thread Rob Clark
On Mon, Apr 9, 2018 at 1:35 AM, Jason Ekstrand  wrote:
> Rather lively discussion we've got going here...
>
> On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:
>>
>> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
>>  wrote:
>> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark  wrote:
>> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
>> >>  wrote:
>> >>> +
>> >>> +   /** The mode of the underlying variable */
>> >>> +   nir_variable_mode mode;
>> >>
>> >> In fact, it seems like deref->mode is unused outside of
>> >> nir_print and
>> >> nir_validate.. for logical addressing we can get the mode from
>> >> the
>> >> deref_var->var at the start of the chain, and deref->mode has
>> >> no
>> >> meaning for physical addressing (where the mode comes from the
>> >> pointer).
>> >>
>> >> So maybe just drop deref->mode?
>> >
>> > Isn't it still useful with logical addressing in case a var is
>> > not
>> > immediately available? (think VK_KHR_variable_pointers)
>
>
> Yes, the idea here is to basically be the NIR equivalent of storage classes.
> For cases where you can chase it all the way back to the variable, this is
> pointless.  For cases where you can't, this can be very useful.
>
>>
>>  not sure, maybe this should just also use fat-pointers like
>>  physical
>>  addressing does??
>
>
> I *think* (without knowing the future with perfect accuracy) that I'd like
> us to start moving away from fat pointers in SPIR-V -> NIR.  They've been
> working ok thus far but we already have had complaints from the radv guys
> that they don't want fat pointers for SLM and I'm not sure there that great
> of a fit for other things.  The big reason is that it's actually fairly
> painful to change from one fat pointer scheme to another if whatever the
> spirv_to_nir authors picked doesn't really fit your hardware.
>
> I'm not 100% sure what to do but the idea I've got at the moment is
> something like this:
>
>  1) For every mode (or storage class), the driver provides to spirv_to_nir a
> glsl_type which is the type it wants the pointer represented as.
>  2) Add a deref_to_pointer intrinsic to convert the deref to that type and
> use casts to convert the fat pointer to a deref again
>
> Why the deref_to_pointer intrinsic?  Because right now some annoying details
> about nir_intrinsic_info force us to have all drefs have a single component.
> If we didn't, then nir_intrinsic_store_var would have two variable-size
> sources which aren't the same size.  We could modify the rules and have a
> concept of a "deref source" and to nir_intrinsic_info and say that a deref
> source can be any size.  That would also work and may actually be a better
> plan.  I'm open to other suggestions as well.
>
> The key point, however, is (1) where we just let the driver choose the
> storage type of pointers and then they can have whatever lowering pass they
> want.  If that's a "standard" fat-pointers pass in nir_lower_io, so be it.
> If they want to translate directly into LLVM derefs, they can do that too, I
> suppose.
>
>> > Also I could see this being useful in physical addressing too to
>> > avoid
>> > all passes working with derefs needing to do the constant
>> > folding?
>
>
> Constant folding is cheap, so I'm not too worried about that.  The bigger
> thing that actual derefs gain us is the ability of the compiler to reason
> about derefs.  Suppose, for instance, that you had the following:
>
> struct S {
>float a;
>float b;
> };
>
> location(set = 0, binding = 0) Block {
>S arr[10];
> } foo;
>
> void
> my_func(int i)
> {
>foo.arr[i].a = make_a_value();
>foo.arr[i].b = make_another_value();
>use_value(foo.arr[i].a);
> }
>
> If everything is lowered to fat pointers, the compiler can't very easily
> tell that the second line of my_func() didn't stomp foo.arr[i].a and so it
> has to re-load the value in the third line.  With derefs, we can easily see
> that the second line didn't stomp it and just re-use the re-use the result
> of the make_a_value() call.  Yes, you may be able to tell the difference
> between foo.arr[i].a and foo.arr[i].b with some fancy analysis and
> arithmetic tracking but it'd be very painful to do.  I've given this
> substantial thought for almost two years now and haven't actually come up
> with a good way to do it without the information provided by derefs.
>
> The biggest thing that I think we will gain from deref instructions isn't
> fancy syntax sugar and better nir_printability of pointers.  It also isn't
> the removal of explicit of fat pointers in spirv_to_nir (thought that is
> also nice).  The biggest thing I'm going for is improving NIR's ability to
> optimize UBO, SSBO, and SLM access.  We're pretty good today for local
> variables (though there are a couple of known places for improvements) but

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-09 Thread Rob Clark
On Mon, Apr 9, 2018 at 1:35 AM, Jason Ekstrand  wrote:
> Rather lively discussion we've got going here...
>
> On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:
>>
>> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
>>  wrote:
>> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark  wrote:
>> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
>> >>  wrote:
>> >>> +
>> >>> +   /** The mode of the underlying variable */
>> >>> +   nir_variable_mode mode;
>> >>
>> >> In fact, it seems like deref->mode is unused outside of
>> >> nir_print and
>> >> nir_validate.. for logical addressing we can get the mode from
>> >> the
>> >> deref_var->var at the start of the chain, and deref->mode has
>> >> no
>> >> meaning for physical addressing (where the mode comes from the
>> >> pointer).
>> >>
>> >> So maybe just drop deref->mode?
>> >
>> > Isn't it still useful with logical addressing in case a var is
>> > not
>> > immediately available? (think VK_KHR_variable_pointers)
>
>
> Yes, the idea here is to basically be the NIR equivalent of storage classes.
> For cases where you can chase it all the way back to the variable, this is
> pointless.  For cases where you can't, this can be very useful.
>
>>
>>  not sure, maybe this should just also use fat-pointers like
>>  physical
>>  addressing does??
>
>
> I *think* (without knowing the future with perfect accuracy) that I'd like
> us to start moving away from fat pointers in SPIR-V -> NIR.  They've been
> working ok thus far but we already have had complaints from the radv guys
> that they don't want fat pointers for SLM and I'm not sure there that great
> of a fit for other things.  The big reason is that it's actually fairly
> painful to change from one fat pointer scheme to another if whatever the
> spirv_to_nir authors picked doesn't really fit your hardware.
>
> I'm not 100% sure what to do but the idea I've got at the moment is
> something like this:
>
>  1) For every mode (or storage class), the driver provides to spirv_to_nir a
> glsl_type which is the type it wants the pointer represented as.
>  2) Add a deref_to_pointer intrinsic to convert the deref to that type and
> use casts to convert the fat pointer to a deref again
>
> Why the deref_to_pointer intrinsic?  Because right now some annoying details
> about nir_intrinsic_info force us to have all drefs have a single component.
> If we didn't, then nir_intrinsic_store_var would have two variable-size
> sources which aren't the same size.  We could modify the rules and have a
> concept of a "deref source" and to nir_intrinsic_info and say that a deref
> source can be any size.  That would also work and may actually be a better
> plan.  I'm open to other suggestions as well.
>
> The key point, however, is (1) where we just let the driver choose the
> storage type of pointers and then they can have whatever lowering pass they
> want.  If that's a "standard" fat-pointers pass in nir_lower_io, so be it.
> If they want to translate directly into LLVM derefs, they can do that too, I
> suppose.
>
>> > Also I could see this being useful in physical addressing too to
>> > avoid
>> > all passes working with derefs needing to do the constant
>> > folding?
>
>
> Constant folding is cheap, so I'm not too worried about that.  The bigger
> thing that actual derefs gain us is the ability of the compiler to reason
> about derefs.  Suppose, for instance, that you had the following:
>
> struct S {
>float a;
>float b;
> };
>
> location(set = 0, binding = 0) Block {
>S arr[10];
> } foo;
>
> void
> my_func(int i)
> {
>foo.arr[i].a = make_a_value();
>foo.arr[i].b = make_another_value();
>use_value(foo.arr[i].a);
> }
>
> If everything is lowered to fat pointers, the compiler can't very easily
> tell that the second line of my_func() didn't stomp foo.arr[i].a and so it
> has to re-load the value in the third line.  With derefs, we can easily see
> that the second line didn't stomp it and just re-use the re-use the result
> of the make_a_value() call.  Yes, you may be able to tell the difference
> between foo.arr[i].a and foo.arr[i].b with some fancy analysis and
> arithmetic tracking but it'd be very painful to do.  I've given this
> substantial thought for almost two years now and haven't actually come up
> with a good way to do it without the information provided by derefs.

note here that fat pointers are *only* for *pointers*, not for
anything to do with logical addressing.  And that for pointers, fat
pointers are way easier to CF and CSE/DCE than some magic trick like
stashing local in 0x (and private somewhere else, I
guess).  (Not to mention the pain that is when you have to do math in
get_io_offset())

I've thought of various alternatives to try to avoid fat pointers.
They are all worse.  Sorry.

BR,

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Jason Ekstrand
Rather lively discussion we've got going here...

On Sun, Apr 8, 2018 at 3:23 PM, Rob Clark  wrote:

> On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
>  wrote:
> > On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark  wrote:
> >> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
> >>  wrote:
> >>> +
> >>> +   /** The mode of the underlying variable */
> >>> +   nir_variable_mode mode;
> >>
> >> In fact, it seems like deref->mode is unused outside of
> nir_print and
> >> nir_validate.. for logical addressing we can get the mode from
> the
> >> deref_var->var at the start of the chain, and deref->mode has no
> >> meaning for physical addressing (where the mode comes from the
> >> pointer).
> >>
> >> So maybe just drop deref->mode?
> >
> > Isn't it still useful with logical addressing in case a var is
> not
> > immediately available? (think VK_KHR_variable_pointers)
>

Yes, the idea here is to basically be the NIR equivalent of storage
classes.  For cases where you can chase it all the way back to the
variable, this is pointless.  For cases where you can't, this can be very
useful.


>  not sure, maybe this should just also use fat-pointers like
> physical
>  addressing does??
>

I *think* (without knowing the future with perfect accuracy) that I'd like
us to start moving away from fat pointers in SPIR-V -> NIR.  They've been
working ok thus far but we already have had complaints from the radv guys
that they don't want fat pointers for SLM and I'm not sure there that great
of a fit for other things.  The big reason is that it's actually fairly
painful to change from one fat pointer scheme to another if whatever the
spirv_to_nir authors picked doesn't really fit your hardware.

I'm not 100% sure what to do but the idea I've got at the moment is
something like this:

 1) For every mode (or storage class), the driver provides to spirv_to_nir
a glsl_type which is the type it wants the pointer represented as.
 2) Add a deref_to_pointer intrinsic to convert the deref to that type and
use casts to convert the fat pointer to a deref again

Why the deref_to_pointer intrinsic?  Because right now some annoying
details about nir_intrinsic_info force us to have all drefs have a single
component.  If we didn't, then nir_intrinsic_store_var would have two
variable-size sources which aren't the same size.  We could modify the
rules and have a concept of a "deref source" and to nir_intrinsic_info and
say that a deref source can be any size.  That would also work and may
actually be a better plan.  I'm open to other suggestions as well.

The key point, however, is (1) where we just let the driver choose the
storage type of pointers and then they can have whatever lowering pass they
want.  If that's a "standard" fat-pointers pass in nir_lower_io, so be it.
If they want to translate directly into LLVM derefs, they can do that too,
I suppose.

> Also I could see this being useful in physical addressing too to
> avoid
> > all passes working with derefs needing to do the constant
> folding?
>

Constant folding is cheap, so I'm not too worried about that.  The bigger
thing that actual derefs gain us is the ability of the compiler to reason
about derefs.  Suppose, for instance, that you had the following:

struct S {
   float a;
   float b;
};

location(set = 0, binding = 0) Block {
   S arr[10];
} foo;

void
my_func(int i)
{
   foo.arr[i].a = make_a_value();
   foo.arr[i].b = make_another_value();
   use_value(foo.arr[i].a);
}

If everything is lowered to fat pointers, the compiler can't very easily
tell that the second line of my_func() didn't stomp foo.arr[i].a and so it
has to re-load the value in the third line.  With derefs, we can easily see
that the second line didn't stomp it and just re-use the re-use the result
of the make_a_value() call.  Yes, you may be able to tell the difference
between foo.arr[i].a and foo.arr[i].b with some fancy analysis and
arithmetic tracking but it'd be very painful to do.  I've given this
substantial thought for almost two years now and haven't actually come up
with a good way to do it without the information provided by derefs.

The biggest thing that I think we will gain from deref instructions isn't
fancy syntax sugar and better nir_printability of pointers.  It also isn't
the removal of explicit of fat pointers in spirv_to_nir (thought that is
also nice).  The biggest thing I'm going for is improving NIR's ability to
optimize UBO, SSBO, and SLM access.  We're pretty good today for local
variables (though there are a couple of known places for improvements) but
we're utterly rubbish for anything that leaves the current shader
invocation.  If we want competent compute performance (which we do!) we
need to solve that problem.


>  The problem is that you don't necessarily know the type at compile
>  time (and in the case where you do, you need 

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Rob Clark
On Sun, Apr 8, 2018 at 5:54 PM, Bas Nieuwenhuizen
 wrote:
> On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark  wrote:
>> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
>>  wrote:
>>> +
>>> +   /** The mode of the underlying variable */
>>> +   nir_variable_mode mode;
>>
>> In fact, it seems like deref->mode is unused outside of nir_print and
>> nir_validate.. for logical addressing we can get the mode from the
>> deref_var->var at the start of the chain, and deref->mode has no
>> meaning for physical addressing (where the mode comes from the
>> pointer).
>>
>> So maybe just drop deref->mode?
>
> Isn't it still useful with logical addressing in case a var is not
> immediately available? (think VK_KHR_variable_pointers)

 not sure, maybe this should just also use fat-pointers like physical
 addressing does??

> Also I could see this being useful in physical addressing too to avoid
> all passes working with derefs needing to do the constant folding?

 The problem is that you don't necessarily know the type at compile
 time (and in the case where you do, you need to do constant folding to
 figure it out)
>>>
>>> So I have two considerations here
>>>
>>> 1) for vulkan you always know the mode, even when you don't know the 
>>> var.
>>> 2)  In CL the mode can still get annotated in the source program (CL C
>>> non-generic pointers) in cases in which we cannot reasonably figure it
>>> out with just constant folding. In those cases the mode is extra
>>> information that you really lose.
>>
>> so, even in cl 1.x, you could do things like 'somefxn(foo ? global_ptr
>> : local_ptr)'.. depending on how much we inline all the things, that
>> might not get CF'd away.
>>>
>>> How does this even work btw? somefxn has a definition, and the
>>> definition specifies a mode for the argument right? (which is
>>> implicitly __private if the app does not specify anything?)
>>
>> iirc, the cl spec has an example something along these lines..
>>
>> it doesn't require *physical* storage for anything where you don't
>> know what the ptr type is, however.. so fat ptrs in ssa space works
>> out
>>
>
> But something like
> __constant int *ptr_value = ...;
> store ptr in complex data structure.
> __constant int* ptr2 = load from complex data structure.
>
> Without explicitly annotating ptr2 it is unlikely that constant
> folding would find that ptr2 is pointing to __constant address space.
> Hence removing the modes loses valuable information that you cannot
> get back by constant folding. However, if you have a pointer with
> unknown mode, we could have a special mode (or mode_all?) and you can
> use the uvec2 representation in that case?

 hmm, I'm not really getting how deref->mode could magically have
 information that fatptr.y doesn't have.. if the mode is known, vtn
 could stash it in fatptr.y and everyone is happy?  If vtn doesn't know
 this, then I don't see how deref->mode helps..
>>>
>>> You mean insert it into the fatptr every time deref_cast is called?
>>>
>>> Wouldn't that blow up the IR size significantly for very little benefit?
>>
>> in an easy to clean up way, so meh?
>
> We can't clean it up if we want to keep the information. Also nir is
> pretty slow to compile already, so I'd like not to add a significant
> number of instruction for very little benefit.

I guess I'm failing to see what information you'd loose.. or how there
would be a problem..

I'm not strictly against having nir_var_unknown as a possible value
for deref->mode, but I'm not really seeing how that helps anything,
other than adding extra complexity to the IR..


>>
>> I think I'm leaning towards using fat ptrs for the vk case, since I
>> guess that is a case where you could always expect
>> nir_src_as_const_value() to work, to get the variable mode.  If for no
>> other reason than I guess these deref's, if the var is not known,
>> start w/ deref_cast, and it would be ugly for deref_cast to have to
>> work differently for compute vs vk.  But maybe Jason already has some
>> thoughts about it?
>
> I'd like to avoid fat pointers alltogether on AMD since we would not
> use it even for CL. a generic pointer is just a uint64_t for us, with
> no bitfield in there for the address space.
>
> I think we may need to think a bit more about representation however,
> as e.g. for AMD a pointer is typically 64-bits (but we can do e.g.
> 32-bits for known workgroup pointers), the current deref instructions
> return 32-bit, and you want something like a uvec2 as pointer
> representation?

 afaiu, newer AMD (and NV) hw can remap shared/private into a single
 global address space..  But I

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
On Sun, Apr 8, 2018 at 11:40 PM, Rob Clark  wrote:
> On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
>  wrote:
>> +
>> +   /** The mode of the underlying variable */
>> +   nir_variable_mode mode;
>
> In fact, it seems like deref->mode is unused outside of nir_print and
> nir_validate.. for logical addressing we can get the mode from the
> deref_var->var at the start of the chain, and deref->mode has no
> meaning for physical addressing (where the mode comes from the
> pointer).
>
> So maybe just drop deref->mode?

 Isn't it still useful with logical addressing in case a var is not
 immediately available? (think VK_KHR_variable_pointers)
>>>
>>> not sure, maybe this should just also use fat-pointers like physical
>>> addressing does??
>>>
 Also I could see this being useful in physical addressing too to avoid
 all passes working with derefs needing to do the constant folding?
>>>
>>> The problem is that you don't necessarily know the type at compile
>>> time (and in the case where you do, you need to do constant folding to
>>> figure it out)
>>
>> So I have two considerations here
>>
>> 1) for vulkan you always know the mode, even when you don't know the var.
>> 2)  In CL the mode can still get annotated in the source program (CL C
>> non-generic pointers) in cases in which we cannot reasonably figure it
>> out with just constant folding. In those cases the mode is extra
>> information that you really lose.
>
> so, even in cl 1.x, you could do things like 'somefxn(foo ? global_ptr
> : local_ptr)'.. depending on how much we inline all the things, that
> might not get CF'd away.
>>
>> How does this even work btw? somefxn has a definition, and the
>> definition specifies a mode for the argument right? (which is
>> implicitly __private if the app does not specify anything?)
>
> iirc, the cl spec has an example something along these lines..
>
> it doesn't require *physical* storage for anything where you don't
> know what the ptr type is, however.. so fat ptrs in ssa space works
> out
>

 But something like
 __constant int *ptr_value = ...;
 store ptr in complex data structure.
 __constant int* ptr2 = load from complex data structure.

 Without explicitly annotating ptr2 it is unlikely that constant
 folding would find that ptr2 is pointing to __constant address space.
 Hence removing the modes loses valuable information that you cannot
 get back by constant folding. However, if you have a pointer with
 unknown mode, we could have a special mode (or mode_all?) and you can
 use the uvec2 representation in that case?
>>>
>>> hmm, I'm not really getting how deref->mode could magically have
>>> information that fatptr.y doesn't have.. if the mode is known, vtn
>>> could stash it in fatptr.y and everyone is happy?  If vtn doesn't know
>>> this, then I don't see how deref->mode helps..
>>
>> You mean insert it into the fatptr every time deref_cast is called?
>>
>> Wouldn't that blow up the IR size significantly for very little benefit?
>
> in an easy to clean up way, so meh?

We can't clean it up if we want to keep the information. Also nir is
pretty slow to compile already, so I'd like not to add a significant
number of instruction for very little benefit.

>
>>
>>>
>
> I think I'm leaning towards using fat ptrs for the vk case, since I
> guess that is a case where you could always expect
> nir_src_as_const_value() to work, to get the variable mode.  If for no
> other reason than I guess these deref's, if the var is not known,
> start w/ deref_cast, and it would be ugly for deref_cast to have to
> work differently for compute vs vk.  But maybe Jason already has some
> thoughts about it?

 I'd like to avoid fat pointers alltogether on AMD since we would not
 use it even for CL. a generic pointer is just a uint64_t for us, with
 no bitfield in there for the address space.

 I think we may need to think a bit more about representation however,
 as e.g. for AMD a pointer is typically 64-bits (but we can do e.g.
 32-bits for known workgroup pointers), the current deref instructions
 return 32-bit, and you want something like a uvec2 as pointer
 representation?
>>>
>>> afaiu, newer AMD (and NV) hw can remap shared/private into a single
>>> global address space..  But I guess that is an easy subset of the
>>> harder case where drivers need to use different instructions.. so a
>>> pretty simple lowering pass run before lower_io could remap things
>>> that use fatptrs into something that ignores fatptr.y.  Then opt
>>> passes make fatptr.y go away.  So both AMD and hw that doesn't have a
>>> flat address space are happy.
>>
>> But then you run into other issues, like how are you going to stuff a
>> 64

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Rob Clark
On Sun, Apr 8, 2018 at 5:20 PM, Bas Nieuwenhuizen
 wrote:
> +
> +   /** The mode of the underlying variable */
> +   nir_variable_mode mode;

 In fact, it seems like deref->mode is unused outside of nir_print and
 nir_validate.. for logical addressing we can get the mode from the
 deref_var->var at the start of the chain, and deref->mode has no
 meaning for physical addressing (where the mode comes from the
 pointer).

 So maybe just drop deref->mode?
>>>
>>> Isn't it still useful with logical addressing in case a var is not
>>> immediately available? (think VK_KHR_variable_pointers)
>>
>> not sure, maybe this should just also use fat-pointers like physical
>> addressing does??
>>
>>> Also I could see this being useful in physical addressing too to avoid
>>> all passes working with derefs needing to do the constant folding?
>>
>> The problem is that you don't necessarily know the type at compile
>> time (and in the case where you do, you need to do constant folding to
>> figure it out)
>
> So I have two considerations here
>
> 1) for vulkan you always know the mode, even when you don't know the var.
> 2)  In CL the mode can still get annotated in the source program (CL C
> non-generic pointers) in cases in which we cannot reasonably figure it
> out with just constant folding. In those cases the mode is extra
> information that you really lose.

 so, even in cl 1.x, you could do things like 'somefxn(foo ? global_ptr
 : local_ptr)'.. depending on how much we inline all the things, that
 might not get CF'd away.
>
> How does this even work btw? somefxn has a definition, and the
> definition specifies a mode for the argument right? (which is
> implicitly __private if the app does not specify anything?)

iirc, the cl spec has an example something along these lines..

it doesn't require *physical* storage for anything where you don't
know what the ptr type is, however.. so fat ptrs in ssa space works
out

>>>
>>> But something like
>>> __constant int *ptr_value = ...;
>>> store ptr in complex data structure.
>>> __constant int* ptr2 = load from complex data structure.
>>>
>>> Without explicitly annotating ptr2 it is unlikely that constant
>>> folding would find that ptr2 is pointing to __constant address space.
>>> Hence removing the modes loses valuable information that you cannot
>>> get back by constant folding. However, if you have a pointer with
>>> unknown mode, we could have a special mode (or mode_all?) and you can
>>> use the uvec2 representation in that case?
>>
>> hmm, I'm not really getting how deref->mode could magically have
>> information that fatptr.y doesn't have.. if the mode is known, vtn
>> could stash it in fatptr.y and everyone is happy?  If vtn doesn't know
>> this, then I don't see how deref->mode helps..
>
> You mean insert it into the fatptr every time deref_cast is called?
>
> Wouldn't that blow up the IR size significantly for very little benefit?

in an easy to clean up way, so meh?

>
>>

 I think I'm leaning towards using fat ptrs for the vk case, since I
 guess that is a case where you could always expect
 nir_src_as_const_value() to work, to get the variable mode.  If for no
 other reason than I guess these deref's, if the var is not known,
 start w/ deref_cast, and it would be ugly for deref_cast to have to
 work differently for compute vs vk.  But maybe Jason already has some
 thoughts about it?
>>>
>>> I'd like to avoid fat pointers alltogether on AMD since we would not
>>> use it even for CL. a generic pointer is just a uint64_t for us, with
>>> no bitfield in there for the address space.
>>>
>>> I think we may need to think a bit more about representation however,
>>> as e.g. for AMD a pointer is typically 64-bits (but we can do e.g.
>>> 32-bits for known workgroup pointers), the current deref instructions
>>> return 32-bit, and you want something like a uvec2 as pointer
>>> representation?
>>
>> afaiu, newer AMD (and NV) hw can remap shared/private into a single
>> global address space..  But I guess that is an easy subset of the
>> harder case where drivers need to use different instructions.. so a
>> pretty simple lowering pass run before lower_io could remap things
>> that use fatptrs into something that ignores fatptr.y.  Then opt
>> passes make fatptr.y go away.  So both AMD and hw that doesn't have a
>> flat address space are happy.
>
> But then you run into other issues, like how are you going to stuff a
> 64-bit fatptr.x + a ?-bit fatptr.y into a 64-bit value for Physical64
> addressing? Also this means we have to track to the sources back to
> the cast/var any time we want to do anything at all with any deref
> which seems less efficient to me than just stuffing the deref in
> there.

so fat ptrs only have to exist in ssa space, not be stored to
som

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
On Sun, Apr 8, 2018 at 10:43 PM, Bas Nieuwenhuizen
 wrote:
> On Sun, Apr 8, 2018 at 10:23 PM, Bas Nieuwenhuizen
>  wrote:
>> On Tue, Apr 3, 2018 at 8:32 PM, Jason Ekstrand  wrote:
>>> This commit adds a new instruction type to NIR for handling derefs.
>>> Nothing uses it yet but this adds the data structure as well as all of
>>> the code to validate, print, clone, and [de]serialize them.
>>> ---
>>>  src/compiler/nir/nir.c| 50 +++
>>>  src/compiler/nir/nir.h| 58 -
>>>  src/compiler/nir/nir_clone.c  | 42 
>>>  src/compiler/nir/nir_instr_set.c  | 78 
>>> +
>>>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>>>  src/compiler/nir/nir_opt_dce.c|  7 +++
>>>  src/compiler/nir/nir_print.c  | 56 +
>>>  src/compiler/nir/nir_serialize.c  | 81 
>>> ++
>>>  src/compiler/nir/nir_validate.c   | 83 
>>> +++
>>>  9 files changed, 506 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
>>> index 8364197..a538f22 100644
>>> --- a/src/compiler/nir/nir.c
>>> +++ b/src/compiler/nir/nir.c
>>> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
>>> return instr;
>>>  }
>>>
>>> +nir_deref_instr *
>>> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
>>> +{
>>> +   nir_deref_instr *instr =
>>> +  rzalloc_size(shader, sizeof(nir_deref_instr));
>>> +
>>> +   instr_init(&instr->instr, nir_instr_type_deref);
>>> +
>>> +   instr->deref_type = deref_type;
>>> +   if (deref_type != nir_deref_type_var)
>>> +  src_init(&instr->parent);
>>> +
>>> +   if (deref_type == nir_deref_type_array)
>>> +  src_init(&instr->arr.index);
>>> +
>>> +   dest_init(&instr->dest);
>>> +
>>> +   return instr;
>>> +}
>>> +
>>>  nir_jump_instr *
>>>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>>>  {
>>> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
>>> nir_foreach_dest_cb cb, void *state)
>>>  }
>>>
>>>  static bool
>>> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
>>> *state)
>>> +{
>>> +   return cb(&instr->dest, state);
>>> +}
>>> +
>>> +static bool
>>>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>>>   void *state)
>>>  {
>>> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, 
>>> nir_foreach_dest_cb cb, void *state)
>>> switch (instr->type) {
>>> case nir_instr_type_alu:
>>>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
>>> +   case nir_instr_type_deref:
>>> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
>>> case nir_instr_type_intrinsic:
>>>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, 
>>> state);
>>> case nir_instr_type_tex:
>>> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
>>> nir_foreach_ssa_def_cb cb, void *state)
>>>  {
>>> switch (instr->type) {
>>> case nir_instr_type_alu:
>>> +   case nir_instr_type_deref:
>>> case nir_instr_type_tex:
>>> case nir_instr_type_intrinsic:
>>> case nir_instr_type_phi:
>>> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
>>> nir_foreach_src_cb cb, void *state)
>>>  }
>>>
>>>  static bool
>>> +visit_deref_instr_src(nir_deref_instr *instr,
>>> +  nir_foreach_src_cb cb, void *state)
>>> +{
>>> +   if (instr->deref_type != nir_deref_type_var) {
>>> +  if (!visit_src(&instr->parent, cb, state))
>>> + return false;
>>> +   }
>>> +
>>> +   if (instr->deref_type == nir_deref_type_array) {
>>> +  if (!visit_src(&instr->arr.index, cb, state))
>>> + return false;
>>> +   }
>>> +
>>> +   return true;
>>> +}
>>> +
>>> +static bool
>>>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>>>  {
>>> for (unsigned i = 0; i < instr->num_srcs; i++) {
>>> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb 
>>> cb, void *state)
>>>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>>>   return false;
>>>break;
>>> +   case nir_instr_type_deref:
>>> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
>>> + return false;
>>> +  break;
>>> case nir_instr_type_intrinsic:
>>>if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
>>>   return false;
>>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>>> index cc7c401..db7dc91 100644
>>> --- a/src/compiler/nir/nir.h
>>> +++ b/src/compiler/nir/nir.h
>>> @@ -427,6 +427,7 @@ typedef struct nir_register {
>>>
>>>  typedef enum {
>>> nir_instr_type_alu,
>>> +   nir_instr_type_deref,
>>> nir_instr_type_call,
>>> nir_instr_type_tex,
>>> nir_instr_type_intrinsic,
>>> @@

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
 +
 +   /** The mode of the underlying variable */
 +   nir_variable_mode mode;
>>>
>>> In fact, it seems like deref->mode is unused outside of nir_print and
>>> nir_validate.. for logical addressing we can get the mode from the
>>> deref_var->var at the start of the chain, and deref->mode has no
>>> meaning for physical addressing (where the mode comes from the
>>> pointer).
>>>
>>> So maybe just drop deref->mode?
>>
>> Isn't it still useful with logical addressing in case a var is not
>> immediately available? (think VK_KHR_variable_pointers)
>
> not sure, maybe this should just also use fat-pointers like physical
> addressing does??
>
>> Also I could see this being useful in physical addressing too to avoid
>> all passes working with derefs needing to do the constant folding?
>
> The problem is that you don't necessarily know the type at compile
> time (and in the case where you do, you need to do constant folding to
> figure it out)

 So I have two considerations here

 1) for vulkan you always know the mode, even when you don't know the var.
 2)  In CL the mode can still get annotated in the source program (CL C
 non-generic pointers) in cases in which we cannot reasonably figure it
 out with just constant folding. In those cases the mode is extra
 information that you really lose.
>>>
>>> so, even in cl 1.x, you could do things like 'somefxn(foo ? global_ptr
>>> : local_ptr)'.. depending on how much we inline all the things, that
>>> might not get CF'd away.

How does this even work btw? somefxn has a definition, and the
definition specifies a mode for the argument right? (which is
implicitly __private if the app does not specify anything?)

>>
>> But something like
>> __constant int *ptr_value = ...;
>> store ptr in complex data structure.
>> __constant int* ptr2 = load from complex data structure.
>>
>> Without explicitly annotating ptr2 it is unlikely that constant
>> folding would find that ptr2 is pointing to __constant address space.
>> Hence removing the modes loses valuable information that you cannot
>> get back by constant folding. However, if you have a pointer with
>> unknown mode, we could have a special mode (or mode_all?) and you can
>> use the uvec2 representation in that case?
>
> hmm, I'm not really getting how deref->mode could magically have
> information that fatptr.y doesn't have.. if the mode is known, vtn
> could stash it in fatptr.y and everyone is happy?  If vtn doesn't know
> this, then I don't see how deref->mode helps..

You mean insert it into the fatptr every time deref_cast is called?

Wouldn't that blow up the IR size significantly for very little benefit?

>
>>>
>>> I think I'm leaning towards using fat ptrs for the vk case, since I
>>> guess that is a case where you could always expect
>>> nir_src_as_const_value() to work, to get the variable mode.  If for no
>>> other reason than I guess these deref's, if the var is not known,
>>> start w/ deref_cast, and it would be ugly for deref_cast to have to
>>> work differently for compute vs vk.  But maybe Jason already has some
>>> thoughts about it?
>>
>> I'd like to avoid fat pointers alltogether on AMD since we would not
>> use it even for CL. a generic pointer is just a uint64_t for us, with
>> no bitfield in there for the address space.
>>
>> I think we may need to think a bit more about representation however,
>> as e.g. for AMD a pointer is typically 64-bits (but we can do e.g.
>> 32-bits for known workgroup pointers), the current deref instructions
>> return 32-bit, and you want something like a uvec2 as pointer
>> representation?
>
> afaiu, newer AMD (and NV) hw can remap shared/private into a single
> global address space..  But I guess that is an easy subset of the
> harder case where drivers need to use different instructions.. so a
> pretty simple lowering pass run before lower_io could remap things
> that use fatptrs into something that ignores fatptr.y.  Then opt
> passes make fatptr.y go away.  So both AMD and hw that doesn't have a
> flat address space are happy.

But then you run into other issues, like how are you going to stuff a
64-bit fatptr.x + a ?-bit fatptr.y into a 64-bit value for Physical64
addressing? Also this means we have to track to the sources back to
the cast/var any time we want to do anything at all with any deref
which seems less efficient to me than just stuffing the deref in
there.

Also, what would the something which ignores fatptr.y be? I'd assume
that would be the normal deref based stuff, but requiring fatptr
contradicts that?

>
> BR,
> -R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Rob Clark
On Sun, Apr 8, 2018 at 4:26 PM, Bas Nieuwenhuizen
 wrote:
> On Sun, Apr 8, 2018 at 6:06 PM, Rob Clark  wrote:
>> On Sun, Apr 8, 2018 at 11:15 AM, Bas Nieuwenhuizen
>>  wrote:
>>> On Sun, Apr 8, 2018 at 3:29 PM, Rob Clark  wrote:
 On Sun, Apr 8, 2018 at 8:58 AM, Bas Nieuwenhuizen
  wrote:
> On Sun, Apr 8, 2018 at 1:38 PM, Rob Clark  wrote:
>> On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  
>> wrote:
>>> This commit adds a new instruction type to NIR for handling derefs.
>>> Nothing uses it yet but this adds the data structure as well as all of
>>> the code to validate, print, clone, and [de]serialize them.
>>> ---
>>>  src/compiler/nir/nir.c| 50 +++
>>>  src/compiler/nir/nir.h| 58 -
>>>  src/compiler/nir/nir_clone.c  | 42 
>>>  src/compiler/nir/nir_instr_set.c  | 78 
>>> +
>>>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>>>  src/compiler/nir/nir_opt_dce.c|  7 +++
>>>  src/compiler/nir/nir_print.c  | 56 +
>>>  src/compiler/nir/nir_serialize.c  | 81 
>>> ++
>>>  src/compiler/nir/nir_validate.c   | 83 
>>> +++
>>>  9 files changed, 506 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
>>> index 8364197..a538f22 100644
>>> --- a/src/compiler/nir/nir.c
>>> +++ b/src/compiler/nir/nir.c
>>> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
>>> return instr;
>>>  }
>>>
>>> +nir_deref_instr *
>>> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
>>> +{
>>> +   nir_deref_instr *instr =
>>> +  rzalloc_size(shader, sizeof(nir_deref_instr));
>>> +
>>> +   instr_init(&instr->instr, nir_instr_type_deref);
>>> +
>>> +   instr->deref_type = deref_type;
>>> +   if (deref_type != nir_deref_type_var)
>>> +  src_init(&instr->parent);
>>> +
>>> +   if (deref_type == nir_deref_type_array)
>>> +  src_init(&instr->arr.index);
>>> +
>>> +   dest_init(&instr->dest);
>>> +
>>> +   return instr;
>>> +}
>>> +
>>>  nir_jump_instr *
>>>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>>>  {
>>> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
>>> nir_foreach_dest_cb cb, void *state)
>>>  }
>>>
>>>  static bool
>>> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
>>> *state)
>>> +{
>>> +   return cb(&instr->dest, state);
>>> +}
>>> +
>>> +static bool
>>>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb 
>>> cb,
>>>   void *state)
>>>  {
>>> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, 
>>> nir_foreach_dest_cb cb, void *state)
>>> switch (instr->type) {
>>> case nir_instr_type_alu:
>>>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
>>> +   case nir_instr_type_deref:
>>> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
>>> case nir_instr_type_intrinsic:
>>>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, 
>>> state);
>>> case nir_instr_type_tex:
>>> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
>>> nir_foreach_ssa_def_cb cb, void *state)
>>>  {
>>> switch (instr->type) {
>>> case nir_instr_type_alu:
>>> +   case nir_instr_type_deref:
>>> case nir_instr_type_tex:
>>> case nir_instr_type_intrinsic:
>>> case nir_instr_type_phi:
>>> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
>>> nir_foreach_src_cb cb, void *state)
>>>  }
>>>
>>>  static bool
>>> +visit_deref_instr_src(nir_deref_instr *instr,
>>> +  nir_foreach_src_cb cb, void *state)
>>> +{
>>> +   if (instr->deref_type != nir_deref_type_var) {
>>> +  if (!visit_src(&instr->parent, cb, state))
>>> + return false;
>>> +   }
>>> +
>>> +   if (instr->deref_type == nir_deref_type_array) {
>>> +  if (!visit_src(&instr->arr.index, cb, state))
>>> + return false;
>>> +   }
>>> +
>>> +   return true;
>>> +}
>>> +
>>> +static bool
>>>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>>>  {
>>> for (unsigned i = 0; i < instr->num_srcs; i++) {
>>> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, 
>>> nir_foreach_src_cb cb, void *state)
>>>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>>>   return false;
>>>b

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
On Sun, Apr 8, 2018 at 10:23 PM, Bas Nieuwenhuizen
 wrote:
> On Tue, Apr 3, 2018 at 8:32 PM, Jason Ekstrand  wrote:
>> This commit adds a new instruction type to NIR for handling derefs.
>> Nothing uses it yet but this adds the data structure as well as all of
>> the code to validate, print, clone, and [de]serialize them.
>> ---
>>  src/compiler/nir/nir.c| 50 +++
>>  src/compiler/nir/nir.h| 58 -
>>  src/compiler/nir/nir_clone.c  | 42 
>>  src/compiler/nir/nir_instr_set.c  | 78 +
>>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>>  src/compiler/nir/nir_opt_dce.c|  7 +++
>>  src/compiler/nir/nir_print.c  | 56 +
>>  src/compiler/nir/nir_serialize.c  | 81 
>> ++
>>  src/compiler/nir/nir_validate.c   | 83 
>> +++
>>  9 files changed, 506 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
>> index 8364197..a538f22 100644
>> --- a/src/compiler/nir/nir.c
>> +++ b/src/compiler/nir/nir.c
>> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
>> return instr;
>>  }
>>
>> +nir_deref_instr *
>> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
>> +{
>> +   nir_deref_instr *instr =
>> +  rzalloc_size(shader, sizeof(nir_deref_instr));
>> +
>> +   instr_init(&instr->instr, nir_instr_type_deref);
>> +
>> +   instr->deref_type = deref_type;
>> +   if (deref_type != nir_deref_type_var)
>> +  src_init(&instr->parent);
>> +
>> +   if (deref_type == nir_deref_type_array)
>> +  src_init(&instr->arr.index);
>> +
>> +   dest_init(&instr->dest);
>> +
>> +   return instr;
>> +}
>> +
>>  nir_jump_instr *
>>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>>  {
>> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
>> nir_foreach_dest_cb cb, void *state)
>>  }
>>
>>  static bool
>> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
>> *state)
>> +{
>> +   return cb(&instr->dest, state);
>> +}
>> +
>> +static bool
>>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>>   void *state)
>>  {
>> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, nir_foreach_dest_cb 
>> cb, void *state)
>> switch (instr->type) {
>> case nir_instr_type_alu:
>>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
>> +   case nir_instr_type_deref:
>> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
>> case nir_instr_type_intrinsic:
>>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, state);
>> case nir_instr_type_tex:
>> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
>> nir_foreach_ssa_def_cb cb, void *state)
>>  {
>> switch (instr->type) {
>> case nir_instr_type_alu:
>> +   case nir_instr_type_deref:
>> case nir_instr_type_tex:
>> case nir_instr_type_intrinsic:
>> case nir_instr_type_phi:
>> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
>> nir_foreach_src_cb cb, void *state)
>>  }
>>
>>  static bool
>> +visit_deref_instr_src(nir_deref_instr *instr,
>> +  nir_foreach_src_cb cb, void *state)
>> +{
>> +   if (instr->deref_type != nir_deref_type_var) {
>> +  if (!visit_src(&instr->parent, cb, state))
>> + return false;
>> +   }
>> +
>> +   if (instr->deref_type == nir_deref_type_array) {
>> +  if (!visit_src(&instr->arr.index, cb, state))
>> + return false;
>> +   }
>> +
>> +   return true;
>> +}
>> +
>> +static bool
>>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>>  {
>> for (unsigned i = 0; i < instr->num_srcs; i++) {
>> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb 
>> cb, void *state)
>>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>>   return false;
>>break;
>> +   case nir_instr_type_deref:
>> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
>> + return false;
>> +  break;
>> case nir_instr_type_intrinsic:
>>if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
>>   return false;
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index cc7c401..db7dc91 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -427,6 +427,7 @@ typedef struct nir_register {
>>
>>  typedef enum {
>> nir_instr_type_alu,
>> +   nir_instr_type_deref,
>> nir_instr_type_call,
>> nir_instr_type_tex,
>> nir_instr_type_intrinsic,
>> @@ -894,7 +895,9 @@ bool nir_alu_srcs_equal(const nir_alu_instr *alu1, const 
>> nir_alu_instr *alu2,
>>  typedef enum {
>> nir_deref_type_var,
>> nir_deref_type_array,
>> -   nir_deref_type_struct
>>

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
On Sun, Apr 8, 2018 at 6:06 PM, Rob Clark  wrote:
> On Sun, Apr 8, 2018 at 11:15 AM, Bas Nieuwenhuizen
>  wrote:
>> On Sun, Apr 8, 2018 at 3:29 PM, Rob Clark  wrote:
>>> On Sun, Apr 8, 2018 at 8:58 AM, Bas Nieuwenhuizen
>>>  wrote:
 On Sun, Apr 8, 2018 at 1:38 PM, Rob Clark  wrote:
> On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  
> wrote:
>> This commit adds a new instruction type to NIR for handling derefs.
>> Nothing uses it yet but this adds the data structure as well as all of
>> the code to validate, print, clone, and [de]serialize them.
>> ---
>>  src/compiler/nir/nir.c| 50 +++
>>  src/compiler/nir/nir.h| 58 -
>>  src/compiler/nir/nir_clone.c  | 42 
>>  src/compiler/nir/nir_instr_set.c  | 78 
>> +
>>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>>  src/compiler/nir/nir_opt_dce.c|  7 +++
>>  src/compiler/nir/nir_print.c  | 56 +
>>  src/compiler/nir/nir_serialize.c  | 81 
>> ++
>>  src/compiler/nir/nir_validate.c   | 83 
>> +++
>>  9 files changed, 506 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
>> index 8364197..a538f22 100644
>> --- a/src/compiler/nir/nir.c
>> +++ b/src/compiler/nir/nir.c
>> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
>> return instr;
>>  }
>>
>> +nir_deref_instr *
>> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
>> +{
>> +   nir_deref_instr *instr =
>> +  rzalloc_size(shader, sizeof(nir_deref_instr));
>> +
>> +   instr_init(&instr->instr, nir_instr_type_deref);
>> +
>> +   instr->deref_type = deref_type;
>> +   if (deref_type != nir_deref_type_var)
>> +  src_init(&instr->parent);
>> +
>> +   if (deref_type == nir_deref_type_array)
>> +  src_init(&instr->arr.index);
>> +
>> +   dest_init(&instr->dest);
>> +
>> +   return instr;
>> +}
>> +
>>  nir_jump_instr *
>>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>>  {
>> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
>> nir_foreach_dest_cb cb, void *state)
>>  }
>>
>>  static bool
>> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
>> *state)
>> +{
>> +   return cb(&instr->dest, state);
>> +}
>> +
>> +static bool
>>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>>   void *state)
>>  {
>> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, 
>> nir_foreach_dest_cb cb, void *state)
>> switch (instr->type) {
>> case nir_instr_type_alu:
>>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
>> +   case nir_instr_type_deref:
>> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
>> case nir_instr_type_intrinsic:
>>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, 
>> state);
>> case nir_instr_type_tex:
>> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
>> nir_foreach_ssa_def_cb cb, void *state)
>>  {
>> switch (instr->type) {
>> case nir_instr_type_alu:
>> +   case nir_instr_type_deref:
>> case nir_instr_type_tex:
>> case nir_instr_type_intrinsic:
>> case nir_instr_type_phi:
>> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
>> nir_foreach_src_cb cb, void *state)
>>  }
>>
>>  static bool
>> +visit_deref_instr_src(nir_deref_instr *instr,
>> +  nir_foreach_src_cb cb, void *state)
>> +{
>> +   if (instr->deref_type != nir_deref_type_var) {
>> +  if (!visit_src(&instr->parent, cb, state))
>> + return false;
>> +   }
>> +
>> +   if (instr->deref_type == nir_deref_type_array) {
>> +  if (!visit_src(&instr->arr.index, cb, state))
>> + return false;
>> +   }
>> +
>> +   return true;
>> +}
>> +
>> +static bool
>>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>>  {
>> for (unsigned i = 0; i < instr->num_srcs; i++) {
>> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, 
>> nir_foreach_src_cb cb, void *state)
>>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>>   return false;
>>break;
>> +   case nir_instr_type_deref:
>> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
>> + return false;
>> +  break;
>> case

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
On Tue, Apr 3, 2018 at 8:32 PM, Jason Ekstrand  wrote:
> This commit adds a new instruction type to NIR for handling derefs.
> Nothing uses it yet but this adds the data structure as well as all of
> the code to validate, print, clone, and [de]serialize them.
> ---
>  src/compiler/nir/nir.c| 50 +++
>  src/compiler/nir/nir.h| 58 -
>  src/compiler/nir/nir_clone.c  | 42 
>  src/compiler/nir/nir_instr_set.c  | 78 +
>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>  src/compiler/nir/nir_opt_dce.c|  7 +++
>  src/compiler/nir/nir_print.c  | 56 +
>  src/compiler/nir/nir_serialize.c  | 81 ++
>  src/compiler/nir/nir_validate.c   | 83 
> +++
>  9 files changed, 506 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index 8364197..a538f22 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
> return instr;
>  }
>
> +nir_deref_instr *
> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
> +{
> +   nir_deref_instr *instr =
> +  rzalloc_size(shader, sizeof(nir_deref_instr));
> +
> +   instr_init(&instr->instr, nir_instr_type_deref);
> +
> +   instr->deref_type = deref_type;
> +   if (deref_type != nir_deref_type_var)
> +  src_init(&instr->parent);
> +
> +   if (deref_type == nir_deref_type_array)
> +  src_init(&instr->arr.index);
> +
> +   dest_init(&instr->dest);
> +
> +   return instr;
> +}
> +
>  nir_jump_instr *
>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>  {
> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
> nir_foreach_dest_cb cb, void *state)
>  }
>
>  static bool
> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void *state)
> +{
> +   return cb(&instr->dest, state);
> +}
> +
> +static bool
>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>   void *state)
>  {
> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, nir_foreach_dest_cb 
> cb, void *state)
> switch (instr->type) {
> case nir_instr_type_alu:
>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
> +   case nir_instr_type_deref:
> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
> case nir_instr_type_intrinsic:
>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, state);
> case nir_instr_type_tex:
> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
> nir_foreach_ssa_def_cb cb, void *state)
>  {
> switch (instr->type) {
> case nir_instr_type_alu:
> +   case nir_instr_type_deref:
> case nir_instr_type_tex:
> case nir_instr_type_intrinsic:
> case nir_instr_type_phi:
> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, nir_foreach_src_cb 
> cb, void *state)
>  }
>
>  static bool
> +visit_deref_instr_src(nir_deref_instr *instr,
> +  nir_foreach_src_cb cb, void *state)
> +{
> +   if (instr->deref_type != nir_deref_type_var) {
> +  if (!visit_src(&instr->parent, cb, state))
> + return false;
> +   }
> +
> +   if (instr->deref_type == nir_deref_type_array) {
> +  if (!visit_src(&instr->arr.index, cb, state))
> + return false;
> +   }
> +
> +   return true;
> +}
> +
> +static bool
>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>  {
> for (unsigned i = 0; i < instr->num_srcs; i++) {
> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb 
> cb, void *state)
>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>   return false;
>break;
> +   case nir_instr_type_deref:
> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
> + return false;
> +  break;
> case nir_instr_type_intrinsic:
>if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
>   return false;
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index cc7c401..db7dc91 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -427,6 +427,7 @@ typedef struct nir_register {
>
>  typedef enum {
> nir_instr_type_alu,
> +   nir_instr_type_deref,
> nir_instr_type_call,
> nir_instr_type_tex,
> nir_instr_type_intrinsic,
> @@ -894,7 +895,9 @@ bool nir_alu_srcs_equal(const nir_alu_instr *alu1, const 
> nir_alu_instr *alu2,
>  typedef enum {
> nir_deref_type_var,
> nir_deref_type_array,
> -   nir_deref_type_struct
> +   nir_deref_type_array_wildcard,
> +   nir_deref_type_struct,
> +   nir_deref_type_cast,
>  } nir_deref_type;
>
>  typedef struct nir_deref {
> @@ -956,6 +959,56 @@ nir_deref_tail(nir_deref *deref)
>  t

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Rob Clark
On Sun, Apr 8, 2018 at 11:15 AM, Bas Nieuwenhuizen
 wrote:
> On Sun, Apr 8, 2018 at 3:29 PM, Rob Clark  wrote:
>> On Sun, Apr 8, 2018 at 8:58 AM, Bas Nieuwenhuizen
>>  wrote:
>>> On Sun, Apr 8, 2018 at 1:38 PM, Rob Clark  wrote:
 On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  
 wrote:
> This commit adds a new instruction type to NIR for handling derefs.
> Nothing uses it yet but this adds the data structure as well as all of
> the code to validate, print, clone, and [de]serialize them.
> ---
>  src/compiler/nir/nir.c| 50 +++
>  src/compiler/nir/nir.h| 58 -
>  src/compiler/nir/nir_clone.c  | 42 
>  src/compiler/nir/nir_instr_set.c  | 78 
> +
>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>  src/compiler/nir/nir_opt_dce.c|  7 +++
>  src/compiler/nir/nir_print.c  | 56 +
>  src/compiler/nir/nir_serialize.c  | 81 
> ++
>  src/compiler/nir/nir_validate.c   | 83 
> +++
>  9 files changed, 506 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index 8364197..a538f22 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
> return instr;
>  }
>
> +nir_deref_instr *
> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
> +{
> +   nir_deref_instr *instr =
> +  rzalloc_size(shader, sizeof(nir_deref_instr));
> +
> +   instr_init(&instr->instr, nir_instr_type_deref);
> +
> +   instr->deref_type = deref_type;
> +   if (deref_type != nir_deref_type_var)
> +  src_init(&instr->parent);
> +
> +   if (deref_type == nir_deref_type_array)
> +  src_init(&instr->arr.index);
> +
> +   dest_init(&instr->dest);
> +
> +   return instr;
> +}
> +
>  nir_jump_instr *
>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>  {
> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
> nir_foreach_dest_cb cb, void *state)
>  }
>
>  static bool
> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
> *state)
> +{
> +   return cb(&instr->dest, state);
> +}
> +
> +static bool
>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>   void *state)
>  {
> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, 
> nir_foreach_dest_cb cb, void *state)
> switch (instr->type) {
> case nir_instr_type_alu:
>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
> +   case nir_instr_type_deref:
> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
> case nir_instr_type_intrinsic:
>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, 
> state);
> case nir_instr_type_tex:
> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
> nir_foreach_ssa_def_cb cb, void *state)
>  {
> switch (instr->type) {
> case nir_instr_type_alu:
> +   case nir_instr_type_deref:
> case nir_instr_type_tex:
> case nir_instr_type_intrinsic:
> case nir_instr_type_phi:
> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
> nir_foreach_src_cb cb, void *state)
>  }
>
>  static bool
> +visit_deref_instr_src(nir_deref_instr *instr,
> +  nir_foreach_src_cb cb, void *state)
> +{
> +   if (instr->deref_type != nir_deref_type_var) {
> +  if (!visit_src(&instr->parent, cb, state))
> + return false;
> +   }
> +
> +   if (instr->deref_type == nir_deref_type_array) {
> +  if (!visit_src(&instr->arr.index, cb, state))
> + return false;
> +   }
> +
> +   return true;
> +}
> +
> +static bool
>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>  {
> for (unsigned i = 0; i < instr->num_srcs; i++) {
> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, 
> nir_foreach_src_cb cb, void *state)
>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>   return false;
>break;
> +   case nir_instr_type_deref:
> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
> + return false;
> +  break;
> case nir_instr_type_intrinsic:
>if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
>   return false;
> diff --git a/src/compiler/nir/nir.h

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
On Sun, Apr 8, 2018 at 3:29 PM, Rob Clark  wrote:
> On Sun, Apr 8, 2018 at 8:58 AM, Bas Nieuwenhuizen
>  wrote:
>> On Sun, Apr 8, 2018 at 1:38 PM, Rob Clark  wrote:
>>> On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  wrote:
 This commit adds a new instruction type to NIR for handling derefs.
 Nothing uses it yet but this adds the data structure as well as all of
 the code to validate, print, clone, and [de]serialize them.
 ---
  src/compiler/nir/nir.c| 50 +++
  src/compiler/nir/nir.h| 58 -
  src/compiler/nir/nir_clone.c  | 42 
  src/compiler/nir/nir_instr_set.c  | 78 
 +
  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
  src/compiler/nir/nir_opt_dce.c|  7 +++
  src/compiler/nir/nir_print.c  | 56 +
  src/compiler/nir/nir_serialize.c  | 81 
 ++
  src/compiler/nir/nir_validate.c   | 83 
 +++
  9 files changed, 506 insertions(+), 11 deletions(-)

 diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
 index 8364197..a538f22 100644
 --- a/src/compiler/nir/nir.c
 +++ b/src/compiler/nir/nir.c
 @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
 return instr;
  }

 +nir_deref_instr *
 +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
 +{
 +   nir_deref_instr *instr =
 +  rzalloc_size(shader, sizeof(nir_deref_instr));
 +
 +   instr_init(&instr->instr, nir_instr_type_deref);
 +
 +   instr->deref_type = deref_type;
 +   if (deref_type != nir_deref_type_var)
 +  src_init(&instr->parent);
 +
 +   if (deref_type == nir_deref_type_array)
 +  src_init(&instr->arr.index);
 +
 +   dest_init(&instr->dest);
 +
 +   return instr;
 +}
 +
  nir_jump_instr *
  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
  {
 @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
 nir_foreach_dest_cb cb, void *state)
  }

  static bool
 +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
 *state)
 +{
 +   return cb(&instr->dest, state);
 +}
 +
 +static bool
  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
   void *state)
  {
 @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, 
 nir_foreach_dest_cb cb, void *state)
 switch (instr->type) {
 case nir_instr_type_alu:
return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
 +   case nir_instr_type_deref:
 +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
 case nir_instr_type_intrinsic:
return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, 
 state);
 case nir_instr_type_tex:
 @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
 nir_foreach_ssa_def_cb cb, void *state)
  {
 switch (instr->type) {
 case nir_instr_type_alu:
 +   case nir_instr_type_deref:
 case nir_instr_type_tex:
 case nir_instr_type_intrinsic:
 case nir_instr_type_phi:
 @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
 nir_foreach_src_cb cb, void *state)
  }

  static bool
 +visit_deref_instr_src(nir_deref_instr *instr,
 +  nir_foreach_src_cb cb, void *state)
 +{
 +   if (instr->deref_type != nir_deref_type_var) {
 +  if (!visit_src(&instr->parent, cb, state))
 + return false;
 +   }
 +
 +   if (instr->deref_type == nir_deref_type_array) {
 +  if (!visit_src(&instr->arr.index, cb, state))
 + return false;
 +   }
 +
 +   return true;
 +}
 +
 +static bool
  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
  {
 for (unsigned i = 0; i < instr->num_srcs; i++) {
 @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, 
 nir_foreach_src_cb cb, void *state)
if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
   return false;
break;
 +   case nir_instr_type_deref:
 +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
 + return false;
 +  break;
 case nir_instr_type_intrinsic:
if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
   return false;
 diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
 index cc7c401..db7dc91 100644
 --- a/src/compiler/nir/nir.h
 +++ b/src/compiler/nir/nir.h
 @@ -427,6 +427,7 @@ typedef struct nir_register {


Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Rob Clark
On Sun, Apr 8, 2018 at 8:58 AM, Bas Nieuwenhuizen
 wrote:
> On Sun, Apr 8, 2018 at 1:38 PM, Rob Clark  wrote:
>> On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  wrote:
>>> This commit adds a new instruction type to NIR for handling derefs.
>>> Nothing uses it yet but this adds the data structure as well as all of
>>> the code to validate, print, clone, and [de]serialize them.
>>> ---
>>>  src/compiler/nir/nir.c| 50 +++
>>>  src/compiler/nir/nir.h| 58 -
>>>  src/compiler/nir/nir_clone.c  | 42 
>>>  src/compiler/nir/nir_instr_set.c  | 78 
>>> +
>>>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>>>  src/compiler/nir/nir_opt_dce.c|  7 +++
>>>  src/compiler/nir/nir_print.c  | 56 +
>>>  src/compiler/nir/nir_serialize.c  | 81 
>>> ++
>>>  src/compiler/nir/nir_validate.c   | 83 
>>> +++
>>>  9 files changed, 506 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
>>> index 8364197..a538f22 100644
>>> --- a/src/compiler/nir/nir.c
>>> +++ b/src/compiler/nir/nir.c
>>> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
>>> return instr;
>>>  }
>>>
>>> +nir_deref_instr *
>>> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
>>> +{
>>> +   nir_deref_instr *instr =
>>> +  rzalloc_size(shader, sizeof(nir_deref_instr));
>>> +
>>> +   instr_init(&instr->instr, nir_instr_type_deref);
>>> +
>>> +   instr->deref_type = deref_type;
>>> +   if (deref_type != nir_deref_type_var)
>>> +  src_init(&instr->parent);
>>> +
>>> +   if (deref_type == nir_deref_type_array)
>>> +  src_init(&instr->arr.index);
>>> +
>>> +   dest_init(&instr->dest);
>>> +
>>> +   return instr;
>>> +}
>>> +
>>>  nir_jump_instr *
>>>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>>>  {
>>> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
>>> nir_foreach_dest_cb cb, void *state)
>>>  }
>>>
>>>  static bool
>>> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
>>> *state)
>>> +{
>>> +   return cb(&instr->dest, state);
>>> +}
>>> +
>>> +static bool
>>>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>>>   void *state)
>>>  {
>>> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, 
>>> nir_foreach_dest_cb cb, void *state)
>>> switch (instr->type) {
>>> case nir_instr_type_alu:
>>>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
>>> +   case nir_instr_type_deref:
>>> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
>>> case nir_instr_type_intrinsic:
>>>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, 
>>> state);
>>> case nir_instr_type_tex:
>>> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
>>> nir_foreach_ssa_def_cb cb, void *state)
>>>  {
>>> switch (instr->type) {
>>> case nir_instr_type_alu:
>>> +   case nir_instr_type_deref:
>>> case nir_instr_type_tex:
>>> case nir_instr_type_intrinsic:
>>> case nir_instr_type_phi:
>>> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
>>> nir_foreach_src_cb cb, void *state)
>>>  }
>>>
>>>  static bool
>>> +visit_deref_instr_src(nir_deref_instr *instr,
>>> +  nir_foreach_src_cb cb, void *state)
>>> +{
>>> +   if (instr->deref_type != nir_deref_type_var) {
>>> +  if (!visit_src(&instr->parent, cb, state))
>>> + return false;
>>> +   }
>>> +
>>> +   if (instr->deref_type == nir_deref_type_array) {
>>> +  if (!visit_src(&instr->arr.index, cb, state))
>>> + return false;
>>> +   }
>>> +
>>> +   return true;
>>> +}
>>> +
>>> +static bool
>>>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>>>  {
>>> for (unsigned i = 0; i < instr->num_srcs; i++) {
>>> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb 
>>> cb, void *state)
>>>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>>>   return false;
>>>break;
>>> +   case nir_instr_type_deref:
>>> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
>>> + return false;
>>> +  break;
>>> case nir_instr_type_intrinsic:
>>>if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
>>>   return false;
>>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>>> index cc7c401..db7dc91 100644
>>> --- a/src/compiler/nir/nir.h
>>> +++ b/src/compiler/nir/nir.h
>>> @@ -427,6 +427,7 @@ typedef struct nir_register {
>>>
>>>  typedef enum {
>>> nir_instr_type_alu,
>>> +   nir_instr_type_deref,
>>> nir_instr_type_call,
>>> nir_instr_type_tex,
>>> nir_instr_type_intrinsic,
>>> @@ -894,7 +895

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Bas Nieuwenhuizen
On Sun, Apr 8, 2018 at 1:38 PM, Rob Clark  wrote:
> On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  wrote:
>> This commit adds a new instruction type to NIR for handling derefs.
>> Nothing uses it yet but this adds the data structure as well as all of
>> the code to validate, print, clone, and [de]serialize them.
>> ---
>>  src/compiler/nir/nir.c| 50 +++
>>  src/compiler/nir/nir.h| 58 -
>>  src/compiler/nir/nir_clone.c  | 42 
>>  src/compiler/nir/nir_instr_set.c  | 78 +
>>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>>  src/compiler/nir/nir_opt_dce.c|  7 +++
>>  src/compiler/nir/nir_print.c  | 56 +
>>  src/compiler/nir/nir_serialize.c  | 81 
>> ++
>>  src/compiler/nir/nir_validate.c   | 83 
>> +++
>>  9 files changed, 506 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
>> index 8364197..a538f22 100644
>> --- a/src/compiler/nir/nir.c
>> +++ b/src/compiler/nir/nir.c
>> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
>> return instr;
>>  }
>>
>> +nir_deref_instr *
>> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
>> +{
>> +   nir_deref_instr *instr =
>> +  rzalloc_size(shader, sizeof(nir_deref_instr));
>> +
>> +   instr_init(&instr->instr, nir_instr_type_deref);
>> +
>> +   instr->deref_type = deref_type;
>> +   if (deref_type != nir_deref_type_var)
>> +  src_init(&instr->parent);
>> +
>> +   if (deref_type == nir_deref_type_array)
>> +  src_init(&instr->arr.index);
>> +
>> +   dest_init(&instr->dest);
>> +
>> +   return instr;
>> +}
>> +
>>  nir_jump_instr *
>>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>>  {
>> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
>> nir_foreach_dest_cb cb, void *state)
>>  }
>>
>>  static bool
>> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void 
>> *state)
>> +{
>> +   return cb(&instr->dest, state);
>> +}
>> +
>> +static bool
>>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>>   void *state)
>>  {
>> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, nir_foreach_dest_cb 
>> cb, void *state)
>> switch (instr->type) {
>> case nir_instr_type_alu:
>>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
>> +   case nir_instr_type_deref:
>> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
>> case nir_instr_type_intrinsic:
>>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, state);
>> case nir_instr_type_tex:
>> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
>> nir_foreach_ssa_def_cb cb, void *state)
>>  {
>> switch (instr->type) {
>> case nir_instr_type_alu:
>> +   case nir_instr_type_deref:
>> case nir_instr_type_tex:
>> case nir_instr_type_intrinsic:
>> case nir_instr_type_phi:
>> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, 
>> nir_foreach_src_cb cb, void *state)
>>  }
>>
>>  static bool
>> +visit_deref_instr_src(nir_deref_instr *instr,
>> +  nir_foreach_src_cb cb, void *state)
>> +{
>> +   if (instr->deref_type != nir_deref_type_var) {
>> +  if (!visit_src(&instr->parent, cb, state))
>> + return false;
>> +   }
>> +
>> +   if (instr->deref_type == nir_deref_type_array) {
>> +  if (!visit_src(&instr->arr.index, cb, state))
>> + return false;
>> +   }
>> +
>> +   return true;
>> +}
>> +
>> +static bool
>>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>>  {
>> for (unsigned i = 0; i < instr->num_srcs; i++) {
>> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb 
>> cb, void *state)
>>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>>   return false;
>>break;
>> +   case nir_instr_type_deref:
>> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
>> + return false;
>> +  break;
>> case nir_instr_type_intrinsic:
>>if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
>>   return false;
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index cc7c401..db7dc91 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -427,6 +427,7 @@ typedef struct nir_register {
>>
>>  typedef enum {
>> nir_instr_type_alu,
>> +   nir_instr_type_deref,
>> nir_instr_type_call,
>> nir_instr_type_tex,
>> nir_instr_type_intrinsic,
>> @@ -894,7 +895,9 @@ bool nir_alu_srcs_equal(const nir_alu_instr *alu1, const 
>> nir_alu_instr *alu2,
>>  typedef enum {
>> nir_deref_type_var,
>> nir_deref_type_array,
>> -   nir_deref_type_struct
>> +   nir_

Re: [Mesa-dev] [PATCH v3 002/104] nir: Add a deref instruction type

2018-04-08 Thread Rob Clark
On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  wrote:
> This commit adds a new instruction type to NIR for handling derefs.
> Nothing uses it yet but this adds the data structure as well as all of
> the code to validate, print, clone, and [de]serialize them.
> ---
>  src/compiler/nir/nir.c| 50 +++
>  src/compiler/nir/nir.h| 58 -
>  src/compiler/nir/nir_clone.c  | 42 
>  src/compiler/nir/nir_instr_set.c  | 78 +
>  src/compiler/nir/nir_opt_copy_propagate.c | 62 +++
>  src/compiler/nir/nir_opt_dce.c|  7 +++
>  src/compiler/nir/nir_print.c  | 56 +
>  src/compiler/nir/nir_serialize.c  | 81 ++
>  src/compiler/nir/nir_validate.c   | 83 
> +++
>  9 files changed, 506 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index 8364197..a538f22 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -470,6 +470,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
> return instr;
>  }
>
> +nir_deref_instr *
> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
> +{
> +   nir_deref_instr *instr =
> +  rzalloc_size(shader, sizeof(nir_deref_instr));
> +
> +   instr_init(&instr->instr, nir_instr_type_deref);
> +
> +   instr->deref_type = deref_type;
> +   if (deref_type != nir_deref_type_var)
> +  src_init(&instr->parent);
> +
> +   if (deref_type == nir_deref_type_array)
> +  src_init(&instr->arr.index);
> +
> +   dest_init(&instr->dest);
> +
> +   return instr;
> +}
> +
>  nir_jump_instr *
>  nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
>  {
> @@ -1199,6 +1219,12 @@ visit_alu_dest(nir_alu_instr *instr, 
> nir_foreach_dest_cb cb, void *state)
>  }
>
>  static bool
> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void *state)
> +{
> +   return cb(&instr->dest, state);
> +}
> +
> +static bool
>  visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
>   void *state)
>  {
> @@ -1239,6 +1265,8 @@ nir_foreach_dest(nir_instr *instr, nir_foreach_dest_cb 
> cb, void *state)
> switch (instr->type) {
> case nir_instr_type_alu:
>return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
> +   case nir_instr_type_deref:
> +  return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
> case nir_instr_type_intrinsic:
>return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, state);
> case nir_instr_type_tex:
> @@ -1284,6 +1312,7 @@ nir_foreach_ssa_def(nir_instr *instr, 
> nir_foreach_ssa_def_cb cb, void *state)
>  {
> switch (instr->type) {
> case nir_instr_type_alu:
> +   case nir_instr_type_deref:
> case nir_instr_type_tex:
> case nir_instr_type_intrinsic:
> case nir_instr_type_phi:
> @@ -1350,6 +1379,23 @@ visit_alu_src(nir_alu_instr *instr, nir_foreach_src_cb 
> cb, void *state)
>  }
>
>  static bool
> +visit_deref_instr_src(nir_deref_instr *instr,
> +  nir_foreach_src_cb cb, void *state)
> +{
> +   if (instr->deref_type != nir_deref_type_var) {
> +  if (!visit_src(&instr->parent, cb, state))
> + return false;
> +   }
> +
> +   if (instr->deref_type == nir_deref_type_array) {
> +  if (!visit_src(&instr->arr.index, cb, state))
> + return false;
> +   }
> +
> +   return true;
> +}
> +
> +static bool
>  visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
>  {
> for (unsigned i = 0; i < instr->num_srcs; i++) {
> @@ -1437,6 +1483,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb 
> cb, void *state)
>if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
>   return false;
>break;
> +   case nir_instr_type_deref:
> +  if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
> + return false;
> +  break;
> case nir_instr_type_intrinsic:
>if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
>   return false;
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index cc7c401..db7dc91 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -427,6 +427,7 @@ typedef struct nir_register {
>
>  typedef enum {
> nir_instr_type_alu,
> +   nir_instr_type_deref,
> nir_instr_type_call,
> nir_instr_type_tex,
> nir_instr_type_intrinsic,
> @@ -894,7 +895,9 @@ bool nir_alu_srcs_equal(const nir_alu_instr *alu1, const 
> nir_alu_instr *alu2,
>  typedef enum {
> nir_deref_type_var,
> nir_deref_type_array,
> -   nir_deref_type_struct
> +   nir_deref_type_array_wildcard,
> +   nir_deref_type_struct,
> +   nir_deref_type_cast,
>  } nir_deref_type;
>
>  typedef struct nir_deref {
> @@ -956,6 +959,56 @@ nir_deref_tail(nir_deref *deref)
>  t