Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Roland Scheidegger]

Am 06.02.2016 um 22:30 schrieb Marek Olšák:
> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
>> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
>>> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
>>> +   nir_options.lower_fpow = true;
>>> +   nir_options.lower_fsat = true;
>>> +   nir_options.lower_scmp = true;
>>> +   nir_options.lower_flrp = true;
>>> +   nir_options.lower_ffract = true;
>>> +   nir_options.native_integers = true;
>>> +
>>
>>
>> btw, one of the few remaining things to tackle is how to handle
>> nir_shader_compiler_options struct.  To follow the existing approach
>> of shader caps, I'd have to add a big pile of caps now, and then keep
>> adding them as nir_shader_compiler_options struct grows.  Which seems
>> sub-optimal.
>>
>> How do people feel about adding a screen->get_shader_paramp() which,
>> along the lines of get_paramf, returns a 'const void *'?  Then we
>> could add a single cap to return the whole compiler-options struct.
>> (And maybe if at some point there was direct support for LLVM as an
>> IR, it might need something similar??)
>>
>> Other possibility is just a pipe->get_nir_compiler_options() type
>> hook.  A bit more of a point solution, but might make sense if we
>> can't think of any other plausible uses for ->get_shader_paramp()..
>> and less churn since it would only need to be implemented by drivers
>> consuming NIR..
>>
>> Thoughts/opinions?
> 
> pipe->get_nir_compiler_options() sounds good.
> 
> Maybe wait for VMWare guys' opinion as well.

Looks usable to me, albeit I'm not sure you really need NIR-specific
options as such? That is those options above don't really look nir
specific - maybe they aren't used with just glsl->tgsi, but it looks to
me like they would in theory be applicable to other IR as well. Though I
suppose if you just had compiler_otions it would be a bit confusing if
you had entries which then may not be used...

Roland


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

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Rob Clark]

On Mon, Feb 8, 2016 at 6:58 AM, Roland Scheidegger  wrote:
> Am 06.02.2016 um 22:30 schrieb Marek Olšák:
>> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
>>> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
 +   // XXX get from pipe_screen?  Or just let pipe driver provide?
 +   nir_options.lower_fpow = true;
 +   nir_options.lower_fsat = true;
 +   nir_options.lower_scmp = true;
 +   nir_options.lower_flrp = true;
 +   nir_options.lower_ffract = true;
 +   nir_options.native_integers = true;
 +
>>>
>>>
>>> btw, one of the few remaining things to tackle is how to handle
>>> nir_shader_compiler_options struct.  To follow the existing approach
>>> of shader caps, I'd have to add a big pile of caps now, and then keep
>>> adding them as nir_shader_compiler_options struct grows.  Which seems
>>> sub-optimal.
>>>
>>> How do people feel about adding a screen->get_shader_paramp() which,
>>> along the lines of get_paramf, returns a 'const void *'?  Then we
>>> could add a single cap to return the whole compiler-options struct.
>>> (And maybe if at some point there was direct support for LLVM as an
>>> IR, it might need something similar??)
>>>
>>> Other possibility is just a pipe->get_nir_compiler_options() type
>>> hook.  A bit more of a point solution, but might make sense if we
>>> can't think of any other plausible uses for ->get_shader_paramp()..
>>> and less churn since it would only need to be implemented by drivers
>>> consuming NIR..
>>>
>>> Thoughts/opinions?
>>
>> pipe->get_nir_compiler_options() sounds good.
>>
>> Maybe wait for VMWare guys' opinion as well.
>
> Looks usable to me, albeit I'm not sure you really need NIR-specific
> options as such? That is those options above don't really look nir
> specific - maybe they aren't used with just glsl->tgsi, but it looks to
> me like they would in theory be applicable to other IR as well. Though I
> suppose if you just had compiler_otions it would be a bit confusing if
> you had entries which then may not be used...

Yeah, not really NIR specific (and there are a couple that overlap w/
existing caps), other than being used only by NIR..  although it would
be a lot of churn to keep adding caps when the compiler_options struct
is extended, and it might be confusing that some of the lowering
options aren't supported in the TGSI path..

