Re: [Mesa-dev] [PATCH v3 34/48] intel/fs: Rework zero-length URB write handling

On Wed, 2017-10-25 at 16:26 -0700, Jason Ekstrand wrote: > Originally we tried to handle this case based on > slots_valid.  However, > there are a number of ways that this can go wrong.  For one, we throw > away any trailing slots which either aren't written or are set to > VARYING_SLOT_PAD.  I

Re: [Mesa-dev] [PATCH v3 31/48] intel/cs: Re-run final NIR optimizations for each SIMD size

This should be squashed into the previous commit On Wed, 2017-10-25 at 16:26 -0700, Jason Ekstrand wrote: > With the advent of SPIR-V subgroup operations, compute shaders will > have > to be slightly different depending on the SIMD size at which they > execute.  In order to allow us to do

Re: [Mesa-dev] [PATCH v3 30/48] intel/cs: Re-run final NIR optimizations for each SIMD size

On Wed, 2017-10-25 at 16:26 -0700, Jason Ekstrand wrote: > With the advent of SPIR-V subgroup operations, compute shaders will > have > to be slightly different depending on the SIMD size at which they > execute.  In order to allow us to do dispatch-width specific things > in > NIR, we re-run the

Re: [Mesa-dev] [PATCH v3 29/48] intel/cs: Rework the way thread local ID is handled

On Wed, 2017-10-25 at 16:26 -0700, Jason Ekstrand wrote: > Previously, brw_nir_lower_intrinsics added the param and then emitted > a > load_uniform intrinsic to load it directly.  This commit switches > things > over to use a specific NIR intrinsic for the thread id.  The one > thing I > don't

Re: [Mesa-dev] [PATCH v3 25/48] intel/cs: Drop max_dispatch_width checks from compile_cs

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > The only things that adjust fs_visitor::max_dispatch_width are render > target writes which don't happen in compute shaders so they're > pointless. > --- >  src/intel/compiler/brw_fs.cpp | 6 ++ >  1 file changed, 2 insertions(+), 4

Re: [Mesa-dev] [PATCH v3 23/48] intel/fs: Assign constant locations if they haven't been assigned

This sounds good to me, but I guess it is not really fixing anything, right? I ask because the subject claims that this patch does something that the original code was already supposed to be doing. On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > Before, we bailing in

Re: [Mesa-dev] [PATCH v3 20/48] intel/fs: Protect opt_algebraic from OOB BROADCAST indices

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > --- >  src/intel/compiler/brw_fs.cpp | 10 -- >  1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/src/intel/compiler/brw_fs.cpp > b/src/intel/compiler/brw_fs.cpp > index 1c4351b..52079d3 100644 > ---

Re: [Mesa-dev] [PATCH v3 19/48] i965/fs/nir: Don't stomp 64-bit values to D in get_nir_src

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > --- >  src/intel/compiler/brw_fs_nir.cpp | 33 +-- > -- >  1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > b/src/intel/compiler/brw_fs_nir.cpp > index

Re: [Mesa-dev] [PATCH v3 18/48] i965/fs/nir: Minor refactor of store_output

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > Stop retyping the output of shuffle_64bit_data_for_32bit_write.  It's > always BRW_REGISTER_TYPE_D which is perfectly fine for writing out. > Also, when we change get_nir_src to return something with a 64-bit > type > for 64-bit values,

Re: [Mesa-dev] [PATCH v3 00/48] nir, intel: Prerequisites for subgroups

I left a few minor comments in patches 1, 2, 8 and 14. Otherwise patches 1-2, 4-5 and 7-14 (3 and 6 already have Rb) are: Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> I feel like patches 10, 11 could maybe use another extra review if there is someone who wants to do it, sinc

Re: [Mesa-dev] [PATCH v3 14/48] i965/fs: Extend the live ranges of VGRFs which leave loops

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > No Shader-db changes. > > Cc: mesa-sta...@lists.freedesktop.org > --- >  src/intel/compiler/brw_fs_live_variables.cpp | 55 > >  1 file changed, 55 insertions(+) > > diff --git

Re: [Mesa-dev] [PATCH v3 10/48] intel/eu: Fix broadcast instruction for

The subject line is incomplete, it misses the '64-bit types' at the end. On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > We're not using broadcast for any 32-bit types right now since we ^^ 64-bit I guess >

Re: [Mesa-dev] [PATCH v3 08/48] intel/eu: Just modify the offset in brw_broadcast

I am not sure I get the purpose of this, there is nothing wrong with the change, but the subject suggests that was so that we modified that offset only inside brw_broadcast()... but that was already happening and in fact this patch only changes code inside that function so I wonder if there is

Re: [Mesa-dev] [PATCH v3 02/48] intel/fs: Be more explicit about our placement of [un]zip

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > Before, we were careful to place the zip after the last of the split > instructions but did unzip on-demand.  This changes things so that > the > unzips go before all of the split instructions and the unzip comes > explicitly after all the

Re: [Mesa-dev] [PATCH v3 01/48] intel/fs: Pass builders instead of blocks into emit_[un]zip

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > This makes it far more explicit where we're inserting the > instructions > rather than the magic "before and after" stuff that the emit_[un]zip > helpers did based on block and inst. > > Cc: mesa-sta...@lists.freedesktop.org > --- >  

Re: [Mesa-dev] [PATCH v2 9/8] glsl/linker: refactor check_location_aliasing

hecked this (and a few other variations) and everything seems to work as expected. > Acked-by: Ilia Mirkin <imir...@alum.mit.edu> Thanks to you and Timothy for all the reviews! Iago > > On Wed, Oct 25, 2017 at 5:15 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: >

Re: [Mesa-dev] [PATCH v2 10/8] glsl/linker: Fix type checks for location aliasing

On Wed, 2017-10-25 at 08:30 -0400, Ilia Mirkin wrote: > On Wed, Oct 25, 2017 at 5:15 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: > > From the OpenGL 4.6 spec, section 4.4.1 Input Layout Qualifiers, > > Page 68, > > (Location aliasing): > > >

Re: [Mesa-dev] [PATCH v2 8/8] glsl/linker: validate explicit locations for SSO programs

okay. Iago On Tue, 2017-10-24 at 23:47 -0400, Ilia Mirkin wrote: > For all the patches in the series that I didn't have comments on, > > Reviewed-by: Ilia Mirkin <imir...@alum.mit.edu> > > On Tue, Oct 24, 2017 at 5:28 AM, Iago Toral Quiroga <ito...@igalia.co > m> wro

Re: [Mesa-dev] [PATCH v2 4/8] glsl/linker: outputs in the same location must share interpolation

On Tue, 2017-10-24 at 23:45 -0400, Ilia Mirkin wrote: > On Tue, Oct 24, 2017 at 5:28 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: > > From ARB_enhanced_layouts: > > > > "[...]when location aliasing, the aliases sharing the location > >  mus

Re: [Mesa-dev] [PATCH v2 2/8] glsl/linker: refactor link-time validation of output locations

On Tue, 2017-10-24 at 23:40 -0400, Ilia Mirkin wrote: > On Tue, Oct 24, 2017 at 5:28 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: > > Move the checks for explicit locations to a separate function. We > > will use this in a follow-up patch to validate loc

[Mesa-dev] [PATCH v2 9/8] glsl/linker: refactor check_location_aliasing

Mostly, this merges the type checks with all the other checks so we only have a single loop for this. --- src/compiler/glsl/link_varyings.cpp | 110 +++- 1 file changed, 46 insertions(+), 64 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp

[Mesa-dev] [PATCH v2 10/8] glsl/linker: Fix type checks for location aliasing

From the OpenGL 4.6 spec, section 4.4.1 Input Layout Qualifiers, Page 68, (Location aliasing): "Further, when location aliasing, the aliases sharing the location must have the same underlying numerical type (floating-point or integer)." The current implementation is too strict, since

Re: [Mesa-dev] [PATCH] i965: Don't disable CCS for RT dependencies when dispatching compute.

Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> BTW, thanks for fixing the original dependency issue, I was about to start looking into those test failures when you landed the fix :) Iago On Mon, 2017-10-23 at 22:21 -0700, Kenneth Graunke wrote: > Compute shaders don't ha

[Mesa-dev] [PATCH v2 5/8] glsl/linker: outputs in the same location must share auxiliary storage

From ARB_enhanced_layouts: "[...]when location aliasing, the aliases sharing the location must have the same underlying numerical type (floating-point or integer) and the same auxiliary storage and interpolation qualification.[...]" Add code to the linker to validate that aliased locations

[Mesa-dev] [PATCH v2 8/8] glsl/linker: validate explicit locations for SSO programs

v2: - we only need to validate inputs to the first stage and outputs from the last stage, everything else has already been validated during cross_validate_outputs_to_inputs (Timothy). - Use MAX_VARYING instead of MAX_VARYINGS_INCL_PATCH (Illia) --- src/compiler/glsl/link_varyings.cpp | 55

[Mesa-dev] [PATCH v2 7/8] glsl/linker: generalize validate_explicit_variable_location for SSO