I guess right now it really only matters for two drivers, and down the
road I think we won't have more than 3 or 4 drivers using NIR, so I
suppose it is also an option to start w/
screen->get_nir_compiler_options() for now and revisit later.  If we
get to the point where we are always doing glsl->nir and then
optionally nir->tgsi for drivers that don't consume NIR directly,
maybe then it would make more sense to expose everything as caps?

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

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Rob Clark]

On Mon, Feb 8, 2016 at 1:01 PM, Ilia Mirkin  wrote:
> On Mon, Feb 8, 2016 at 8:59 AM, Rob Clark  wrote:
>> On Mon, Feb 8, 2016 at 6:58 AM, Roland Scheidegger  
>> wrote:
>>> Am 06.02.2016 um 22:30 schrieb Marek Olšák:
 On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
>> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
>> +   nir_options.lower_fpow = true;
>> +   nir_options.lower_fsat = true;
>> +   nir_options.lower_scmp = true;
>> +   nir_options.lower_flrp = true;
>> +   nir_options.lower_ffract = true;
>> +   nir_options.native_integers = true;
>> +
>
>
> btw, one of the few remaining things to tackle is how to handle
> nir_shader_compiler_options struct.  To follow the existing approach
> of shader caps, I'd have to add a big pile of caps now, and then keep
> adding them as nir_shader_compiler_options struct grows.  Which seems
> sub-optimal.
>
> How do people feel about adding a screen->get_shader_paramp() which,
> along the lines of get_paramf, returns a 'const void *'?  Then we
> could add a single cap to return the whole compiler-options struct.
> (And maybe if at some point there was direct support for LLVM as an
> IR, it might need something similar??)
>
> Other possibility is just a pipe->get_nir_compiler_options() type
> hook.  A bit more of a point solution, but might make sense if we
> can't think of any other plausible uses for ->get_shader_paramp()..
> and less churn since it would only need to be implemented by drivers
> consuming NIR..
>
> Thoughts/opinions?

 pipe->get_nir_compiler_options() sounds good.

 Maybe wait for VMWare guys' opinion as well.
>>>
>>> Looks usable to me, albeit I'm not sure you really need NIR-specific
>>> options as such? That is those options above don't really look nir
>>> specific - maybe they aren't used with just glsl->tgsi, but it looks to
>>> me like they would in theory be applicable to other IR as well. Though I
>>> suppose if you just had compiler_otions it would be a bit confusing if
>>> you had entries which then may not be used...
>>
>> Yeah, not really NIR specific (and there are a couple that overlap w/
>> existing caps), other than being used only by NIR..  although it would
>> be a lot of churn to keep adding caps when the compiler_options struct
>> is extended, and it might be confusing that some of the lowering
>> options aren't supported in the TGSI path..
>>
>> I guess right now it really only matters for two drivers, and down the
>> road I think we won't have more than 3 or 4 drivers using NIR, so I
>> suppose it is also an option to start w/
>> screen->get_nir_compiler_options() for now and revisit later.  If we
>> get to the point where we are always doing glsl->nir and then
>> optionally nir->tgsi for drivers that don't consume NIR directly,
>> maybe then it would make more sense to expose everything as caps?
>
> I actually kinda want this for TGSI as well, eventually. Perhaps something 
> like
>
> bool get_compiler_options(pipe_shader_ir, void *)

perhaps:

  const struct pipe_compiler_options * (*get_compiler_options)(struct
pipe_screen *, unsigned shader)

imo, it should take shader stage as arg so we can have different
config per stage, and return a const ptr.. and I think it could
directly return options struct (no need for bool return)..

I suppose if you plan to add lots of knobs to twiddle for TGSI then
shader cap for each would be annoying.  Although not super-thrilled
about having to translate from generic(ish) pipe struct to nir struct,
since the driver will already want a const version of the nir struct
for the tgsi_to_nir case.