For non-SSO programs, we only need to validate outputs, since the cross validation of outputs to inputs will ensure that we produce linker errors for invalid inputs too. Hoever, for the SSO path there is no output to input validation, so we need to validate inputs explicitly. Generalize the

[Mesa-dev] [PATCH v2 6/8] glsl/linker: create a helper function to validate explicit locations

Currently, we only validate explicit locations for non-SSO programs. This creates a helper that we can call from both SSO and non-SSO paths directly, so we can reuse all the logic behind this. Reviewed-by: Timothy Arceri --- src/compiler/glsl/link_varyings.cpp | 94

[Mesa-dev] [PATCH v2 3/8] glsl/linker: fix location aliasing checks for interface variables

The existing code was checking the whole interface variable rather than its members, which is not what we want: we want to check aliasing for each member in the interface variable. Surprisingly, there are piglit tests that verify this and were passing due to a bug in the existing code: when we

[Mesa-dev] [PATCH v2 2/8] glsl/linker: refactor link-time validation of output locations

Move the checks for explicit locations to a separate function. We will use this in a follow-up patch to validate locations for interface variables where we need to validate each interface member rather than the interface variable itself. Reviewed-by: Timothy Arceri ---

[Mesa-dev] [PATCH v2 4/8] glsl/linker: outputs in the same location must share interpolation

From ARB_enhanced_layouts: "[...]when location aliasing, the aliases sharing the location must have the same underlying numerical type (floating-point or integer) and the same auxiliary storage and interpolation qualification.[...]" Add code to the linker to validate that aliased locations do

[Mesa-dev] [PATCH v2 0/8] glsl/linker: fix location aliasing checks

variables (these start at VARYING_SLOT_PATCH0 not VARYING_SLOT_VAR0). Illia: I also checked your original patch and I think this series includes all functional changes you had. Patches 2-7 are already reviewed, changes 1) and 3) affect some of them but trivially so. Patches 1 (new in this series) and 8 still n

[Mesa-dev] [PATCH v2 1/8] glsl/linker: report linker errors for invalid explicit locations on inputs

We were assuming that if an input has an invalid explicit location it would fail to link because it would not find the corresponding output, however, since we look for the matching output by indexing the explicit_locations array with the input location, we still need to ensure that we don't index

Re: [Mesa-dev] [PATCH 3/3] glsl/linker: validate explicit locations for SSO programs

On Fri, 2017-10-20 at 22:29 +1100, Timothy Arceri wrote: > On 20/10/17 22:15, Iago Toral wrote: > > On Fri, 2017-10-20 at 13:07 +0200, Iago Toral wrote: > > > On Fri, 2017-10-20 at 22:03 +1100, Timothy Arceri wrote: > > > > > > > > O

Re: [Mesa-dev] [PATCH 3/3] glsl/linker: validate explicit locations for SSO programs

On Fri, 2017-10-20 at 13:07 +0200, Iago Toral wrote: > On Fri, 2017-10-20 at 22:03 +1100, Timothy Arceri wrote: > > > > On 20/10/17 21:46, Iago Toral Quiroga wrote: > > > --- > > >   src/compiler/glsl/link_varyings.cpp | 53 > > > +

Re: [Mesa-dev] [PATCH 3/3] glsl/linker: validate explicit locations for SSO programs

On Fri, 2017-10-20 at 22:03 +1100, Timothy Arceri wrote: > > On 20/10/17 21:46, Iago Toral Quiroga wrote: > > --- > >   src/compiler/glsl/link_varyings.cpp | 53 > > + > >   src/compiler/glsl/link_varyings.h   |  4 +++ >

[Mesa-dev] [PATCH 1/3] glsl/linker: create a helper function to validate explicit locations

Currently, we only validate explicit locations for non-SSO programs. This creates a helper that we can call from both SSO and non-SSO paths directly, so we can reuse all the logic behind this. --- src/compiler/glsl/link_varyings.cpp | 92 ++--- 1 file changed, 54

[Mesa-dev] [PATCH 2/3] glsl/linker: generalize validate_explicit_variable_location for SSO

For non-SSO programs, we only need to validate outputs, since the cross validation of outputs to inputs will ensure that we produce linker errors for invalid inputs too. Hoever,for the SSO path there is no output to input validation, so we need to validate inputs explicitly. Generalize the

[Mesa-dev] [PATCH 0/3] Implement location aliasing checks for SSO programs

-dev/2017-October/173503.html Iago Toral Quiroga (3): glsl/linker: create a helper function to validate explicit locations glsl/linker: generalize validate_explicit_variable_location for SSO glsl/linker: validate explicit locations for SSO programs src/compiler/glsl/link_varyings.cpp | 159