I guess we could do:

  const void * (*get_compiler_options)(struct pipe_screen *, unsigned
shader, enum pipe_shader_ir type)

where the return value could be 'struct tgsi_shader_options *' or
'struct nir_shader_compiler_options *', etc..

hmm.. also not sure how to roll that out without a flag day.  Perhaps
keep the shader params for now (for tgsi) with a helper to populate a
tgsi_compiler_options struct for drivers where
screen->get_compiler_options() is null.. (and then what about other
st's?)

BR,
-R

> or perhaps struct pipe_compiler_options * (which would contain some
> common stuff + a union of the per-ir options) instead of the void *
> would make more sense? The reason I'm interested is to be able to
> indicate that various frontend opts should be disabled. Also it could
> be used to get rid of a bunch of PIPE_CAP_TGSI_XYZ's, which are a huge
> pain to add all the time.
>
>   -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Roland Scheidegger]

Am 08.02.2016 um 14:59 schrieb Rob Clark:
> On Mon, Feb 8, 2016 at 6:58 AM, Roland Scheidegger  wrote:
>> Am 06.02.2016 um 22:30 schrieb Marek Olšák:
>>> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
 On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
> +   nir_options.lower_fpow = true;
> +   nir_options.lower_fsat = true;
> +   nir_options.lower_scmp = true;
> +   nir_options.lower_flrp = true;
> +   nir_options.lower_ffract = true;
> +   nir_options.native_integers = true;
> +


 btw, one of the few remaining things to tackle is how to handle
 nir_shader_compiler_options struct.  To follow the existing approach
 of shader caps, I'd have to add a big pile of caps now, and then keep
 adding them as nir_shader_compiler_options struct grows.  Which seems
 sub-optimal.

 How do people feel about adding a screen->get_shader_paramp() which,
 along the lines of get_paramf, returns a 'const void *'?  Then we
 could add a single cap to return the whole compiler-options struct.
 (And maybe if at some point there was direct support for LLVM as an
 IR, it might need something similar??)

 Other possibility is just a pipe->get_nir_compiler_options() type
 hook.  A bit more of a point solution, but might make sense if we
 can't think of any other plausible uses for ->get_shader_paramp()..
 and less churn since it would only need to be implemented by drivers
 consuming NIR..

 Thoughts/opinions?
>>>
>>> pipe->get_nir_compiler_options() sounds good.
>>>
>>> Maybe wait for VMWare guys' opinion as well.
>>
>> Looks usable to me, albeit I'm not sure you really need NIR-specific
>> options as such? That is those options above don't really look nir
>> specific - maybe they aren't used with just glsl->tgsi, but it looks to
>> me like they would in theory be applicable to other IR as well. Though I
>> suppose if you just had compiler_otions it would be a bit confusing if
>> you had entries which then may not be used...
> 
> Yeah, not really NIR specific (and there are a couple that overlap w/
> existing caps), other than being used only by NIR..  although it would
> be a lot of churn to keep adding caps when the compiler_options struct
> is extended, and it might be confusing that some of the lowering
> options aren't supported in the TGSI path..
> 
> I guess right now it really only matters for two drivers, and down the
> road I think we won't have more than 3 or 4 drivers using NIR, so I
> suppose it is also an option to start w/
> screen->get_nir_compiler_options() for now and revisit later.  If we
> get to the point where we are always doing glsl->nir and then
> optionally nir->tgsi for drivers that don't consume NIR directly,
> maybe then it would make more sense to expose everything as caps?
> 

Probably. In any case, it looks like it would be easy enough to change
later, so whatever solution looks good now is ok.

Roland


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

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Ilia Mirkin]

On Mon, Feb 8, 2016 at 8:59 AM, Rob Clark  wrote:
> On Mon, Feb 8, 2016 at 6:58 AM, Roland Scheidegger  wrote:
>> Am 06.02.2016 um 22:30 schrieb Marek Olšák:
>>> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
 On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