[Mesa-dev] [PATCH 3/3] glsl/linker: validate explicit locations for SSO programs

--- src/compiler/glsl/link_varyings.cpp | 53 + src/compiler/glsl/link_varyings.h | 4 +++ src/compiler/glsl/linker.cpp| 6 + 3 files changed, 63 insertions(+) diff --git a/src/compiler/glsl/link_varyings.cpp

Re: [Mesa-dev] [PATCH 4/4] glsl/linker: outputs in the same location must share auxiliary storage

On Fri, 2017-10-20 at 15:06 +1100, Timothy Arceri wrote: > I only really skimmed over them but these look fine to me 3-4: > > Reviewed-by: Timothy Arceri > > Thanks! > Thanks for the reviews Timothy. Illia: do you have any plans to work on the SSO path? if not, I am

Re: [Mesa-dev] [PATCH 2/4] glsl/linker: fix location aliasing checks for interface variables

On Fri, 2017-10-20 at 13:18 +1100, Timothy Arceri wrote: > > On 20/10/17 03:31, Iago Toral Quiroga wrote: > > The existing code was checking the whole interface variable rather > > than its members, which is not what we want: we want to check > > aliasing for each member i

Re: [Mesa-dev] [PATCH 2/4] glsl/linker: fix location aliasing checks for interface variables

this. See for example: tests/spec/arb_enhanced_layouts/linker/block-member-locations/named- block-member-location-overlap.shader_test Iago > On Thu, Oct 19, 2017 at 12:31 PM, Iago Toral Quiroga > <ito...@igalia.com> wrote: > > The existing code was checking the whole interface v

[Mesa-dev] [PATCH 2/4] glsl/linker: fix location aliasing checks for interface variables

The existing code was checking the whole interface variable rather than its members, which is not what we want: we want to check aliasing for each member in the interface variable. Surprisingly, there are piglit tests that verify this and were passing due to a bug in the existing code: when we

[Mesa-dev] [PATCH 4/4] glsl/linker: outputs in the same location must share auxiliary storage

From ARB_enhanced_layouts: "[...]when location aliasing, the aliases sharing the location must have the same underlying numerical type (floating-point or integer) and the same auxiliary storage and interpolation qualification.[...]" Add code to the linker to validate that aliased locations

[Mesa-dev] [PATCH 0/4] glsl/linker: fix location aliasing checks

onent layout qualifier." At least uur glsl_struct_field, which we use to store data for interface block members only has location data, but not component information so I guess we should fix that some day too. Iago Toral Quiroga (4): glsl/linker: refactor link-time validation of output

[Mesa-dev] [PATCH 1/4] glsl/linker: refactor link-time validation of output locations

Move the checks for explicit locations to a separate function. We will use this in a follow-up patch to validate locations for interface variables where we need to validate each interface member rather than the interface variable itself. --- src/compiler/glsl/link_varyings.cpp | 128

[Mesa-dev] [PATCH 3/4] glsl/linker: outputs in the same location must share interpolation

From ARB_enhanced_layouts: "[...]when location aliasing, the aliases sharing the location must have the same underlying numerical type (floating-point or integer) and the same auxiliary storage and interpolation qualification.[...]" Add code to the linker to validate that aliased locations do

Re: [Mesa-dev] [PATCH] glsl/linker: outputs in the same location must share interpolation

On Thu, 2017-10-19 at 08:29 +0200, Iago Toral wrote: > On Thu, 2017-10-19 at 17:14 +1100, Timothy Arceri wrote: > > > > On 19/10/17 16:57, Iago Toral Quiroga wrote: > > >  From ARB_enhanced_layouts: > > > > > > "[...]when location aliasing, the

Re: [Mesa-dev] [PATCH] glsl/linker: outputs in the same location must share interpolation

On Thu, 2017-10-19 at 17:14 +1100, Timothy Arceri wrote: > > On 19/10/17 16:57, Iago Toral Quiroga wrote: > >  From ARB_enhanced_layouts: > > > > "[...]when location aliasing, the aliases sharing the location > >   must have the same underlying numerical ty

[Mesa-dev] [PATCH] glsl/linker: outputs in the same location must share interpolation

From ARB_enhanced_layouts: "[...]when location aliasing, the aliases sharing the location must have the same underlying numerical type (floating-point or integer) and the same auxiliary storage and interpolation qualification.[...]" Add code to the linker to validate that aliased locations do

Re: [Mesa-dev] [PATCH] i965/sbe: fix active components for SSO programs with over 16 inputs