> +   nir_options.lower_fpow = true;
> +   nir_options.lower_fsat = true;
> +   nir_options.lower_scmp = true;
> +   nir_options.lower_flrp = true;
> +   nir_options.lower_ffract = true;
> +   nir_options.native_integers = true;
> +


 btw, one of the few remaining things to tackle is how to handle
 nir_shader_compiler_options struct.  To follow the existing approach
 of shader caps, I'd have to add a big pile of caps now, and then keep
 adding them as nir_shader_compiler_options struct grows.  Which seems
 sub-optimal.

 How do people feel about adding a screen->get_shader_paramp() which,
 along the lines of get_paramf, returns a 'const void *'?  Then we
 could add a single cap to return the whole compiler-options struct.
 (And maybe if at some point there was direct support for LLVM as an
 IR, it might need something similar??)

 Other possibility is just a pipe->get_nir_compiler_options() type
 hook.  A bit more of a point solution, but might make sense if we
 can't think of any other plausible uses for ->get_shader_paramp()..
 and less churn since it would only need to be implemented by drivers
 consuming NIR..

 Thoughts/opinions?
>>>
>>> pipe->get_nir_compiler_options() sounds good.
>>>
>>> Maybe wait for VMWare guys' opinion as well.
>>
>> Looks usable to me, albeit I'm not sure you really need NIR-specific
>> options as such? That is those options above don't really look nir
>> specific - maybe they aren't used with just glsl->tgsi, but it looks to
>> me like they would in theory be applicable to other IR as well. Though I
>> suppose if you just had compiler_otions it would be a bit confusing if
>> you had entries which then may not be used...
>
> Yeah, not really NIR specific (and there are a couple that overlap w/
> existing caps), other than being used only by NIR..  although it would
> be a lot of churn to keep adding caps when the compiler_options struct
> is extended, and it might be confusing that some of the lowering
> options aren't supported in the TGSI path..
>
> I guess right now it really only matters for two drivers, and down the
> road I think we won't have more than 3 or 4 drivers using NIR, so I
> suppose it is also an option to start w/
> screen->get_nir_compiler_options() for now and revisit later.  If we
> get to the point where we are always doing glsl->nir and then
> optionally nir->tgsi for drivers that don't consume NIR directly,
> maybe then it would make more sense to expose everything as caps?

I actually kinda want this for TGSI as well, eventually. Perhaps something like

bool get_compiler_options(pipe_shader_ir, void *)

or perhaps struct pipe_compiler_options * (which would contain some
common stuff + a union of the per-ir options) instead of the void *
would make more sense? The reason I'm interested is to be able to
indicate that various frontend opts should be disabled. Also it could
be used to get rid of a bunch of PIPE_CAP_TGSI_XYZ's, which are a huge
pain to add all the time.

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

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Ilia Mirkin]

On Mon, Feb 8, 2016 at 1:55 PM, Rob Clark  wrote:
> On Mon, Feb 8, 2016 at 1:01 PM, Ilia Mirkin  wrote:
>> On Mon, Feb 8, 2016 at 8:59 AM, Rob Clark  wrote:
>>> On Mon, Feb 8, 2016 at 6:58 AM, Roland Scheidegger  
>>> wrote:
 Am 06.02.2016 um 22:30 schrieb Marek Olšák:
> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
>> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
>>> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
>>> +   nir_options.lower_fpow = true;
>>> +   nir_options.lower_fsat = true;
>>> +   nir_options.lower_scmp = true;
>>> +   nir_options.lower_flrp = true;
>>> +   nir_options.lower_ffract = true;
>>> +   nir_options.native_integers = true;
>>> +
>>
>>
>> btw, one of the few remaining things to tackle is how to handle
>> nir_shader_compiler_options struct.  To follow the existing approach
>> of shader caps, I'd have to add a big pile of caps now, and then keep
>> adding them as nir_shader_compiler_options struct grows.  Which seems
>> sub-optimal.
>>
>> How do people feel about adding a screen->get_shader_paramp() which,
>> along the lines of get_paramf, returns a 'const void *'?  Then we
>> could add a single cap to return the whole compiler-options struct.
>> (And maybe if at some point there was direct support for LLVM as an
>> IR, it might need something similar??)
>>
>> Other possibility is just a pipe->get_nir_compiler_options() type
>> hook.  A bit more of a point solution, but might make sense if we
>> can't think of any other plausible uses for ->get_shader_paramp()..
>> and less churn since it would only need to be implemented by drivers
>> consuming NIR..
>>
>> Thoughts/opinions?
>
> pipe->get_nir_compiler_options() sounds good.
>
> Maybe wait for VMWare guys' opinion as well.

 Looks usable to me, albeit I'm not sure you really need NIR-specific
 options as such? That is those options above don't really look nir
 specific - maybe they aren't used with just glsl->tgsi, but it looks to
 me like they would in theory be applicable to other IR as well. Though I
 suppose if you just had compiler_otions it would be a bit confusing if
 you had entries which then may not be used...