Ken, do you have any thoughts on this patch? Iago On Fri, 2017-10-13 at 11:10 +0200, Iago Toral Quiroga wrote: > When we have up to 16 FS inputs, the SF unit will reorder our inputs > to be consecutive, however, when we have more than 16 we need to > to read our inputs from the UR

Re: [Mesa-dev] [PATCH] glsl/linker: produce error when invalid explicit locations are used

that I had missed when I sent the v1. Does your review stand for the v2? Iago On Tue, 2017-10-17 at 21:22 +0200, Nicolai Hähnle wrote: > On 16.10.2017 14:06, Iago Toral Quiroga wrote: > > We only need to add a check to validate output locations here. For > > inputs with invalid locatio

[Mesa-dev] [PATCH v2] glsl/linker: produce error when invalid explicit locations are used

We only need to add a check to validate output locations here. For inputs with invalid locations we will fail to link when we can't find a matching output in the same (invalid) location. v2: compute location slots properly depending on shader stage and variable type / direction Fixes:

[Mesa-dev] [PATCH] glsl/linker: produce error when invalid explicit locations are used

We only need to add a check to validate output locations here. For inputs with invalid locations we will fail to link when we can't find a matching output in the same (invalid) location. Fixes: KHR-GL45.enhanced_layouts.varying_location_limit --- src/compiler/glsl/link_varyings.cpp | 12

[Mesa-dev] [PATCH] i965/sbe: fix active components for SSO programs with over 16 inputs

When we have up to 16 FS inputs, the SF unit will reorder our inputs to be consecutive, however, when we have more than 16 we need to to read our inputs from the URB exactly as they have been output from the previous stage. This means that for SSO we have to consider if we have URB padding due to

[Mesa-dev] [PATCH] i965/tes: account for the fact that dvec3/4 inputs take two slots

When computing the total size of the URB for tessellation evaluation inputs we were not accounting for this, and instead we were always assuming that each input would take a single vec4 slot, which could lead to computing a smaller read size than required. Specifically, this is a problem when the

Re: [Mesa-dev] [PATCH 07/12] i965: Add and use STRIDE and WIDTH macros

Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> On Thu, 2017-09-28 at 23:05 -0700, Matt Turner wrote: > You'll notice there were bugs in some of the code being replaced. > --- >  src/intel/compiler/brw_eu_validate.c | 33 +++--- > --- >  1 file c

Re: [Mesa-dev] [PATCH 06/12] i965: Add parentheses around usage of macro arguments

Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> On Thu, 2017-09-28 at 23:05 -0700, Matt Turner wrote: > Otherwise I cannot use this macro in test_eu_validate.cpp > --- >  src/intel/common/gen_device_info.h | 2 +- >  1 file changed, 1 insertion(+), 1 deletion(-) > >

Re: [Mesa-dev] [PATCH 03/12] i965: Fix support for disassembling 64-bit integer immediates

Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> On Thu, 2017-09-28 at 23:05 -0700, Matt Turner wrote: > The type suffixes were wrong, and the 16 was missing the 0 prefix. > > Fixes: 92f787ff86ab ("i965: Add support for disassembling 64-bit > integer immediates")

Re: [Mesa-dev] [PATCH 02/12] i965/fs: Rewrite fsign64 to skip the float -> double conversion