>>>
>>> Yeah, not really NIR specific (and there are a couple that overlap w/
>>> existing caps), other than being used only by NIR..  although it would
>>> be a lot of churn to keep adding caps when the compiler_options struct
>>> is extended, and it might be confusing that some of the lowering
>>> options aren't supported in the TGSI path..
>>>
>>> I guess right now it really only matters for two drivers, and down the
>>> road I think we won't have more than 3 or 4 drivers using NIR, so I
>>> suppose it is also an option to start w/
>>> screen->get_nir_compiler_options() for now and revisit later.  If we
>>> get to the point where we are always doing glsl->nir and then
>>> optionally nir->tgsi for drivers that don't consume NIR directly,
>>> maybe then it would make more sense to expose everything as caps?
>>
>> I actually kinda want this for TGSI as well, eventually. Perhaps something 
>> like
>>
>> bool get_compiler_options(pipe_shader_ir, void *)
>
> perhaps:
>
>   const struct pipe_compiler_options * (*get_compiler_options)(struct
> pipe_screen *, unsigned shader)
>
> imo, it should take shader stage as arg so we can have different
> config per stage, and return a const ptr.. and I think it could
> directly return options struct (no need for bool return)..
>
> I suppose if you plan to add lots of knobs to twiddle for TGSI then
> shader cap for each would be annoying.  Although not super-thrilled
> about having to translate from generic(ish) pipe struct to nir struct,
> since the driver will already want a const version of the nir struct
> for the tgsi_to_nir case.
>
> I guess we could do:
>
>   const void * (*get_compiler_options)(struct pipe_screen *, unsigned
> shader, enum pipe_shader_ir type)

That means the driver has to allocate and store the options somewhere.
This can get annoying... I'd rather just have it fill in a struct
that's passed in.

>
> where the return value could be 'struct tgsi_shader_options *' or
> 'struct nir_shader_compiler_options *', etc..

Well I was thinking that there'd be a

struct pipe_compiler_options {
  some common stuff?
  union {
struct nir_options;
struct tgsi_options;
  }
}

>
> hmm.. also not sure how to roll that out without a flag day.  Perhaps
> keep the shader params for now (for tgsi) with a helper to populate a
> tgsi_compiler_options struct for drivers where
> screen->get_compiler_options() is null.. (and then what about other
> st's?)

Yeah, definitely some sort of transition plan would have to happen.
And maybe leave all the current 

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Rob Clark]

On Mon, Feb 8, 2016 at 2:00 PM, Ilia Mirkin  wrote:
> On Mon, Feb 8, 2016 at 1:55 PM, Rob Clark  wrote:
>> On Mon, Feb 8, 2016 at 1:01 PM, Ilia Mirkin  wrote:
>>> On Mon, Feb 8, 2016 at 8:59 AM, Rob Clark  wrote:
 On Mon, Feb 8, 2016 at 6:58 AM, Roland Scheidegger  
 wrote:
> Am 06.02.2016 um 22:30 schrieb Marek Olšák:
>> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
>>> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
 +   // XXX get from pipe_screen?  Or just let pipe driver provide?
 +   nir_options.lower_fpow = true;
 +   nir_options.lower_fsat = true;
 +   nir_options.lower_scmp = true;
 +   nir_options.lower_flrp = true;
 +   nir_options.lower_ffract = true;
 +   nir_options.native_integers = true;
 +
>>>
>>>
>>> btw, one of the few remaining things to tackle is how to handle
>>> nir_shader_compiler_options struct.  To follow the existing approach
>>> of shader caps, I'd have to add a big pile of caps now, and then keep
>>> adding them as nir_shader_compiler_options struct grows.  Which seems
>>> sub-optimal.
>>>
>>> How do people feel about adding a screen->get_shader_paramp() which,
>>> along the lines of get_paramf, returns a 'const void *'?  Then we
>>> could add a single cap to return the whole compiler-options struct.
>>> (And maybe if at some point there was direct support for LLVM as an
>>> IR, it might need something similar??)
>>>
>>> Other possibility is just a pipe->get_nir_compiler_options() type
>>> hook.  A bit more of a point solution, but might make sense if we
>>> can't think of any other plausible uses for ->get_shader_paramp()..
>>> and less churn since it would only need to be implemented by drivers
>>> consuming NIR..
>>>
>>> Thoughts/opinions?
>>
>> pipe->get_nir_compiler_options() sounds good.
>>
>> Maybe wait for VMWare guys' opinion as well.
>
> Looks usable to me, albeit I'm not sure you really need NIR-specific
> options as such? That is those options above don't really look nir
> specific - maybe they aren't used with just glsl->tgsi, but it looks to
> me like they would in theory be applicable to other IR as well. Though I
> suppose if you just had compiler_otions it would be a bit confusing if
> you had entries which then may not be used...

 Yeah, not really NIR specific (and there are a couple that overlap w/
 existing caps), other than being used only by NIR..  although it would
 be a lot of churn to keep adding caps when the compiler_options struct
 is extended, and it might be confusing that some of the lowering
 options aren't supported in the TGSI path..

 I guess right now it really only matters for two drivers, and down the
 road I think we won't have more than 3 or 4 drivers using NIR, so I
 suppose it is also an option to start w/
 screen->get_nir_compiler_options() for now and revisit later.  If we
 get to the point where we are always doing glsl->nir and then
 optionally nir->tgsi for drivers that don't consume NIR directly,
 maybe then it would make more sense to expose everything as caps?
>>>
>>> I actually kinda want this for TGSI as well, eventually. Perhaps something 
>>> like
>>>
>>> bool get_compiler_options(pipe_shader_ir, void *)
>>
>> perhaps:
>>
>>   const struct pipe_compiler_options * (*get_compiler_options)(struct
>> pipe_screen *, unsigned shader)
>>
>> imo, it should take shader stage as arg so we can have different
>> config per stage, and return a const ptr.. and I think it could
>> directly return options struct (no need for bool return)..
>>
>> I suppose if you plan to add lots of knobs to twiddle for TGSI then
>> shader cap for each would be annoying.  Although not super-thrilled
>> about having to translate from generic(ish) pipe struct to nir struct,
>> since the driver will already want a const version of the nir struct
>> for the tgsi_to_nir case.
>>
>> I guess we could do:
>>
>>   const void * (*get_compiler_options)(struct pipe_screen *, unsigned
>> shader, enum pipe_shader_ir type)
>
> That means the driver has to allocate and store the options somewhere.
> This can get annoying... I'd rather just have it fill in a struct
> that's passed in.

If you really have enough diff settings between gen's then just embed
the struct in your pipe_screen and populate it at screen init..

Currently we've managed to keep nir_shader_compiler_options as
something that can be const (by keeping options that depend on draw
state out).  It would be annoying to loose that.  Not to mention that
it would be duplication for the NIR drivers, since they already need
the nir options sturct in tgsi_to_nir case.

>>
>> where the return value 

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Marek Olšák]