gt; + bld.AND(r, subscript(op[0], BRW_REGISTER_TYPE_UD, 1), > + brw_imm_ud(0x8000u)); >   > - /* Convert from 32-bit float to 64-bit double */ > - result.type = BRW_REGISTER_TYPE_DF; > - inst = bld.MOV(result, rety

Re: [Mesa-dev] [PATCH v3] i965: skip reading unused slots at the begining of the URB for the FS

On Fri, 2017-09-29 at 03:53 -0700, Kenneth Graunke wrote: > On Friday, September 29, 2017 3:36:34 AM PDT Iago Toral Quiroga > wrote: > > We can start reading the URB at the first offset that contains > > varyings > > that are actually read in the URB. We still need

[Mesa-dev] [PATCH v3] i965: skip reading unused slots at the begining of the URB for the FS

We can start reading the URB at the first offset that contains varyings that are actually read in the URB. We still need to make sure that we read at least one varying to honor hardware requirements. This helps alleviate a problem introduced with 99df02ca26f61 for separate shader objects: without

Re: [Mesa-dev] [PATCH v3] i965/fs: force pull model for 64-bit GS inputs

On Thu, 2017-09-28 at 21:19 -0700, Kenneth Graunke wrote: > On Thursday, September 28, 2017 1:24:21 AM PDT Iago Toral Quiroga > wrote: > > Triggering the push model when 64-bit inputs are involved is not > > easy due to > > the constrains on the maximum number of

Re: [Mesa-dev] [PATCH v2] i965: skip reading clip distances from the URB for the FS if possible

On Thu, 2017-09-28 at 21:55 -0700, Kenneth Graunke wrote: > On Thursday, September 28, 2017 3:33:12 AM PDT Iago Toral Quiroga > wrote: > > we can skip these slots when they are not read in the fragment > > shader > > and they are positioned right after a VUE header

Re: [Mesa-dev] [PATCH v2] i965: skip reading clip distances from the URB for the FS if possible

On Thu, 2017-09-28 at 07:24 -0400, Ilia Mirkin wrote: > On Thu, Sep 28, 2017 at 6:33 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: > > we can skip these slots when they are not read in the fragment > > shader > > and they are positioned right after a VUE

[Mesa-dev] [PATCH v2] i965: skip reading clip distances from the URB for the FS if possible

we can skip these slots when they are not read in the fragment shader and they are positioned right after a VUE header that we are already skipping. We also need to ensure that we are passing at least one other varying, since that is a hardware requirement. This helps alleviate a problem

[Mesa-dev] [PATCH v3] i965/fs: force pull model for 64-bit GS inputs

Triggering the push model when 64-bit inputs are involved is not easy due to the constrains on the maximum number of registers that we allow for this mode, however, for GS with 'points' primitive type and just a couple of double varyings we can trigger this and it just doesn't work because the

Re: [Mesa-dev] [PATCH v2] i965/fs: force pull model for 64-bit GS inputs

On Thu, 2017-09-28 at 00:59 -0700, Kenneth Graunke wrote: > On Thursday, September 28, 2017 12:36:27 AM PDT Iago Toral Quiroga > wrote: > > Triggering the push model when 64-bit inputs are involved is not > > easy due to > > the constrains on the maximum number of

[Mesa-dev] [PATCH v2] i965/fs: force pull model for 64-bit GS inputs

Triggering the push model when 64-bit inputs are involved is not easy due to the constrains on the maximum number of registers that we allow for this mode, however, for GS with 'points' primitive type and just a couple of double varyings we can trigger this and it just doesn't work because the

[Mesa-dev] [RFC PATCH] i965: enable up to 32 inputs for geometry shaders in gen8+

We have been exposing only 16 since 1e3e72e3054de with arguments based on register pressure and the number of available GRFs, however, our scalar backend will always limit the number of push registers for GS threads to 24 and fallback to pull model for anything else, so there is really no reason

[Mesa-dev] [PATCH] i965/fs: force pull model for 64-bit GS inputs

Triggering the push model when 64-bit inputs are involved is not easy due to the constrains on the maximum number of registers that we allow for this mode, however, for GS with 'points' primitive type and just a couple of double varyings we can trigger this and it just doesn't work because the

Re: [Mesa-dev] [PATCH] i965: skip reading clip distances from the URB for the FS if possible

Sure, I can do that. Iago On Tue, 2017-09-26 at 06:32 -0400, Ilia Mirkin wrote: > Perhaps a debug message would be warranted in such a situation? I > suspect it would be difficult to debug, esp if it came up in a > regular application. > On Sep 26, 2017 3:50 AM, "Iago To

[Mesa-dev] [PATCH] i965: skip reading clip distances from the URB for the FS if possible

we can skip these slots when they are not read in the fragment shader and they are positioned right after a VUE header that we are already skipping. We also need to ensure that we are passing at least one other varying, since that is a hardware requirement. This helps alleviate a problem

Re: [Mesa-dev] [PATCH v2 0/6] Flag new aux state on aux state updates

omment on patch 4.  Other than > that, 1, 3, 4, and 5 are > > Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net> > > On Fri, Sep 15, 2017 at 3:02 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: > > Jason, Ken: I think this series addresses all your feedbac

[Mesa-dev] [PATCH v2 3/6] i965: emit BRW_NEW_AUX_STATE if we drop the aux surface

--- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 8a809a7320..0328a4604d 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++

[Mesa-dev] [PATCH v2 4/6] i965: emit BRW_NEW_AUX_STATE when we change the fast clear value

--- src/mesa/drivers/dri/i965/brw_blorp.c | 12 ++-- src/mesa/drivers/dri/i965/brw_clear.c | 2 +- src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 28 +++ 3 files changed, 31 insertions(+), 11 deletions(-) diff --git

[Mesa-dev] [PATCH v2 1/6] i965: rename BRW_NEW_FAST_CLEAR_COLOR to BRW_NEW_AUX_STATE

We want to use this flag to signal changes to the aux surfaces, so let's not make it about fast clearing only. Suggested by Jason. --- src/mesa/drivers/dri/i965/brw_blorp.c | 2 +- src/mesa/drivers/dri/i965/brw_context.h | 4 ++--

[Mesa-dev] [PATCH v2 5/6] i965: emit BRW_NEW_AUX_STATE on aux state changes

--- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 0328a4604d..ca1f30200b 100644 ---

[Mesa-dev] [PATCH v2 6/6] i965: emit BRW_NEW_AUX_STATE for textures without unresolved colors

intel_miptree_texture_aux_usage() will report ISL_AUX_USAGE_NONE for textures that have a fast clear surface when they have been fully resolved in order to avoid unnecesary aux surface lookups and save bandwidth. In this case we want to signal new AUX state so we update the surface state

[Mesa-dev] [PATCH v2 0/6] Flag new aux state on aux state updates

Jason, Ken: I think this series addresses all your feedback, let me know if you think I missed anything. Maybe you also prefer to squash some of the patches, let me know if that is the case. Iago Toral Quiroga (6): i965: rename BRW_NEW_FAST_CLEAR_COLOR to BRW_NEW_AUX_STATE i965: emit

[Mesa-dev] [PATCH v2 2/6] i965: emit BRW_NEW_AUX_STATE when we allocate aux surfaces

Fixes a regression introduced with b96313c0e1289b296d7, which removed BRW_NEW_BLORP for a bunch of SURFACE_STATE setup code, including render targets, on the basis that blorp invalidates binding tables but not surface states, however, at least on Broadwell, this caused a regression in a CTS test,

Re: [Mesa-dev] [PATCH 1/5] mesa: align atomic buffer handling code with ubo/ssbo

Thanks for cleaning this up, I left some minor comments in this patch but otherwise patches 1-4 are: Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> Regarding patch 5, I have not worked on the state tracker before, but the change seems straight forward enough that I think you can also

Re: [Mesa-dev] [PATCH 1/5] mesa: align atomic buffer handling code with ubo/ssbo

On Fri, 2017-09-15 at 13:57 +1000, Dave Airlie wrote: > From: Dave Airlie > > this adds automatic size support to the atomic buffer code, > but also realigns the code to act like the ubo/ssbo code. > > Signed-off-by: Dave Airlie > --- >  

Re: [Mesa-dev] [PATCH 5/5] i965/tex: Unify the TexImage and TexSubImage code

eBuffer for us, while _mesa_store_texsubimage > doesn't, but we don't need that anyway - intelTexImage already does > it. Right, we were allocating memory twice for that code path... For the series: Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> > Reviewed-by: Kenneth Graunke

[Mesa-dev] [RFC 2/3] i965: emit BRW_NEW_AUX_STATE when we allocate aux surfaces

Fixes a regression introduced with b96313c0e1289b296d7, which removed BRW_NEW_BLORP for a bunch of SURFACE_STATE setup code, including render targets, on the basis that blorp invalidates binding tables but not surface states, however, at least on Broadwell, this caused a regression in a CTS test,

[Mesa-dev] [RFC 3/3] i965: flag BRW_NEW_AUX_STATE if we drop the aux buffer

--- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 8a809a7320d..0328a4604dd 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++

[Mesa-dev] [RFC 0/3] Flag new aux state when we create/drop aux surfaces

we don't have aux, but I decided to split it because we have not been signaling anything for dropped aux surfaces before. Iago Toral Quiroga (3): i965: rename BRW_NEW_FAST_CLEAR_COLOR to BRW_NEW_AUX_STATE i965: emit BRW_NEW_AUX_STATE when we allocate aux surfaces i965: flag BRW_NEW_AUX_STATE

[Mesa-dev] [RFC 1/3] i965: rename BRW_NEW_FAST_CLEAR_COLOR to BRW_NEW_AUX_STATE

We want to use this flag to signal changes to the aux surfaces, so let's not make it about fast clearing only. Suggested by Jason. --- src/mesa/drivers/dri/i965/brw_blorp.c | 2 +- src/mesa/drivers/dri/i965/brw_context.h | 4 ++--

Re: [Mesa-dev] [PATCH 1/3] glsl: use 'declared_var' instead of 'var' after checking redeclarations

at 12:46 +0200, Iago Toral Quiroga wrote: > Since the original 'var' might have been deleted from this point > forward. > > Bugzila: https://bugs.freedesktop.org/show_bug.cgi?id=102685 > Fixes: 51bf007d2c27fba (glsl: Disallow unsized array of atomic_uint) > --- >  src/compile

[Mesa-dev] [PATCH 3/3] glsl: avoid accessing invalid memory after get_variable_being_redeclared()

After get_variable_being_redeclared() has been called, it is no longer safe to access the original variable pointer, since its memory might have been freed. Aavoid potential bugs by re-assigning the original variable pointer to the result of the function call, making it impossible for the

[Mesa-dev] [PATCH 1/3] glsl: use 'declared_var' instead of 'var' after checking redeclarations

Since the original 'var' might have been deleted from this point forward. Bugzila: https://bugs.freedesktop.org/show_bug.cgi?id=102685 Fixes: 51bf007d2c27fba (glsl: Disallow unsized array of atomic_uint) --- src/compiler/glsl/ast_to_hir.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2

[Mesa-dev] [PATCH 2/3] glsl: make the redeclared variable NULL if it is deleted

get_variable_being_redeclared() can delete the original variable in a specific scenario. The code sets it to NULL after this so other code in that same function doesn't try to access trashed memory after the fact, however, the copy of that variable in the caller code won't see any of this making

[Mesa-dev] [PATCH] i965: add BRW_NEW_BLORP to the list of dirty state for render targets

Commit b96313c0e1289b removed BRW_NEW_BLORP for a bunch of SURFACE_STATE setup code, including render targets, on the basis that blorp invalidates binding tables but not surface states, however, at least on Broadwell, this seems to be causing a regression in a CTS test that seems related to render

Re: [Mesa-dev] [PATCH] glsl: Disallow unsized array of atomic_uint

On Tue, 2017-09-12 at 00:03 -0700, Kenneth Graunke wrote: > On Monday, September 11, 2017 11:15:05 PM PDT Iago Toral Quiroga > wrote: > > This was a bugfix to the spec addressed in OpenGL 4.5 and there is > > a CTS test to check this. > > > > Fixes: > > KHR-

[Mesa-dev] [PATCH] glsl: Disallow unsized array of atomic_uint

This was a bugfix to the spec addressed in OpenGL 4.5 and there is a CTS test to check this. Fixes: KHR-GL45.shader_atomic_counters.negative-unsized-array --- src/compiler/glsl/ast_to_hir.cpp | 11 +++ 1 file changed, 11 insertions(+) diff --git a/src/compiler/glsl/ast_to_hir.cpp

Re: [Mesa-dev] [PATCH v2] i965: do not fallback to linear tiling for stencil surfaces

On Mon, 2017-09-11 at 15:55 +0300, Pohjolainen, Topi wrote: > On Mon, Sep 11, 2017 at 02:33:09PM +0200, Iago Toral Quiroga wrote: > > We were skipping this fallback for depth, but not for stencil > > which the hardware always requires to be W-tiled. > > > > Also, mak

[Mesa-dev] [PATCH v2] i965: do not fallback to linear tiling for stencil surfaces

We were skipping this fallback for depth, but not for stencil which the hardware always requires to be W-tiled. Also, make the checks for whether we need to apply retiling strategies based on usage instead of tiling flags, which is safer and more explicit. This fixes a regression in a CTS test

Re: [Mesa-dev] [PATCH] i965: do not fallback to linear tiling for depth/stencil surfaces

On Mon, 2017-09-11 at 12:15 +0300, Pohjolainen, Topi wrote: > On Mon, Sep 11, 2017 at 10:55:36AM +0200, Iago Toral wrote: > > On Fri, 2017-09-08 at 11:44 +0300, Pohjolainen, Topi wrote: > > > On Fri, Sep 08, 2017 at 09:41:11AM +0200, Iago Toral wrote: > > > > On

Re: [Mesa-dev] [PATCH] i965: do not fallback to linear tiling for depth/stencil surfaces

On Fri, 2017-09-08 at 11:44 +0300, Pohjolainen, Topi wrote: > On Fri, Sep 08, 2017 at 09:41:11AM +0200, Iago Toral wrote: > > On Fri, 2017-09-08 at 00:34 -0700, Kenneth Graunke wrote: > > > On Thursday, September 7, 2017 11:44:04 PM PDT Iago Toral Quiroga > > > wrote:

Re: [Mesa-dev] [PATCH] i965: do not fallback to linear tiling for depth/stencil surfaces

On Fri, 2017-09-08 at 11:44 +0300, Pohjolainen, Topi wrote: > On Fri, Sep 08, 2017 at 09:41:11AM +0200, Iago Toral wrote: > > On Fri, 2017-09-08 at 00:34 -0700, Kenneth Graunke wrote: > > > On Thursday, September 7, 2017 11:44:04 PM PDT Iago Toral Quiroga > > > wrote:

<    3   4   5   6   7   8   9   10   11   12   >