On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
>> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
>> +   nir_options.lower_fpow = true;
>> +   nir_options.lower_fsat = true;
>> +   nir_options.lower_scmp = true;
>> +   nir_options.lower_flrp = true;
>> +   nir_options.lower_ffract = true;
>> +   nir_options.native_integers = true;
>> +
>
>
> btw, one of the few remaining things to tackle is how to handle
> nir_shader_compiler_options struct.  To follow the existing approach
> of shader caps, I'd have to add a big pile of caps now, and then keep
> adding them as nir_shader_compiler_options struct grows.  Which seems
> sub-optimal.
>
> How do people feel about adding a screen->get_shader_paramp() which,
> along the lines of get_paramf, returns a 'const void *'?  Then we
> could add a single cap to return the whole compiler-options struct.
> (And maybe if at some point there was direct support for LLVM as an
> IR, it might need something similar??)
>
> Other possibility is just a pipe->get_nir_compiler_options() type
> hook.  A bit more of a point solution, but might make sense if we
> can't think of any other plausible uses for ->get_shader_paramp()..
> and less churn since it would only need to be implemented by drivers
> consuming NIR..
>
> Thoughts/opinions?

pipe->get_nir_compiler_options() sounds good.

Maybe wait for VMWare guys' opinion as well.

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

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Marek Olšák]

On Sat, Feb 6, 2016 at 10:30 PM, Marek Olšák  wrote:
> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark  wrote:
>> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
>>> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
>>> +   nir_options.lower_fpow = true;
>>> +   nir_options.lower_fsat = true;
>>> +   nir_options.lower_scmp = true;
>>> +   nir_options.lower_flrp = true;
>>> +   nir_options.lower_ffract = true;
>>> +   nir_options.native_integers = true;
>>> +
>>
>>
>> btw, one of the few remaining things to tackle is how to handle
>> nir_shader_compiler_options struct.  To follow the existing approach
>> of shader caps, I'd have to add a big pile of caps now, and then keep
>> adding them as nir_shader_compiler_options struct grows.  Which seems
>> sub-optimal.
>>
>> How do people feel about adding a screen->get_shader_paramp() which,
>> along the lines of get_paramf, returns a 'const void *'?  Then we
>> could add a single cap to return the whole compiler-options struct.
>> (And maybe if at some point there was direct support for LLVM as an
>> IR, it might need something similar??)
>>
>> Other possibility is just a pipe->get_nir_compiler_options() type
>> hook.  A bit more of a point solution, but might make sense if we
>> can't think of any other plausible uses for ->get_shader_paramp()..
>> and less churn since it would only need to be implemented by drivers
>> consuming NIR..
>>
>> Thoughts/opinions?
>
> pipe->get_nir_compiler_options() sounds good.

Well, it should be screen->...

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

Re: [Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Rob Clark]

On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark  wrote:
> +   // XXX get from pipe_screen?  Or just let pipe driver provide?
> +   nir_options.lower_fpow = true;
> +   nir_options.lower_fsat = true;
> +   nir_options.lower_scmp = true;
> +   nir_options.lower_flrp = true;
> +   nir_options.lower_ffract = true;
> +   nir_options.native_integers = true;
> +


btw, one of the few remaining things to tackle is how to handle
nir_shader_compiler_options struct.  To follow the existing approach
of shader caps, I'd have to add a big pile of caps now, and then keep
adding them as nir_shader_compiler_options struct grows.  Which seems
sub-optimal.

How do people feel about adding a screen->get_shader_paramp() which,
along the lines of get_paramf, returns a 'const void *'?  Then we
could add a single cap to return the whole compiler-options struct.
(And maybe if at some point there was direct support for LLVM as an
IR, it might need something similar??)

Other possibility is just a pipe->get_nir_compiler_options() type
hook.  A bit more of a point solution, but might make sense if we
can't think of any other plausible uses for ->get_shader_paramp()..
and less churn since it would only need to be implemented by drivers
consuming NIR..

Thoughts/opinions?

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

[Mesa-dev] [RFCv3 11/11] mesa/st: add support for NIR as possible driver IR

[Rob Clark]

From: Rob Clark 

Signed-off-by: Rob Clark 
---
 src/compiler/nir/nir.h |   2 +
 src/mesa/Makefile.sources  |   1 +
 src/mesa/state_tracker/st_glsl_to_nir.cpp  | 407 +
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  40 ++-
 src/mesa/state_tracker/st_glsl_to_tgsi.h   |   5 +
 src/mesa/state_tracker/st_nir.h|  23 ++
 src/mesa/state_tracker/st_program.c| 171 ++--
 src/mesa/state_tracker/st_program.h|   6 +
 8 files changed, 623 insertions(+), 32 deletions(-)
 create mode 100644 src/mesa/state_tracker/st_glsl_to_nir.cpp

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 29c8631..148fe68 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -348,6 +348,8 @@ typedef struct nir_variable {
 
 #define nir_foreach_variable(var, var_list) \
foreach_list_typed(nir_variable, var, node, var_list)
+#define nir_foreach_variable_safe(var, var_list) \
+   foreach_list_typed_safe(nir_variable, var, node, var_list)
 
 typedef struct nir_register {
struct exec_node node;
diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index 500a5c0..b712bad 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -481,6 +481,7 @@ STATETRACKER_FILES = \
state_tracker/st_gen_mipmap.c \
state_tracker/st_gen_mipmap.h \
state_tracker/st_gl_api.h \
+   state_tracker/st_glsl_to_nir.cpp \
state_tracker/st_glsl_to_tgsi.cpp \
state_tracker/st_glsl_to_tgsi.h \
state_tracker/st_manager.c \
diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker/st_glsl_to_nir.cpp
new file mode 100644
index 000..8722102
--- /dev/null
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -0,0 +1,407 @@
+/*
+ * Copyright © 2015 Red Hat
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
THE
+ * SOFTWARE.
+ */
+
+#include "st_nir.h"
+
+#include "pipe/p_defines.h"
+#include "pipe/p_screen.h"
+#include "pipe/p_context.h"
+
+#include "program/program.h"
+#include "program/prog_statevars.h"
+#include "program/prog_parameter.h"
+#include "program/ir_to_mesa.h"
+#include "main/mtypes.h"
+#include "main/errors.h"
+#include "main/shaderapi.h"
+#include "main/uniforms.h"
+
+#include "st_context.h"
+#include "st_program.h"
+
+#include "compiler/nir/nir.h"
+#include "compiler/nir/glsl_to_nir.h"
+#include "compiler/glsl_types.h"
+
+
+/* Depending on PIPE_CAP_TGSI_TEXCOORD (st->needs_texcoord_semantic) we
+ * may need to fix up varying slots so the glsl->nir path is aligned
+ * with the anything->tgsi->nir path.
+ */
+static void
+st_nir_fixup_varying_slots(struct st_context *st, struct exec_list *var_list)
+{
+   if (st->needs_texcoord_semantic)
+  return;
+
+   nir_foreach_variable(var, var_list) {
+  if (var->data.location >= VARYING_SLOT_VAR0) {
+ var->data.location += 9;
+  } else if ((var->data.location >= VARYING_SLOT_TEX0) &&
+   (var->data.location <= VARYING_SLOT_TEX7)) {
+ var->data.location += VARYING_SLOT_VAR0 - VARYING_SLOT_TEX0;
+  }
+   }
+}
+
+/* input location assignment for VS inputs must be handled specially, so
+ * that it is aligned w/ st's vbo state.
+ * (This isn't the case with, for ex, FS inputs, which only need to agree
+ * on varying-slot w/ the VS outputs)
+ */
+static void
+st_nir_assign_vs_in_locations(struct gl_program *prog,
+  struct exec_list *var_list, unsigned *size)
+{
+   unsigned attr, num_inputs = 0;
+   unsigned input_to_index[VERT_ATTRIB_MAX] = {0};
+
+   /* TODO de-duplicate w/ similar code in st_translate_vertex_program()? */
+   for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) {
+  if ((prog->InputsRead & BITFIELD64_BIT(attr)) != 0) {
+ input_to_index[attr] = num_inputs;
+ num_inputs++;
+ if