[Mesa-dev] [PATCH] i965/gen7: Only advertise 4 samples for RGBA32F on GLES

[Jordan Justen]

We can't render to 8x MSAA if the width is greater than 64 bits. (see
brw_render_target_supported)

Fixes ES31-CTS.sample_variables.mask.rgba32f.samples_8.mask_*

Signed-off-by: Jordan Justen 
---
 src/mesa/drivers/dri/i965/brw_formatquery.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_formatquery.c 
b/src/mesa/drivers/dri/i965/brw_formatquery.c
index 8f7a910..96cc6e0 100644
--- a/src/mesa/drivers/dri/i965/brw_formatquery.c
+++ b/src/mesa/drivers/dri/i965/brw_formatquery.c
@@ -23,6 +23,7 @@
 
 #include "brw_context.h"
 #include "brw_state.h"
+#include "main/context.h"
 #include "main/formatquery.h"
 #include "main/glformats.h"
 
@@ -50,9 +51,24 @@ brw_query_samples_for_format(struct gl_context *ctx, GLenum 
target,
   return 3;
 
case 7:
-  samples[0] = 8;
-  samples[1] = 4;
-  return 2;
+  if (internalFormat == GL_RGBA32F && _mesa_is_gles(ctx)) {
+ /* For GLES, we are allowed to return a smaller number of samples for
+  * GL_RGBA32F. See OpenGLES 3.2 spec, section 20.3.1 Internal Format
+  * Query Parameters, under SAMPLES:
+  *
+  * "A value less than or equal to the value of MAX_SAMPLES, if
+  *  internalformat is RGBA16F, R32F, RG32F, or RGBA32F."
+  *
+  * In brw_render_target_supported, we prevent formats with a size
+  * greater than 8 bytes from using 8x MSAA on gen7.
+  */
+ samples[0] = 4;
+ return 1;
+  } else {
+ samples[0] = 8;
+ samples[1] = 4;
+ return 2;
+  }
 
case 6:
   samples[0] = 4;
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH] mesa/glsl: remove unused uses_builtin_functions field

[Samuel Iglesias Gonsálvez]

Reviewed-by: Samuel Iglesias Gonsálvez 

On Tue, 2016-11-22 at 18:01 +1100, Timothy Arceri wrote:
> This has been unused since 943b69cddd
> ---
>  src/compiler/glsl/glsl_parser_extras.cpp | 1 -
>  src/mesa/main/ff_fragment_shader.cpp | 1 -
>  src/mesa/main/mtypes.h   | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index 88c7822..4f76815 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -1984,7 +1984,6 @@ _mesa_glsl_compile_shader(struct gl_context
> *ctx, struct gl_shader *shader,
> shader->InfoLog = state->info_log;
> shader->Version = state->language_version;
> shader->IsES = state->es_shader;
> -   shader->info.uses_builtin_functions = state-
> >uses_builtin_functions;
>  
> /* Retain any live IR, but trash the rest. */
> reparent_ir(shader->ir, shader->ir);
> diff --git a/src/mesa/main/ff_fragment_shader.cpp
> b/src/mesa/main/ff_fragment_shader.cpp
> index c2618b7..036c5a3 100644
> --- a/src/mesa/main/ff_fragment_shader.cpp
> +++ b/src/mesa/main/ff_fragment_shader.cpp
> @@ -1256,7 +1256,6 @@ create_new_program(struct gl_context *ctx,
> struct state_key *key)
>  
> p.shader->CompileStatus = true;
> p.shader->Version = state->language_version;
> -   p.shader->info.uses_builtin_functions = state-
> >uses_builtin_functions;
> p.shader_program->Shaders =
>    (gl_shader **)malloc(sizeof(*p.shader_program->Shaders));
> p.shader_program->Shaders[0] = p.shader;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 0b38b4f..93a70b4 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2234,7 +2234,6 @@ struct gl_subroutine_function
>   */
>  struct gl_shader_info
>  {
> -   bool uses_builtin_functions;
> bool uses_gl_fragcoord;
> bool redeclares_gl_fragcoord;
> bool ARB_fragment_coord_conventions_enable;

signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 07/13] glsl: assert on incoherent point mode layout-id-qualifier validation

[Andres Gomez]

The point mode value in an ast_type_qualifier can only be true if the
flag is already set since this layout-id-qualifier can only be or not
be present in a shader.

Hence, it is useless to check for its value if the flag is already
set. Just replaced with an assert.

V2: assert instead of checking for coherence and raising a compilation
error. Suggested by Timothy.

Signed-off-by: Andres Gomez 
---
 src/compiler/glsl/ast_type.cpp | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp
index 6c1efc2..a21480d 100644
--- a/src/compiler/glsl/ast_type.cpp
+++ b/src/compiler/glsl/ast_type.cpp
@@ -316,10 +316,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
}
 
if (q.flags.q.point_mode) {
-  if (this->flags.q.point_mode && this->point_mode != q.point_mode) {
- _mesa_glsl_error(loc, state, "conflicting point mode used");
- return false;
-  }
+  /* Point mode can only be true if the flag is set. */
+  assert (!this->flags.q.point_mode || (this->point_mode && q.point_mode));
   this->flags.q.point_mode = 1;
   this->point_mode = q.point_mode;
}
@@ -581,12 +579,10 @@ ast_type_qualifier::validate_in_qualifier(YYLTYPE *loc,
"conflicting ordering specified");
}
 
-   if (state->in_qualifier->flags.q.point_mode && this->flags.q.point_mode
-   && state->in_qualifier->point_mode != this->point_mode) {
-  r = false;
-  _mesa_glsl_error(loc, state,
-   "conflicting point mode specified");
-   }
+   /* Point mode can only be true if the flag is set. */
+   assert (!state->in_qualifier->flags.q.point_mode
+   || !this->flags.q.point_mode
+   || (state->in_qualifier->point_mode && this->point_mode));
 
return r;
 }
-- 
2.9.3

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

Re: [Mesa-dev] [PATCH 3/3] radv: Incorporate GPU family into cache UUID.

[Mike Lothian]

Would it be possible to have a cache for each family? I imagine this might
cause issues when someone has multiple cards in their system

On Tue, 22 Nov 2016 at 01:20 Bas Nieuwenhuizen 
wrote:

> Invalidates the cache when someone switches cards.
>
> Signed-off-by: Bas Nieuwenhuizen 
> ---
>  src/amd/vulkan/radv_device.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 04c0bdc..8595973 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -61,9 +61,10 @@ radv_get_function_timestamp(void *ptr, uint32_t*
> timestamp)
>  }
>
>  static int
> -radv_device_get_cache_uuid(void *uuid)
> +radv_device_get_cache_uuid(enum radeon_family family, void *uuid)
>  {
> uint32_t mesa_timestamp, llvm_timestamp;
> +   uint16_t f = family;
> memset(uuid, 0, VK_UUID_SIZE);
> if (radv_get_function_timestamp(radv_device_get_cache_uuid,
> &mesa_timestamp) ||
> radv_get_function_timestamp(LLVMInitializeAMDGPUTargetInfo,
> &llvm_timestamp))
> @@ -71,7 +72,8 @@ radv_device_get_cache_uuid(void *uuid)
>
> memcpy(uuid, &mesa_timestamp, 4);
> memcpy((char*)uuid + 4, &llvm_timestamp, 4);
> -   snprintf((char*)uuid + 8, VK_UUID_SIZE - 8, "radv");
> +   memcpy((char*)uuid + 8, &f, 2);
> +   snprintf((char*)uuid + 10, VK_UUID_SIZE - 10, "radv");
> return 0;
>  }
>
> @@ -120,7 +122,7 @@ radv_physical_device_init(struct radv_physical_device
> *device,
> goto fail;
> }
>
> -   if (radv_device_get_cache_uuid(device->uuid)) {
> +   if (radv_device_get_cache_uuid(device->rad_info.family,
> device->uuid)) {
> radv_finish_wsi(device);
> device->ws->destroy(device->ws);
> goto fail;
> --
> 2.10.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965/gen7: Only advertise 4 samples for RGBA32F on GLES

[Samuel Iglesias Gonsálvez]

Reviewed-by: Samuel Iglesias Gonsálvez 

On Tue, 2016-11-22 at 00:17 -0800, Jordan Justen wrote:
> We can't render to 8x MSAA if the width is greater than 64 bits. (see
> brw_render_target_supported)
> 
> Fixes ES31-CTS.sample_variables.mask.rgba32f.samples_8.mask_*
> 
> Signed-off-by: Jordan Justen 
> ---
>  src/mesa/drivers/dri/i965/brw_formatquery.c | 22
> +++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_formatquery.c
> b/src/mesa/drivers/dri/i965/brw_formatquery.c
> index 8f7a910..96cc6e0 100644
> --- a/src/mesa/drivers/dri/i965/brw_formatquery.c
> +++ b/src/mesa/drivers/dri/i965/brw_formatquery.c
> @@ -23,6 +23,7 @@
>  
>  #include "brw_context.h"
>  #include "brw_state.h"
> +#include "main/context.h"
>  #include "main/formatquery.h"
>  #include "main/glformats.h"
>  
> @@ -50,9 +51,24 @@ brw_query_samples_for_format(struct gl_context
> *ctx, GLenum target,
>    return 3;
>  
> case 7:
> -  samples[0] = 8;
> -  samples[1] = 4;
> -  return 2;
> +  if (internalFormat == GL_RGBA32F && _mesa_is_gles(ctx)) {
> + /* For GLES, we are allowed to return a smaller number of
> samples for
> +  * GL_RGBA32F. See OpenGLES 3.2 spec, section 20.3.1
> Internal Format
> +  * Query Parameters, under SAMPLES:
> +  *
> +  * "A value less than or equal to the value of MAX_SAMPLES,
> if
> +  *  internalformat is RGBA16F, R32F, RG32F, or RGBA32F."
> +  *
> +  * In brw_render_target_supported, we prevent formats with
> a size
> +  * greater than 8 bytes from using 8x MSAA on gen7.
> +  */
> + samples[0] = 4;
> + return 1;
> +  } else {
> + samples[0] = 8;
> + samples[1] = 4;
> + return 2;
> +  }
>  
> case 6:
>    samples[0] = 4;

signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/aubinator: Use the correct length for MEDIA commands

[Lionel Landwerlin]

On 22/11/16 02:58, Jason Ekstrand wrote:

---
  src/intel/tools/decoder.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c
index 6bd02bf..55488eb 100644
--- a/src/intel/tools/decoder.c
+++ b/src/intel/tools/decoder.c
@@ -612,8 +612,8 @@ gen_group_get_length(struct gen_group *group, const 
uint32_t *p)
   return field(h, 0, 7) + 2;
case 1:
   return 1;
-  case 2:
- return 2;
+  case 2: /* MEDIA */
+ return field(h, 0, 7) + 2;


The documentation seems to indicate 0..15 as bits for DWord Length.
Am I missing something?


case 3:
   return field(h, 0, 7) + 2;
}



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

Re: [Mesa-dev] [PATCH 2/4] anv/cmd_buffer: Handle running out of binding tables in compute shaders

[Lionel Landwerlin]

On 22/11/16 04:26, Jason Ekstrand wrote:

If we try to allocate a binding table and fail, we have to get a new
binding table block, re-emit STATE_BASE_ADDRESS, and then try again.  We
already handle this correctly for 3D and blorp but it never got handled for
CS.  This fixes the new stress.lots-of-surface-state.cs.static crucible test.

Cc: "13.0" 
Cc: Jordan Justen 
---
  src/intel/vulkan/genX_cmd_buffer.c | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index de3253f..6645d1b 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -1674,12 +1674,22 @@ flush_compute_descriptor_set(struct anv_cmd_buffer 
*cmd_buffer)
 struct anv_state surfaces = { 0, }, samplers = { 0, };
 VkResult result;
  
-   result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, &samplers);

-   if (result != VK_SUCCESS)
-  return result;
 result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE, &surfaces);
-   if (result != VK_SUCCESS)
-  return result;
+   if (result != VK_SUCCESS) {


Would it make sense to have the same assert(result == 
VK_ERROR_OUT_OF_DEVICE_MEMORY); here?

Anyhow,

Reviewed-by: Lionel Landwerlin 


+  result = anv_cmd_buffer_new_binding_table_block(cmd_buffer);
+  assert(result == VK_SUCCESS);
+
+  /* Re-emit state base addresses so we get the new surface state base
+   * address before we start emitting binding tables etc.
+   */
+  genX(cmd_buffer_emit_state_base_address)(cmd_buffer);
+
+  result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE, &surfaces);
+  assert(result == VK_SUCCESS);
+   }
+   result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, &samplers);
+   assert(result == VK_SUCCESS);
+
  
 struct anv_state push_state = anv_cmd_buffer_cs_push_constants(cmd_buffer);
  



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

Re: [Mesa-dev] [PATCH 3/4] anv/cmd_buffer: Re-emit MEDIA_CURBE_LOAD when CS push constants are dirty

[Lionel Landwerlin]

Reviewed-by: Lionel Landwerlin 

On 22/11/16 04:26, Jason Ekstrand wrote:

This can happen even if the binding table isn't changed.  For instance, you
could have dynamic offsets with your descriptor set.  This fixes the new
stress.lots-of-surface-state.cs.dynamic cricible test.

Cc: "13.0" 
Cc: Jordan Justen 
---
  src/intel/vulkan/genX_cmd_buffer.c | 23 +--
  1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index 6645d1b..eded1c9 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -1687,19 +1687,10 @@ flush_compute_descriptor_set(struct anv_cmd_buffer 
*cmd_buffer)
result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE, &surfaces);
assert(result == VK_SUCCESS);
 }
+
 result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, &samplers);
 assert(result == VK_SUCCESS);
  
-

-   struct anv_state push_state = anv_cmd_buffer_cs_push_constants(cmd_buffer);
-
-   if (push_state.alloc_size) {
-  anv_batch_emit(&cmd_buffer->batch, GENX(MEDIA_CURBE_LOAD), curbe) {
- curbe.CURBETotalDataLength= push_state.alloc_size;
- curbe.CURBEDataStartAddress   = push_state.offset;
-  }
-   }
-
 uint32_t iface_desc_data_dw[GENX(INTERFACE_DESCRIPTOR_DATA_length)];
 struct GENX(INTERFACE_DESCRIPTOR_DATA) desc = {
.BindingTablePointer = surfaces.offset,
@@ -1738,6 +1729,18 @@ genX(cmd_buffer_flush_compute_state)(struct 
anv_cmd_buffer *cmd_buffer)
 if (cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE)
anv_batch_emit_batch(&cmd_buffer->batch, &pipeline->batch);
  
+   if (cmd_buffer->state.push_constants_dirty & VK_SHADER_STAGE_COMPUTE_BIT) {

+  struct anv_state push_state =
+ anv_cmd_buffer_cs_push_constants(cmd_buffer);
+
+  if (push_state.alloc_size) {
+ anv_batch_emit(&cmd_buffer->batch, GENX(MEDIA_CURBE_LOAD), curbe) {
+curbe.CURBETotalDataLength= push_state.alloc_size;
+curbe.CURBEDataStartAddress   = push_state.offset;
+ }
+  }
+   }
+
 if ((cmd_buffer->state.descriptors_dirty & VK_SHADER_STAGE_COMPUTE_BIT) ||
 (cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE)) {
/* FIXME: figure out descriptors for gen7 */



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

Re: [Mesa-dev] [PATCH 4/4] anv/cmd_buffer: Emit a CS stall before setting a CS pipeline

[Lionel Landwerlin]

Reviewed-by: Lionel Landwerlin 

On 22/11/16 04:26, Jason Ekstrand wrote:

Cc: "13.0" 
Cc: Jordan Justen 
---
  src/intel/vulkan/genX_cmd_buffer.c | 14 +-
  1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index eded1c9..6af2a18 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -1726,8 +1726,20 @@ genX(cmd_buffer_flush_compute_state)(struct 
anv_cmd_buffer *cmd_buffer)
  
 genX(flush_pipeline_select_gpgpu)(cmd_buffer);
  
-   if (cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE)

+   if (cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE) {
+  /* From the Sky Lake PRM Vol 2a, MEDIA_VFE_STATE:
+   *
+   *"A stalling PIPE_CONTROL is required before MEDIA_VFE_STATE unless
+   *the only bits that are changed are scoreboard related: Scoreboard
+   *Enable, Scoreboard Type, Scoreboard Mask, Scoreboard * Delta. For
+   *these scoreboard related states, a MEDIA_STATE_FLUSH is
+   *sufficient."
+   */
+  cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
+  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
+
anv_batch_emit_batch(&cmd_buffer->batch, &pipeline->batch);
+   }
  
 if (cmd_buffer->state.push_constants_dirty & VK_SHADER_STAGE_COMPUTE_BIT) {

struct anv_state push_state =



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

[Mesa-dev] [PATCH] gbm: request correct version of the DRI2_FENCE extension

[Lucas Stach]

There is no version 2 of the DRI2_FENCE extension. So only a request
for version 1 has a chance to succeed.

Fixes: 74b1969d717f (gbm: wire up fence extension)
Cc: "13.0" 
Signed-off-by: Lucas Stach 
---
 src/gbm/backends/dri/gbm_dri.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index e9b104e13aca..676abce37d1c 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -245,7 +245,7 @@ struct dri_extension_match {
 static struct dri_extension_match dri_core_extensions[] = {
{ __DRI2_FLUSH, 1, offsetof(struct gbm_dri_device, flush) },
{ __DRI_IMAGE, 1, offsetof(struct gbm_dri_device, image) },
-   { __DRI2_FENCE, 2, offsetof(struct gbm_dri_device, fence), 1 },
+   { __DRI2_FENCE, 1, offsetof(struct gbm_dri_device, fence), 1 },
{ NULL, 0, 0 }
 };
 
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH 04/13] glsl: Split default in layout qualifier merge

[Timothy Arceri]

On Mon, 2016-11-21 at 18:08 +0200, Andres Gomez wrote:
> On Thu, 2016-11-17 at 16:56 +1100, Timothy Arceri wrote:
> > 
> > On Mon, 2016-11-14 at 19:15 +0200, Andres Gomez wrote:
> ... 
> > 
> > > 
> > > diff --git a/src/compiler/glsl/glsl_parser.yy
> > > b/src/compiler/glsl/glsl_parser.yy
> > > index 50f7097..9f18c15 100644
> > > --- a/src/compiler/glsl/glsl_parser.yy
> > > +++ b/src/compiler/glsl/glsl_parser.yy
> > > @@ -2913,8 +2913,10 @@ layout_in_defaults:
> > >   _mesa_glsl_error(&@1, state, "duplicate layout(...)
> > > qualifiers");
> > >   YYERROR;
> > >    } else {
> > > - if (!state->in_qualifier->
> > > -merge_in_qualifier(& @1, state, $1, $$, false))
> > > {
> > > + if (!$1.validate_in_qualifier(& @1, state)) {
> > > +YYERROR;
> > > + }
> > > + if (!$1.merge_into_in_qualifier(& @1, state, $$,
> > > false)) {
> > >  YYERROR;
> > >   }
> > 
> > 
> > To avoid regressions between patches the order should be merge then
> > validate right?
> 
> No, the original code does first the validation and then the merge.
> 
> This swaps when the merging behavior changes, merging the default in
> qualifier as the last step in patch 08/13. Then, we first merge and
> afterwards validate on the resulting layout-qualifier.
> 

Yes. But in this patch aren't you changing the validation to check the
current (already merged) layout rather than the new layout so you need
to switch the order the original code does validation right?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] radv: Incorporate GPU family into cache UUID.

[Bas Nieuwenhuizen]

The cache is actually stored by the application, and the UUID of the
cache is application visible, so it should be able to store multiple
caches. We might want to use different files for the driver cache of
builtin shaders though.

And as I'm writing this, I realized that PCI vendor and device are
also stored in the cache header, so this patch is probably redundant
...

- Bas

On Tue, Nov 22, 2016 at 9:49 AM, Mike Lothian  wrote:
> Would it be possible to have a cache for each family? I imagine this might
> cause issues when someone has multiple cards in their system
>
> On Tue, 22 Nov 2016 at 01:20 Bas Nieuwenhuizen 
> wrote:
>>
>> Invalidates the cache when someone switches cards.
>>
>> Signed-off-by: Bas Nieuwenhuizen 
>> ---
>>  src/amd/vulkan/radv_device.c | 8 +---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
>> index 04c0bdc..8595973 100644
>> --- a/src/amd/vulkan/radv_device.c
>> +++ b/src/amd/vulkan/radv_device.c
>> @@ -61,9 +61,10 @@ radv_get_function_timestamp(void *ptr, uint32_t*
>> timestamp)
>>  }
>>
>>  static int
>> -radv_device_get_cache_uuid(void *uuid)
>> +radv_device_get_cache_uuid(enum radeon_family family, void *uuid)
>>  {
>> uint32_t mesa_timestamp, llvm_timestamp;
>> +   uint16_t f = family;
>> memset(uuid, 0, VK_UUID_SIZE);
>> if (radv_get_function_timestamp(radv_device_get_cache_uuid,
>> &mesa_timestamp) ||
>> radv_get_function_timestamp(LLVMInitializeAMDGPUTargetInfo,
>> &llvm_timestamp))
>> @@ -71,7 +72,8 @@ radv_device_get_cache_uuid(void *uuid)
>>
>> memcpy(uuid, &mesa_timestamp, 4);
>> memcpy((char*)uuid + 4, &llvm_timestamp, 4);
>> -   snprintf((char*)uuid + 8, VK_UUID_SIZE - 8, "radv");
>> +   memcpy((char*)uuid + 8, &f, 2);
>> +   snprintf((char*)uuid + 10, VK_UUID_SIZE - 10, "radv");
>> return 0;
>>  }
>>
>> @@ -120,7 +122,7 @@ radv_physical_device_init(struct radv_physical_device
>> *device,
>> goto fail;
>> }
>>
>> -   if (radv_device_get_cache_uuid(device->uuid)) {
>> +   if (radv_device_get_cache_uuid(device->rad_info.family,
>> device->uuid)) {
>> radv_finish_wsi(device);
>> device->ws->destroy(device->ws);
>> goto fail;
>> --
>> 2.10.2
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/13] glsl: simplifies the merge of the default in layout qualifier

[Timothy Arceri]

On Mon, 2016-11-21 at 18:08 +0200, Andres Gomez wrote:
> On Thu, 2016-11-17 at 16:17 +1100, Timothy Arceri wrote:
> > 
> > On Mon, 2016-11-14 at 19:15 +0200, Andres Gomez wrote:
> ...
> > 
> > > 
> > > diff --git a/src/compiler/glsl/ast_type.cpp
> > > b/src/compiler/glsl/ast_type.cpp
> > > index 803d952..064c63b 100644
> > > --- a/src/compiler/glsl/ast_type.cpp
> > > +++ b/src/compiler/glsl/ast_type.cpp
> ...
> > 
> > > 
> > > @@ -534,6 +549,45 @@
> > > ast_type_qualifier::validate_in_qualifier(YYLTYPE *loc,
> > >    _mesa_glsl_error(loc, state, "invalid input layout
> > > qualifiers
> > > used");
> > > }
> > >  
> > > +   /* The checks below are also performed when merging but we
> > > want
> > > to spit an
> > > +* error against the default global input qualifier as soon
> > > as we
> > > can, with
> > > +* the closest error location in the shader.
> > > +*/
> > 
> > 
> > This comment looks like it should be removed? Don't the below
> > changes
> > remove the validation from the merge?
> 
> I don't think I follow what you mean.
> 
> The comment applies in the sense that the validation below is done
> against the default in qualifier while the validation done in the
> merge
> is between the 2 layout-qualifiers merging.
> 
> That validation of the merge will happen against the default in
> qualifier in any case, in the last step of the merging. However,
> since
> we want to fail ASAP and with the proper failing column, we have to
> validate explicitly against the default in qualifier every time after
> a
> merge.
> 
> In my opinion, the comment stands, although maybe I should rephrase.
> Dunno.

Ok. I see now. I needed to take a look at the final output of the
series to see what was going on. You are just moving these checks in
this patch not removing duplicates.

Can I ask that you write a follow up patch for this series that creates
a helper function for each of these validations. For example:

static bool
validate_ordering(loc, state, qualifier, new_qualifier)
{
   if (qualifier->flags.q.ordering && new_qualifier->flags.q.ordering
   && qualifier->ordering != new_qualifier->ordering) {
  _mesa_glsl_error(loc, state,
   "conflicting ordering specified");
   }

   return true;
}

In merge_qualifier() just put the call in the outer if. For example:

if (q.flags.q.ordering && validate_ordering(...))

This will be better IMO as the code currently exists early when there
is no reason we shouldn't continue on and check the other layout
qualifiers before reporting errors.

This patch is:

Reviewed-by: Timothy Arceri 

> 
> > 
> > 
> > 
> > > 
> > > +
> > > +   /* Input layout qualifiers can be specified multiple
> > > +* times in separate declarations, as long as they match.
> > > +*/
> > > +   if (state->in_qualifier->flags.q.prim_type && this-
> > > > 
> > > > flags.q.prim_type
> > > 
> > > +   && state->in_qualifier->prim_type != this->prim_type) {
> > > +  r = false;
> > > +  _mesa_glsl_error(loc, state,
> > > +   "conflicting input primitive %s
> > > specified",
> > > +   state->stage == MESA_SHADER_GEOMETRY ?
> > > +   "type" : "mode");
> > > +   }
> > > +
> > > +   if (state->in_qualifier->flags.q.vertex_spacing
> > > +   && this->flags.q.vertex_spacing
> > > +   && state->in_qualifier->vertex_spacing != this-
> > > > 
> > > > vertex_spacing) {
> > > 
> > > +  r = false;
> > > +  _mesa_glsl_error(loc, state,
> > > +   "conflicting vertex spacing specified");
> > > +   }
> > > +
> > > +   if (state->in_qualifier->flags.q.ordering && this-
> > > > 
> > > > flags.q.ordering
> > > 
> > > +   && state->in_qualifier->ordering != this->ordering) {
> > > +  r = false;
> > > +  _mesa_glsl_error(loc, state,
> > > +   "conflicting ordering specified");
> > > +   }
> > > +
> > > +   if (state->in_qualifier->flags.q.point_mode && this-
> > > > 
> > > > flags.q.point_mode
> > > 
> > > +   && state->in_qualifier->point_mode != this->point_mode) {
> > > +  r  = false;
> > 
> >            ^
> > There is an extra space here          
> 
> I'll correct this.
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] docs: get rid of duplicated description from sourcetree.html

[Mun Gwan-gyeong]

Signed-off-by: Mun Gwan-gyeong 
---
 docs/sourcetree.html | 1 -
 1 file changed, 1 deletion(-)

diff --git a/docs/sourcetree.html b/docs/sourcetree.html
index aee3321..7013f65 100644
--- a/docs/sourcetree.html
+++ b/docs/sourcetree.html
@@ -140,7 +140,6 @@ each directory.
clover - OpenCL state tracker
dri - Meta state tracker for DRI drivers
glx - Meta state tracker for GLX
-   vdpau - VDPAU state tracker
wgl - Windows WGL state tracker
xa - XA state tracker
xvmc - XvMC state tracker
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH v2] nir/spirv: implement ordered / unordered floating point comparisons properly

[Lionel Landwerlin]

Sounds good to me, thanks!

Reviewed-by: Lionel Landwerlin 

On 17/11/16 08:36, Iago Toral Quiroga wrote:

Besides the logical operation involved, these also require that we test if the
operands are ordered / unordered.

For ordered operations, both operands must be ordered (and they must pass the
conditional test) while for unordered operations it is sufficient if only one
of the operands is unordered (or they pass the logical test).

Fixes the following Vulkan CTS tests:

dEQP-VK.spirv_assembly.instruction.compute.opfunord.equal
dEQP-VK.spirv_assembly.instruction.compute.opfunord.greater
dEQP-VK.spirv_assembly.instruction.compute.opfunord.greaterequal
dEQP-VK.spirv_assembly.instruction.compute.opfunord.less
dEQP-VK.spirv_assembly.instruction.compute.opfunord.lessequal

v2: Fixed typo: s/nir_eq/nir_feq
---
  src/compiler/spirv/vtn_alu.c | 54 +++-
  1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c
index 6d98a62..6d97f6c 100644
--- a/src/compiler/spirv/vtn_alu.c
+++ b/src/compiler/spirv/vtn_alu.c
@@ -257,7 +257,10 @@ vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap)
 case SpvOpBitReverse:return nir_op_bitfield_reverse;
 case SpvOpBitCount:  return nir_op_bit_count;
  
-   /* Comparisons: (TODO: How do we want to handled ordered/unordered?) */

+   /* The ordered / unordered operators need special implementation besides
+* the logical operator to use since they also need to check if operands are
+* ordered.
+*/
 case SpvOpFOrdEqual:return nir_op_feq;
 case SpvOpFUnordEqual:  return nir_op_feq;
 case SpvOpINotEqual:return nir_op_ine;
@@ -447,6 +450,55 @@ vtn_handle_alu(struct vtn_builder *b, SpvOp opcode,
nir_imm_float(&b->nb, INFINITY));
break;
  

Maybe drop the additional empty line?

+
+   case SpvOpFUnordEqual:
+   case SpvOpFUnordNotEqual:
+   case SpvOpFUnordLessThan:
+   case SpvOpFUnordGreaterThan:
+   case SpvOpFUnordLessThanEqual:
+   case SpvOpFUnordGreaterThanEqual: {
+  bool swap;
+  nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, &swap);
+
+  if (swap) {
+ nir_ssa_def *tmp = src[0];
+ src[0] = src[1];
+ src[1] = tmp;
+  }
+
+  val->ssa->def =
+ nir_ior(&b->nb,
+ nir_build_alu(&b->nb, op, src[0], src[1], NULL, NULL),
+ nir_ior(&b->nb,
+ nir_fne(&b->nb, src[0], src[0]),
+ nir_fne(&b->nb, src[1], src[1])));
+  break;
+   }
+
+   case SpvOpFOrdEqual:
+   case SpvOpFOrdNotEqual:
+   case SpvOpFOrdLessThan:
+   case SpvOpFOrdGreaterThan:
+   case SpvOpFOrdLessThanEqual:
+   case SpvOpFOrdGreaterThanEqual: {
+  bool swap;
+  nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, &swap);
+
+  if (swap) {
+ nir_ssa_def *tmp = src[0];
+ src[0] = src[1];
+ src[1] = tmp;
+  }
+
+  val->ssa->def =
+ nir_iand(&b->nb,
+  nir_build_alu(&b->nb, op, src[0], src[1], NULL, NULL),
+  nir_iand(&b->nb,
+  nir_feq(&b->nb, src[0], src[0]),
+  nir_feq(&b->nb, src[1], src[1])));
+  break;
+   }
+
 default: {
bool swap;
nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, &swap);



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

Re: [Mesa-dev] [PATCH v2] nir/spirv: implement ordered / unordered floating point comparisons properly

[Iago Toral]

On Tue, 2016-11-22 at 12:15 +, Lionel Landwerlin wrote:
> > 
> Sounds good to me, thanks!
> 
> Reviewed-by: Lionel Landwerlin 
> 
On 17/11/16 08:36, Iago Toral Quiroga wrote:
> > 
> > Besides the logical operation involved, these also require that we
> > test if the
> > operands are ordered / unordered.
> > 
> > For ordered operations, both operands must be ordered (and they
> > must pass the
> > conditional test) while for unordered operations it is sufficient
> > if only one
> > of the operands is unordered (or they pass the logical test).
> > 
> > Fixes the following Vulkan CTS tests:
> > 
> > dEQP-VK.spirv_assembly.instruction.compute.opfunord.equal
> > dEQP-VK.spirv_assembly.instruction.compute.opfunord.greater
> > dEQP-VK.spirv_assembly.instruction.compute.opfunord.greaterequal
> > dEQP-VK.spirv_assembly.instruction.compute.opfunord.less
> > dEQP-VK.spirv_assembly.instruction.compute.opfunord.lessequal
> > 
> > v2: Fixed typo: s/nir_eq/nir_feq
> > ---
> >   src/compiler/spirv/vtn_alu.c | 54
> > +++-
> >   1 file changed, 53 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/compiler/spirv/vtn_alu.c
> > b/src/compiler/spirv/vtn_alu.c
> > index 6d98a62..6d97f6c 100644
> > --- a/src/compiler/spirv/vtn_alu.c
> > +++ b/src/compiler/spirv/vtn_alu.c
> > @@ -257,7 +257,10 @@ vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode,
> > bool *swap)
> >  case SpvOpBitReverse:return
> > nir_op_bitfield_reverse;
> >  case SpvOpBitCount:  return nir_op_bit_count;
> >   
> > -   /* Comparisons: (TODO: How do we want to handled
> > ordered/unordered?) */
> > +   /* The ordered / unordered operators need special
> > implementation besides
> > +* the logical operator to use since they also need to check if
> > operands are
> > +* ordered.
> > +*/
> >  case SpvOpFOrdEqual:return
> > nir_op_feq;
> >  case SpvOpFUnordEqual:  return
> > nir_op_feq;
> >  case SpvOpINotEqual:return
> > nir_op_ine;
> > @@ -447,6 +450,55 @@ vtn_handle_alu(struct vtn_builder *b, SpvOp
> > opcode,
> > nir_imm_float(&b->nb,
> > INFINITY));
> > break;
> >   
> Maybe drop the additional empty line?

Yeah, that should not be there, I'll drop it before pushing. Thanks!

Iago

> > +
> > +   case SpvOpFUnordEqual:
> > +   case SpvOpFUnordNotEqual:
> > +   case SpvOpFUnordLessThan:
> > +   case SpvOpFUnordGreaterThan:
> > +   case SpvOpFUnordLessThanEqual:
> > +   case SpvOpFUnordGreaterThanEqual: {
> > +  bool swap;
> > +  nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, &swap);
> > +
> > +  if (swap) {
> > + nir_ssa_def *tmp = src[0];
> > + src[0] = src[1];
> > + src[1] = tmp;
> > +  }
> > +
> > +  val->ssa->def =
> > + nir_ior(&b->nb,
> > + nir_build_alu(&b->nb, op, src[0], src[1], NULL,
> > NULL),
> > + nir_ior(&b->nb,
> > + nir_fne(&b->nb, src[0], src[0]),
> > + nir_fne(&b->nb, src[1], src[1])));
> > +  break;
> > +   }
> > +
> > +   case SpvOpFOrdEqual:
> > +   case SpvOpFOrdNotEqual:
> > +   case SpvOpFOrdLessThan:
> > +   case SpvOpFOrdGreaterThan:
> > +   case SpvOpFOrdLessThanEqual:
> > +   case SpvOpFOrdGreaterThanEqual: {
> > +  bool swap;
> > +  nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, &swap);
> > +
> > +  if (swap) {
> > + nir_ssa_def *tmp = src[0];
> > + src[0] = src[1];
> > + src[1] = tmp;
> > +  }
> > +
> > +  val->ssa->def =
> > + nir_iand(&b->nb,
> > +  nir_build_alu(&b->nb, op, src[0], src[1], NULL,
> > NULL),
> > +  nir_iand(&b->nb,
> > +  nir_feq(&b->nb, src[0], src[0]),
> > +  nir_feq(&b->nb, src[1], src[1])));
> > +  break;
> > +   }
> > +
> >  default: {
> > bool swap;
> > nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, &swap);
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] radv: Use library mtime for cache UUID.

[Emil Velikov]

On 22 November 2016 at 01:19, Bas Nieuwenhuizen  
wrote:
> We want to also invalidate the cache when LLVM gets changed. As the
> specific LLVM revision is not fixed at build time, we will need to
> check at runtime. Computing a checksum for LLVM is going to be very
> expensive, so just use the mtime.
Good catch Bas ! Also - woohoo, no more RADV_TIMESTAMP.

> +static int
>  radv_device_get_cache_uuid(void *uuid)
>  {
> +   uint32_t mesa_timestamp, llvm_timestamp;
> memset(uuid, 0, VK_UUID_SIZE);
> -   snprintf(uuid, VK_UUID_SIZE, "radv-%s", RADV_TIMESTAMP);
> +   if (radv_get_function_timestamp(radv_device_get_cache_uuid, 
> &mesa_timestamp) ||
> +   radv_get_function_timestamp(LLVMInitializeAMDGPUTargetInfo, 
> &llvm_timestamp))
Can you please double-check that this works correctly with static LLVM ?

> +   return -1;
> +
> +   memcpy(uuid, &mesa_timestamp, 4);
> +   memcpy((char*)uuid + 4, &llvm_timestamp, 4);
> +   snprintf((char*)uuid + 8, VK_UUID_SIZE - 8, "radv");
Since radv_timestamp.h and RADV_TIMESTAMP are no longer used can I ask
you to remove the machinery which sets up ? And/Or update ANV?
With follow-up patch(es) of course.

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

Re: [Mesa-dev] [PATCH 05/13] glsl: simplifies the merge of the default in layout qualifier

[Andres Gomez]

On Tue, 2016-11-22 at 22:22 +1100, Timothy Arceri wrote:
...
> 
> Can I ask that you write a follow up patch for this series that creates
> a helper function for each of these validations. For example:
> 
> static bool
> validate_ordering(loc, state, qualifier, new_qualifier)
> {
>    if (qualifier->flags.q.ordering && new_qualifier->flags.q.ordering
>    && qualifier->ordering != new_qualifier->ordering) {
>   _mesa_glsl_error(loc, state,
>    "conflicting ordering specified");
>    }
> 
>    return true;
> }
> 
> In merge_qualifier() just put the call in the outer if. For example:
> 
> if (q.flags.q.ordering && validate_ordering(...))
> 
> This will be better IMO as the code currently exists early when there
> is no reason we shouldn't continue on and check the other layout
> qualifiers before reporting errors.

OK, I will continue with this follow up but I don't think I can just
put the check in the outer if since, then, I will modify the returning
value of the function in some cases. I will work on that.

I also think that continuing without exiting may cause an unexpected
problem but I will do a piglit and cts run to check.

-- 
Br,

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

Re: [Mesa-dev] [PATCH] i965/gen7: Only advertise 4 samples for RGBA32F on GLES

[Iago Toral]

Reviewed-by: Iago Toral Quiroga 

On Tue, 2016-11-22 at 00:17 -0800, Jordan Justen wrote:
> We can't render to 8x MSAA if the width is greater than 64 bits. (see
> brw_render_target_supported)
> 
> Fixes ES31-CTS.sample_variables.mask.rgba32f.samples_8.mask_*
> 
> Signed-off-by: Jordan Justen 
> ---
>  src/mesa/drivers/dri/i965/brw_formatquery.c | 22
> +++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_formatquery.c
> b/src/mesa/drivers/dri/i965/brw_formatquery.c
> index 8f7a910..96cc6e0 100644
> --- a/src/mesa/drivers/dri/i965/brw_formatquery.c
> +++ b/src/mesa/drivers/dri/i965/brw_formatquery.c
> @@ -23,6 +23,7 @@
>  
>  #include "brw_context.h"
>  #include "brw_state.h"
> +#include "main/context.h"
>  #include "main/formatquery.h"
>  #include "main/glformats.h"
>  
> @@ -50,9 +51,24 @@ brw_query_samples_for_format(struct gl_context
> *ctx, GLenum target,
>    return 3;
>  
> case 7:
> -  samples[0] = 8;
> -  samples[1] = 4;
> -  return 2;
> +  if (internalFormat == GL_RGBA32F && _mesa_is_gles(ctx)) {
> + /* For GLES, we are allowed to return a smaller number of
> samples for
> +  * GL_RGBA32F. See OpenGLES 3.2 spec, section 20.3.1
> Internal Format
> +  * Query Parameters, under SAMPLES:
> +  *
> +  * "A value less than or equal to the value of MAX_SAMPLES,
> if
> +  *  internalformat is RGBA16F, R32F, RG32F, or RGBA32F."
> +  *
> +  * In brw_render_target_supported, we prevent formats with
> a size
> +  * greater than 8 bytes from using 8x MSAA on gen7.
> +  */
> + samples[0] = 4;
> + return 1;
> +  } else {
> + samples[0] = 8;
> + samples[1] = 4;
> + return 2;
> +  }
>  
> case 6:
>    samples[0] = 4;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: Add missing error-checking to anv_block_pool_init

[Emil Velikov]

Hi Mun,

On 20 November 2016 at 11:11, Mun Gwan-gyeong  wrote:
> When the allocation fails on u_vector_init(), it returns 0
> This fixes u_vector_init failure path on anv_block_pool_init
>
> CID 1394319
>
> Signed-off-by: Mun Gwan-gyeong 
> ---
>  src/intel/vulkan/anv_allocator.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_allocator.c 
> b/src/intel/vulkan/anv_allocator.c
> index f472213..30bbd69 100644
> --- a/src/intel/vulkan/anv_allocator.c
> +++ b/src/intel/vulkan/anv_allocator.c
> @@ -269,8 +269,10 @@ anv_block_pool_init(struct anv_block_pool *pool,
> if (ftruncate(pool->fd, BLOCK_POOL_MEMFD_SIZE) == -1)
>return;
>
> -   u_vector_init(&pool->mmap_cleanups,
> -   round_to_power_of_two(sizeof(struct anv_mmap_cleanup)), 
> 128);
> +   if (!u_vector_init(&pool->mmap_cleanups,
> +  round_to_power_of_two(sizeof(struct anv_mmap_cleanup)),
> +  128))
> +  return;
>
Seems like we don't do any proper teardown in the error paths in this
function. Can you address that first ?
As-is even though we return here, we'll crash shortly after since the
error is not propagated through anv_CreateDevice().
Having a look at the function it doesn't check anv_bo_init_new() either.

At the same time we can drop the (always true) return type for
anv_queue_init() and radv_queue_init() (in src/amd/vulkan).

Most of the above will be fixes, so please include (in the commit message):
Cc: "13.0" 

Keeping Jason in the CC list will be nice.
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: Fix unintentional integer overflow in anv_CreateDmaBufImageINTEL

[Emil Velikov]

On 20 November 2016 at 11:44, Mun Gwan-gyeong  wrote:
> pCreateInfo->strideInBytes * pCreateInfo->extent.height with type
> "unsigned int" is evaluated using 32-bit arithmetic.
> This fixes unintentional integer overflow by casting to uint64_t before
> multifying.
>
> CID 1394321
>

Thanks for the patch Mun, I've applied some minor polish and pushed
everything but the anv_block_pool_init patch.
That one requires a bit more work, such that it doesn't paper over
(too much) the other issues.

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

Re: [Mesa-dev] [PATCH 1/2] intel/common: Add an is_kabylake field to gen_device_info

[Jason Ekstrand]

On Mon, Nov 21, 2016 at 2:49 PM, Matt Turner  wrote:

> On Tue, Nov 8, 2016 at 1:21 PM, Jason Ekstrand 
> wrote:
> > Most of the 3-D engine Kaby Lake is identical to Sky Lake.  However,
> there
> > are a few small differences that we need to be able to detect.
> >
> > Signed-off-by: Jason Ekstrand 
>
> I noticed this patch has never landed.
>

Dug out, dusted off, and landed!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] anv/cmd_buffer: Handle running out of binding tables in compute shaders

[Jason Ekstrand]

On Tue, Nov 22, 2016 at 2:08 AM, Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> On 22/11/16 04:26, Jason Ekstrand wrote:
>
>> If we try to allocate a binding table and fail, we have to get a new
>> binding table block, re-emit STATE_BASE_ADDRESS, and then try again.  We
>> already handle this correctly for 3D and blorp but it never got handled
>> for
>> CS.  This fixes the new stress.lots-of-surface-state.cs.static crucible
>> test.
>>
>> Cc: "13.0" 
>> Cc: Jordan Justen 
>> ---
>>   src/intel/vulkan/genX_cmd_buffer.c | 20 +++-
>>   1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
>> b/src/intel/vulkan/genX_cmd_buffer.c
>> index de3253f..6645d1b 100644
>> --- a/src/intel/vulkan/genX_cmd_buffer.c
>> +++ b/src/intel/vulkan/genX_cmd_buffer.c
>> @@ -1674,12 +1674,22 @@ flush_compute_descriptor_set(struct
>> anv_cmd_buffer *cmd_buffer)
>>  struct anv_state surfaces = { 0, }, samplers = { 0, };
>>  VkResult result;
>>   -   result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, &samplers);
>> -   if (result != VK_SUCCESS)
>> -  return result;
>>  result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE,
>> &surfaces);
>> -   if (result != VK_SUCCESS)
>> -  return result;
>> +   if (result != VK_SUCCESS) {
>>
>
> Would it make sense to have the same assert(result ==
> VK_ERROR_OUT_OF_DEVICE_MEMORY); here?
> Anyhow,
>

Yes, but I forgot to make the change before pushing. :(  I'll push another
patch.


> Reviewed-by: Lionel Landwerlin 
>

Thanks!


>
> +  result = anv_cmd_buffer_new_binding_table_block(cmd_buffer);
>> +  assert(result == VK_SUCCESS);
>> +
>> +  /* Re-emit state base addresses so we get the new surface state
>> base
>> +   * address before we start emitting binding tables etc.
>> +   */
>> +  genX(cmd_buffer_emit_state_base_address)(cmd_buffer);
>> +
>> +  result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE,
>> &surfaces);
>> +  assert(result == VK_SUCCESS);
>> +   }
>> +   result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, &samplers);
>> +   assert(result == VK_SUCCESS);
>> +
>>struct anv_state push_state = anv_cmd_buffer_cs_push_constan
>> ts(cmd_buffer);
>>
>>
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gbm: request correct version of the DRI2_FENCE extension

[Emil Velikov]

On 22 November 2016 at 10:12, Lucas Stach  wrote:
> There is no version 2 of the DRI2_FENCE extension. So only a request
> for version 1 has a chance to succeed.
>
> Fixes: 74b1969d717f (gbm: wire up fence extension)
> Cc: "13.0" 
> Signed-off-by: Lucas Stach 
> ---
Nicely spotted ! R-b and pushed to master.
For the future, please base your patches against master, this one is
13.0 since it's missing __DRI2_INTEROP.

Can I interest you in refactoring things a bit, thus dropping the
duplication (and chances of such bugs) ?
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 4/6] swr: remove unnecessary -1 entries in format mapping table

[Cherniak, Bruce]

Reviewed-by: Bruce Cherniak 

> On Nov 17, 2016, at 10:56 PM, Ilia Mirkin  wrote:
> 
> Signed-off-by: Ilia Mirkin 
> ---
> src/gallium/drivers/swr/swr_screen.cpp | 126 -
> 1 file changed, 126 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_screen.cpp 
> b/src/gallium/drivers/swr/swr_screen.cpp
> index bc219bd..81b64a3 100644
> --- a/src/gallium/drivers/swr/swr_screen.cpp
> +++ b/src/gallium/drivers/swr/swr_screen.cpp
> @@ -422,11 +422,8 @@ SWR_FORMAT
> mesa_to_swr_format(enum pipe_format format)
> {
>static const std::map mesa2swr = {
> -  {PIPE_FORMAT_NONE,   (SWR_FORMAT)-1},
>   {PIPE_FORMAT_B8G8R8A8_UNORM, B8G8R8A8_UNORM},
>   {PIPE_FORMAT_B8G8R8X8_UNORM, B8G8R8X8_UNORM},
> -  {PIPE_FORMAT_A8R8G8B8_UNORM, (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_X8R8G8B8_UNORM, (SWR_FORMAT)-1},
>   {PIPE_FORMAT_B5G5R5A1_UNORM, B5G5R5A1_UNORM},
>   {PIPE_FORMAT_B4G4R4A4_UNORM, B4G4R4A4_UNORM},
>   {PIPE_FORMAT_B5G6R5_UNORM,   B5G6R5_UNORM},
> @@ -437,35 +434,18 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_L8A8_UNORM, L8A8_UNORM},
>   {PIPE_FORMAT_L16_UNORM,  L16_UNORM},
>   {PIPE_FORMAT_UYVY,   YCRCB_SWAPUVY},
> -  {PIPE_FORMAT_YUYV,   (SWR_FORMAT)-1},
>   {PIPE_FORMAT_Z16_UNORM,  R16_UNORM}, // z
> -  {PIPE_FORMAT_Z32_UNORM,  (SWR_FORMAT)-1},
>   {PIPE_FORMAT_Z32_FLOAT,  R32_FLOAT}, // z
>   {PIPE_FORMAT_Z24_UNORM_S8_UINT,  R24_UNORM_X8_TYPELESS}, // z
> -  {PIPE_FORMAT_S8_UINT_Z24_UNORM,  (SWR_FORMAT)-1},
>   {PIPE_FORMAT_Z24X8_UNORM,R24_UNORM_X8_TYPELESS}, // z
> -  {PIPE_FORMAT_X8Z24_UNORM,(SWR_FORMAT)-1},
> -  {PIPE_FORMAT_S8_UINT,(SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R64_FLOAT,  (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R64G64_FLOAT,   (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R64G64B64_FLOAT,(SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R64G64B64A64_FLOAT, (SWR_FORMAT)-1},
>   {PIPE_FORMAT_R32_FLOAT,  R32_FLOAT},
>   {PIPE_FORMAT_R32G32_FLOAT,   R32G32_FLOAT},
>   {PIPE_FORMAT_R32G32B32_FLOAT,R32G32B32_FLOAT},
>   {PIPE_FORMAT_R32G32B32A32_FLOAT, R32G32B32A32_FLOAT},
> -  {PIPE_FORMAT_R32_UNORM,  (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32_UNORM,   (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32B32_UNORM,(SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32B32A32_UNORM, (SWR_FORMAT)-1},
>   {PIPE_FORMAT_R32_USCALED,R32_USCALED},
>   {PIPE_FORMAT_R32G32_USCALED, R32G32_USCALED},
>   {PIPE_FORMAT_R32G32B32_USCALED,  R32G32B32_USCALED},
>   {PIPE_FORMAT_R32G32B32A32_USCALED,   R32G32B32A32_USCALED},
> -  {PIPE_FORMAT_R32_SNORM,  (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32_SNORM,   (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32B32_SNORM,(SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32B32A32_SNORM, (SWR_FORMAT)-1},
>   {PIPE_FORMAT_R32_SSCALED,R32_SSCALED},
>   {PIPE_FORMAT_R32G32_SSCALED, R32G32_SSCALED},
>   {PIPE_FORMAT_R32G32B32_SSCALED,  R32G32B32_SSCALED},
> @@ -490,7 +470,6 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_R8G8_UNORM, R8G8_UNORM},
>   {PIPE_FORMAT_R8G8B8_UNORM,   R8G8B8_UNORM},
>   {PIPE_FORMAT_R8G8B8A8_UNORM, R8G8B8A8_UNORM},
> -  {PIPE_FORMAT_X8B8G8R8_UNORM, (SWR_FORMAT)-1},
>   {PIPE_FORMAT_R8_USCALED, R8_USCALED},
>   {PIPE_FORMAT_R8G8_USCALED,   R8G8_USCALED},
>   {PIPE_FORMAT_R8G8B8_USCALED, R8G8B8_USCALED},
> @@ -503,10 +482,6 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_R8G8_SSCALED,   R8G8_SSCALED},
>   {PIPE_FORMAT_R8G8B8_SSCALED, R8G8B8_SSCALED},
>   {PIPE_FORMAT_R8G8B8A8_SSCALED,   R8G8B8A8_SSCALED},
> -  {PIPE_FORMAT_R32_FIXED,  (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32_FIXED,   (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32B32_FIXED,(SWR_FORMAT)-1},
> -  {PIPE_FORMAT_R32G32B32A32_FIXED, (SWR_FORMAT)-1},
>   {PIPE_FORMAT_R16_FLOAT,  R16_FLOAT},
>   {PIPE_FORMAT_R16G16_FLOAT,   R16G16_FLOAT},
>   {PIPE_FORMAT_R16G16B16_FLOAT,R16G16B16_FLOAT},
> @@ -515,20 +490,14 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_L8_SRGB,L8_UNORM_SRGB},
>   {PIPE_FORMAT_L8A8_SRGB,  L8A8_UNORM_SRGB},
>   {PIPE_FORMAT_R8G8B8_SRGB,R8G8B8_UNORM_SRGB},
> -  {PIPE_FORMAT_A8B8G8R8_SRGB,  (SWR_FORMAT)-1},
> -  {PIPE_FORMAT_X8B8G8R8_SRGB,  (SWR_FORMAT)-1},
>   {PIPE_FORMAT_B8G8R8A8_SRGB,  B8G8R8A8_UNORM_SRGB},
>   {PIPE_FOR

Re: [Mesa-dev] [PATCH v2 3/6] swr: rework resource layout and surface setup

[Cherniak, Bruce]

Reviewed-by: Bruce Cherniak  

> On Nov 17, 2016, at 10:56 PM, Ilia Mirkin  wrote:
> 
> This is a bit of a mega-commit, but unfortunately there's no great way
> to break this up since a lot of different pieces have to match up. Here
> we do the following:
> - change surface layout to match swr's Load/StoreTile expectations
> - fix sampler settings to respect all sampler view parameters
> - fix stencil sampling to read from secondary resource
> - respect pipe surface format, level, and layer settings
> - fix resource map/unmap based on the new layout logic
> - fix resource map/unmap to copy proper parts of stencil values in and
>   out of the matching depth texture
> 
> These fix a massive quantity of piglits, including all the
> tex-miplevel-selection ones.
> 
> Note that the swr native miptree layout isn't extremely space-efficient,
> and we end up using it for all textures, not just the renderable ones. A
> back-of-the-envelope calculation suggests about 10%-25% increased memory
> usage for miptrees, depending on the number of LODs. Single-LOD textures
> should be unaffected.
> 
> There are a handful of regressions as a result of this change:
> - Some textureGrad tests, these failures match llvmpipe. (There are
>   debug settings allowing improved gallivm sampling accurancy.)
> - Some layered clearing tests as swr doesn't currently support that. It
>   was getting lucky before because enough other things were broken.
> 
> Signed-off-by: Ilia Mirkin 
> ---
> src/gallium/drivers/swr/swr_context.cpp | 103 
> src/gallium/drivers/swr/swr_draw.cpp|   4 +-
> src/gallium/drivers/swr/swr_resource.h  |   8 +-
> src/gallium/drivers/swr/swr_screen.cpp  | 203 ++--
> src/gallium/drivers/swr/swr_shader.cpp  |  28 -
> src/gallium/drivers/swr/swr_state.cpp   | 166 --
> 6 files changed, 352 insertions(+), 160 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_context.cpp 
> b/src/gallium/drivers/swr/swr_context.cpp
> index a5ab236..e42686e 100644
> --- a/src/gallium/drivers/swr/swr_context.cpp
> +++ b/src/gallium/drivers/swr/swr_context.cpp
> @@ -139,21 +139,35 @@ swr_transfer_map(struct pipe_context *pipe,
>if (!pt)
>   return NULL;
>pipe_resource_reference(&pt->resource, resource);
> +   pt->usage = (pipe_transfer_usage)usage;
>pt->level = level;
>pt->box = *box;
> -   pt->stride = spr->row_stride[level];
> -   pt->layer_stride = spr->img_stride[level];
> -
> -   /* if we're mapping the depth/stencil, copy in stencil */
> -   if (spr->base.format == PIPE_FORMAT_Z24_UNORM_S8_UINT
> -   && spr->has_stencil) {
> -  for (unsigned i = 0; i < spr->alignedWidth * spr->alignedHeight; i++) {
> - spr->swr.pBaseAddress[4 * i + 3] = spr->secondary.pBaseAddress[i];
> -  }
> -   } else if (spr->base.format == PIPE_FORMAT_Z32_FLOAT_S8X24_UINT
> -  && spr->has_stencil) {
> -  for (unsigned i = 0; i < spr->alignedWidth * spr->alignedHeight; i++) {
> - spr->swr.pBaseAddress[8 * i + 4] = spr->secondary.pBaseAddress[i];
> +   pt->stride = spr->swr.pitch;
> +   pt->layer_stride = spr->swr.qpitch * spr->swr.pitch;
> +
> +   /* if we're mapping the depth/stencil, copy in stencil for the section
> +* being read in
> +*/
> +   if (usage & PIPE_TRANSFER_READ && spr->has_depth && spr->has_stencil) {
> +  size_t zbase, sbase;
> +  for (int z = box->z; z < box->z + box->depth; z++) {
> + zbase = (z * spr->swr.qpitch + box->y) * spr->swr.pitch +
> +spr->mip_offsets[level];
> + sbase = (z * spr->secondary.qpitch + box->y) * spr->secondary.pitch 
> +
> +spr->secondary_mip_offsets[level];
> + for (int y = box->y; y < box->y + box->height; y++) {
> +if (spr->base.format == PIPE_FORMAT_Z24_UNORM_S8_UINT) {
> +   for (int x = box->x; x < box->x + box->width; x++)
> +  spr->swr.pBaseAddress[zbase + 4 * x + 3] =
> + spr->secondary.pBaseAddress[sbase + x];
> +} else if (spr->base.format == PIPE_FORMAT_Z32_FLOAT_S8X24_UINT) 
> {
> +   for (int x = box->x; x < box->x + box->width; x++)
> +  spr->swr.pBaseAddress[zbase + 8 * x + 4] =
> + spr->secondary.pBaseAddress[sbase + x];
> +}
> +zbase += spr->swr.pitch;
> +sbase += spr->secondary.pitch;
> + }
>   }
>}
> 
> @@ -167,23 +181,60 @@ swr_transfer_map(struct pipe_context *pipe,
> }
> 
> static void
> -swr_transfer_unmap(struct pipe_context *pipe, struct pipe_transfer *transfer)
> +swr_transfer_flush_region(struct pipe_context *pipe,
> +  struct pipe_transfer *transfer,
> +  const struct pipe_box *flush_box)
> {
>assert(transfer->resource);
> +   assert(transfer->usage & PIPE_TRANSFER_WRITE);
> 
> -   struct swr_resource *res = swr_resource(transfer->resource);
> -   /* if we're mappin

Re: [Mesa-dev] [PATCH v2 6/6] swr: avoid using exceptions for expected condition handling

[Cherniak, Bruce]

Reviewed-by: Bruce Cherniak 

> On Nov 17, 2016, at 10:56 PM, Ilia Mirkin  wrote:
> 
> I was getting a weird segfault from GCC 4.9.3:
> 
> 0x754f27aa in strlen () from /lib64/libc.so.6
> (gdb) bt
> #0  0x754f27aa in strlen () from /lib64/libc.so.6
> #1  0x74f128e5 in get_cie_encoding (cie=cie@entry=0x76e09813)
> at /gcc-4.9.3/libgcc/unwind-dw2-fde.c:272
> #2  0x74f1318e in classify_object_over_fdes (ob=ob@entry=0xd7bb90, 
> this_fde=0x77f11010)
> at /gcc-4.9.3/libgcc/unwind-dw2-fde.c:628
> #3  0x74f135ba in init_object (ob=0xd7bb90)
> at /gcc-4.9.3/libgcc/unwind-dw2-fde.c:749
> #4  search_object (ob=ob@entry=0xd7bb90, pc=pc@entry=0x74f11f4d 
> <_Unwind_RaiseException+61>)
> at /gcc-4.9.3/libgcc/unwind-dw2-fde.c:961
> #5  0x74f13e62 in _Unwind_Find_registered_FDE (bases=0x7fffd358, 
> pc=0x74f11f4d <_Unwind_RaiseException+61>)
> at /gcc-4.9.3/libgcc/unwind-dw2-fde.c:1025
> #6  _Unwind_Find_FDE (pc=0x74f11f4d <_Unwind_RaiseException+61>, 
> bases=bases@entry=0x7fffd358)
> at /gcc-4.9.3/libgcc/unwind-dw2-fde-dip.c:450
> #7  0x74f11197 in uw_frame_state_for 
> (context=context@entry=0x7fffd2b0, fs=fs@entry=0x7fffd100)
> at /gcc-4.9.3/libgcc/unwind-dw2.c:1245
> #8  0x74f11b15 in uw_init_context_1 
> (context=context@entry=0x7fffd2b0, 
> outer_cfa=outer_cfa@entry=0x7fffd660, outer_ra=0x7518d23b 
> <__cxa_throw+91>)
> at /gcc-4.9.3/libgcc/unwind-dw2.c:1566
> #9  0x74f11f4e in _Unwind_RaiseException (exc=0xd7c250)
> at /gcc-4.9.3/libgcc/unwind.inc:88
> #10 0x7518d23b in __cxa_throw () from 
> /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/libstdc++.so.6
> #11 0x751ed556 in std::__throw_out_of_range(char const*) ()
>from /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/libstdc++.so.6
> #12 0x7fffea778be0 in std::map std::less, std::allocator SWR_FORMAT> > >::at (
> this=0x7fffebeb4c40 ,
> __k=@0x7fffd73c: PIPE_FORMAT_RGTC1_UNORM)
> at 
> /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/include/g++-v4/bits/stl_map.h:549
> #13 0x7fffea776aee in mesa_to_swr_format (format=PIPE_FORMAT_RGTC1_UNORM) 
> at swr_screen.cpp:597
> 
> We can just void this whole issue by not using exceptions in the
> first place.
> 
> Signed-off-by: Ilia Mirkin 
> ---
> src/gallium/drivers/swr/swr_screen.cpp | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_screen.cpp 
> b/src/gallium/drivers/swr/swr_screen.cpp
> index aede662..36afcc3 100644
> --- a/src/gallium/drivers/swr/swr_screen.cpp
> +++ b/src/gallium/drivers/swr/swr_screen.cpp
> @@ -594,12 +594,11 @@ mesa_to_swr_format(enum pipe_format format)
>   */
>};
> 
> -   try {
> -  return mesa2swr.at(format);
> -   }
> -   catch (std::out_of_range) {
> +   auto it = mesa2swr.find(format);
> +   if (it == mesa2swr.end())
>   return (SWR_FORMAT)-1;
> -   }
> +   else
> +  return it->second;
> }
> 
> static boolean
> -- 
> 2.7.3
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

Re: [Mesa-dev] [PATCH v2 5/6] swr: remove formats from mapping table that don't have StoreTile impls

[Cherniak, Bruce]

Reviewed-by: Bruce Cherniak 

> On Nov 17, 2016, at 10:56 PM, Ilia Mirkin  wrote:
> 
> This table exists for the purpose of determining renderable formats.
> Without a StoreTile implementation, that can't happen.
> 
> This basically removes rendering support to all L/LA/I formats. They can
> be re-added when/if StoreTile implementations are added.
> 
> Signed-off-by: Ilia Mirkin 
> ---
> src/gallium/drivers/swr/swr_screen.cpp | 86 +++---
> 1 file changed, 48 insertions(+), 38 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_screen.cpp 
> b/src/gallium/drivers/swr/swr_screen.cpp
> index 81b64a3..aede662 100644
> --- a/src/gallium/drivers/swr/swr_screen.cpp
> +++ b/src/gallium/drivers/swr/swr_screen.cpp
> @@ -428,12 +428,7 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_B4G4R4A4_UNORM, B4G4R4A4_UNORM},
>   {PIPE_FORMAT_B5G6R5_UNORM,   B5G6R5_UNORM},
>   {PIPE_FORMAT_R10G10B10A2_UNORM,  R10G10B10A2_UNORM},
> -  {PIPE_FORMAT_L8_UNORM,   L8_UNORM},
>   {PIPE_FORMAT_A8_UNORM,   A8_UNORM},
> -  {PIPE_FORMAT_I8_UNORM,   I8_UNORM},
> -  {PIPE_FORMAT_L8A8_UNORM, L8A8_UNORM},
> -  {PIPE_FORMAT_L16_UNORM,  L16_UNORM},
> -  {PIPE_FORMAT_UYVY,   YCRCB_SWAPUVY},
>   {PIPE_FORMAT_Z16_UNORM,  R16_UNORM}, // z
>   {PIPE_FORMAT_Z32_FLOAT,  R32_FLOAT}, // z
>   {PIPE_FORMAT_Z24_UNORM_S8_UINT,  R24_UNORM_X8_TYPELESS}, // z
> @@ -487,26 +482,11 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_R16G16B16_FLOAT,R16G16B16_FLOAT},
>   {PIPE_FORMAT_R16G16B16A16_FLOAT, R16G16B16A16_FLOAT},
> 
> -  {PIPE_FORMAT_L8_SRGB,L8_UNORM_SRGB},
> -  {PIPE_FORMAT_L8A8_SRGB,  L8A8_UNORM_SRGB},
>   {PIPE_FORMAT_R8G8B8_SRGB,R8G8B8_UNORM_SRGB},
>   {PIPE_FORMAT_B8G8R8A8_SRGB,  B8G8R8A8_UNORM_SRGB},
>   {PIPE_FORMAT_B8G8R8X8_SRGB,  B8G8R8X8_UNORM_SRGB},
>   {PIPE_FORMAT_R8G8B8A8_SRGB,  R8G8B8A8_UNORM_SRGB},
> 
> -  {PIPE_FORMAT_DXT1_RGBA,  BC1_UNORM},
> -  {PIPE_FORMAT_DXT3_RGBA,  BC2_UNORM},
> -  {PIPE_FORMAT_DXT5_RGBA,  BC3_UNORM},
> -
> -  {PIPE_FORMAT_DXT1_SRGBA, BC1_UNORM_SRGB},
> -  {PIPE_FORMAT_DXT3_SRGBA, BC2_UNORM_SRGB},
> -  {PIPE_FORMAT_DXT5_SRGBA, BC3_UNORM_SRGB},
> -
> -  {PIPE_FORMAT_RGTC1_UNORM,BC4_UNORM},
> -  {PIPE_FORMAT_RGTC1_SNORM,BC4_SNORM},
> -  {PIPE_FORMAT_RGTC2_UNORM,BC5_UNORM},
> -  {PIPE_FORMAT_RGTC2_SNORM,BC5_SNORM},
> -
>   {PIPE_FORMAT_B5G5R5X1_UNORM, B5G5R5X1_UNORM},
>   {PIPE_FORMAT_R10G10B10A2_USCALED,R10G10B10A2_USCALED},
>   {PIPE_FORMAT_R11G11B10_FLOAT,R11G11B10_FLOAT},
> @@ -515,18 +495,9 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_B10G10R10A2_UNORM,  B10G10R10A2_UNORM},
>   {PIPE_FORMAT_R8G8B8X8_UNORM, R8G8B8X8_UNORM},
> 
> -  {PIPE_FORMAT_L16A16_UNORM,   L16A16_UNORM},
>   {PIPE_FORMAT_A16_UNORM,  A16_UNORM},
> -  {PIPE_FORMAT_I16_UNORM,  I16_UNORM},
> -
>   {PIPE_FORMAT_A16_FLOAT,  A16_FLOAT},
> -  {PIPE_FORMAT_L16_FLOAT,  L16_FLOAT},
> -  {PIPE_FORMAT_L16A16_FLOAT,   L16A16_FLOAT},
> -  {PIPE_FORMAT_I16_FLOAT,  I16_FLOAT},
>   {PIPE_FORMAT_A32_FLOAT,  A32_FLOAT},
> -  {PIPE_FORMAT_L32_FLOAT,  L32_FLOAT},
> -  {PIPE_FORMAT_L32A32_FLOAT,   L32A32_FLOAT},
> -  {PIPE_FORMAT_I32_FLOAT,  I32_FLOAT},
> 
>   {PIPE_FORMAT_R10G10B10A2_SSCALED,R10G10B10A2_SSCALED},
>   {PIPE_FORMAT_R10G10B10A2_SNORM,  R10G10B10A2_SNORM},
> @@ -565,14 +536,6 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_R32G32B32_SINT, R32G32B32_SINT},
>   {PIPE_FORMAT_R32G32B32A32_SINT,  R32G32B32A32_SINT},
> 
> -  {PIPE_FORMAT_I8_UINT,I8_UINT},
> -  {PIPE_FORMAT_L8_UINT,L8_UINT},
> -  {PIPE_FORMAT_L8A8_UINT,  L8A8_UINT},
> -
> -  {PIPE_FORMAT_I8_SINT,I8_SINT},
> -  {PIPE_FORMAT_L8_SINT,L8_SINT},
> -  {PIPE_FORMAT_L8A8_SINT,  L8A8_SINT},
> -
>   {PIPE_FORMAT_B10G10R10A2_UINT,   B10G10R10A2_UINT},
> 
>   {PIPE_FORMAT_B10G10R10X2_UNORM,  B10G10R10X2_UNORM},
> @@ -581,7 +544,54 @@ mesa_to_swr_format(enum pipe_format format)
>   {PIPE_FORMAT_R32G32B32X32_FLOAT, R32G32B32X32_FLOAT},
>   {PIPE_FORMAT_R10G10B10A2_UINT,   R10G10B10A2_UINT},
> 
> -  {PIPE_FORMAT_B5G6R5_SRGB,B5G6R5_UNORM_SRGB}
> +  {PIPE_FORMAT_B5G6R5_SRGB,B5G6R5_UNORM_SRGB},
> +
> +  /* These formats have entries in SWR but don't have Lo

Re: [Mesa-dev] [PATCH v2 0/6] texture layout improvements

[Cherniak, Bruce]

The entire set has now been r-v-b.
Reviewed-by: Bruce Cherniak 

> On Nov 17, 2016, at 10:56 PM, Ilia Mirkin  wrote:
> 
> Some of these are new, others have had some changes done to them. Most 
> notably,
> I've fixed the 1D layout in both the TilingFunctions logic as well as the
> frontend.
> 
> An update piglit run is available at
> 
> https://people.freedesktop.org/~imirkin/swr/problems.html
> 
> which includes all but the last 2 patches of my tree available at
> 
> https://github.com/imirkin/mesa/commits/swr
> 
> The "swr-6" that is the old version was the output of the last time I sent 
> this
> texture layout series. Some of the fixes come from improvements in these 
> patches
> while others are from unrelated patches that made it into the repo in the
> meanwhile.
> 
> Ilia Mirkin (6):
>  swr: [rasterizer memory] minify original sizes for block formats
>  swr: [rasterizer memory] minify texture width before alignment
>  swr: rework resource layout and surface setup
>  swr: remove unnecessary -1 entries in format mapping table
>  swr: remove formats from mapping table that don't have StoreTile impls
>  swr: avoid using exceptions for expected condition handling
> 
> .../swr/rasterizer/memory/TilingFunctions.h|  40 +-
> src/gallium/drivers/swr/swr_context.cpp| 103 +++--
> src/gallium/drivers/swr/swr_draw.cpp   |   4 +-
> src/gallium/drivers/swr/swr_resource.h |   8 +-
> src/gallium/drivers/swr/swr_screen.cpp | 420 ++---
> src/gallium/drivers/swr/swr_shader.cpp |  28 +-
> src/gallium/drivers/swr/swr_state.cpp  | 166 +---
> 7 files changed, 429 insertions(+), 340 deletions(-)
> 
> -- 
> 2.7.3
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

Re: [Mesa-dev] [PATCH v2 0/6] texture layout improvements

[Ilia Mirkin]

Great, thanks! I'll push these out tonight.

On Tue, Nov 22, 2016 at 11:44 AM, Cherniak, Bruce
 wrote:
> The entire set has now been r-v-b.
> Reviewed-by: Bruce Cherniak 
>
>> On Nov 17, 2016, at 10:56 PM, Ilia Mirkin  wrote:
>>
>> Some of these are new, others have had some changes done to them. Most 
>> notably,
>> I've fixed the 1D layout in both the TilingFunctions logic as well as the
>> frontend.
>>
>> An update piglit run is available at
>>
>> https://people.freedesktop.org/~imirkin/swr/problems.html
>>
>> which includes all but the last 2 patches of my tree available at
>>
>> https://github.com/imirkin/mesa/commits/swr
>>
>> The "swr-6" that is the old version was the output of the last time I sent 
>> this
>> texture layout series. Some of the fixes come from improvements in these 
>> patches
>> while others are from unrelated patches that made it into the repo in the
>> meanwhile.
>>
>> Ilia Mirkin (6):
>>  swr: [rasterizer memory] minify original sizes for block formats
>>  swr: [rasterizer memory] minify texture width before alignment
>>  swr: rework resource layout and surface setup
>>  swr: remove unnecessary -1 entries in format mapping table
>>  swr: remove formats from mapping table that don't have StoreTile impls
>>  swr: avoid using exceptions for expected condition handling
>>
>> .../swr/rasterizer/memory/TilingFunctions.h|  40 +-
>> src/gallium/drivers/swr/swr_context.cpp| 103 +++--
>> src/gallium/drivers/swr/swr_draw.cpp   |   4 +-
>> src/gallium/drivers/swr/swr_resource.h |   8 +-
>> src/gallium/drivers/swr/swr_screen.cpp | 420 
>> ++---
>> src/gallium/drivers/swr/swr_shader.cpp |  28 +-
>> src/gallium/drivers/swr/swr_state.cpp  | 166 +---
>> 7 files changed, 429 insertions(+), 340 deletions(-)
>>
>> --
>> 2.7.3
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] i965/miptree: Don't shrink textures when augmenting for more levels

[Topi Pohjolainen]

This was detected when examining CCS_E failures with piglit test:
"fbo-generatemipmap-formats". Test creates a 2D texture with
dimensions 293x277. It manually loops over all levels and calls
glTexImage2D(). Level one triggers creation of full miptree:
intel_alloc_texture_image_buffer() realizes that there is only one
level in the miptree and calls intel_miptree_create_for_teximage()
to re-allocate the miptree with all 9 levels. However, the end result
is a miptree with level zero dimensions of 292x276.

Related, and possibly calling for treatment of its own is mip-map
generation:
After calling glTexImage2D() against every level test continues by
replacing content for levels one to eight with data derived from level
zero by calling glGenerateMipmapEXT(). This results into the miptree
being allocated anew for every level:
Mip-map generation goes thru meta which ends up validating the texture
(brw_validate_textures()->intel_finalize_mipmap_tree()->
intel_miptree_match_image()) where one finds texture with base level
size 292:276. This results into new miptree being created for the npot
size 293:277. Only here intel_finalize_mipmap_tree() is asked for only
one level, and therefore such is created. Generation for level one in
turn finds right base level size but only one level when two is needed.
And the same goes on for all eight levels.

This patch prevents the shrink maintaining the NPOT size of 293x277.

Signed-off-by: Topi Pohjolainen 
CC: Jason Ekstrand 
CC: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/intel_tex_image.c | 29 +
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c 
b/src/mesa/drivers/dri/i965/intel_tex_image.c
index 4454e53..cbff58a 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -29,6 +29,22 @@
 
 #define FILE_DEBUG_FLAG DEBUG_TEXTURE
 
+/* Make sure one doesn't end up shrinking base level zero unnecessarily.
+ * Determining the base level dimension by shifting higher level dimension
+ * ends up in off-by-one value in case base level has NPOT size (for example,
+ * 293 != 146 << 1).
+ * Choose the original base level dimension when shifted dimensions agree.
+ * Otherwise assume real resize is intended and use the new shifted value.
+ */
+static unsigned 
+get_base_dim(unsigned old_base_dim, unsigned new_level_dim, unsigned level)
+{
+   const unsigned old_level_dim = old_base_dim >> level;
+   const unsigned new_base_dim = new_level_dim << level;
+
+   return old_level_dim == new_level_dim ? old_base_dim : new_base_dim;
+}
+
 /* Work back from the specified level of the image to the baselevel and create 
a
  * miptree of that size.
  */
@@ -40,6 +56,8 @@ intel_miptree_create_for_teximage(struct brw_context *brw,
 {
GLuint lastLevel;
int width, height, depth;
+   const struct intel_mipmap_tree *old_mt = intelObj->mt;
+   const unsigned level = intelImage->base.Base.Level;
 
intel_get_image_dims(&intelImage->base.Base, &width, &height, &depth);
 
@@ -51,20 +69,23 @@ intel_miptree_create_for_teximage(struct brw_context *brw,
case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
case GL_TEXTURE_RECTANGLE:
case GL_TEXTURE_EXTERNAL_OES:
-  assert(intelImage->base.Base.Level == 0);
+  assert(level == 0);
   break;
case GL_TEXTURE_3D:
-  depth <<= intelImage->base.Base.Level;
+  depth = old_mt ? get_base_dim(old_mt->logical_depth0, depth, level) :
+   depth << level;
   /* Fall through */
case GL_TEXTURE_2D:
case GL_TEXTURE_2D_ARRAY:
case GL_TEXTURE_CUBE_MAP:
case GL_TEXTURE_CUBE_MAP_ARRAY:
-  height <<= intelImage->base.Base.Level;
+  height = old_mt ? get_base_dim(old_mt->logical_height0, height, level) :
+height << level;
   /* Fall through */
case GL_TEXTURE_1D:
case GL_TEXTURE_1D_ARRAY:
-  width <<= intelImage->base.Base.Level;
+  width = old_mt ? get_base_dim(old_mt->logical_width0, width, level) :
+   width << level;
   break;
default:
   unreachable("Unexpected target");
-- 
2.5.5

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

[Mesa-dev] [PATCH 2/2] intel/blorp: Fix rectangle size for level-not-zero resolves

[Topi Pohjolainen]

Needed to prevent gpu hangs when mip-mapped compression gets
enabled.

Signed-off-by: Topi Pohjolainen 
CC: Jason Ekstrand 
---
 src/intel/blorp/blorp_clear.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/blorp/blorp_clear.c b/src/intel/blorp/blorp_clear.c
index 83ec57e..9132c3c 100644
--- a/src/intel/blorp/blorp_clear.c
+++ b/src/intel/blorp/blorp_clear.c
@@ -581,8 +581,8 @@ blorp_ccs_resolve(struct blorp_batch *batch,
   y_scaledown = aux_fmtl->bh / 2;
}
params.x0 = params.y0 = 0;
-   params.x1 = params.dst.aux_surf.logical_level0_px.width;
-   params.y1 = params.dst.aux_surf.logical_level0_px.height;
+   params.x1 = params.dst.aux_surf.logical_level0_px.width >> level;
+   params.y1 = params.dst.aux_surf.logical_level0_px.height >> level;
params.x1 = ALIGN(params.x1, x_scaledown) / x_scaledown;
params.y1 = ALIGN(params.y1, y_scaledown) / y_scaledown;
 
-- 
2.5.5

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

[Mesa-dev] [PATCH] main/getteximage: Take y-offset into account for memcpy size

[Eduardo Lima Mitev]

In get_tex_memcpy, when copying texture data directly from source
to destination (when row strides match for both src and dst), the
block size is currently calculated as 'bytes-per-row * image-height',
ignoring the given y-offset argument.

This can cause a read past the end of the mapped buffer, leading to
a segfault.

Fixes CTS test (from crash to pass):
* GL45-CTS/get_texture_sub_image/functional_test
---
 src/mesa/main/texgetimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index b900278..a783ed5 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -654,7 +654,7 @@ get_tex_memcpy(struct gl_context *ctx,
 
   if (src) {
  if (bytesPerRow == dstRowStride && bytesPerRow == srcRowStride) {
-memcpy(dst, src, bytesPerRow * texImage->Height);
+memcpy(dst, src, bytesPerRow * (texImage->Height - yoffset));
  }
  else {
 GLuint row;
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH 1/2] i965/miptree: Don't shrink textures when augmenting for more levels

[Jason Ekstrand]

On Tue, Nov 22, 2016 at 9:09 AM, Topi Pohjolainen <
topi.pohjolai...@gmail.com> wrote:

> This was detected when examining CCS_E failures with piglit test:
> "fbo-generatemipmap-formats". Test creates a 2D texture with
> dimensions 293x277. It manually loops over all levels and calls
> glTexImage2D(). Level one triggers creation of full miptree:
> intel_alloc_texture_image_buffer() realizes that there is only one
> level in the miptree and calls intel_miptree_create_for_teximage()
> to re-allocate the miptree with all 9 levels. However, the end result
> is a miptree with level zero dimensions of 292x276.
>

Man... Who decided that you should be able to resize a texture by uploading
something to miplevel 2... It was a bad idea...

Both are

Reviewed-by: Jason Ekstrand 


> Related, and possibly calling for treatment of its own is mip-map
> generation:
> After calling glTexImage2D() against every level test continues by
> replacing content for levels one to eight with data derived from level
> zero by calling glGenerateMipmapEXT(). This results into the miptree
> being allocated anew for every level:
> Mip-map generation goes thru meta which ends up validating the texture
> (brw_validate_textures()->intel_finalize_mipmap_tree()->
> intel_miptree_match_image()) where one finds texture with base level
> size 292:276. This results into new miptree being created for the npot
> size 293:277. Only here intel_finalize_mipmap_tree() is asked for only
> one level, and therefore such is created. Generation for level one in
> turn finds right base level size but only one level when two is needed.
> And the same goes on for all eight levels.
>
> This patch prevents the shrink maintaining the NPOT size of 293x277.
>
> Signed-off-by: Topi Pohjolainen 
> CC: Jason Ekstrand 
> CC: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/intel_tex_image.c | 29
> +
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index 4454e53..cbff58a 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -29,6 +29,22 @@
>
>  #define FILE_DEBUG_FLAG DEBUG_TEXTURE
>
> +/* Make sure one doesn't end up shrinking base level zero unnecessarily.
> + * Determining the base level dimension by shifting higher level dimension
> + * ends up in off-by-one value in case base level has NPOT size (for
> example,
> + * 293 != 146 << 1).
> + * Choose the original base level dimension when shifted dimensions agree.
> + * Otherwise assume real resize is intended and use the new shifted value.
> + */
> +static unsigned
> +get_base_dim(unsigned old_base_dim, unsigned new_level_dim, unsigned
> level)
> +{
> +   const unsigned old_level_dim = old_base_dim >> level;
> +   const unsigned new_base_dim = new_level_dim << level;
> +
> +   return old_level_dim == new_level_dim ? old_base_dim : new_base_dim;
>
+}
> +
>  /* Work back from the specified level of the image to the baselevel and
> create a
>   * miptree of that size.
>   */
> @@ -40,6 +56,8 @@ intel_miptree_create_for_teximage(struct brw_context
> *brw,
>  {
> GLuint lastLevel;
> int width, height, depth;
> +   const struct intel_mipmap_tree *old_mt = intelObj->mt;
> +   const unsigned level = intelImage->base.Base.Level;
>
> intel_get_image_dims(&intelImage->base.Base, &width, &height, &depth);
>
> @@ -51,20 +69,23 @@ intel_miptree_create_for_teximage(struct brw_context
> *brw,
> case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> case GL_TEXTURE_RECTANGLE:
> case GL_TEXTURE_EXTERNAL_OES:
> -  assert(intelImage->base.Base.Level == 0);
> +  assert(level == 0);
>break;
> case GL_TEXTURE_3D:
> -  depth <<= intelImage->base.Base.Level;
> +  depth = old_mt ? get_base_dim(old_mt->logical_depth0, depth,
> level) :
> +   depth << level;
>/* Fall through */
> case GL_TEXTURE_2D:
> case GL_TEXTURE_2D_ARRAY:
> case GL_TEXTURE_CUBE_MAP:
> case GL_TEXTURE_CUBE_MAP_ARRAY:
> -  height <<= intelImage->base.Base.Level;
> +  height = old_mt ? get_base_dim(old_mt->logical_height0, height,
> level) :
> +height << level;
>/* Fall through */
> case GL_TEXTURE_1D:
> case GL_TEXTURE_1D_ARRAY:
> -  width <<= intelImage->base.Base.Level;
> +  width = old_mt ? get_base_dim(old_mt->logical_width0, width,
> level) :
> +   width << level;
>break;
> default:
>unreachable("Unexpected target");
> --
> 2.5.5
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [AppVeyor] mesa master #2773 failed

[AppVeyor]

Build mesa 2773 failed


Commit d219720d19 by Marek Olšák on 11/18/2016 8:00 PM:

mesa: use special checksums for unset checksums and fixed-func shaders\n\nfor debugging\n\nReviewed-by: Timothy Arceri 


Configure your notification preferences

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

[Mesa-dev] [AppVeyor] mesa master #2774 completed

[AppVeyor]

Build mesa 2774 completed



Commit a3f6bea69a by Marek Olšák on 11/22/2016 5:28 PM:

gallium: fix more occurences of u_hash.h\n\nthis fixes compile failures since 86514d84e0beec47c82da4888db12bf07f33cb83


Configure your notification preferences

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

Re: [Mesa-dev] [PATCH] main/getteximage: Take y-offset into account for memcpy size

[Jason Ekstrand]

On Tue, Nov 22, 2016 at 9:11 AM, Eduardo Lima Mitev 
wrote:

> In get_tex_memcpy, when copying texture data directly from source
> to destination (when row strides match for both src and dst), the
> block size is currently calculated as 'bytes-per-row * image-height',
> ignoring the given y-offset argument.
>
> This can cause a read past the end of the mapped buffer, leading to
> a segfault.
>
> Fixes CTS test (from crash to pass):
> * GL45-CTS/get_texture_sub_image/functional_test
> ---
>  src/mesa/main/texgetimage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index b900278..a783ed5 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -654,7 +654,7 @@ get_tex_memcpy(struct gl_context *ctx,
>
>if (src) {
>   if (bytesPerRow == dstRowStride && bytesPerRow == srcRowStride) {
> -memcpy(dst, src, bytesPerRow * texImage->Height);
> +memcpy(dst, src, bytesPerRow * (texImage->Height - yoffset));
>

Why not use the height parameter that gets passed in.  That seems more
correct.


>   }
>   else {
>  GLuint row;
> --
> 2.10.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/3] i965/hiz: Add debug dump for depth/hiz buffers around hiz ops

[Matt Turner]

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

[Mesa-dev] [PATCH 3/3] glsl/cache/tests: Cast cache_get result to avoid compiler warning

[Aaron Watry]

disk_cache_get returns void*, but we were storing/comparing a char*.

Signed-off-by: Aaron Watry 
---
Note that this did, and still, segfaults for me when I actually run it...

But at least the compiler is no longer complaining about the type mismatch.
 src/compiler/glsl/tests/cache_test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index 0ef05aa..c663e54 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -208,14 +208,14 @@ test_put_and_get(void)
_mesa_sha1_compute(blob, sizeof(blob), blob_key);
 
/* Ensure that disk_cache_get returns nothing before anything is added. */
-   result = disk_cache_get(cache, blob_key, &size);
+   result = (char*) disk_cache_get(cache, blob_key, &size);
expect_null(result, "disk_cache_get with non-existent item (pointer)");
expect_equal(size, 0, "disk_cache_get with non-existent item (size)");
 
/* Simple test of put and get. */
disk_cache_put(cache, blob_key, blob, sizeof(blob));
 
-   result = disk_cache_get(cache, blob_key, &size);
+   result = (char*) disk_cache_get(cache, blob_key, &size);
expect_equal_str(blob, result, "disk_cache_get of existing item (pointer)");
expect_equal(size, sizeof(blob), "disk_cache_get of existing item (size)");
 
@@ -225,7 +225,7 @@ test_put_and_get(void)
_mesa_sha1_compute(string, sizeof(string), string_key);
disk_cache_put(cache, string_key, string, sizeof(string));
 
-   result = disk_cache_get(cache, string_key, &size);
+   result = (char*) disk_cache_get(cache, string_key, &size);
expect_equal_str(result, string, "2nd disk_cache_get of existing item 
(pointer)");
expect_equal(size, sizeof(string), "2nd disk_cache_get of existing item 
(size)");
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 2/3] compiler/glsl/tests: Fix print format when building 32-bit on 64-bit host

[Aaron Watry]

Avoids two warnings.

Signed-off-by: Aaron Watry 
---
 src/compiler/glsl/tests/cache_test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index ca22605..0ef05aa 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "util/mesa-sha1.h"
 #include "util/disk_cache.h"
@@ -42,7 +43,8 @@ static void
 expect_equal(uint64_t actual, uint64_t expected, const char *test)
 {
if (actual != expected) {
-  fprintf(stderr, "Error: Test '%s' failed: Expected=%ld, Actual=%ld\n",
+  fprintf(stderr, "Error: Test '%s' failed: Expected=%" PRIu64
+  ", Actual=%" PRIu64 "\n",
   test, expected, actual);
   error = true;
}
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/3] compiler/glsl/tests: Fix print format when building 32-bit on 64-bit host

[Aaron Watry]

Avoids three warnings.

Signed-off-by: Aaron Watry 
---
Hope you don't mind my re-wrapping the long lines.

 src/compiler/glsl/tests/blob_test.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/tests/blob_test.c 
b/src/compiler/glsl/tests/blob_test.c
index 4806029..09114b1 100644
--- a/src/compiler/glsl/tests/blob_test.c
+++ b/src/compiler/glsl/tests/blob_test.c
@@ -23,6 +23,7 @@
 
 /* A collection of unit tests for blob.c */
 
+#include 
 #include 
 #include 
 #include 
@@ -49,7 +50,10 @@ static void
 expect_equal(uint64_t expected, uint64_t actual, const char *test)
 {
if (actual != expected) {
-  fprintf (stderr, "Error: Test '%s' failed: Expected=%ld, Actual=%ld\n",
+  fprintf(stderr,
+  "Error: Test '%s' failed: "
+  "Expected=%" PRIu64 ", "
+  "Actual=%" PRIu64 "\n",
test, expected, actual);
   error = true;
}
@@ -59,7 +63,9 @@ static void
 expect_unequal(uint64_t expected, uint64_t actual, const char *test)
 {
if (actual == expected) {
-  fprintf (stderr, "Error: Test '%s' failed: Result=%ld, but expected 
something different.\n",
+  fprintf(stderr,
+  "Error: Test '%s' failed: Result=%" PRIu64 ", "
+  "but expected something different.\n",
test, actual);
   error = true;
}
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 03/58] glsl: use linked_shaders bitmask to iterate stages for subroutine fields

[Emil Velikov]

On 20 November 2016 at 13:28, Timothy Arceri
 wrote:

> @@ -3134,11 +3134,10 @@ check_resources(struct gl_context *ctx, struct 
> gl_shader_program *prog)
>  static void
>  link_calculate_subroutine_compat(struct gl_shader_program *prog)
>  {
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +  const int i = u_bit_scan(&mask);
>struct gl_linked_shader *sh = prog->_LinkedShaders[i];
> -  int count;
> -  if (!sh)
> - continue;
>
>for (unsigned j = 0; j < sh->NumSubroutineUniformRemapTable; j++) {
>   if (sh->SubroutineUniformRemapTable[j] == 
> INACTIVE_UNIFORM_EXPLICIT_LOCATION)
> @@ -3150,7 +3149,7 @@ link_calculate_subroutine_compat(struct 
> gl_shader_program *prog)
>  continue;
>
>   sh->NumSubroutineUniforms++;
> - count = 0;
> + int count = 0;
Nit: Maybe leave this as-is ?

>   if (sh->NumSubroutineFunctions == 0) {
>  linker_error(prog, "subroutine uniform %s defined but no valid 
> functions found\n", uni->type->name);
>  continue;
> @@ -3172,7 +3171,9 @@ link_calculate_subroutine_compat(struct 
> gl_shader_program *prog)
>  static void
>  check_subroutine_resources(struct gl_shader_program *prog)
>  {
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +  const int i = u_bit_scan(&mask);
>struct gl_linked_shader *sh = prog->_LinkedShaders[i];
>
>if (sh) {
Fold this "always true" conditional ?

> @@ -3374,7 +3375,9 @@ check_explicit_uniform_locations(struct gl_context *ctx,
> }
>
> unsigned entries_total = 0;
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +  const int i = u_bit_scan(&mask);
>struct gl_linked_shader *sh = prog->_LinkedShaders[i];
>
>if (!sh)
Drop this if block.

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

[Mesa-dev] [PATCH v2] main/getteximage: Use the height argument to calculate copy size

[Eduardo Lima Mitev]

In get_tex_memcpy, when copying texture data directly from source
to destination (when row strides match for both src and dst), the
copy size is currently calculated using the full texture height
instead of the sub-region height parameter that was passed.

This can cause a read past the end of the mapped buffer when y-offset
is greater than zero, leading to a segfault.

Fixes CTS test (from crash to pass):
* GL45-CTS/get_texture_sub_image/functional_test

v2: (Jason) Use the passed 'height' instead of copying til the
end of the buffer (tex-height - yoffset).
---
 src/mesa/main/texgetimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index b900278..0186819 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -654,7 +654,7 @@ get_tex_memcpy(struct gl_context *ctx,
 
   if (src) {
  if (bytesPerRow == dstRowStride && bytesPerRow == srcRowStride) {
-memcpy(dst, src, bytesPerRow * texImage->Height);
+memcpy(dst, src, bytesPerRow * height);
  }
  else {
 GLuint row;
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH] main/getteximage: Take y-offset into account for memcpy size

[Eduardo Lima Mitev]

On 11/22/2016 06:43 PM, Jason Ekstrand wrote:
> On Tue, Nov 22, 2016 at 9:11 AM, Eduardo Lima Mitev  > wrote:
> 
> In get_tex_memcpy, when copying texture data directly from source
> to destination (when row strides match for both src and dst), the
> block size is currently calculated as 'bytes-per-row * image-height',
> ignoring the given y-offset argument.
> 
> This can cause a read past the end of the mapped buffer, leading to
> a segfault.
> 
> Fixes CTS test (from crash to pass):
> * GL45-CTS/get_texture_sub_image/functional_test
> ---
>  src/mesa/main/texgetimage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index b900278..a783ed5 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -654,7 +654,7 @@ get_tex_memcpy(struct gl_context *ctx,
> 
>if (src) {
>   if (bytesPerRow == dstRowStride && bytesPerRow ==
> srcRowStride) {
> -memcpy(dst, src, bytesPerRow * texImage->Height);
> +memcpy(dst, src, bytesPerRow * (texImage->Height -
> yoffset));
> 
> 
> Why not use the height parameter that gets passed in.  That seems more
> correct.
>  

Indeed, that's the correct thing to do here. Then I wonder if we should
clamp height (and width) to the texture size minus y-offset (and
x-offset). In case the region exceeds texture size, the extra memory
will have undefined data, but we don't crash the driver. Or maybe that's
caught earlier.

Thanks for reviewing!

Eduardo

>   }
>   else {
>  GLuint row;
> --
> 2.10.2
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
> 

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

[Mesa-dev] [PATCH v5] clover: restore support for LLVM <= 3.9

[Vedran Miletić]

The commit 8e430ff8b060b4e8e922bae24b3c57837da6ea77 support for LLVM
3.9 and older versionsin  Clover. This patch restores it and refactors
the support using Clover compatibility layer for LLVM.

v2: merged #ifdef blocks
v3: added support for LLVM 3.6-3.8
v4: add missing #ifdef around 
v5: simplify using templates and lambda

Signed-off-by: Vedran Miletić 
Reviewed-by[v2]: Jan Vesely 
Tested-by[v4]: Vinson Lee 
Tested-by[v4]: Pierre Moreau 
Reviewed-by: Francisco Jerez 
---
 .../state_trackers/clover/llvm/codegen/bitcode.cpp |  9 +++--
 src/gallium/state_trackers/clover/llvm/compat.hpp  | 18 ++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp 
b/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
index 5dcc4f8..d09207b 100644
--- a/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
+++ b/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
@@ -32,6 +32,7 @@
 ///
 
 #include "llvm/codegen.hpp"
+#include "llvm/compat.hpp"
 #include "llvm/metadata.hpp"
 #include "core/error.hpp"
 #include "util/algorithm.hpp"
@@ -99,13 +100,9 @@ clover::llvm::parse_module_library(const module &m, 
::llvm::LLVMContext &ctx,
auto mod = ::llvm::parseBitcodeFile(::llvm::MemoryBufferRef(
 as_string(m.secs[0].data), " "), ctx);
 
-   if (::llvm::Error err = mod.takeError()) {
-  std::string msg;
-  ::llvm::handleAllErrors(std::move(err), [&](::llvm::ErrorInfoBase &EIB) {
- msg = EIB.message();
- fail(r_log, error(CL_INVALID_PROGRAM), msg.c_str());
+   compat::handle_module_error(mod, [&](const std::string &s) {
+ fail(r_log, error(CL_INVALID_PROGRAM), s);
   });
-   }
 
return std::unique_ptr<::llvm::Module>(std::move(*mod));
 }
diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp 
b/src/gallium/state_trackers/clover/llvm/compat.hpp
index a963cff..fc257ec 100644
--- a/src/gallium/state_trackers/clover/llvm/compat.hpp
+++ b/src/gallium/state_trackers/clover/llvm/compat.hpp
@@ -39,6 +39,11 @@
 #include 
 #include 
 #include 
+#if HAVE_LLVM >= 0x0400
+#include 
+#else
+#include 
+#endif
 
 #if HAVE_LLVM >= 0x0307
 #include 
@@ -158,6 +163,19 @@ namespace clover {
 #else
  const auto default_reloc_model = ::llvm::Reloc::Default;
 #endif
+
+ template inline void
+ handle_module_error(M &mod, const F &f) {
+#if HAVE_LLVM >= 0x0400
+if (::llvm::Error err = mod.takeError())
+   ::llvm::handleAllErrors(std::move(err), 
[&](::llvm::ErrorInfoBase &eib) {
+ f(eib.message());
+  });
+#else
+if (!mod)
+   f(mod.getError().message());
+#endif
+ }
   }
}
 }
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 00/13] anv: Implement input attachments

[Jordan Justen]

Series Reviewed-by: Jordan Justen 

On 2016-11-16 11:31:32, Jason Ekstrand wrote:
> This series adds input attachment support to the Intel Vulkan driver.  It
> applies on top of my series to implement CCS.  It can be found here:
> 
> https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/anv-input-att
> 
> Jason Ekstrand (13):
>   compiler: Add the rest of the subpassInput types
>   spirv: Handle the InputAttachmentIndex decoration
>   spirv: Stop warning about input attachments
>   nir: Add a layer_id system value intrinsic
>   i965/fs: Implement load_layer_id for fragment shaders
>   anv/pipeline: Move gather_info further down the compilation process
>   anv: Add an input attachment lowering pass
>   anv/pass: Calculate the combined image usage of attachments
>   anv/pipeline: Add a input_attachment_index to the bindings
>   anv/cmd_buffer: Fix pipeline barriers for input attachments
>   anv: Use pass attachment information to insert flushes
>   anv/pipeline: Handle depth/stencil self-dependencies
>   anv: Set up binding tables and surface states for input attachments
> 
>  src/compiler/builtin_type_macros.h |   7 +-
>  src/compiler/glsl_types.cpp|  20 ++-
>  src/compiler/glsl_types.h  |   1 +
>  src/compiler/nir/nir_intrinsics.h  |   1 +
>  src/compiler/spirv/spirv_to_nir.c  |   2 +-
>  src/compiler/spirv/vtn_private.h   |   1 +
>  src/compiler/spirv/vtn_variables.c |   4 +
>  src/intel/vulkan/Makefile.sources  |   1 +
>  src/intel/vulkan/anv_blorp.c   |  60 +
>  src/intel/vulkan/anv_descriptor_set.c  |   5 +-
>  src/intel/vulkan/anv_image.c   |   9 +-
>  src/intel/vulkan/anv_nir.h |   2 +
>  src/intel/vulkan/anv_nir_apply_pipeline_layout.c   |  27 
>  src/intel/vulkan/anv_nir_lower_input_attachments.c | 141 
> +
>  src/intel/vulkan/anv_pass.c|  16 ++-
>  src/intel/vulkan/anv_pipeline.c|   7 +-
>  src/intel/vulkan/anv_private.h |   8 ++
>  src/intel/vulkan/genX_cmd_buffer.c |  71 ++-
>  src/intel/vulkan/genX_pipeline.c   |  30 -
>  src/intel/vulkan/vk_format_info.h  |   7 +
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   |   5 +
>  src/mesa/program/prog_to_nir.c |   1 +
>  22 files changed, 403 insertions(+), 23 deletions(-)
>  create mode 100644 src/intel/vulkan/anv_nir_lower_input_attachments.c
> 
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/5] swr: only broadcast color0 value, not all color values

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
mailto:timothy.o.row...@intel.com>>

On Nov 21, 2016, at 11:52 AM, Ilia Mirkin 
mailto:imir...@alum.mit.edu>> wrote:

The way that dual-source blending is described for GLES2 is very odd,
and we end up with a shader that both has this property set *and* has a
color1 value to be used as the second source. While changing the state
tracker is an option, it seems more reliable to verify that the
broadcast is only done on color0.

Fixes arb_blend_func_extended-fbo-extended-blend-pattern_gles2

Signed-off-by: Ilia Mirkin mailto:imir...@alum.mit.edu>>
---
src/gallium/drivers/swr/swr_shader.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index e4f9796..2f72239 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -645,7 +645,8 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key &key)

LLVMValueRef out =
   LLVMBuildLoad(gallivm->builder, outputs[attrib][channel], "");
-if 
(swr_fs->info.base.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]) {
+if 
(swr_fs->info.base.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS] &&
+swr_fs->info.base.output_semantic_index[attrib] == 0) {
   for (uint32_t rt = 0; rt < key.nr_cbufs; rt++) {
  STORE(unwrap(out),
pPS,
--
2.7.3


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

Re: [Mesa-dev] [PATCH 2/5] swr: flatshading makes color outputs flat, it doesn't affect others

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
mailto:timothy.o.row...@intel.com>>

On Nov 21, 2016, at 11:52 AM, Ilia Mirkin 
mailto:imir...@alum.mit.edu>> wrote:

We were previously not marking the "regular" flat outputs as flat when
flatshading was enabled.

Signed-off-by: Ilia Mirkin mailto:imir...@alum.mit.edu>>
---
src/gallium/drivers/swr/swr_state.cpp | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_state.cpp 
b/src/gallium/drivers/swr/swr_state.cpp
index dcbe434..8541aca 100644
--- a/src/gallium/drivers/swr/swr_state.cpp
+++ b/src/gallium/drivers/swr/swr_state.cpp
@@ -1490,10 +1490,8 @@ swr_update_derived(struct pipe_context *pipe,
  (ctx->rasterizer->sprite_coord_enable ? 1 : 0);
   for (unsigned i = 0; i < backendState.numAttributes; i++)
  backendState.numComponents[i] = 4;
-   backendState.constantInterpolationMask =
-  ctx->rasterizer->flatshade ?
-  ctx->fs->flatConstantMask :
-  ctx->fs->constantMask;
+   backendState.constantInterpolationMask = ctx->fs->constantMask |
+  (ctx->rasterizer->flatshade ? ctx->fs->flatConstantMask : 0);
   backendState.pointSpriteTexCoordMask = ctx->fs->pointSpriteMask;

   SwrSetBackendState(ctx->swrContext, &backendState);
--
2.7.3


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

Re: [Mesa-dev] [PATCH 3/5] swr: rework vert <-> frag shader linkage logic

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
mailto:timothy.o.row...@intel.com>>

On Nov 21, 2016, at 11:52 AM, Ilia Mirkin 
mailto:imir...@alum.mit.edu>> wrote:

Fixes a few things:
- sprite coords only apply to generic varyings, and are a bitmask
- back color only applies in 2-sided lighting mode
- handle some odd situations between only some front/back colors being
  there. This is only semi-legal in GL, but we shouldn't start
  crashing.

Signed-off-by: Ilia Mirkin mailto:imir...@alum.mit.edu>>
---
src/gallium/drivers/swr/swr_shader.cpp | 93 ++
1 file changed, 50 insertions(+), 43 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index 2f72239..d29f635 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -372,15 +372,6 @@ locate_linkage(ubyte name, ubyte index, struct 
tgsi_shader_info *info)
  }
   }

-   if (name == TGSI_SEMANTIC_COLOR) { // BCOLOR fallback
-  for (int i = 0; i < PIPE_MAX_SHADER_OUTPUTS; i++) {
- if ((info->output_semantic_name[i] == TGSI_SEMANTIC_BCOLOR)
- && (info->output_semantic_index[i] == index)) {
-return i - 1; // position is not part of the linkage
- }
-  }
-   }
-
   return 0x;
}

@@ -523,54 +514,70 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key &key)

  unsigned linkedAttrib =
 locate_linkage(semantic_name, semantic_idx, &ctx->vs->info.base);
-  if (linkedAttrib == 0x) {
- // not found - check for point sprite
- if (ctx->rasterizer->sprite_coord_enable) {
-linkedAttrib = ctx->vs->info.base.num_outputs - 1;
-swr_fs->pointSpriteMask |= (1 << linkedAttrib);
- } else {
-fprintf(stderr,
-"Missing %s[%d]\n",
-tgsi_semantic_names[semantic_name],
-semantic_idx);
-assert(0 && "attribute linkage not found");
+  if (semantic_name == TGSI_SEMANTIC_GENERIC &&
+  ctx->rasterizer->sprite_coord_enable & (1 << semantic_idx)) {
+ /* we add an extra attrib to the backendState in swr_update_derived. 
*/
+ linkedAttrib = ctx->vs->info.base.num_outputs - 1;
+ swr_fs->pointSpriteMask |= (1 << linkedAttrib);
+  } else if (linkedAttrib == 0x) {
+ inputs[attrib][0] = wrap(VIMMED1(0.0f));
+ inputs[attrib][1] = wrap(VIMMED1(0.0f));
+ inputs[attrib][2] = wrap(VIMMED1(0.0f));
+ inputs[attrib][3] = wrap(VIMMED1(1.0f));
+ /* If we're reading in color and 2-sided lighting is enabled, we have
+  * to keep going.
+  */
+ if (semantic_name != TGSI_SEMANTIC_COLOR || !key.light_twoside)
+continue;
+  } else {
+ if (interpMode == TGSI_INTERPOLATE_CONSTANT) {
+swr_fs->constantMask |= 1 << linkedAttrib;
+ } else if (interpMode == TGSI_INTERPOLATE_COLOR) {
+swr_fs->flatConstantMask |= 1 << linkedAttrib;
 }
  }

-  if (interpMode == TGSI_INTERPOLATE_CONSTANT) {
- swr_fs->constantMask |= 1 << linkedAttrib;
-  } else if (interpMode == TGSI_INTERPOLATE_COLOR) {
- swr_fs->flatConstantMask |= 1 << linkedAttrib;
-  }
-
-  for (int channel = 0; channel < TGSI_NUM_CHANNELS; channel++) {
- if (mask & (1 << channel)) {
-Value *indexA = C(linkedAttrib * 12 + channel);
-Value *indexB = C(linkedAttrib * 12 + channel + 4);
-Value *indexC = C(linkedAttrib * 12 + channel + 8);
+  unsigned bcolorAttrib = 0x;
+  Value *offset = NULL;
+  if (semantic_name == TGSI_SEMANTIC_COLOR && key.light_twoside) {
+ bcolorAttrib = locate_linkage(
+   TGSI_SEMANTIC_BCOLOR, semantic_idx, &ctx->vs->info.base);
+ /* Neither front nor back colors were available. Nothing to load. */
+ if (bcolorAttrib == 0x && linkedAttrib == 0x)
+continue;
+ /* If there is no front color, just always use the back color. */
+ if (linkedAttrib == 0x)
+linkedAttrib = bcolorAttrib;

-if ((semantic_name == TGSI_SEMANTIC_COLOR)
-&& ctx->rasterizer->light_twoside) {
-   unsigned bcolorAttrib = locate_linkage(
-  TGSI_SEMANTIC_BCOLOR, semantic_idx, &ctx->vs->info.base);
+ if (bcolorAttrib != 0x) {
+if (interpMode == TGSI_INTERPOLATE_CONSTANT) {
+   swr_fs->constantMask |= 1 << bcolorAttrib;
+} else if (interpMode == TGSI_INTERPOLATE_COLOR) {
+   swr_fs->flatConstantMask |= 1 << bcolorAttrib;
+}

-   unsigned diff = 12 * (bcolorAttrib - linkedAttrib);
+unsigned diff = 12 * (bcolorAttrib - linkedAttrib);

+if (diff) {
   Value *back =
  XOR(C(1), LOAD(pPS, {0, SWR_PS_CONT

Re: [Mesa-dev] [PATCH 4/5] swr: add sprite coord enable mask to fs key

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
mailto:timothy.o.row...@intel.com>>

On Nov 21, 2016, at 11:52 AM, Ilia Mirkin 
mailto:imir...@alum.mit.edu>> wrote:

This fixes gl-coord-replace-doesnt-eliminate-frag-tex-coords

Signed-off-by: Ilia Mirkin mailto:imir...@alum.mit.edu>>
---
src/gallium/drivers/swr/swr_shader.cpp | 3 ++-
src/gallium/drivers/swr/swr_shader.h   | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index d29f635..428c9b3 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -131,6 +131,7 @@ swr_generate_fs_key(struct swr_jit_fs_key &key,

   key.nr_cbufs = ctx->framebuffer.nr_cbufs;
   key.light_twoside = ctx->rasterizer->light_twoside;
+   key.sprite_coord_enable = ctx->rasterizer->sprite_coord_enable;
   memcpy(&key.vs_output_semantic_name,
  &ctx->vs->info.base.output_semantic_name,
  sizeof(key.vs_output_semantic_name));
@@ -515,7 +516,7 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key &key)
  unsigned linkedAttrib =
 locate_linkage(semantic_name, semantic_idx, &ctx->vs->info.base);
  if (semantic_name == TGSI_SEMANTIC_GENERIC &&
-  ctx->rasterizer->sprite_coord_enable & (1 << semantic_idx)) {
+  key.sprite_coord_enable & (1 << semantic_idx)) {
 /* we add an extra attrib to the backendState in swr_update_derived. */
 linkedAttrib = ctx->vs->info.base.num_outputs - 1;
 swr_fs->pointSpriteMask |= (1 << linkedAttrib);
diff --git a/src/gallium/drivers/swr/swr_shader.h 
b/src/gallium/drivers/swr/swr_shader.h
index ccdda44..7e3399c 100644
--- a/src/gallium/drivers/swr/swr_shader.h
+++ b/src/gallium/drivers/swr/swr_shader.h
@@ -51,6 +51,7 @@ struct swr_jit_sampler_key {
struct swr_jit_fs_key : swr_jit_sampler_key {
   unsigned nr_cbufs;
   unsigned light_twoside;
+   unsigned sprite_coord_enable;
   ubyte vs_output_semantic_name[PIPE_MAX_SHADER_OUTPUTS];
   ubyte vs_output_semantic_idx[PIPE_MAX_SHADER_OUTPUTS];
};
--
2.7.3


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

Re: [Mesa-dev] [PATCH 5/5] swr: color interpolation is also supposed to get perspective division

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
mailto:timothy.o.row...@intel.com>>

On Nov 21, 2016, at 11:52 AM, Ilia Mirkin 
mailto:imir...@alum.mit.edu>> wrote:

Signed-off-by: Ilia Mirkin mailto:imir...@alum.mit.edu>>
---
src/gallium/drivers/swr/swr_shader.cpp | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index 428c9b3..294a568 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -457,7 +457,8 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key &key)

  // load/compute w
  Value *vw = nullptr, *pAttribs;
-  if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE) {
+  if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE ||
+  interpMode == TGSI_INTERPOLATE_COLOR) {
 pAttribs = pPerspAttribs;
 switch (interpLoc) {
 case TGSI_INTERPOLATE_LOC_CENTER:
@@ -596,7 +597,8 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key &key)
   Value *interp1 = FMUL(vb, vj);
   interp = FADD(interp, interp1);
   interp = FADD(interp, vc);
-   if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE)
+   if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE ||
+   interpMode == TGSI_INTERPOLATE_COLOR)
  interp = FMUL(interp, vw);
   inputs[attrib][channel] = wrap(interp);
}
--
2.7.3


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

[Mesa-dev] [PATCH 02/18] i965: Use W-typed immediate in brw_F32TO16().

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_emit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index cfb3fa0..3146271 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -1239,7 +1239,7 @@ brw_F32TO16(struct brw_codegen *p, struct brw_reg dst, 
struct brw_reg src)
 
if (needs_zero_fill) {
   brw_inst_set_no_dd_clear(devinfo, inst, true);
-  inst = brw_MOV(p, suboffset(dst, 1), brw_imm_ud(0u));
+  inst = brw_MOV(p, suboffset(dst, 1), brw_imm_w(0));
   brw_inst_set_no_dd_check(devinfo, inst, true);
}
 
-- 
2.7.3

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

[Mesa-dev] [PATCH 04/18] i965: Mark error annotation on correct SIMD16 inst.

[Matt Turner]

inst, whose assignment can be seen in the last line of context pointed
to the correct instruction in the SIMD16 program, but src_offset was the
offset from the beginning of the SIMD16 program.

So if an instruction at offset 0x100 in the SIMD16 program was illegal,
we would mark an error on the instruction at offset 0x100 (which is
likely in the SIMD8 program).
---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index 0e736ed..fa1d67c 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -135,10 +135,10 @@ brw_validate_instructions(const struct brw_codegen *p, 
int start_offset,
   struct annotation_info *annotation)
 {
const struct gen_device_info *devinfo = p->devinfo;
-   const void *store = p->store + start_offset / 16;
+   const void *store = p->store;
bool valid = true;
 
-   for (int src_offset = 0; src_offset < p->next_insn_offset - start_offset;
+   for (int src_offset = start_offset; src_offset < p->next_insn_offset;
 src_offset += sizeof(brw_inst)) {
   struct string error_msg = { .str = NULL, .len = 0 };
   const brw_inst *inst = store + src_offset;
-- 
2.7.3

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

[Mesa-dev] [PATCH 06/18] i965: Add a CHECK macro to call more complicated validation funcs.

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index e23f1ec..3225386 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -53,6 +53,15 @@ cat(struct string *dest, const struct string src)
   }  \
} while(0)
 
+#define CHECK(func, args...) \
+   do {  \
+  struct string __msg = func(devinfo, inst, ##args); \
+  if (__msg.str) {   \
+ cat(&error_msg, __msg); \
+ free(__msg.str);\
+  }  \
+   } while (0)
+
 static bool
 src0_is_null(const struct gen_device_info *devinfo, const brw_inst *inst)
 {
-- 
2.7.3

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

[Mesa-dev] [PATCH 00/18] i965: More shader validation

[Matt Turner]

This series adds the core "Register Region Restrictions" to the shader
validator. I've added a make check test to verify that the validation checks
are correct.

There is still quite a bit more to do, but the next section in the PRM is
"Special Requirements for Handling Double Precision Data Types" and I thought
it would be mean to do so without reviewing Igalia's Gen7 FP64 work first :)

[PATCH 01/18] gtest: Update to 1.8.0.
[PATCH 02/18] i965: Use W-typed immediate in brw_F32TO16().
[PATCH 03/18] i965/vec4: Use UW-typed operands when dest is UW.
[PATCH 04/18] i965: Mark error annotation on correct SIMD16 inst.
[PATCH 05/18] i965: Make ERROR_IF usable from other functions.
[PATCH 06/18] i965: Add a CHECK macro to call more complicated
[PATCH 07/18] i965: Add a test for the EU assembly validator.
[PATCH 08/18] i965: Structure code so unsupported inst will not
[PATCH 09/18] i965: Simplify num_sources_from_inst().
[PATCH 10/18] i965: Factor out sources_not_null() validation
[PATCH 11/18] i965: Factor out send_restrictions() function.
[PATCH 12/18] i965: Claim that SEND/math has two sources.
[PATCH 13/18] i965: Validate math instruction sources.
[PATCH 14/18] i965: Replace reg_type_size[] with a function.
[PATCH 15/18] i965: Validate "General Restrictions on Regioning
[PATCH 16/18] i965: Validate "General Restrictions Based on Operand
[PATCH 17/18] i965: Validate "Region Alignment Rules"
[PATCH 18/18] i965: Validate "Special Cases for Byte Operations"
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 08/18] i965: Structure code so unsupported inst will not generate more errors.

[Matt Turner]

We want to rely on brw_opcode_desc() always returning non-NULL in other
validation functions. Other validation functions will be in the else
case of the block added in this patch.
---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index 3225386..d3c15da 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -46,6 +46,7 @@ cat(struct string *dest, const struct string src)
 
 #define error(str) "\tERROR: " str "\n"
 
+#define ERROR(msg) ERROR_IF(true, msg)
 #define ERROR_IF(cond, msg)  \
do {  \
   if (cond) {\
@@ -168,8 +169,10 @@ brw_validate_instructions(const struct brw_codegen *p, int 
start_offset,
  break;
   }
 
-  ERROR_IF(is_unsupported_inst(devinfo, inst),
-   "Instruction not supported on this Gen");
+  if (is_unsupported_inst(devinfo, inst)) {
+ ERROR("Instruction not supported on this Gen");
+  } else {
+  }
 
   if (brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SEND) {
  ERROR_IF(brw_inst_src0_address_mode(devinfo, inst) !=
-- 
2.7.3

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

[Mesa-dev] [PATCH 05/18] i965: Make ERROR_IF usable from other functions.

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index fa1d67c..e23f1ec 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -50,7 +50,6 @@ cat(struct string *dest, const struct string src)
do {  \
   if (cond) {\
  CAT(error_msg, error(msg)); \
- valid = false;  \
   }  \
} while(0)
 
@@ -178,6 +177,7 @@ brw_validate_instructions(const struct brw_codegen *p, int 
start_offset,
   if (error_msg.str && annotation) {
  annotation_insert_error(annotation, src_offset, error_msg.str);
   }
+  valid = valid && error_msg.len == 0;
   free(error_msg.str);
}
 
-- 
2.7.3

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

[Mesa-dev] [PATCH 09/18] i965: Simplify num_sources_from_inst().

[Matt Turner]

desc will always be non-NULL, because brw_validate_instructions() does
not attempt to validate any instructions that fail the
is_unsupported_inst() check.
---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index d3c15da..efb1f1c 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -103,10 +103,8 @@ num_sources_from_inst(const struct gen_device_info 
*devinfo,
   */
  return 0;
   }
-   } else if (desc) {
-  return desc->nsrc;
} else {
-  return 0;
+  return desc->nsrc;
}
 
switch (math_function) {
-- 
2.7.3

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

[Mesa-dev] [PATCH 03/18] i965/vec4: Use UW-typed operands when dest is UW.

[Matt Turner]

Using a UD-typed operand makes the execution size D, and if the size of
the execution type is greater than the size of the destination type, the
destination must be appropriately strided.

We actually just want UW-types all around.
---
 src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
index bb18479..c3a130f 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
@@ -828,13 +828,15 @@ generate_tcs_input_urb_offsets(struct brw_codegen *p,
   struct brw_reg addr = brw_address_reg(0);
 
   /* bottom half: m0.0 = g[1.0 + vertex.0]UD */
-  brw_ADD(p, addr, get_element_ud(vertex, 0), brw_imm_uw(0x8));
-  brw_SHL(p, addr, addr, brw_imm_ud(2));
+  brw_ADD(p, addr, retype(get_element_ud(vertex, 0), BRW_REGISTER_TYPE_UW),
+  brw_imm_uw(0x8));
+  brw_SHL(p, addr, addr, brw_imm_uw(2));
   brw_MOV(p, get_element_ud(dst, 0), deref_1ud(brw_indirect(0, 0), 0));
 
   /* top half: m0.1 = g[1.0 + vertex.4]UD */
-  brw_ADD(p, addr, get_element_ud(vertex, 4), brw_imm_uw(0x8));
-  brw_SHL(p, addr, addr, brw_imm_ud(2));
+  brw_ADD(p, addr, retype(get_element_ud(vertex, 4), BRW_REGISTER_TYPE_UW),
+  brw_imm_uw(0x8));
+  brw_SHL(p, addr, addr, brw_imm_uw(2));
   brw_MOV(p, get_element_ud(dst, 1), deref_1ud(brw_indirect(0, 0), 0));
}
 
-- 
2.7.3

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

[Mesa-dev] [PATCH 10/18] i965: Factor out sources_not_null() validation function.

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 40 +
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index efb1f1c..d03ed71 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -130,6 +130,28 @@ num_sources_from_inst(const struct gen_device_info 
*devinfo,
}
 }
 
+static struct string
+sources_not_null(const struct gen_device_info *devinfo,
+ const brw_inst *inst)
+{
+   unsigned num_sources = num_sources_from_inst(devinfo, inst);
+   struct string error_msg = { .str = NULL, .len = 0 };
+
+   /* Nothing to test. 3-src instructions can only have GRF sources, and
+* there's no bit to control the file.
+*/
+   if (num_sources == 3)
+  return (struct string){};
+
+   if (num_sources >= 1)
+  ERROR_IF(src0_is_null(devinfo, inst), "src0 is null");
+
+   if (num_sources == 2)
+  ERROR_IF(src1_is_null(devinfo, inst), "src1 is null");
+
+   return error_msg;
+}
+
 static bool
 is_unsupported_inst(const struct gen_device_info *devinfo,
 const brw_inst *inst)
@@ -150,26 +172,10 @@ brw_validate_instructions(const struct brw_codegen *p, 
int start_offset,
   struct string error_msg = { .str = NULL, .len = 0 };
   const brw_inst *inst = store + src_offset;
 
-  switch (num_sources_from_inst(devinfo, inst)) {
-  case 3:
- /* Nothing to test. 3-src instructions can only have GRF sources, and
-  * there's no bit to control the file.
-  */
- break;
-  case 2:
- ERROR_IF(src1_is_null(devinfo, inst), "src1 is null");
- /* fallthrough */
-  case 1:
- ERROR_IF(src0_is_null(devinfo, inst), "src0 is null");
- break;
-  case 0:
-  default:
- break;
-  }
-
   if (is_unsupported_inst(devinfo, inst)) {
  ERROR("Instruction not supported on this Gen");
   } else {
+ CHECK(sources_not_null);
   }
 
   if (brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SEND) {
-- 
2.7.3

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

[Mesa-dev] [PATCH 12/18] i965: Claim that SEND/math has two sources.

[Matt Turner]

src1 must be a descriptor (including the information to determine that
the SEND is doing an extended math operation), but src0 can actually be
null since it serves as the source of the implicit GRF -> MRF move.
---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index 8c2eb99..9648f0d 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -96,7 +96,14 @@ num_sources_from_inst(const struct gen_device_info *devinfo,
} else if (devinfo->gen < 6 &&
   brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SEND) {
   if (brw_inst_sfid(devinfo, inst) == BRW_SFID_MATH) {
- math_function = brw_inst_math_msg_function(devinfo, inst);
+ /* src1 must be a descriptor (including the information to determine
+  * that the SEND is doing an extended math operation), but src0 can
+  * actually be null since it serves as the source of the implicit GRF
+  * to MRF move.
+  *
+  * If we stop using that functionality, we'll have to revisit this.
+  */
+ return 2;
   } else {
  /* Send instructions are allowed to have null sources since they use
   * the base_mrf field to specify which message register source.
-- 
2.7.3

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

[Mesa-dev] [PATCH 18/18] i965: Validate "Special Cases for Byte Operations"

[Matt Turner]

Do this in general_restrictions_based_on_operand_types() because the two
rules that "Special Cases for Byte Operations" relax are checked there.
---
 src/mesa/drivers/dri/i965/brw_eu_validate.c| 70 +---
 src/mesa/drivers/dri/i965/test_eu_validate.cpp | 89 ++
 2 files changed, 150 insertions(+), 9 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index e9b55ed..7d800c1 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -78,6 +78,34 @@ inst_is_send(const struct gen_device_info *devinfo, const 
brw_inst *inst)
}
 }
 
+static unsigned
+signed_type(unsigned type)
+{
+   switch (type) {
+   case BRW_HW_REG_TYPE_UD: return BRW_HW_REG_TYPE_D;
+   case BRW_HW_REG_TYPE_UW: return BRW_HW_REG_TYPE_W;
+   case BRW_HW_REG_NON_IMM_TYPE_UB: return BRW_HW_REG_NON_IMM_TYPE_B;
+   case GEN8_HW_REG_TYPE_UQ:return GEN8_HW_REG_TYPE_Q;
+   default: return type;
+   }
+}
+
+static bool
+inst_is_raw_move(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   unsigned dst_type = signed_type(brw_inst_dst_reg_type(devinfo, inst));
+   unsigned src_type = signed_type(brw_inst_src0_reg_type(devinfo, inst));
+
+   if (brw_inst_src0_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE &&
+   (brw_inst_src0_negate(devinfo, inst) ||
+brw_inst_src0_abs(devinfo, inst)))
+  return false;
+
+   return brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV &&
+  brw_inst_saturate(devinfo, inst) == 0 &&
+  dst_type == src_type;
+}
+
 static bool
 dst_is_null(const struct gen_device_info *devinfo, const brw_inst *inst)
 {
@@ -417,10 +445,21 @@ general_restrictions_based_on_operand_types(const struct 
gen_device_info *devinf
if (desc->ndst == 0)
   return (struct string){};
 
-   /* FINISHME: check special cases for byte operations */
-   if (brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_B ||
-   brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_UB)
-  return (struct string){};
+   unsigned dst_stride = 1 << (brw_inst_dst_hstride(devinfo, inst) - 1);
+   bool dst_type_is_byte =
+  brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_B ||
+  brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_UB;
+
+   if (dst_type_is_byte) {
+  if (is_packed(exec_size * dst_stride, exec_size, dst_stride)) {
+ if (!inst_is_raw_move(devinfo, inst)) {
+ERROR("Only raw MOV supports a packed-byte destination");
+return error_msg;
+ } else {
+return (struct string){};
+ }
+  }
+   }
 
unsigned exec_type = execution_type(devinfo, inst);
unsigned exec_type_size =
@@ -428,17 +467,30 @@ general_restrictions_based_on_operand_types(const struct 
gen_device_info *devinf
unsigned dst_type_size = brw_element_size(devinfo, inst, dst);
 
if (exec_type_size > dst_type_size) {
-  unsigned dst_stride = 1 << (brw_inst_dst_hstride(devinfo, inst) - 1);
   ERROR_IF(dst_stride * dst_type_size != exec_type_size,
"Destination stride must be equal to the ratio of the sizes of "
"the execution data type to the destination type");
 
+  unsigned subreg = brw_inst_dst_da1_subreg_nr(devinfo, inst);
+
   if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1 &&
   brw_inst_dst_address_mode(devinfo, inst) == BRW_ADDRESS_DIRECT) {
- unsigned subreg = brw_inst_dst_da1_subreg_nr(devinfo, inst);
- ERROR_IF(subreg % exec_type_size != 0,
-  "Destination subreg must be aligned to the size of the "
-  "execution data type");
+ /* The i965 PRM says:
+  *
+  *Implementation Restriction: The relaxed alignment rule for byte
+  *destination (#10.5) is not supported.
+  */
+ if ((devinfo->gen > 4 || devinfo->is_g4x) && dst_type_is_byte) {
+ERROR_IF(subreg % exec_type_size != 0 &&
+ subreg % exec_type_size != 1,
+ "Destination subreg must be aligned to the size of the "
+ "execution data type (or to the next lowest byte for byte 
"
+ "destinations)");
+ } else {
+ERROR_IF(subreg % exec_type_size != 0,
+ "Destination subreg must be aligned to the size of the "
+ "execution data type");
+ }
   }
}
 
diff --git a/src/mesa/drivers/dri/i965/test_eu_validate.cpp 
b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
index f4d0e94..3e045f3 100644
--- a/src/mesa/drivers/dri/i965/test_eu_validate.cpp
+++ b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
@@ -756,3 +756,92 @@ TEST_P(validation_test, one_src_two_dst)
   EXPECT_FALSE(validate(p));
}
 }
+
+TEST_P(validation_test, packed_byte_

[Mesa-dev] [PATCH 07/18] i965: Add a test for the EU assembly validator.

[Matt Turner]

---
 src/mesa/drivers/dri/i965/Makefile.am  |   7 +
 src/mesa/drivers/dri/i965/test_eu_validate.cpp | 169 +
 2 files changed, 176 insertions(+)
 create mode 100644 src/mesa/drivers/dri/i965/test_eu_validate.cpp

diff --git a/src/mesa/drivers/dri/i965/Makefile.am 
b/src/mesa/drivers/dri/i965/Makefile.am
index a192fc0..4e94ee5 100644
--- a/src/mesa/drivers/dri/i965/Makefile.am
+++ b/src/mesa/drivers/dri/i965/Makefile.am
@@ -108,6 +108,7 @@ TESTS = \
test_fs_cmod_propagation \
test_fs_saturate_propagation \
 test_eu_compact \
+   test_eu_validate \
test_vf_float_conversions \
test_vec4_cmod_propagation \
 test_vec4_copy_propagation \
@@ -155,3 +156,9 @@ test_eu_compact_SOURCES = \
test_eu_compact.c
 nodist_EXTRA_test_eu_compact_SOURCES = dummy.cpp
 test_eu_compact_LDADD = $(TEST_LIBS)
+
+test_eu_validate_SOURCES = \
+   test_eu_validate.cpp
+test_eu_validate_LDADD = \
+   $(top_builddir)/src/gtest/libgtest.la \
+   $(TEST_LIBS)
diff --git a/src/mesa/drivers/dri/i965/test_eu_validate.cpp 
b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
new file mode 100644
index 000..13e9ba4
--- /dev/null
+++ b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
@@ -0,0 +1,169 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * 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 
+#include "brw_eu.h"
+#include "util/ralloc.h"
+
+enum subgen {
+   IS_G45 = 1,
+   IS_BYT,
+   IS_HSW,
+   IS_CHV,
+   IS_BXT,
+   IS_KBL,
+};
+
+static const struct gen_info {
+   const char *name;
+   int gen;
+   enum subgen subgen;
+} gens[] = {
+   { "brw", 4 },
+   { "g45", 4, IS_G45 },
+   { "ilk", 5 },
+   { "snb", 6 },
+   { "ivb", 7 },
+   { "byt", 7, IS_BYT },
+   { "hsw", 7, IS_HSW },
+   { "bdw", 8 },
+   { "chv", 8, IS_CHV },
+   { "skl", 9 },
+   { "bxt", 9, IS_BXT },
+   { "kbl", 9, IS_KBL },
+};
+
+class validation_test: public ::testing::TestWithParam {
+   virtual void SetUp();
+
+public:
+   validation_test();
+   virtual ~validation_test();
+
+   struct brw_codegen *p;
+   struct gen_device_info devinfo;
+};
+
+validation_test::validation_test()
+{
+   p = rzalloc(NULL, struct brw_codegen);
+   memset(&devinfo, 0, sizeof(devinfo));
+}
+
+validation_test::~validation_test()
+{
+   ralloc_free(p);
+}
+
+void validation_test::SetUp()
+{
+   struct gen_info info = GetParam();
+
+   devinfo.gen   = info.gen;
+   devinfo.is_g4x= info.subgen == IS_G45;
+   devinfo.is_baytrail   = info.subgen == IS_BYT;
+   devinfo.is_haswell= info.subgen == IS_HSW;
+   devinfo.is_cherryview = info.subgen == IS_CHV;
+   devinfo.is_broxton= info.subgen == IS_BXT;
+   devinfo.is_kabylake   = info.subgen == IS_KBL;
+
+   brw_init_codegen(&devinfo, p, p);
+}
+
+struct gen_name {
+   template 
+   std::string
+   operator()(const ::testing::TestParamInfo& info) const {
+  return info.param.name;
+   }
+};
+
+INSTANTIATE_TEST_CASE_P(eu_assembly, validation_test,
+::testing::ValuesIn(gens),
+gen_name());
+
+static bool
+validate(struct brw_codegen *p)
+{
+   const bool print = getenv("TEST_DEBUG");
+   struct annotation_info annotation;
+   memset(&annotation, 0, sizeof(annotation));
+
+   if (print) {
+  annotation.mem_ctx = ralloc_context(NULL);
+  annotation.ann_count = 1;
+  annotation.ann_size = 2;
+  annotation.ann = rzalloc_array(annotation.mem_ctx, struct annotation,
+ annotation.ann_size);
+  annotation.ann[annotation.ann_count].offset = p->next_insn_offset;
+   }
+
+   bool ret = brw_validate_instructions(p, 0, &annotation);
+
+   if (print) {
+  dump_assembly(p->store, annotation.ann_count, annotation.ann, 
p->devinfo);
+  ralloc_free(annotation.mem_ctx);
+   }
+
+   return ret;
+}
+
+#define g0   brw_vec8_grf(0, 0)
+#define null   

[Mesa-dev] [PATCH 11/18] i965: Factor out send_restrictions() function.

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_validate.c | 34 +++--
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index d03ed71..8c2eb99 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -152,6 +152,27 @@ sources_not_null(const struct gen_device_info *devinfo,
return error_msg;
 }
 
+static struct string
+send_restrictions(const struct gen_device_info *devinfo,
+  const brw_inst *inst)
+{
+   struct string error_msg = { .str = NULL, .len = 0 };
+
+   if (brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SEND) {
+  ERROR_IF(brw_inst_src0_address_mode(devinfo, inst) != BRW_ADDRESS_DIRECT,
+   "send must use direct addressing");
+
+  if (devinfo->gen >= 7) {
+ ERROR_IF(!src0_is_grf(devinfo, inst), "send from non-GRF");
+ ERROR_IF(brw_inst_eot(devinfo, inst) &&
+  brw_inst_src0_da_reg_nr(devinfo, inst) < 112,
+  "send with EOT must use g112-g127");
+  }
+   }
+
+   return error_msg;
+}
+
 static bool
 is_unsupported_inst(const struct gen_device_info *devinfo,
 const brw_inst *inst)
@@ -176,18 +197,7 @@ brw_validate_instructions(const struct brw_codegen *p, int 
start_offset,
  ERROR("Instruction not supported on this Gen");
   } else {
  CHECK(sources_not_null);
-  }
-
-  if (brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SEND) {
- ERROR_IF(brw_inst_src0_address_mode(devinfo, inst) !=
-  BRW_ADDRESS_DIRECT, "send must use direct addressing");
-
- if (devinfo->gen >= 7) {
-ERROR_IF(!src0_is_grf(devinfo, inst), "send from non-GRF");
-ERROR_IF(brw_inst_eot(devinfo, inst) &&
- brw_inst_src0_da_reg_nr(devinfo, inst) < 112,
- "send with EOT must use g112-g127");
- }
+ CHECK(send_restrictions);
   }
 
   if (error_msg.str && annotation) {
-- 
2.7.3

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

[Mesa-dev] [PATCH 15/18] i965: Validate "General Restrictions on Regioning Parameters"

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_validate.c| 139 
 src/mesa/drivers/dri/i965/test_eu_validate.cpp | 220 +
 2 files changed, 359 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index 9648f0d..bdb1273 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -64,6 +64,13 @@ cat(struct string *dest, const struct string src)
} while (0)
 
 static bool
+dst_is_null(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   return brw_inst_dst_reg_file(devinfo, inst) == 
BRW_ARCHITECTURE_REGISTER_FILE &&
+  brw_inst_dst_da_reg_nr(devinfo, inst) == BRW_ARF_NULL;
+}
+
+static bool
 src0_is_null(const struct gen_device_info *devinfo, const brw_inst *inst)
 {
return brw_inst_src0_reg_file(devinfo, inst) == 
BRW_ARCHITECTURE_REGISTER_FILE &&
@@ -187,6 +194,137 @@ is_unsupported_inst(const struct gen_device_info *devinfo,
return brw_opcode_desc(devinfo, brw_inst_opcode(devinfo, inst)) == NULL;
 }
 
+/**
+ * Checks restrictions listed in "General Restrictions on Regioning Parameters"
+ * in the "Register Region Restrictions" section.
+ */
+static struct string
+general_restrictions_on_region_parameters(const struct gen_device_info 
*devinfo,
+  const brw_inst *inst)
+{
+   const struct opcode_desc *desc =
+  brw_opcode_desc(devinfo, brw_inst_opcode(devinfo, inst));
+   unsigned num_sources = num_sources_from_inst(devinfo, inst);
+   unsigned exec_size = 1 << brw_inst_exec_size(devinfo, inst);
+   struct string error_msg = { .str = NULL, .len = 0 };
+
+   if (num_sources == 3)
+  return (struct string){};
+
+   if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_16) {
+  if (desc->ndst != 0 && !dst_is_null(devinfo, inst))
+ ERROR_IF(brw_inst_dst_hstride(devinfo, inst) != 
BRW_HORIZONTAL_STRIDE_1,
+  "Destination Horizontal Stride must be 1");
+
+  if (num_sources >= 1)
+ ERROR_IF(brw_inst_src0_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE 
&&
+  brw_inst_src0_vstride(devinfo, inst) != 
BRW_VERTICAL_STRIDE_0 &&
+  brw_inst_src0_vstride(devinfo, inst) != 
BRW_VERTICAL_STRIDE_4,
+  "In Align16 mode, only VertStride of 0 or 4 is allowed");
+
+  if (num_sources == 2)
+ ERROR_IF(brw_inst_src1_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE 
&&
+  brw_inst_src1_vstride(devinfo, inst) != 
BRW_VERTICAL_STRIDE_0 &&
+  brw_inst_src1_vstride(devinfo, inst) != 
BRW_VERTICAL_STRIDE_4,
+  "In Align16 mode, only VertStride of 0 or 4 is allowed");
+
+  return error_msg;
+   }
+
+   for (unsigned i = 0; i < num_sources; i++) {
+  unsigned vstride, width, hstride, element_size, subreg;
+
+#define DO_SRC(n)  
\
+  if (brw_inst_src ## n ## _reg_file(devinfo, inst) == 
\
+  BRW_IMMEDIATE_VALUE) 
\
+ continue; 
\
+   
\
+  vstride = brw_inst_src ## n ## _vstride(devinfo, inst) ? 
\
+(1 << (brw_inst_src ## n ## _vstride(devinfo, inst) - 1)) : 0; 
\
+  width = 1 << brw_inst_src ## n ## _width(devinfo, inst); 
\
+  hstride = brw_inst_src ## n ## _hstride(devinfo, inst) ? 
\
+(1 << (brw_inst_src ## n ## _hstride(devinfo, inst) - 1)) : 0; 
\
+  element_size = brw_element_size(devinfo, inst, src ## n);
\
+  subreg = brw_inst_src ## n ## _da1_subreg_nr(devinfo, inst)
+
+  if (i == 0) {
+ DO_SRC(0);
+  } else if (i == 1) {
+ DO_SRC(1);
+  }
+#undef DO_SRC
+
+  /* ExecSize must be greater than or equal to Width. */
+  ERROR_IF(exec_size < width, "ExecSize must be greater than or equal "
+  "to Width");
+
+  /* If ExecSize = Width and HorzStride ≠ 0,
+   * VertStride must be set to Width * HorzStride.
+   */
+  if (exec_size == width && hstride != 0) {
+ ERROR_IF(vstride != width * hstride,
+  "If ExecSize = Width and HorzStride ≠ 0, "
+  "VertStride must be set to Width * HorzStride");
+  }
+
+  /* If Width = 1, HorzStride must be 0 regardless of the values of
+   * ExecSize and VertStride.
+   */
+  if (width == 1) {
+ ERROR_IF(hstride != 0,
+  "If Width = 1, HorzStride must be 0 regardless "
+  "of the values of ExecSize and VertStride");
+  }
+
+  /* If ExecSize = Width = 1, both VertStride and HorzStride must be 0. */
+  if (exec_size == 1 && width == 1) {
+   

[Mesa-dev] [PATCH 13/18] i965: Validate math instruction sources.

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_emit.c|  9 -
 src/mesa/drivers/dri/i965/test_eu_validate.cpp | 23 +++
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 3146271..f3aa2bc 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -2005,8 +2005,6 @@ void gen6_math(struct brw_codegen *p,
 
assert(dest.file == BRW_GENERAL_REGISTER_FILE ||
   (devinfo->gen >= 7 && dest.file == BRW_MESSAGE_REGISTER_FILE));
-   assert(src0.file == BRW_GENERAL_REGISTER_FILE ||
-  (devinfo->gen >= 8 && src0.file == BRW_IMMEDIATE_VALUE));
 
assert(dest.hstride == BRW_HORIZONTAL_STRIDE_1);
if (devinfo->gen == 6) {
@@ -2024,13 +2022,6 @@ void gen6_math(struct brw_codegen *p,
} else {
   assert(src0.type == BRW_REGISTER_TYPE_F);
   assert(src1.type == BRW_REGISTER_TYPE_F);
-  if (function == BRW_MATH_FUNCTION_POW) {
- assert(src1.file == BRW_GENERAL_REGISTER_FILE ||
-(devinfo->gen >= 8 && src1.file == BRW_IMMEDIATE_VALUE));
-  } else {
- assert(src1.file == BRW_ARCHITECTURE_REGISTER_FILE &&
-src1.nr == BRW_ARF_NULL);
-  }
}
 
/* Source modifiers are ignored for extended math instructions on Gen6. */
diff --git a/src/mesa/drivers/dri/i965/test_eu_validate.cpp 
b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
index 13e9ba4..8276171 100644
--- a/src/mesa/drivers/dri/i965/test_eu_validate.cpp
+++ b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
@@ -152,6 +152,29 @@ TEST_P(validation_test, src1_null_reg)
EXPECT_FALSE(validate(p));
 }
 
+TEST_P(validation_test, math_src0_null_reg)
+{
+   if (devinfo.gen >= 6) {
+  gen6_math(p, g0, BRW_MATH_FUNCTION_SIN, null, null);
+   } else {
+  gen4_math(p, g0, BRW_MATH_FUNCTION_SIN, 0, null, 
BRW_MATH_PRECISION_FULL);
+   }
+
+   EXPECT_FALSE(validate(p));
+}
+
+TEST_P(validation_test, math_src1_null_reg)
+{
+   if (devinfo.gen >= 6) {
+  gen6_math(p, g0, BRW_MATH_FUNCTION_POW, g0, null);
+  EXPECT_FALSE(validate(p));
+   } else {
+  /* Math instructions on Gen4/5 are actually SEND messages with payloads.
+   * src1 is an immediate message descriptor set by gen4_math.
+   */
+   }
+}
+
 TEST_P(validation_test, opcode46)
 {
/* opcode 46 is "push" on Gen 4 and 5
-- 
2.7.3

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

[Mesa-dev] [PATCH 14/18] i965: Replace reg_type_size[] with a function.

[Matt Turner]

A function is necessary to handle immediate types.
---
 src/mesa/drivers/dri/i965/brw_disasm.c  | 35 
 src/mesa/drivers/dri/i965/brw_eu_emit.c | 58 +++--
 src/mesa/drivers/dri/i965/brw_reg.h |  8 +
 3 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c 
b/src/mesa/drivers/dri/i965/brw_disasm.c
index 5e51be7..3786e4b 100644
--- a/src/mesa/drivers/dri/i965/brw_disasm.c
+++ b/src/mesa/drivers/dri/i965/brw_disasm.c
@@ -257,20 +257,6 @@ static const char *const three_source_reg_encoding[] = {
[BRW_3SRC_TYPE_DF] = "DF",
 };
 
-const int reg_type_size[] = {
-   [BRW_HW_REG_TYPE_UD]  = 4,
-   [BRW_HW_REG_TYPE_D]   = 4,
-   [BRW_HW_REG_TYPE_UW]  = 2,
-   [BRW_HW_REG_TYPE_W]   = 2,
-   [BRW_HW_REG_NON_IMM_TYPE_UB]  = 1,
-   [BRW_HW_REG_NON_IMM_TYPE_B]   = 1,
-   [GEN7_HW_REG_NON_IMM_TYPE_DF] = 8,
-   [BRW_HW_REG_TYPE_F]   = 4,
-   [GEN8_HW_REG_TYPE_UQ] = 8,
-   [GEN8_HW_REG_TYPE_Q]  = 8,
-   [GEN8_HW_REG_NON_IMM_TYPE_HF] = 2,
-};
-
 static const char *const reg_file[4] = {
[0] = "A",
[1] = "g",
@@ -734,6 +720,7 @@ reg(FILE *file, unsigned _reg_file, unsigned _reg_nr)
 static int
 dest(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
 {
+   unsigned elem_size = brw_element_size(devinfo, inst, dst);
int err = 0;
 
if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) {
@@ -744,7 +731,7 @@ dest(FILE *file, const struct gen_device_info *devinfo, 
brw_inst *inst)
 return 0;
  if (brw_inst_dst_da1_subreg_nr(devinfo, inst))
 format(file, ".%"PRIu64, brw_inst_dst_da1_subreg_nr(devinfo, inst) 
/
-   reg_type_size[brw_inst_dst_reg_type(devinfo, inst)]);
+   elem_size);
  string(file, "<");
  err |= control(file, "horiz stride", horiz_stride,
 brw_inst_dst_hstride(devinfo, inst), NULL);
@@ -755,7 +742,7 @@ dest(FILE *file, const struct gen_device_info *devinfo, 
brw_inst *inst)
  string(file, "g[a0");
  if (brw_inst_dst_ia_subreg_nr(devinfo, inst))
 format(file, ".%"PRIu64, brw_inst_dst_ia_subreg_nr(devinfo, inst) /
-   reg_type_size[brw_inst_dst_reg_type(devinfo, inst)]);
+   elem_size);
  if (brw_inst_dst_ia1_addr_imm(devinfo, inst))
 format(file, " %d", brw_inst_dst_ia1_addr_imm(devinfo, inst));
  string(file, "]<");
@@ -773,7 +760,7 @@ dest(FILE *file, const struct gen_device_info *devinfo, 
brw_inst *inst)
 return 0;
  if (brw_inst_dst_da16_subreg_nr(devinfo, inst))
 format(file, ".%"PRIu64, brw_inst_dst_da16_subreg_nr(devinfo, 
inst) /
-   reg_type_size[brw_inst_dst_reg_type(devinfo, inst)]);
+   elem_size);
  string(file, "<1>");
  err |= control(file, "writemask", writemask,
 brw_inst_da16_writemask(devinfo, inst), NULL);
@@ -850,8 +837,10 @@ src_da1(FILE *file,
err |= reg(file, _reg_file, reg_num);
if (err == -1)
   return 0;
-   if (sub_reg_num)
-  format(file, ".%d", sub_reg_num / reg_type_size[type]);   /* use formal 
style like spec */
+   if (sub_reg_num) {
+  unsigned elem_size = brw_hw_reg_type_to_size(devinfo, type, _reg_file);
+  format(file, ".%d", sub_reg_num / elem_size);   /* use formal style like 
spec */
+   }
src_align1_region(file, _vert_stride, _width, _horiz_stride);
err |= control(file, "src reg encoding", reg_encoding, type, NULL);
return err;
@@ -936,10 +925,14 @@ src_da16(FILE *file,
err |= reg(file, _reg_file, _reg_nr);
if (err == -1)
   return 0;
-   if (_subreg_nr)
+   if (_subreg_nr) {
+  unsigned elem_size =
+ brw_hw_reg_type_to_size(devinfo, _reg_type, _reg_file);
+
   /* bit4 for subreg number byte addressing. Make this same meaning as
  in da1 case, so output looks consistent. */
-  format(file, ".%d", 16 / reg_type_size[_reg_type]);
+  format(file, ".%d", 16 / elem_size);
+   }
string(file, "<");
err |= control(file, "vert stride", vert_stride, _vert_stride, NULL);
string(file, ",4,1>");
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index f3aa2bc..de98102 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -141,6 +141,59 @@ brw_reg_type_to_hw_type(const struct gen_device_info 
*devinfo,
}
 }
 
+/**
+ * Return the element size given a hardware register type and file.
+ *
+ * The hardware encoding may depend on whether the value is an immediate.
+ */
+unsigned
+brw_hw_reg_type_to_size(const struct gen_device_info *devinfo,
+unsigned type, enum brw_reg_file file)
+{
+   if (file == BRW_IMMEDIATE_VALUE) {
+  static const int imm_hw_sizes[] = {
+ [BRW_HW_REG_TYPE_UD]  = 4

[Mesa-dev] [PATCH 16/18] i965: Validate "General Restrictions Based on Operand Types"

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_validate.c| 215 +
 src/mesa/drivers/dri/i965/test_eu_validate.cpp |  58 +++
 2 files changed, 273 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index bdb1273..e01067f 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -64,6 +64,20 @@ cat(struct string *dest, const struct string src)
} while (0)
 
 static bool
+inst_is_send(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   switch (brw_inst_opcode(devinfo, inst)) {
+   case BRW_OPCODE_SEND:
+   case BRW_OPCODE_SENDC:
+   case BRW_OPCODE_SENDS:
+   case BRW_OPCODE_SENDSC:
+  return true;
+   default:
+  return false;
+   }
+}
+
+static bool
 dst_is_null(const struct gen_device_info *devinfo, const brw_inst *inst)
 {
return brw_inst_dst_reg_file(devinfo, inst) == 
BRW_ARCHITECTURE_REGISTER_FILE &&
@@ -194,6 +208,206 @@ is_unsupported_inst(const struct gen_device_info *devinfo,
return brw_opcode_desc(devinfo, brw_inst_opcode(devinfo, inst)) == NULL;
 }
 
+static unsigned
+execution_type_for_type(unsigned type, bool is_immediate)
+{
+   /* The meaning of the type bits is dependent on whether the operand is an
+* immediate, so normalize them first.
+*/
+   if (is_immediate) {
+  switch (type) {
+  case BRW_HW_REG_IMM_TYPE_UV:
+  case BRW_HW_REG_IMM_TYPE_V:
+ type = BRW_HW_REG_TYPE_W;
+ break;
+  case BRW_HW_REG_IMM_TYPE_VF:
+ type = BRW_HW_REG_TYPE_F;
+ break;
+  case GEN8_HW_REG_IMM_TYPE_DF:
+ type = GEN7_HW_REG_NON_IMM_TYPE_DF;
+ break;
+  case GEN8_HW_REG_IMM_TYPE_HF:
+ type = GEN8_HW_REG_NON_IMM_TYPE_HF;
+ break;
+  default:
+ break;
+  }
+   }
+
+   switch (type) {
+   case BRW_HW_REG_TYPE_UD:
+   case BRW_HW_REG_TYPE_D:
+  return BRW_HW_REG_TYPE_D;
+   case BRW_HW_REG_TYPE_UW:
+   case BRW_HW_REG_TYPE_W:
+   case BRW_HW_REG_NON_IMM_TYPE_UB:
+   case BRW_HW_REG_NON_IMM_TYPE_B:
+  return BRW_HW_REG_TYPE_W;
+   case GEN8_HW_REG_TYPE_UQ:
+   case GEN8_HW_REG_TYPE_Q:
+  return GEN8_HW_REG_TYPE_Q;
+   case BRW_HW_REG_TYPE_F:
+   case GEN7_HW_REG_NON_IMM_TYPE_DF:
+   case GEN8_HW_REG_NON_IMM_TYPE_HF:
+  return type;
+   default:
+  unreachable("not reached");
+   }
+}
+
+/**
+ * Returns the execution type of an instruction \p inst
+ */
+static unsigned
+execution_type(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   unsigned num_sources = num_sources_from_inst(devinfo, inst);
+   unsigned src0_exec_type, src1_exec_type;
+   unsigned src0_type = brw_inst_src0_reg_type(devinfo, inst);
+   unsigned src1_type = brw_inst_src1_reg_type(devinfo, inst);
+
+   bool src0_is_immediate =
+  brw_inst_src0_reg_file(devinfo, inst) == BRW_IMMEDIATE_VALUE;
+   bool src1_is_immediate =
+  brw_inst_src1_reg_file(devinfo, inst) == BRW_IMMEDIATE_VALUE;
+
+   /* Execution data type is independent of destination data type, except in
+* mixed F/HF instructions on CHV and SKL+.
+*/
+   unsigned dst_exec_type = brw_inst_dst_reg_type(devinfo, inst);
+
+   src0_exec_type = execution_type_for_type(src0_type, src0_is_immediate);
+   if (num_sources == 1) {
+  if ((devinfo->gen >= 9 || devinfo->is_cherryview) &&
+  src0_exec_type == GEN8_HW_REG_NON_IMM_TYPE_HF) {
+ return dst_exec_type;
+  }
+  return src0_exec_type;
+   }
+
+   src1_exec_type = execution_type_for_type(src1_type, src1_is_immediate);
+   if (src0_exec_type == src1_exec_type)
+  return src0_exec_type;
+
+   /* Mixed operand types where one is float is float on Gen < 6
+* (and not allowed on later platforms)
+*/
+   if (devinfo->gen < 6 &&
+   (src0_exec_type == BRW_HW_REG_TYPE_F ||
+src1_exec_type == BRW_HW_REG_TYPE_F))
+  return BRW_HW_REG_TYPE_F;
+
+   if (src0_exec_type == GEN8_HW_REG_TYPE_Q ||
+   src1_exec_type == GEN8_HW_REG_TYPE_Q)
+  return GEN8_HW_REG_TYPE_Q;
+
+   if (src0_exec_type == BRW_HW_REG_TYPE_D ||
+   src1_exec_type == BRW_HW_REG_TYPE_D)
+  return BRW_HW_REG_TYPE_D;
+
+   if (src0_exec_type == BRW_HW_REG_TYPE_W ||
+   src1_exec_type == BRW_HW_REG_TYPE_W)
+  return BRW_HW_REG_TYPE_W;
+
+   if (src0_exec_type == GEN7_HW_REG_NON_IMM_TYPE_DF ||
+   src1_exec_type == GEN7_HW_REG_NON_IMM_TYPE_DF)
+  return GEN7_HW_REG_NON_IMM_TYPE_DF;
+
+   if (devinfo->gen >= 9 || devinfo->is_cherryview) {
+  if (dst_exec_type == BRW_HW_REG_TYPE_F ||
+  src0_exec_type == BRW_HW_REG_TYPE_F ||
+  src1_exec_type == BRW_HW_REG_TYPE_F) {
+ return BRW_HW_REG_TYPE_F;
+  } else {
+ return GEN8_HW_REG_NON_IMM_TYPE_HF;
+  }
+   }
+
+   assert(src0_exec_type == BRW_HW_REG_TYPE_F);
+   return BRW_HW_REG_TYPE_F;
+}
+
+/**
+ * Checks restrictions listed in "General Restrictions Based on Operand Types"
+ * in the "Regi

[Mesa-dev] [PATCH 17/18] i965: Validate "Region Alignment Rules"

[Matt Turner]

---
 src/mesa/drivers/dri/i965/brw_eu_validate.c| 410 -
 src/mesa/drivers/dri/i965/test_eu_validate.cpp | 288 +
 2 files changed, 697 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c 
b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index e01067f..e9b55ed 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -44,7 +44,8 @@ cat(struct string *dest, const struct string src)
 }
 #define CAT(dest, src) cat(&dest, (struct string){src, strlen(src)})
 
-#define error(str) "\tERROR: " str "\n"
+#define error(str)   "\tERROR: " str "\n"
+#define ERROR_INDENT "\t   "
 
 #define ERROR(msg) ERROR_IF(true, msg)
 #define ERROR_IF(cond, msg)  \
@@ -104,6 +105,22 @@ src0_is_grf(const struct gen_device_info *devinfo, const 
brw_inst *inst)
return brw_inst_src0_reg_file(devinfo, inst) == BRW_GENERAL_REGISTER_FILE;
 }
 
+static bool
+src0_has_scalar_region(const struct gen_device_info *devinfo, const brw_inst 
*inst)
+{
+   return brw_inst_src0_vstride(devinfo, inst) == BRW_VERTICAL_STRIDE_0 &&
+  brw_inst_src0_width(devinfo, inst) == BRW_WIDTH_1 &&
+  brw_inst_src0_hstride(devinfo, inst) == BRW_HORIZONTAL_STRIDE_0;
+}
+
+static bool
+src1_has_scalar_region(const struct gen_device_info *devinfo, const brw_inst 
*inst)
+{
+   return brw_inst_src1_vstride(devinfo, inst) == BRW_VERTICAL_STRIDE_0 &&
+  brw_inst_src1_width(devinfo, inst) == BRW_WIDTH_1 &&
+  brw_inst_src1_hstride(devinfo, inst) == BRW_HORIZONTAL_STRIDE_0;
+}
+
 static unsigned
 num_sources_from_inst(const struct gen_device_info *devinfo,
   const brw_inst *inst)
@@ -328,6 +345,26 @@ execution_type(const struct gen_device_info *devinfo, 
const brw_inst *inst)
 }
 
 /**
+ * Returns whether a region is packed
+ *
+ * A region is packed if its elements are adjacent in memory, with no
+ * intervening space, no overlap, and no replicated values.
+ */
+static bool
+is_packed(unsigned vstride, unsigned width, unsigned hstride)
+{
+   if (vstride == width) {
+  if (vstride == 1) {
+ return hstride == 0;
+  } else {
+ return hstride == 1;
+  }
+   }
+
+   return false;
+}
+
+/**
  * Checks restrictions listed in "General Restrictions Based on Operand Types"
  * in the "Register Region Restrictions" section.
  */
@@ -539,6 +576,376 @@ general_restrictions_on_region_parameters(const struct 
gen_device_info *devinfo,
return error_msg;
 }
 
+/**
+ * Creates an \p access_mask for an \p exec_size, \p element_size, and a region
+ *
+ * An \p access_mask is a 32-element array of uint64_t, where each uint64_t is
+ * a bitmask of bytes accessed by the region.
+ *
+ * For instance the access mask of the source gX.1<4,2,2>F in an exec_size = 4
+ * instruction would be
+ *
+ *access_mask[0] = 0x00F0
+ *access_mask[1] = 0xF000
+ *access_mask[2] = 0x00F0
+ *access_mask[3] = 0xF000
+ *access_mask[4-31] = 0
+ *
+ * because the first execution channel accesses bytes 7-4 and the second
+ * execution channel accesses bytes 15-12, etc.
+ */
+static void
+align1_access_mask(uint64_t access_mask[static 32],
+   unsigned exec_size, unsigned element_size, unsigned subreg,
+   unsigned vstride, unsigned width, unsigned hstride)
+{
+   const uint64_t mask = (1 << element_size) - 1;
+   unsigned rowbase = subreg;
+   unsigned element = 0;
+
+   for (int y = 0; y < exec_size / width; y++) {
+  unsigned offset = rowbase;
+
+  for (int x = 0; x < width; x++) {
+ access_mask[element++] = mask << offset;
+ offset += hstride * element_size;
+  }
+
+  rowbase += vstride * element_size;
+   }
+
+   assert(element == 0 || element == exec_size);
+}
+
+/**
+ * Returns the number of registers accessed according to the \p access_mask
+ */
+static int
+registers_read(const uint64_t access_mask[static 32])
+{
+   int regs_read = 0;
+
+   for (unsigned i = 0; i < 32; i++) {
+  if (access_mask[i] > 0x) {
+ return 2;
+  } else if (access_mask[i]) {
+ regs_read = 1;
+  }
+   }
+
+   return regs_read;
+}
+
+/**
+ * Checks restrictions listed in "Region Alignment Rules" in the "Register
+ * Region Restrictions" section.
+ */
+static struct string
+region_alignment_rules(const struct gen_device_info *devinfo,
+   const brw_inst *inst)
+{
+   const struct opcode_desc *desc =
+  brw_opcode_desc(devinfo, brw_inst_opcode(devinfo, inst));
+   unsigned num_sources = num_sources_from_inst(devinfo, inst);
+   unsigned exec_size = 1 << brw_inst_exec_size(devinfo, inst);
+   uint64_t dst_access_mask[32], src0_access_mask[32], src1_access_mask[32];
+   struct string error_msg = { .str = NULL, .len = 0 };
+
+   if (num_sources == 3)
+  return (struct string){};
+
+   if (brw_inst_access_mode(devinfo, inst) == B

Re: [Mesa-dev] [PATCH] main/getteximage: Take y-offset into account for memcpy size

[Jason Ekstrand]

On Nov 22, 2016 11:03, "Eduardo Lima Mitev"  wrote:
>
> On 11/22/2016 06:43 PM, Jason Ekstrand wrote:
> > On Tue, Nov 22, 2016 at 9:11 AM, Eduardo Lima Mitev  > > wrote:
> >
> > In get_tex_memcpy, when copying texture data directly from source
> > to destination (when row strides match for both src and dst), the
> > block size is currently calculated as 'bytes-per-row *
image-height',
> > ignoring the given y-offset argument.
> >
> > This can cause a read past the end of the mapped buffer, leading to
> > a segfault.
> >
> > Fixes CTS test (from crash to pass):
> > * GL45-CTS/get_texture_sub_image/functional_test
> > ---
> >  src/mesa/main/texgetimage.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/main/texgetimage.c
b/src/mesa/main/texgetimage.c
> > index b900278..a783ed5 100644
> > --- a/src/mesa/main/texgetimage.c
> > +++ b/src/mesa/main/texgetimage.c
> > @@ -654,7 +654,7 @@ get_tex_memcpy(struct gl_context *ctx,
> >
> >if (src) {
> >   if (bytesPerRow == dstRowStride && bytesPerRow ==
> > srcRowStride) {
> > -memcpy(dst, src, bytesPerRow * texImage->Height);
> > +memcpy(dst, src, bytesPerRow * (texImage->Height -
> > yoffset));
> >
> >
> > Why not use the height parameter that gets passed in.  That seems more
> > correct.
> >
>
> Indeed, that's the correct thing to do here. Then I wonder if we should
> clamp height (and width) to the texture size minus y-offset (and
> x-offset). In case the region exceeds texture size, the extra memory
> will have undefined data, but we don't crash the driver. Or maybe that's
> caught earlier.

That's probably caught earlier but it would be worth a crawl through to
make sure.

> Thanks for reviewing!
>
> Eduardo
>
> >   }
> >   else {
> >  GLuint row;
> > --
> > 2.10.2
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org 
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> >
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] compiler/glsl/tests: Fix print format when building 32-bit on 64-bit host

[Matt Turner]

Patches 1 and 2 are

Reviewed-by: Matt Turner 


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

Re: [Mesa-dev] [PATCH 3/3] glsl/cache/tests: Cast cache_get result to avoid compiler warning

[Matt Turner]

On 11/22, Aaron Watry wrote:

disk_cache_get returns void*, but we were storing/comparing a char*.

Signed-off-by: Aaron Watry 
---
Note that this did, and still, segfaults for me when I actually run it...


Strange. It passes for me.



But at least the compiler is no longer complaining about the type mismatch.
src/compiler/glsl/tests/cache_test.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index 0ef05aa..c663e54 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -208,14 +208,14 @@ test_put_and_get(void)
   _mesa_sha1_compute(blob, sizeof(blob), blob_key);

   /* Ensure that disk_cache_get returns nothing before anything is added. */
-   result = disk_cache_get(cache, blob_key, &size);
+   result = (char*) disk_cache_get(cache, blob_key, &size);


void * is implicitly convertable to char *, so I think the code is fine
as is. Maybe if I saw the compiler warning.

"result" could be declared void * to begin with, and that would be fine.
I just tested it, and it seems to work.

Also, please use (char *) for casts and not (char*) in Mesa.


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

Re: [Mesa-dev] [PATCH v5] clover: restore support for LLVM <= 3.9

[Vinson Lee]

On Tue, Nov 22, 2016 at 11:25 AM, Vedran Miletić  wrote:
> The commit 8e430ff8b060b4e8e922bae24b3c57837da6ea77 support for LLVM
> 3.9 and older versionsin  Clover. This patch restores it and refactors
> the support using Clover compatibility layer for LLVM.
>
> v2: merged #ifdef blocks
> v3: added support for LLVM 3.6-3.8
> v4: add missing #ifdef around 
> v5: simplify using templates and lambda
>
> Signed-off-by: Vedran Miletić 
> Reviewed-by[v2]: Jan Vesely 
> Tested-by[v4]: Vinson Lee 
> Tested-by[v4]: Pierre Moreau 
> Reviewed-by: Francisco Jerez 
> ---
>  .../state_trackers/clover/llvm/codegen/bitcode.cpp |  9 +++--
>  src/gallium/state_trackers/clover/llvm/compat.hpp  | 18 
> ++
>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp 
> b/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
> index 5dcc4f8..d09207b 100644
> --- a/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/codegen/bitcode.cpp
> @@ -32,6 +32,7 @@
>  ///
>
>  #include "llvm/codegen.hpp"
> +#include "llvm/compat.hpp"
>  #include "llvm/metadata.hpp"
>  #include "core/error.hpp"
>  #include "util/algorithm.hpp"
> @@ -99,13 +100,9 @@ clover::llvm::parse_module_library(const module &m, 
> ::llvm::LLVMContext &ctx,
> auto mod = ::llvm::parseBitcodeFile(::llvm::MemoryBufferRef(
>  as_string(m.secs[0].data), " "), 
> ctx);
>
> -   if (::llvm::Error err = mod.takeError()) {
> -  std::string msg;
> -  ::llvm::handleAllErrors(std::move(err), [&](::llvm::ErrorInfoBase 
> &EIB) {
> - msg = EIB.message();
> - fail(r_log, error(CL_INVALID_PROGRAM), msg.c_str());
> +   compat::handle_module_error(mod, [&](const std::string &s) {
> + fail(r_log, error(CL_INVALID_PROGRAM), s);
>});
> -   }
>
> return std::unique_ptr<::llvm::Module>(std::move(*mod));
>  }
> diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp 
> b/src/gallium/state_trackers/clover/llvm/compat.hpp
> index a963cff..fc257ec 100644
> --- a/src/gallium/state_trackers/clover/llvm/compat.hpp
> +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp
> @@ -39,6 +39,11 @@
>  #include 
>  #include 
>  #include 
> +#if HAVE_LLVM >= 0x0400
> +#include 
> +#else
> +#include 
> +#endif
>
>  #if HAVE_LLVM >= 0x0307
>  #include 
> @@ -158,6 +163,19 @@ namespace clover {
>  #else
>   const auto default_reloc_model = ::llvm::Reloc::Default;
>  #endif
> +
> + template inline void
> + handle_module_error(M &mod, const F &f) {
> +#if HAVE_LLVM >= 0x0400
> +if (::llvm::Error err = mod.takeError())
> +   ::llvm::handleAllErrors(std::move(err), 
> [&](::llvm::ErrorInfoBase &eib) {
> + f(eib.message());
> +  });
> +#else
> +if (!mod)
> +   f(mod.getError().message());
> +#endif
> + }
>}
> }
>  }
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Tested that this patch fixes build with llvm-3.8.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98740
Tested-by: Vinson Lee 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] main/getteximage: Use the height argument to calculate copy size

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 


On Tue, Nov 22, 2016 at 10:57 AM, Eduardo Lima Mitev 
wrote:

> In get_tex_memcpy, when copying texture data directly from source
> to destination (when row strides match for both src and dst), the
> copy size is currently calculated using the full texture height
> instead of the sub-region height parameter that was passed.
>
> This can cause a read past the end of the mapped buffer when y-offset
> is greater than zero, leading to a segfault.
>
> Fixes CTS test (from crash to pass):
> * GL45-CTS/get_texture_sub_image/functional_test
>
> v2: (Jason) Use the passed 'height' instead of copying til the
> end of the buffer (tex-height - yoffset).
> ---
>  src/mesa/main/texgetimage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index b900278..0186819 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -654,7 +654,7 @@ get_tex_memcpy(struct gl_context *ctx,
>
>if (src) {
>   if (bytesPerRow == dstRowStride && bytesPerRow == srcRowStride) {
> -memcpy(dst, src, bytesPerRow * texImage->Height);
> +memcpy(dst, src, bytesPerRow * height);
>   }
>   else {
>  GLuint row;
> --
> 2.10.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Use 3DSTATE_CLIP's User Clip Distance Enable bitmask on Gen8+.

[Matt Turner]

On 11/15, Kenneth Graunke wrote:

Gen6-7.5 specify the user clip distance enable bitmask in 3DSTATE_CLIP.
Gen8+ normally uses the new internal signalling mechanism to select the
one specified in the last enabled shader stage (3DSTATE_VS, DS, or GS).

This is a pretty good fit for Vulkan, or even newer GL, where the
bitmask comes entirely from the shader.  But with glClipPlane(),
this is dynamic state, and we have to listen to _NEW_TRASNFORM.

Clip plane enables are the only reason the VS/DS/GS atoms need to
listen to _NEW_TRANSFORM.  3DSTATE_CLIP already has to listen to it
in order to support ARB_clip_control settings.

Setting the "Use the 3DSTATE_CLIP bitmask" force enable bit allows
us to drop _NEW_TRANSFORM from all the shader stage atoms, so we can
re-emit them less often.

Improves performance of OglBatch7 (version 6) by 2.70773% +/- 0.491257%
(n = 38) at 1024x768 on Cherryview.

Signed-off-by: Kenneth Graunke 
---


Neat!

Reviewed-by: Matt Turner 


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

[Mesa-dev] [PATCH] util: fix missing swizzle components in the SINT <-> UINT conversion string

[Charmaine Lee]

Fixes tgsi error introduced in commit 3817a7a. The error complains missing
swizzle component in the conversion string "UMIN TEMP[0], TEMP[0], IMM[0].x".
---
 src/gallium/auxiliary/util/u_simple_shaders.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_simple_shaders.c 
b/src/gallium/auxiliary/util/u_simple_shaders.c
index 8be31be..7342b3d 100644
--- a/src/gallium/auxiliary/util/u_simple_shaders.c
+++ b/src/gallium/auxiliary/util/u_simple_shaders.c
@@ -633,14 +633,14 @@ util_make_fs_blit_msaa_color(struct pipe_context *pipe,
 
   if (dtype == TGSI_RETURN_TYPE_SINT) {
  conversion_decl = "IMM[0] UINT32 {2147483647, 0, 0, 0}\n";
- conversion = "UMIN TEMP[0], TEMP[0], IMM[0].x\n";
+ conversion = "UMIN TEMP[0], TEMP[0], IMM[0].\n";
   }
} else if (stype == TGSI_RETURN_TYPE_SINT) {
   samp_type = "SINT";
 
   if (dtype == TGSI_RETURN_TYPE_UINT) {
  conversion_decl = "IMM[0] INT32 {0, 0, 0, 0}\n";
- conversion = "IMAX TEMP[0], TEMP[0], IMM[0].x\n";
+ conversion = "IMAX TEMP[0], TEMP[0], IMM[0].\n";
   }
} else {
   assert(dtype == TGSI_RETURN_TYPE_FLOAT);
-- 
1.9.1

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

Re: [Mesa-dev] [PATCH] i965/vec4: skip registers already marked as no_spill

[Matt Turner]

On 11/11, Juan A. Suarez Romero wrote:

Do not evaluate spill costs for registers that were already marked as
no_spill.
---


Reviewed-by: Matt Turner 


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

Re: [Mesa-dev] [PATCH 05/13] glsl: simplifies the merge of the default in layout qualifier

[Timothy Arceri]

On Tue, 2016-11-22 at 16:07 +0200, Andres Gomez wrote:
> On Tue, 2016-11-22 at 22:22 +1100, Timothy Arceri wrote:
> ...
> > 
> > 
> > Can I ask that you write a follow up patch for this series that
> > creates
> > a helper function for each of these validations. For example:
> > 
> > static bool
> > validate_ordering(loc, state, qualifier, new_qualifier)
> > {
> >    if (qualifier->flags.q.ordering && new_qualifier-
> > >flags.q.ordering
> >    && qualifier->ordering != new_qualifier->ordering) {
> >   _mesa_glsl_error(loc, state,
> >    "conflicting ordering specified");
> >    }
> > 
> >    return true;
> > }
> > 
> > In merge_qualifier() just put the call in the outer if. For
> > example:
> > 
> > if (q.flags.q.ordering && validate_ordering(...))
> > 
> > This will be better IMO as the code currently exists early when
> > there
> > is no reason we shouldn't continue on and check the other layout
> > qualifiers before reporting errors.
> 
> OK, I will continue with this follow up but I don't think I can just
> put the check in the outer if since, then, I will modify the
> returning
> value of the function in some cases. I will work on that.

_mesa_glsl_error() will set state->error to true so you should be able
to use that at the end of merge_qualifier()

> 
> I also think that continuing without exiting may cause an unexpected
> problem but I will do a piglit and cts run to check.
> 

It should be fine but piglit will tell us for sure :)

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

[Mesa-dev] [PATCH 1/2] mesa: Add GL and GLSL plumbing for ARB_post_depth_coverage for i965 (gen9+).

[Plamena Manolova]

This extension allows the fragment shader to control whether values in
gl_SampleMaskIn[] reflect the coverage after application of the early
depth and stencil tests.

Signed-off-by: Plamena Manolova 
---
 src/compiler/glsl/ast.h  |  5 +
 src/compiler/glsl/ast_to_hir.cpp |  5 +
 src/compiler/glsl/ast_type.cpp   |  8 +++-
 src/compiler/glsl/glsl_parser.yy | 11 +++
 src/compiler/glsl/glsl_parser_extras.cpp |  4 
 src/compiler/glsl/glsl_parser_extras.h   |  4 
 src/compiler/glsl/linker.cpp |  4 
 src/compiler/shader_info.h   |  1 +
 src/mesa/main/extensions_table.h |  1 +
 src/mesa/main/mtypes.h   |  2 ++
 src/mesa/main/shaderapi.c|  1 +
 11 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index 55f9a6c..ad19493 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -606,6 +606,11 @@ struct ast_type_qualifier {
  /** \{ */
  unsigned blend_support:1; /**< Are there any blend_support_ 
qualifiers */
  /** \} */
+
+ /**
+  * Flag set if GL_ARB_post_depth_coverage layout qualifier is used.
+  */
+ unsigned post_depth_coverage:1;
   }
   /** \brief Set of flags, accessed by name. */
   q;
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 9b8678c..c31da86 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3632,6 +3632,11 @@ apply_layout_qualifier_to_variable(const struct 
ast_type_qualifier *qual,
   _mesa_glsl_error(loc, state, "early_fragment_tests layout qualifier only 
"
"valid in fragment shader input layout declaration.");
}
+
+   if (qual->flags.q.post_depth_coverage) {
+  _mesa_glsl_error(loc, state, "post_depth_coverage layout qualifier only "
+   "valid in fragment shader input layout declaration.");
+   }
 }
 
 static void
diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp
index 2856f18..1905721 100644
--- a/src/compiler/glsl/ast_type.cpp
+++ b/src/compiler/glsl/ast_type.cpp
@@ -489,6 +489,7 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc,
   break;
case MESA_SHADER_FRAGMENT:
   valid_in_mask.flags.q.early_fragment_tests = 1;
+  valid_in_mask.flags.q.post_depth_coverage = 1;
   break;
case MESA_SHADER_COMPUTE:
   create_cs_ast |=
@@ -540,6 +541,10 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc,
   state->fs_early_fragment_tests = true;
}
 
+   if (q.flags.q.post_depth_coverage) {
+  state->fs_post_depth_coverage = true;
+   }
+
if (this->flags.q.vertex_spacing) {
   if (q.flags.q.vertex_spacing &&
   this->vertex_spacing != q.vertex_spacing) {
@@ -671,7 +676,8 @@ ast_type_qualifier::validate_flags(YYLTYPE *loc,
 bad.flags.q.point_mode ? " point_mode" : "",
 bad.flags.q.vertices ? " vertices" : "",
 bad.flags.q.subroutine ? " subroutine" : "",
-bad.flags.q.subroutine_def ? " subroutine_def" : "");
+bad.flags.q.subroutine_def ? " subroutine_def" : "",
+bad.flags.q.post_depth_coverage ? " post_depth_coverage" : 
"");
return false;
 }
 
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index a48dc68..a53f476 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -1373,6 +1373,17 @@ layout_qualifier_id:
 
 $$.flags.q.early_fragment_tests = 1;
  }
+
+ if (!$$.flags.i &&
+ match_layout_qualifier($1, "post_depth_coverage", state) == 0) {
+if (state->stage != MESA_SHADER_FRAGMENT) {
+   _mesa_glsl_error(& @1, state,
+"post_depth_coverage layout qualifier only "
+"valid in fragment shaders");
+}
+
+$$.flags.q.post_depth_coverage = 1;
+ }
   }
 
   /* Layout qualifiers for tessellation evaluation shaders. */
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 85a2e94..bc252a0 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -295,6 +295,7 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
gl_context *_ctx,
this->in_qualifier = new(this) ast_type_qualifier();
this->out_qualifier = new(this) ast_type_qualifier();
this->fs_early_fragment_tests = false;
+   this->fs_post_depth_coverage = false;
this->fs_blend_support = 0;
memset(this->atomic_counter_offsets, 0,
   sizeof(this->atomic_counter_offsets));
@@ -608,6 +609,7 @@ static const _mesa_glsl_extension 
_mesa_glsl_supported_extensions[] = {
EXT(ARB_fragment_layer_viewport),
 

[Mesa-dev] [PATCH 2/2] i965: Add i965 plumbing for ARB_post_depth_coverage for i965 (gen9+).

[Plamena Manolova]

This extension allows the fragment shader to control whether values in
gl_SampleMaskIn[] reflect the coverage after application of the early
depth and stencil tests.

Signed-off-by: Plamena Manolova 
---
 src/mesa/drivers/dri/i965/brw_compiler.h |  1 +
 src/mesa/drivers/dri/i965/brw_fs.cpp |  1 +
 src/mesa/drivers/dri/i965/gen8_ps_state.c| 13 ++---
 src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h 
b/src/mesa/drivers/dri/i965/brw_compiler.h
index 65a7478..410641f 100644
--- a/src/mesa/drivers/dri/i965/brw_compiler.h
+++ b/src/mesa/drivers/dri/i965/brw_compiler.h
@@ -397,6 +397,7 @@ struct brw_wm_prog_data {
bool computed_stencil;
 
bool early_fragment_tests;
+   bool post_depth_coverage;
bool dispatch_8;
bool dispatch_16;
bool dual_src_blend;
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 3cdf0bf..07ac4bc 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -6455,6 +6455,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void 
*log_data,
shader->info->outputs_read);
 
prog_data->early_fragment_tests = shader->info->fs.early_fragment_tests;
+   prog_data->post_depth_coverage = shader->info->fs.post_depth_coverage;
 
prog_data->barycentric_interp_modes =
   brw_compute_barycentric_interp_modes(compiler->devinfo, shader);
diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c 
b/src/mesa/drivers/dri/i965/gen8_ps_state.c
index a4eb962..33ef023 100644
--- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
@@ -53,10 +53,17 @@ gen8_upload_ps_extra(struct brw_context *brw,
   dw1 |= GEN8_PSX_SHADER_IS_PER_SAMPLE;
 
if (prog_data->uses_sample_mask) {
-  if (brw->gen >= 9)
- dw1 |= BRW_PSICMS_INNER << GEN9_PSX_SHADER_NORMAL_COVERAGE_MASK_SHIFT;
-  else
+  if (brw->gen >= 9) {
+ if (prog_data->post_depth_coverage) {
+dw1 |= BRW_PCICMS_DEPTH << 
GEN9_PSX_SHADER_NORMAL_COVERAGE_MASK_SHIFT;
+ }
+ else {
+dw1 |= BRW_PSICMS_INNER << 
GEN9_PSX_SHADER_NORMAL_COVERAGE_MASK_SHIFT;
+ }
+  }
+  else {
  dw1 |= GEN8_PSX_SHADER_USES_INPUT_COVERAGE_MASK;
+  }
}
 
if (prog_data->uses_omask)
diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 66079b5..19f4684 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -415,6 +415,7 @@ intelInitExtensions(struct gl_context *ctx)
   ctx->Extensions.KHR_texture_compression_astc_ldr = true;
   ctx->Extensions.KHR_texture_compression_astc_sliced_3d = true;
   ctx->Extensions.MESA_shader_framebuffer_fetch = true;
+  ctx->Extensions.ARB_post_depth_coverage = true;
}
 
if (ctx->API == API_OPENGL_CORE)
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 2/3] intel/isl: Add support for saving surface debug info to files

[Jason Ekstrand]

On Wed, Nov 16, 2016 at 12:56 AM, Jordan Justen 
wrote:

> Signed-off-by: Jordan Justen 
> ---
>  src/intel/Makefile.sources |   1 +
>  src/intel/isl/isl.h|  14 +++
>  src/intel/isl/isl_dump.c   | 217 ++
> +++
>  3 files changed, 232 insertions(+)
>  create mode 100644 src/intel/isl/isl_dump.c
>
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index c1740fe..6ea838f 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -33,6 +33,7 @@ AUBINATOR_GENERATED_FILES = \
>  ISL_FILES = \
> isl/isl.c \
> isl/isl.h \
> +   isl/isl_dump.c \
> isl/isl_format.c \
> isl/isl_priv.h \
> isl/isl_storage_image.c
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index 11ad891..4986137 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -1475,6 +1475,20 @@ uint32_t
>  isl_surf_get_depth_format(const struct isl_device *dev,
>const struct isl_surf *surf);
>
> +/**
> + * @brief Save the isl_surf information out to files for debug purposes.
> + *
> + */
> +void
> +isl_dump_surf(const struct isl_device *dev,
> +  const struct isl_surf *surf,
> +  const void *map_addr,
> +  unsigned int map_size,
> +  const struct isl_surf *aux_surf,
> +  const void *aux_map_addr,
> +  unsigned int aux_map_size,
> +  const char *basename);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/src/intel/isl/isl_dump.c b/src/intel/isl/isl_dump.c
> new file mode 100644
> index 000..21df1bb
> --- /dev/null
> +++ b/src/intel/isl/isl_dump.c
> @@ -0,0 +1,217 @@
> +/*
> + * Copyright 2016 Intel Corporation
> + *
> + *  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 
> +#include 
> +#include 
> +#include 
> +
> +#include "isl.h"
> +#include "isl_priv.h"
> +#include "common/gen_device_info.h"
> +
> +#include "util/format_srgb.h"
> +#include "util/ralloc.h"
> +
> +#include "main/macros.h"
> +
> +/**
> + * @brief Determine if a surface should be dumped.
> + *
> + * Since dumping a surface can produce a lot of data and be time
> consuming,
> + * this function allows you to filter whether a surface should actually be
> + * dumped. If the application is deterministic, then you can use the
> sequence
> + * id number to filter output. Other examples are shown commented out.
> + *
> + * Return true when the surface should be dumped.
> + */
> +static inline bool
> +filter_surface_dumping(uint64_t sequence_id,
> +   const struct isl_surf *surf,
> +   const void *map_addr,
> +   unsigned int map_size,
> +   const struct isl_surf *aux_surf,
> +   const void *aux_map_addr,
> +   unsigned int aux_map_size,
> +   const char *basename)
> +{
> +   const uint64_t single_id = 0;
> +   const uint64_t min_id = 0;
> +   const uint64_t max_id = 0;
> +   return
> +  (min_id == 0 || sequence_id >= min_id) &&
> +  (max_id == 0 || sequence_id <= max_id) &&
> +  (single_id == 0 || sequence_id == single_id) &&
> +  /* surf->format == ISL_FORMAT_R8_UINT && */
> +  /* surf->msaa_layout == ISL_MSAA_LAYOUT_NONE && */
> +  true;
> +}
> +
> +static const char *
> +tiling_name(enum isl_tiling tiling)
> +{
> +#define TILENAME(t) case ISL_TILING_##t: return #t
> +   switch(tiling) {
> +   TILENAME(LINEAR);
> +   TILENAME(W);
> +   TILENAME(X);
> +   TILENAME(Y0);
> +   TILENAME(Yf);
> +   TILENAME(Ys);
> +   TILENAME(HIZ);
> +   TILENAME(CCS);
> +   default:
> +  return NULL;
> +   }
> +}
> +
> +static const char *
> +msaa_name(enum isl_msaa_layout layout)
> +{
> +#define MSAA_NAME(l) case ISL_MSAA_LAY

Re: [Mesa-dev] [PATCH 05/13] glsl: simplifies the merge of the default in layout qualifier

[Andres Gomez]

On Wed, 2016-11-23 at 08:46 +1100, Timothy Arceri wrote:
> On Tue, 2016-11-22 at 16:07 +0200, Andres Gomez wrote:
> > On Tue, 2016-11-22 at 22:22 +1100, Timothy Arceri wrote:
> > ...
> > > 
> > > 
> > > Can I ask that you write a follow up patch for this series that
> > > creates
> > > a helper function for each of these validations. For example:
> > > 
> > > static bool
> > > validate_ordering(loc, state, qualifier, new_qualifier)
> > > {
> > >    if (qualifier->flags.q.ordering && new_qualifier-
> > > > flags.q.ordering
> > > 
> > >    && qualifier->ordering != new_qualifier->ordering) {
> > >   _mesa_glsl_error(loc, state,
> > >    "conflicting ordering specified");
> > >    }
> > > 
> > >    return true;
> > > }
> > > 
> > > In merge_qualifier() just put the call in the outer if. For
> > > example:
> > > 
> > > if (q.flags.q.ordering && validate_ordering(...))
> > > 
> > > This will be better IMO as the code currently exists early when
> > > there
> > > is no reason we shouldn't continue on and check the other layout
> > > qualifiers before reporting errors.
> > 
> > 
> > OK, I will continue with this follow up but I don't think I can just
> > put the check in the outer if since, then, I will modify the
> > returning
> > value of the function in some cases. I will work on that.
> 
> 
> _mesa_glsl_error() will set state->error to true so you should be able
> to use that at the end of merge_qualifier()

I thought of it and that approach can be used quite broadly everywhere
else in the parser but it is not nice.

I know that may be far fetched but checking for state->error implies
that the implementation of the caller relies on the actual
implementation of the method called. In other words, it will work
because we assume a specific implementation of the function being
called instead of seeing it as a black box that will just return true
or false. If that implementation changes in the future we may have
undesired and unexpected collateral misbehaviors.

I will just rely on the value returned.

Anyway, running the tests right now. I will send the new series
tomorrow morning.

-- 
Br,

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

Re: [Mesa-dev] [PATCH 1/3] intel/blorp: Support map/unmap of blorp_address

[Jason Ekstrand]

On Wed, Nov 16, 2016 at 12:56 AM, Jordan Justen 
wrote:

> Signed-off-by: Jordan Justen 
> ---
>  src/intel/blorp/blorp.h   |  8 +++
>  src/mesa/drivers/dri/i965/brw_blorp.c | 39 ++
> +
>  2 files changed, 47 insertions(+)
>
> diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
> index 4351cb1..d891ba3 100644
> --- a/src/intel/blorp/blorp.h
> +++ b/src/intel/blorp/blorp.h
> @@ -38,6 +38,7 @@ extern "C" {
>
>  struct blorp_batch;
>  struct blorp_params;
> +struct blorp_address;
>
>  struct blorp_context {
> void *driver_ctx;
> @@ -52,6 +53,13 @@ struct blorp_context {
>uint32_t vb;
> } mocs;
>
> +   void (*map)(const struct blorp_context *blorp,
> +   const struct blorp_address *blorp_addr,
> +   void **addr,
> +   unsigned int *map_size,
> +   bool *mapped_previously);
> +   void (*unmap)(const struct blorp_context *blorp,
> + const struct blorp_address *blorp_addr);
> bool (*lookup_shader)(struct blorp_context *blorp,
>   const void *key, uint32_t key_size,
>   uint32_t *kernel_out, void *prog_data_out);
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index f0ad074..41b6214 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -34,10 +34,47 @@
>  #include "brw_meta_util.h"
>  #include "brw_state.h"
>  #include "intel_fbo.h"
> +#include "intel_batchbuffer.h"
>  #include "intel_debug.h"
>
>  #define FILE_DEBUG_FLAG DEBUG_BLORP
>
> +static void
> +brw_blorp_map(const struct blorp_context *blorp,
> +  const struct blorp_address *blorp_addr,
> +  void **addr,
> +  unsigned int *size,
> +  bool *mapped_previously)
> +{
> +   struct brw_context *brw = blorp->driver_ctx;
> +   drm_intel_bo *bo = (drm_intel_bo *)blorp_addr->buffer;
> +
> +   *mapped_previously = bo->virtual != NULL;
> +
> +   if (!*mapped_previously) {
> +  if (drm_intel_bo_references(brw->batch.bo, bo))
> + intel_batchbuffer_flush(brw);
> +
> +  int ret = drm_intel_bo_map(bo, /*write_enable*/ false);
> +  if (ret == -1) {
> + assert(bo->virtual == NULL);
> + *addr = NULL;
> + return;
> +  }
> +   }
> +
> +   *addr = bo->virtual;
> +   *size = bo->size;
> +}
> +
> +static void
> +brw_blorp_unmap(const struct blorp_context *blorp,
> +const struct blorp_address *blorp_addr)
> +{
> +   drm_intel_bo *bo = (drm_intel_bo *)blorp_addr->buffer;
> +   drm_intel_bo_unmap(bo);
>

In the above code you don't map again if it's already got a map but here
you unmap regardless.  Is this going to result in some sort of nasty
double-umapping?


> +}
> +
>  static bool
>  brw_blorp_lookup_shader(struct blorp_context *blorp,
>  const void *key, uint32_t key_size,
> @@ -102,6 +139,8 @@ brw_blorp_init(struct brw_context *brw)
>unreachable("Invalid gen");
> }
>
> +   brw->blorp.map = brw_blorp_map;
> +   brw->blorp.unmap = brw_blorp_unmap;
> brw->blorp.lookup_shader = brw_blorp_lookup_shader;
> brw->blorp.upload_shader = brw_blorp_upload_shader;
>  }
> --
> 2.10.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] intel/blorp_blit: Add option to dump surfaces on blits

[Jason Ekstrand]

On Wed, Nov 16, 2016 at 12:56 AM, Jordan Justen 
wrote:

> Enabling this option causes the source and destination surfaces to be
> dumped out to debug files. The destination is dumped both before and
> after the blit operation.
>
> Signed-off-by: Jordan Justen 
> ---
>  src/intel/blorp/blorp_blit.c | 53 ++
> ++
>  1 file changed, 53 insertions(+)
>
> diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
> index c0b56c3..c1c4219 100644
> --- a/src/intel/blorp/blorp_blit.c
> +++ b/src/intel/blorp/blorp_blit.c
> @@ -28,6 +28,8 @@
>
>  #define FILE_DEBUG_FLAG DEBUG_BLORP
>
> +static const bool isl_surface_debug_dump = false;
> +
>  /**
>   * Enum to specify the order of arguments in a sampler message
>   */
> @@ -1428,6 +1430,41 @@ surf_retile_w_to_y(const struct isl_device *isl_dev,
>  }
>
>  static void
> +dump_blorp_surf(const struct blorp_context *blorp,
> +const struct brw_blorp_surface_info *surf,
> +const char *basename)
> +{
> +   void *map, *aux_map;
> +   unsigned int size, aux_size;
> +   bool was_mapped, aux_was_mapped;
> +
> +   blorp->map(blorp, &surf->addr, &map, &size, &was_mapped);
> +   if (map == NULL)
> +  return;
> +
> +   if (surf->aux_addr.buffer) {
> +  blorp->map(blorp, &surf->aux_addr, &aux_map, &aux_size,
> &aux_was_mapped);
> +  if (aux_map == NULL) {
> + if (!was_mapped)
> +blorp->unmap(blorp, &surf->addr);
> + return;
> +  }
> +   } else {
> +  aux_map = NULL;
> +  aux_size = 0;
> +   }
> +
> +   isl_dump_surf(blorp->isl_dev, &surf->surf, map, size,
> + aux_map ? &surf->aux_surf : NULL, aux_map, aux_size,
> + basename);
> +
> +   if (!was_mapped)
> +  blorp->unmap(blorp, &surf->addr);
> +   if (surf->aux_addr.buffer && !aux_was_mapped)
> +  blorp->unmap(blorp, &surf->aux_addr);
>

Never mind my earlier comments.  You already thought about it!


> +}
> +
> +static void
>  do_blorp_blit(struct blorp_batch *batch,
>struct blorp_params *params,
>struct brw_blorp_blit_prog_key *wm_prog_key,
> @@ -1657,6 +1694,11 @@ blorp_blit(struct blorp_batch *batch,
> brw_blorp_surface_info_init(batch->blorp, ¶ms.dst, dst_surf,
> dst_level,
> dst_layer, dst_format, true);
>
> +   if (isl_surface_debug_dump) {
> +  dump_blorp_surf(batch->blorp, ¶ms.src, "blorp-blit-src");
> +  dump_blorp_surf(batch->blorp, ¶ms.dst, "blorp-blit-dst-before");
> +   }
> +
> params.src.view.swizzle = src_swizzle;
> params.dst.view.swizzle = dst_swizzle;
>
> @@ -1708,6 +1750,9 @@ blorp_blit(struct blorp_batch *batch,
>   src_x0, src_y0, src_x1, src_y1,
>   dst_x0, dst_y0, dst_x1, dst_y1,
>   mirror_x, mirror_y);
> +
> +   if (isl_surface_debug_dump)
> +  dump_blorp_surf(batch->blorp, ¶ms.dst, "blorp-blit-dst-after");
>  }
>
>  static enum isl_format
> @@ -1850,6 +1895,11 @@ blorp_copy(struct blorp_batch *batch,
> brw_blorp_surface_info_init(batch->blorp, ¶ms.dst, dst_surf,
> dst_level,
> dst_layer, ISL_FORMAT_UNSUPPORTED, true);
>
> +   if (isl_surface_debug_dump) {
> +  dump_blorp_surf(batch->blorp, ¶ms.src, "blorp-copy-src");
> +  dump_blorp_surf(batch->blorp, ¶ms.dst, "blorp-copy-dst-before");
> +   }
> +
> struct brw_blorp_blit_prog_key wm_prog_key;
> memset(&wm_prog_key, 0, sizeof(wm_prog_key));
>
> @@ -1889,4 +1939,7 @@ blorp_copy(struct blorp_batch *batch,
>   src_x, src_y, src_x + src_width, src_y + src_height,
>   dst_x, dst_y, dst_x + dst_width, dst_y + dst_height,
>   false, false);
> +
> +   if (isl_surface_debug_dump)
> +  dump_blorp_surf(batch->blorp, ¶ms.dst, "blorp-copy-dst-after");
>  }
> --
> 2.10.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/3] i965/hiz: Add debug dump for depth/hiz buffers around hiz ops

[Jason Ekstrand]

On Thu, Nov 17, 2016 at 3:27 PM, Jordan Justen 
wrote:

> Signed-off-by: Jordan Justen 
> Cc: Matt Turner 
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c | 83 --
> -
>  1 file changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index 8aab1a7..d66bf14 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -39,6 +39,8 @@
>
>  #define FILE_DEBUG_FLAG DEBUG_BLORP
>
> +static const bool isl_hiz_debug_dump = false;
> +
>  static void
>  brw_blorp_map(const struct blorp_context *blorp,
>const struct blorp_address *blorp_addr,
> @@ -1005,20 +1007,42 @@ brw_blorp_resolve_color(struct brw_context *brw,
> struct intel_mipmap_tree *mt)
>  }
>
>  static void
> -gen6_blorp_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree *mt,
> -unsigned int level, unsigned int layer, enum
> blorp_hiz_op op)
> +hiz_dump_blorp_surf(const struct blorp_context *blorp,
> +const struct blorp_surf *surf,
> +const char *opname, const char *when)
>

How is this code different from the other blorp surface dumping code?  They
seem pretty much the same to me.


>  {
> -   assert(intel_miptree_level_has_hiz(mt, level));
> -
> -   struct isl_surf isl_tmp[2];
> -   struct blorp_surf surf;
> -   blorp_surf_for_miptree(brw, &surf, mt, true, (1 << ISL_AUX_USAGE_HIZ),
> -  &level, layer, 1, isl_tmp);
> +   void *map, *aux_map;
> +   unsigned int size, aux_size;
> +   bool was_mapped, aux_was_mapped;
> +   char *basename;
> +
> +   blorp->map(blorp, &surf->addr, &map, &size, &was_mapped);
> +   if (map == NULL)
> +  return;
> +
> +   if (surf->aux_addr.buffer) {
> +  blorp->map(blorp, &surf->aux_addr, &aux_map, &aux_size,
> &aux_was_mapped);
> +  if (aux_map == NULL) {
> + if (!was_mapped)
> +blorp->unmap(blorp, &surf->addr);
> + return;
> +  }
> +   } else {
> +  aux_map = NULL;
> +  aux_size = 0;
> +   }
>
> -   struct blorp_batch batch;
> -   blorp_batch_init(&brw->blorp, &batch, brw, 0);
> -   blorp_gen6_hiz_op(&batch, &surf, level, layer, op);
> -   blorp_batch_finish(&batch);
> +   basename = ralloc_asprintf(NULL, "%s-%s", opname, when);
> +   assert(basename);
> +   isl_dump_surf(blorp->isl_dev, surf->surf, map, size,
> + aux_map ? surf->aux_surf : NULL, aux_map, aux_size,
> + basename);
> +   ralloc_free(basename);
> +
> +   if (!was_mapped)
> +  blorp->unmap(blorp, &surf->addr);
> +   if (surf->aux_addr.buffer && !aux_was_mapped)
> +  blorp->unmap(blorp, &surf->aux_addr);
>  }
>
>  /**
> @@ -1038,25 +1062,50 @@ intel_hiz_exec(struct brw_context *brw, struct
> intel_mipmap_tree *mt,
>
> switch (op) {
> case BLORP_HIZ_OP_DEPTH_RESOLVE:
> -  opname = "depth resolve";
> +  opname = "depth-resolve";
>break;
> case BLORP_HIZ_OP_HIZ_RESOLVE:
> -  opname = "hiz ambiguate";
> +  opname = "hiz-ambiguate";
>break;
> case BLORP_HIZ_OP_DEPTH_CLEAR:
> -  opname = "depth clear";
> +  opname = "depth-clear";
>break;
> case BLORP_HIZ_OP_NONE:
> -  opname = "noop?";
> +  opname = "hiz-noop";
>break;
> }
>
> DBG("%s %s to mt %p level %d layer %d\n",
> __func__, opname, mt, level, layer);
>
> +   intel_miptree_check_level_layer(mt, level, layer);
> +   intel_miptree_used_for_rendering(mt);
> +
> +   assert(intel_miptree_level_has_hiz(mt, level));
> +
> +   struct isl_surf isl_tmp[2];
> +   struct blorp_surf surf;
> +   struct blorp_batch batch;
> +   const bool need_blorp = brw->gen < 8 || isl_hiz_debug_dump;
> +
> +   if (need_blorp) {
> +  blorp_surf_for_miptree(brw, &surf, mt, true, (1 <<
> ISL_AUX_USAGE_HIZ),
> + &level, layer, 1, isl_tmp);
> +  blorp_batch_init(&brw->blorp, &batch, brw, 0);
> +   }
> +
> +   if (isl_hiz_debug_dump)
> +  hiz_dump_blorp_surf(batch.blorp, &surf, opname, "before");
> +
> if (brw->gen >= 8) {
>gen8_hiz_exec(brw, mt, level, layer, op);
> } else {
> -  gen6_blorp_hiz_exec(brw, mt, level, layer, op);
> +  blorp_gen6_hiz_op(&batch, &surf, level, layer, op);
> }
> +
> +   if (isl_hiz_debug_dump)
> +  hiz_dump_blorp_surf(batch.blorp, &surf, opname, "after");
> +
> +   if (need_blorp)
> +  blorp_batch_finish(&batch);
>  }
> --
> 2.10.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] intel/blorp_blit: Add option to dump surfaces on blits

[Jason Ekstrand]

The first three are

Reviewed-by: Jason Ekstrand 

The last one seems really clunky to me.  I sort-of get why since we don't
actually use blorp for gen8 HiZ ops.  I think it would be better if you
split it into two patches, one which merges intel_hiz_exec with
gen6_blorp_hiz_exec and a second which adds the dumping and try to re-use
the dumping from blits.

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

Re: [Mesa-dev] [PATCH 2/2] i965: Add i965 plumbing for ARB_post_depth_coverage for i965 (gen9+).

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

Of course, the other has to land first. :-)

On Tue, Nov 22, 2016 at 1:55 PM, Plamena Manolova <
plamena.manol...@intel.com> wrote:

> This extension allows the fragment shader to control whether values in
> gl_SampleMaskIn[] reflect the coverage after application of the early
> depth and stencil tests.
>
> Signed-off-by: Plamena Manolova 
> ---
>  src/mesa/drivers/dri/i965/brw_compiler.h |  1 +
>  src/mesa/drivers/dri/i965/brw_fs.cpp |  1 +
>  src/mesa/drivers/dri/i965/gen8_ps_state.c| 13 ++---
>  src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
>  4 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h
> b/src/mesa/drivers/dri/i965/brw_compiler.h
> index 65a7478..410641f 100644
> --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> @@ -397,6 +397,7 @@ struct brw_wm_prog_data {
> bool computed_stencil;
>
> bool early_fragment_tests;
> +   bool post_depth_coverage;
> bool dispatch_8;
> bool dispatch_16;
> bool dual_src_blend;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 3cdf0bf..07ac4bc 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -6455,6 +6455,7 @@ brw_compile_fs(const struct brw_compiler *compiler,
> void *log_data,
> shader->info->outputs_read);
>
> prog_data->early_fragment_tests = shader->info->fs.early_
> fragment_tests;
> +   prog_data->post_depth_coverage = shader->info->fs.post_depth_coverage;
>
> prog_data->barycentric_interp_modes =
>brw_compute_barycentric_interp_modes(compiler->devinfo, shader);
> diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> index a4eb962..33ef023 100644
> --- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> @@ -53,10 +53,17 @@ gen8_upload_ps_extra(struct brw_context *brw,
>dw1 |= GEN8_PSX_SHADER_IS_PER_SAMPLE;
>
> if (prog_data->uses_sample_mask) {
> -  if (brw->gen >= 9)
> - dw1 |= BRW_PSICMS_INNER << GEN9_PSX_SHADER_NORMAL_
> COVERAGE_MASK_SHIFT;
> -  else
> +  if (brw->gen >= 9) {
> + if (prog_data->post_depth_coverage) {
> +dw1 |= BRW_PCICMS_DEPTH << GEN9_PSX_SHADER_NORMAL_
> COVERAGE_MASK_SHIFT;
> + }
> + else {
> +dw1 |= BRW_PSICMS_INNER << GEN9_PSX_SHADER_NORMAL_
> COVERAGE_MASK_SHIFT;
> + }
> +  }
> +  else {
>   dw1 |= GEN8_PSX_SHADER_USES_INPUT_COVERAGE_MASK;
> +  }
> }
>
> if (prog_data->uses_omask)
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 66079b5..19f4684 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -415,6 +415,7 @@ intelInitExtensions(struct gl_context *ctx)
>ctx->Extensions.KHR_texture_compression_astc_ldr = true;
>ctx->Extensions.KHR_texture_compression_astc_sliced_3d = true;
>ctx->Extensions.MESA_shader_framebuffer_fetch = true;
> +  ctx->Extensions.ARB_post_depth_coverage = true;
> }
>
> if (ctx->API == API_OPENGL_CORE)
> --
> 2.7.4
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] More shader state clean-ups

[Timothy Arceri]

This series applies on top of my series to rework the way we
handle the gl shader/program structs [1].

These changes allow us to stop passing duplicate fields around
for no good reason, and allow us to simplify the on disk shader 
cache by getting all the values from a single location we can
cache less.

[1] https://lists.freedesktop.org/archives/mesa-dev/2016-November/135916.html

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

[Mesa-dev] [PATCH 01/14] i965: get outputs_written from gl_program

[Timothy Arceri]

There is no need to go via the pointer in nir_shader. This change
is required for the shader cache as we don't create a nir_shader.
---
 src/mesa/drivers/dri/i965/brw_vs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vs.c 
b/src/mesa/drivers/dri/i965/brw_vs.c
index 8f7dc73..82dcdb3 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -328,7 +328,7 @@ brw_vs_populate_key(struct brw_context *brw,
   }
}
 
-   if (prog->nir->info->outputs_written &
+   if (prog->info.outputs_written &
(VARYING_BIT_COL0 | VARYING_BIT_COL1 | VARYING_BIT_BFC0 |
 VARYING_BIT_BFC1)) {
   /* _NEW_LIGHT | _NEW_BUFFERS */
@@ -382,7 +382,7 @@ brw_vs_precompile(struct gl_context *ctx, struct gl_program 
*prog)
brw_setup_tex_for_precompile(brw, &key.tex, prog);
key.program_string_id = bvp->id;
key.clamp_vertex_color =
-  (prog->nir->info->outputs_written &
+  (prog->info.outputs_written &
(VARYING_BIT_COL0 | VARYING_BIT_COL1 | VARYING_BIT_BFC0 |
 VARYING_BIT_BFC1));
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 05/14] mesa/glsl: move redeclares_gl_fragcoord to gl_shader

[Timothy Arceri]

This is never used in gl_linked_shader other than as a temp
during linking so just use a temp instead.
---
 src/compiler/glsl/glsl_parser_extras.cpp |  3 +--
 src/compiler/glsl/linker.cpp | 21 -
 src/mesa/main/mtypes.h   |  3 ++-
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index ad13436..7e62eaa 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1804,8 +1804,7 @@ set_shader_inout_layout(struct gl_shader *shader,
   break;
 
case MESA_SHADER_FRAGMENT:
-  shader->info.redeclares_gl_fragcoord =
- state->fs_redeclares_gl_fragcoord;
+  shader->redeclares_gl_fragcoord = state->fs_redeclares_gl_fragcoord;
   shader->info.uses_gl_fragcoord = state->fs_uses_gl_fragcoord;
   shader->info.pixel_center_integer = state->fs_pixel_center_integer;
   shader->info.origin_upper_left = state->fs_origin_upper_left;
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index fe90afc..b31c055 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1825,7 +1825,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
 struct gl_shader **shader_list,
 unsigned num_shaders)
 {
-   linked_shader->info.redeclares_gl_fragcoord = false;
+   bool redeclares_gl_fragcoord = false;
linked_shader->info.uses_gl_fragcoord = false;
linked_shader->info.origin_upper_left = false;
linked_shader->info.pixel_center_integer = false;
@@ -1843,12 +1843,10 @@ link_fs_inout_layout_qualifiers(struct 
gl_shader_program *prog,
*it must be redeclared in all the fragment shaders in that program
*that have a static use gl_FragCoord."
*/
-  if ((linked_shader->info.redeclares_gl_fragcoord
-   && !shader->info.redeclares_gl_fragcoord
-   && shader->info.uses_gl_fragcoord)
-  || (shader->info.redeclares_gl_fragcoord
-  && !linked_shader->info.redeclares_gl_fragcoord
-  && linked_shader->info.uses_gl_fragcoord)) {
+  if ((redeclares_gl_fragcoord && !shader->redeclares_gl_fragcoord &&
+   shader->info.uses_gl_fragcoord)
+  || (shader->redeclares_gl_fragcoord && !redeclares_gl_fragcoord &&
+  linked_shader->info.uses_gl_fragcoord)) {
  linker_error(prog, "fragment shader defined with conflicting "
  "layout qualifiers for gl_FragCoord\n");
   }
@@ -1858,8 +1856,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
*   "All redeclarations of gl_FragCoord in all fragment shaders in a
*single program must have the same set of qualifiers."
*/
-  if (linked_shader->info.redeclares_gl_fragcoord &&
-  shader->info.redeclares_gl_fragcoord &&
+  if (redeclares_gl_fragcoord && shader->redeclares_gl_fragcoord &&
   (shader->info.origin_upper_left !=
linked_shader->info.origin_upper_left ||
shader->info.pixel_center_integer !=
@@ -1873,10 +1870,8 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
* are multiple redeclarations, all the fields except uses_gl_fragcoord
* are already known to be the same.
*/
-  if (shader->info.redeclares_gl_fragcoord ||
-  shader->info.uses_gl_fragcoord) {
- linked_shader->info.redeclares_gl_fragcoord =
-shader->info.redeclares_gl_fragcoord;
+  if (shader->redeclares_gl_fragcoord || shader->info.uses_gl_fragcoord) {
+ redeclares_gl_fragcoord = shader->redeclares_gl_fragcoord;
  linked_shader->info.uses_gl_fragcoord =
 linked_shader->info.uses_gl_fragcoord ||
 shader->info.uses_gl_fragcoord;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index c4fdbf3..b8d9fcf 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2235,7 +2235,6 @@ struct gl_subroutine_function
 struct gl_shader_info
 {
bool uses_gl_fragcoord;
-   bool redeclares_gl_fragcoord;
 
/**
 * Fragment shader state from GLSL 1.50 layout qualifiers.
@@ -2421,6 +2420,8 @@ struct gl_shader
 
bool ARB_fragment_coord_conventions_enable;
 
+   bool redeclares_gl_fragcoord;
+
struct gl_shader_info info;
 };
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 03/14] st/mesa/glsl: set early_fragment_tests directly in shader_info

[Timothy Arceri]

We also move EarlyFragmentTests out of the gl_shader_info struct
as it is now only used by gl_shader.
---
 src/compiler/glsl/glsl_parser_extras.cpp   |  2 +-
 src/compiler/glsl/linker.cpp   |  4 ++--
 src/mesa/main/mtypes.h | 12 ++--
 src/mesa/main/shaderapi.c  |  1 -
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  2 +-
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 4f76815..0efcfe9 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1811,7 +1811,7 @@ set_shader_inout_layout(struct gl_shader *shader,
   shader->info.origin_upper_left = state->fs_origin_upper_left;
   shader->info.ARB_fragment_coord_conventions_enable =
  state->ARB_fragment_coord_conventions_enable;
-  shader->info.EarlyFragmentTests = state->fs_early_fragment_tests;
+  shader->EarlyFragmentTests = state->fs_early_fragment_tests;
   shader->BlendSupport = state->fs_blend_support;
   break;
 
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index cec4cab..a98ffc0 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1886,8 +1886,8 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
 shader->info.pixel_center_integer;
   }
 
-  linked_shader->info.EarlyFragmentTests |=
- shader->info.EarlyFragmentTests;
+  linked_shader->Program->info.fs.early_fragment_tests |=
+ shader->EarlyFragmentTests;
   linked_shader->Program->sh.fs.BlendSupport |= shader->BlendSupport;
}
 }
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 93a70b4..a149d0a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2308,12 +2308,6 @@ struct gl_shader_info
} Geom;
 
/**
-* Whether early fragment tests are enabled as defined by
-* ARB_shader_image_load_store.
-*/
-   bool EarlyFragmentTests;
-
-   /**
 * Compute shader state from ARB_compute_shader and
 * ARB_compute_variable_group_size layout qualifiers.
 */
@@ -2420,6 +2414,12 @@ struct gl_shader
 */
GLbitfield BlendSupport;
 
+   /**
+* Whether early fragment tests are enabled as defined by
+* ARB_shader_image_load_store.
+*/
+   bool EarlyFragmentTests;
+
struct gl_shader_info info;
 };
 
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 6740f9a..9d2fdde 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -2200,7 +2200,6 @@ _mesa_copy_linked_program_data(const struct 
gl_shader_program *src,
}
case MESA_SHADER_FRAGMENT: {
   dst->info.fs.depth_layout = src->FragDepthLayout;
-  dst->info.fs.early_fragment_tests = dst_sh->info.EarlyFragmentTests;
   break;
}
case MESA_SHADER_COMPUTE: {
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index c5ba53a..c66eae9 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -6098,7 +6098,7 @@ st_translate_program(
}
 
if (procType == PIPE_SHADER_FRAGMENT) {
-  if (program->shader->info.EarlyFragmentTests)
+  if (program->shader->Program->info.fs.early_fragment_tests)
  ureg_property(ureg, TGSI_PROPERTY_FS_EARLY_DEPTH_STENCIL, 1);
 
   if (proginfo->info.inputs_read & VARYING_BIT_POS) {
-- 
2.7.4

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

[Mesa-dev] [PATCH 07/14] mesa/glsl: move origin_upper_left to gl_shader

[Timothy Arceri]

This is only used by gl_linked_shader as a temp during linking
so use a temp there instead.
---
 src/compiler/glsl/glsl_parser_extras.cpp | 2 +-
 src/compiler/glsl/linker.cpp | 8 +++-
 src/mesa/main/mtypes.h   | 9 +
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 7d100bb..6d7da29 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1807,7 +1807,7 @@ set_shader_inout_layout(struct gl_shader *shader,
   shader->redeclares_gl_fragcoord = state->fs_redeclares_gl_fragcoord;
   shader->uses_gl_fragcoord = state->fs_uses_gl_fragcoord;
   shader->info.pixel_center_integer = state->fs_pixel_center_integer;
-  shader->info.origin_upper_left = state->fs_origin_upper_left;
+  shader->origin_upper_left = state->fs_origin_upper_left;
   shader->ARB_fragment_coord_conventions_enable =
  state->ARB_fragment_coord_conventions_enable;
   shader->EarlyFragmentTests = state->fs_early_fragment_tests;
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 5201005..a8fe922 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1827,7 +1827,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
 {
bool redeclares_gl_fragcoord = false;
bool uses_gl_fragcoord = false;
-   linked_shader->info.origin_upper_left = false;
+   bool origin_upper_left = false;
linked_shader->info.pixel_center_integer = false;
 
if (linked_shader->Stage != MESA_SHADER_FRAGMENT ||
@@ -1857,8 +1857,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
*single program must have the same set of qualifiers."
*/
   if (redeclares_gl_fragcoord && shader->redeclares_gl_fragcoord &&
-  (shader->info.origin_upper_left !=
-   linked_shader->info.origin_upper_left ||
+  (shader->origin_upper_left != origin_upper_left ||
shader->info.pixel_center_integer !=
linked_shader->info.pixel_center_integer)) {
  linker_error(prog, "fragment shader defined with conflicting "
@@ -1873,8 +1872,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
   if (shader->redeclares_gl_fragcoord || shader->uses_gl_fragcoord) {
  redeclares_gl_fragcoord = shader->redeclares_gl_fragcoord;
  uses_gl_fragcoord |= shader->uses_gl_fragcoord;
- linked_shader->info.origin_upper_left =
-shader->info.origin_upper_left;
+ origin_upper_left = shader->origin_upper_left;
  linked_shader->info.pixel_center_integer =
 shader->info.pixel_center_integer;
   }
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 3882571..610285d 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2234,10 +2234,6 @@ struct gl_subroutine_function
  */
 struct gl_shader_info
 {
-   /**
-* Fragment shader state from GLSL 1.50 layout qualifiers.
-*/
-   bool origin_upper_left;
bool pixel_center_integer;
 
struct {
@@ -2421,6 +2417,11 @@ struct gl_shader
bool redeclares_gl_fragcoord;
bool uses_gl_fragcoord;
 
+   /**
+* Fragment shader state from GLSL 1.50 layout qualifiers.
+*/
+   bool origin_upper_left;
+
struct gl_shader_info info;
 };
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 08/14] mesa/glsl: move pixel_center_integer to gl_shader

[Timothy Arceri]

This is only used by gl_linked_shader as a temp during linking
so use a temp there instead.
---
 src/compiler/glsl/glsl_parser_extras.cpp | 2 +-
 src/compiler/glsl/linker.cpp | 8 +++-
 src/mesa/main/mtypes.h   | 3 +--
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 6d7da29..2711b71 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1806,7 +1806,7 @@ set_shader_inout_layout(struct gl_shader *shader,
case MESA_SHADER_FRAGMENT:
   shader->redeclares_gl_fragcoord = state->fs_redeclares_gl_fragcoord;
   shader->uses_gl_fragcoord = state->fs_uses_gl_fragcoord;
-  shader->info.pixel_center_integer = state->fs_pixel_center_integer;
+  shader->pixel_center_integer = state->fs_pixel_center_integer;
   shader->origin_upper_left = state->fs_origin_upper_left;
   shader->ARB_fragment_coord_conventions_enable =
  state->ARB_fragment_coord_conventions_enable;
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index a8fe922..8bbbf7a 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1828,7 +1828,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
bool redeclares_gl_fragcoord = false;
bool uses_gl_fragcoord = false;
bool origin_upper_left = false;
-   linked_shader->info.pixel_center_integer = false;
+   bool pixel_center_integer = false;
 
if (linked_shader->Stage != MESA_SHADER_FRAGMENT ||
(prog->data->Version < 150 &&
@@ -1858,8 +1858,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
*/
   if (redeclares_gl_fragcoord && shader->redeclares_gl_fragcoord &&
   (shader->origin_upper_left != origin_upper_left ||
-   shader->info.pixel_center_integer !=
-   linked_shader->info.pixel_center_integer)) {
+   shader->pixel_center_integer != pixel_center_integer)) {
  linker_error(prog, "fragment shader defined with conflicting "
   "layout qualifiers for gl_FragCoord\n");
   }
@@ -1873,8 +1872,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
  redeclares_gl_fragcoord = shader->redeclares_gl_fragcoord;
  uses_gl_fragcoord |= shader->uses_gl_fragcoord;
  origin_upper_left = shader->origin_upper_left;
- linked_shader->info.pixel_center_integer =
-shader->info.pixel_center_integer;
+ pixel_center_integer = shader->pixel_center_integer;
   }
 
   linked_shader->Program->info.fs.early_fragment_tests |=
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 610285d..1a38830 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2234,8 +2234,6 @@ struct gl_subroutine_function
  */
 struct gl_shader_info
 {
-   bool pixel_center_integer;
-
struct {
   /** Global xfb_stride out qualifier if any */
   GLuint BufferStride[MAX_FEEDBACK_BUFFERS];
@@ -2421,6 +2419,7 @@ struct gl_shader
 * Fragment shader state from GLSL 1.50 layout qualifiers.
 */
bool origin_upper_left;
+   bool pixel_center_integer;
 
struct gl_shader_info info;
 };
-- 
2.7.4

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

[Mesa-dev] [PATCH 02/14] mesa/glsl/i965: set and use tcs vertices_out directly

[Timothy Arceri]

---
 src/compiler/glsl/linker.cpp| 25 -
 src/mesa/drivers/dri/i965/brw_tcs.c |  6 ++
 src/mesa/main/shaderapi.c   |  6 +-
 3 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index e0523b1..cec4cab 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1659,15 +1659,15 @@ link_xfb_stride_layout_qualifiers(struct gl_context 
*ctx,
  */
 static void
 link_tcs_out_layout_qualifiers(struct gl_shader_program *prog,
-   struct gl_linked_shader *linked_shader,
+   struct gl_program *gl_prog,
struct gl_shader **shader_list,
unsigned num_shaders)
 {
-   linked_shader->info.TessCtrl.VerticesOut = 0;
-
-   if (linked_shader->Stage != MESA_SHADER_TESS_CTRL)
+   if (gl_prog->info.stage != MESA_SHADER_TESS_CTRL)
   return;
 
+   gl_prog->info.tcs.vertices_out = 0;
+
/* From the GLSL 4.0 spec (chapter 4.3.8.2):
 *
 * "All tessellation control shader layout declarations in a program
@@ -1682,17 +1682,16 @@ link_tcs_out_layout_qualifiers(struct gl_shader_program 
*prog,
   struct gl_shader *shader = shader_list[i];
 
   if (shader->info.TessCtrl.VerticesOut != 0) {
- if (linked_shader->info.TessCtrl.VerticesOut != 0 &&
- linked_shader->info.TessCtrl.VerticesOut !=
- shader->info.TessCtrl.VerticesOut) {
+ if (gl_prog->info.tcs.vertices_out != 0 &&
+ gl_prog->info.tcs.vertices_out !=
+ (unsigned) shader->info.TessCtrl.VerticesOut) {
 linker_error(prog, "tessellation control shader defined with "
  "conflicting output vertex count (%d and %d)\n",
- linked_shader->info.TessCtrl.VerticesOut,
+ gl_prog->info.tcs.vertices_out,
  shader->info.TessCtrl.VerticesOut);
 return;
  }
- linked_shader->info.TessCtrl.VerticesOut =
-shader->info.TessCtrl.VerticesOut;
+ gl_prog->info.tcs.vertices_out = shader->info.TessCtrl.VerticesOut;
   }
}
 
@@ -1700,7 +1699,7 @@ link_tcs_out_layout_qualifiers(struct gl_shader_program 
*prog,
 * since we already know we're in the right type of shader program
 * for doing it.
 */
-   if (linked_shader->info.TessCtrl.VerticesOut == 0) {
+   if (gl_prog->info.tcs.vertices_out == 0) {
   linker_error(prog, "tessellation control shader didn't declare "
"vertices out layout qualifier\n");
   return;
@@ -2215,7 +2214,7 @@ link_intrastage_shaders(void *mem_ctx,
clone_ir_list(mem_ctx, linked->ir, main->ir);
 
link_fs_inout_layout_qualifiers(prog, linked, shader_list, num_shaders);
-   link_tcs_out_layout_qualifiers(prog, linked, shader_list, num_shaders);
+   link_tcs_out_layout_qualifiers(prog, gl_prog, shader_list, num_shaders);
link_tes_in_layout_qualifiers(prog, linked, shader_list, num_shaders);
link_gs_inout_layout_qualifiers(prog, linked, shader_list, num_shaders);
link_cs_input_layout_qualifiers(prog, linked, shader_list, num_shaders);
@@ -2418,7 +2417,7 @@ resize_tes_inputs(struct gl_context *ctx,
 * known until draw time.
 */
const int num_vertices = tcs
-  ? tcs->info.TessCtrl.VerticesOut
+  ? tcs->Program->info.tcs.vertices_out
   : ctx->Const.MaxPatchVertices;
 
array_resize_visitor input_resize_visitor(num_vertices, prog,
diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c 
b/src/mesa/drivers/dri/i965/brw_tcs.c
index ef7d94b..d5f3d3b 100644
--- a/src/mesa/drivers/dri/i965/brw_tcs.c
+++ b/src/mesa/drivers/dri/i965/brw_tcs.c
@@ -394,10 +394,8 @@ brw_tcs_precompile(struct gl_context *ctx,
brw_setup_tex_for_precompile(brw, &key.tex, prog);
 
/* Guess that the input and output patches have the same dimensionality. */
-   if (brw->gen < 8) {
-  key.input_vertices = shader_prog->
- _LinkedShaders[MESA_SHADER_TESS_CTRL]->info.TessCtrl.VerticesOut;
-   }
+   if (brw->gen < 8)
+  key.input_vertices = prog->info.tcs.vertices_out;
 
struct brw_program *btep;
if (tes) {
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 24026bd..6740f9a 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -820,7 +820,7 @@ get_programiv(struct gl_context *ctx, GLuint program, 
GLenum pname,
  break;
   if (check_tcs_query(ctx, shProg)) {
  *params = shProg->_LinkedShaders[MESA_SHADER_TESS_CTRL]->
-info.TessCtrl.VerticesOut;
+Program->info.tcs.vertices_out;
   }
   return;
case GL_TESS_GEN_MODE:
@@ -2181,10 +2181,6 @@ _mesa_copy_linked_program_data(const struct 
gl_shader_program *src,
dst->info.separate_shader = src->SeparateShader;
 
switch (dst_sh->Stage) {
-   case MESA_SHADER_TESS_CTRL: {
-   

[Mesa-dev] [PATCH 11/14] mesa/glsl/i965: set and get tes layouts directly to and from shader_info

[Timothy Arceri]

---
 src/compiler/glsl/linker.cpp| 60 ++---
 src/mesa/drivers/dri/i965/brw_tcs.c |  6 ++--
 src/mesa/main/shaderapi.c   | 15 +++---
 3 files changed, 37 insertions(+), 44 deletions(-)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index d0029fb..f9821a5 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1712,18 +1712,20 @@ link_tcs_out_layout_qualifiers(struct gl_shader_program 
*prog,
  */
 static void
 link_tes_in_layout_qualifiers(struct gl_shader_program *prog,
-  struct gl_linked_shader *linked_shader,
+  struct gl_program *gl_prog,
   struct gl_shader **shader_list,
   unsigned num_shaders)
 {
-   linked_shader->info.TessEval.PrimitiveMode = PRIM_UNKNOWN;
-   linked_shader->info.TessEval.Spacing = 0;
-   linked_shader->info.TessEval.VertexOrder = 0;
-   linked_shader->info.TessEval.PointMode = -1;
-
-   if (linked_shader->Stage != MESA_SHADER_TESS_EVAL)
+   if (gl_prog->info.stage != MESA_SHADER_TESS_EVAL)
   return;
 
+   int point_mode = -1;
+
+   gl_prog->info.tes.primitive_mode = PRIM_UNKNOWN;
+   gl_prog->info.tes.spacing = 0;
+   gl_prog->info.tes.vertex_order = 0;
+   gl_prog->info.tes.point_mode = false;
+
/* From the GLSL 4.0 spec (chapter 4.3.8.1):
 *
 * "At least one tessellation evaluation shader (compilation unit) in
@@ -1742,49 +1744,45 @@ link_tes_in_layout_qualifiers(struct gl_shader_program 
*prog,
   struct gl_shader *shader = shader_list[i];
 
   if (shader->info.TessEval.PrimitiveMode != PRIM_UNKNOWN) {
- if (linked_shader->info.TessEval.PrimitiveMode != PRIM_UNKNOWN &&
- linked_shader->info.TessEval.PrimitiveMode !=
+ if (gl_prog->info.tes.primitive_mode != PRIM_UNKNOWN &&
+ gl_prog->info.tes.primitive_mode !=
  shader->info.TessEval.PrimitiveMode) {
 linker_error(prog, "tessellation evaluation shader defined with "
  "conflicting input primitive modes.\n");
 return;
  }
- linked_shader->info.TessEval.PrimitiveMode = 
shader->info.TessEval.PrimitiveMode;
+ gl_prog->info.tes.primitive_mode = 
shader->info.TessEval.PrimitiveMode;
   }
 
   if (shader->info.TessEval.Spacing != 0) {
- if (linked_shader->info.TessEval.Spacing != 0 &&
- linked_shader->info.TessEval.Spacing !=
+ if (gl_prog->info.tes.spacing != 0 && gl_prog->info.tes.spacing !=
  shader->info.TessEval.Spacing) {
 linker_error(prog, "tessellation evaluation shader defined with "
  "conflicting vertex spacing.\n");
 return;
  }
- linked_shader->info.TessEval.Spacing = shader->info.TessEval.Spacing;
+ gl_prog->info.tes.spacing = shader->info.TessEval.Spacing;
   }
 
   if (shader->info.TessEval.VertexOrder != 0) {
- if (linked_shader->info.TessEval.VertexOrder != 0 &&
- linked_shader->info.TessEval.VertexOrder !=
+ if (gl_prog->info.tes.vertex_order != 0 &&
+ gl_prog->info.tes.vertex_order !=
  shader->info.TessEval.VertexOrder) {
 linker_error(prog, "tessellation evaluation shader defined with "
  "conflicting ordering.\n");
 return;
  }
- linked_shader->info.TessEval.VertexOrder =
-shader->info.TessEval.VertexOrder;
+ gl_prog->info.tes.vertex_order = shader->info.TessEval.VertexOrder;
   }
 
   if (shader->info.TessEval.PointMode != -1) {
- if (linked_shader->info.TessEval.PointMode != -1 &&
- linked_shader->info.TessEval.PointMode !=
- shader->info.TessEval.PointMode) {
+ if (point_mode != -1 &&
+ point_mode != shader->info.TessEval.PointMode) {
 linker_error(prog, "tessellation evaluation shader defined with "
  "conflicting point modes.\n");
 return;
  }
- linked_shader->info.TessEval.PointMode =
-shader->info.TessEval.PointMode;
+ point_mode = shader->info.TessEval.PointMode;
   }
 
}
@@ -1793,21 +1791,23 @@ link_tes_in_layout_qualifiers(struct gl_shader_program 
*prog,
 * since we already know we're in the right type of shader program
 * for doing it.
 */
-   if (linked_shader->info.TessEval.PrimitiveMode == PRIM_UNKNOWN) {
+   if (gl_prog->info.tes.primitive_mode == PRIM_UNKNOWN) {
   linker_error(prog,
"tessellation evaluation shader didn't declare input "
"primitive modes.\n");
   return;
}
 
-   if (linked_shader->info.TessEval.Spacing == 0)
-  linked_shader->info.TessEval.Spacing = GL_EQUAL;
+   if (gl_prog->info.tes.spacing == 0)
+  gl_prog->info.tes.spacing = GL_EQ

[Mesa-dev] [PATCH 04/14] mesa/glsl: move ARB_fragment_coord_conventions_enable field

[Timothy Arceri]

This is only used by gl_shader not gl_linked_shader so move it
there.
---
 src/compiler/glsl/glsl_parser_extras.cpp | 2 +-
 src/compiler/glsl/linker.cpp | 2 +-
 src/mesa/main/mtypes.h   | 3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 0efcfe9..ad13436 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1809,7 +1809,7 @@ set_shader_inout_layout(struct gl_shader *shader,
   shader->info.uses_gl_fragcoord = state->fs_uses_gl_fragcoord;
   shader->info.pixel_center_integer = state->fs_pixel_center_integer;
   shader->info.origin_upper_left = state->fs_origin_upper_left;
-  shader->info.ARB_fragment_coord_conventions_enable =
+  shader->ARB_fragment_coord_conventions_enable =
  state->ARB_fragment_coord_conventions_enable;
   shader->EarlyFragmentTests = state->fs_early_fragment_tests;
   shader->BlendSupport = state->fs_blend_support;
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index a98ffc0..fe90afc 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -4787,7 +4787,7 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
  goto done;
   }
 
-  if (prog->Shaders[i]->info.ARB_fragment_coord_conventions_enable) {
+  if (prog->Shaders[i]->ARB_fragment_coord_conventions_enable) {
  prog->ARB_fragment_coord_conventions_enable = true;
   }
 
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index a149d0a..c4fdbf3 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2236,7 +2236,6 @@ struct gl_shader_info
 {
bool uses_gl_fragcoord;
bool redeclares_gl_fragcoord;
-   bool ARB_fragment_coord_conventions_enable;
 
/**
 * Fragment shader state from GLSL 1.50 layout qualifiers.
@@ -2420,6 +2419,8 @@ struct gl_shader
 */
bool EarlyFragmentTests;
 
+   bool ARB_fragment_coord_conventions_enable;
+
struct gl_shader_info info;
 };
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 09/14] glsl: exit loop early if we find xfb layout qualifers

[Timothy Arceri]

---
 src/compiler/glsl/link_varyings.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index da51fd8..398e1da 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -120,6 +120,7 @@ process_xfb_layout_qualifiers(void *mem_ctx, const 
gl_linked_shader *sh,
for (unsigned j = 0; j < MAX_FEEDBACK_BUFFERS; j++) {
   if (sh->info.TransformFeedback.BufferStride[j]) {
  has_xfb_qualifiers = true;
+ break;
   }
}
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 10/14] mesa/glsl: move TransformFeedbackBufferStride to gl_shader

[Timothy Arceri]

Here we remove the single use of this field in gl_linked_shader
which allows us to move the field out of gl_shader_info

While we are at it we rewrite link_xfb_stride_layout_qualifiers()
to be more clear.
---
 src/compiler/glsl/glsl_parser_extras.cpp |  2 +-
 src/compiler/glsl/link_varyings.cpp  |  3 +-
 src/compiler/glsl/link_varyings.h|  1 +
 src/compiler/glsl/linker.cpp | 73 +++-
 src/mesa/main/mtypes.h   |  8 ++--
 5 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 2711b71..54fe24a 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1700,7 +1700,7 @@ set_shader_inout_layout(struct gl_shader *shader,
  if (state->out_qualifier->out_xfb_stride[i]->
 process_qualifier_constant(state, "xfb_stride", &xfb_stride,
 true)) {
-shader->info.TransformFeedback.BufferStride[i] = xfb_stride;
+shader->TransformFeedbackBufferStride[i] = xfb_stride;
  }
   }
}
diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 398e1da..f032f2a 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -108,6 +108,7 @@ create_xfb_varying_names(void *mem_ctx, const glsl_type *t, 
char **name,
 
 bool
 process_xfb_layout_qualifiers(void *mem_ctx, const gl_linked_shader *sh,
+  struct gl_shader_program *prog,
   unsigned *num_tfeedback_decls,
   char ***varying_names)
 {
@@ -118,7 +119,7 @@ process_xfb_layout_qualifiers(void *mem_ctx, const 
gl_linked_shader *sh,
 * xfb_stride to interface block members so this will catch that case also.
 */
for (unsigned j = 0; j < MAX_FEEDBACK_BUFFERS; j++) {
-  if (sh->info.TransformFeedback.BufferStride[j]) {
+  if (prog->TransformFeedback.BufferStride[j]) {
  has_xfb_qualifiers = true;
  break;
   }
diff --git a/src/compiler/glsl/link_varyings.h 
b/src/compiler/glsl/link_varyings.h
index afce56e..2abe3ca 100644
--- a/src/compiler/glsl/link_varyings.h
+++ b/src/compiler/glsl/link_varyings.h
@@ -301,6 +301,7 @@ parse_tfeedback_decls(struct gl_context *ctx, struct 
gl_shader_program *prog,
 
 bool
 process_xfb_layout_qualifiers(void *mem_ctx, const gl_linked_shader *sh,
+  struct gl_shader_program *prog,
   unsigned *num_tfeedback_decls,
   char ***varying_names);
 
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 8bbbf7a..d0029fb 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1587,6 +1587,29 @@ private:
hash_table *unnamed_interfaces;
 };
 
+static bool
+validate_xfb_buffer_stride(struct gl_context *ctx, unsigned idx,
+   struct gl_shader_program *prog)
+{
+   /* We will validate doubles at a later stage */
+   if (prog->TransformFeedback.BufferStride[idx] % 4) {
+  linker_error(prog, "invalid qualifier xfb_stride=%d must be a "
+   "multiple of 4 or if its applied to a type that is "
+   "or contains a double a multiple of 8.",
+   prog->TransformFeedback.BufferStride[idx]);
+  return false;
+   }
+
+   if (prog->TransformFeedback.BufferStride[idx] / 4 >
+   ctx->Const.MaxTransformFeedbackInterleavedComponents) {
+  linker_error(prog, "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
+   "limit has been exceeded.");
+  return false;
+   }
+
+   return true;
+}
+
 /**
  * Check for conflicting xfb_stride default qualifiers and store buffer stride
  * for later use.
@@ -1599,54 +1622,28 @@ link_xfb_stride_layout_qualifiers(struct gl_context 
*ctx,
   unsigned num_shaders)
 {
for (unsigned i = 0; i < MAX_FEEDBACK_BUFFERS; i++) {
-  linked_shader->info.TransformFeedback.BufferStride[i] = 0;
+  prog->TransformFeedback.BufferStride[i] = 0;
}
 
for (unsigned i = 0; i < num_shaders; i++) {
   struct gl_shader *shader = shader_list[i];
 
   for (unsigned j = 0; j < MAX_FEEDBACK_BUFFERS; j++) {
- if (shader->info.TransformFeedback.BufferStride[j]) {
-if (linked_shader->info.TransformFeedback.BufferStride[j] != 0 &&
-shader->info.TransformFeedback.BufferStride[j] != 0 &&
-linked_shader->info.TransformFeedback.BufferStride[j] !=
-   shader->info.TransformFeedback.BufferStride[j]) {
+ if (shader->TransformFeedbackBufferStride[j]) {
+if (prog->TransformFeedback.BufferStride[j] == 0) {
+   prog->TransformFeedback.BufferStride[j] =
+  shader->TransformFeedbackBufferStride[j];
+

[Mesa-dev] [PATCH 06/14] mesa/glsl: move uses_gl_fragcoord to gl_shader

[Timothy Arceri]

This is only used by gl_linked_shader as a temp during linking
so use a temp there instead.
---
 src/compiler/glsl/glsl_parser_extras.cpp |  2 +-
 src/compiler/glsl/linker.cpp | 12 +---
 src/mesa/main/mtypes.h   |  3 +--
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 7e62eaa..7d100bb 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1805,7 +1805,7 @@ set_shader_inout_layout(struct gl_shader *shader,
 
case MESA_SHADER_FRAGMENT:
   shader->redeclares_gl_fragcoord = state->fs_redeclares_gl_fragcoord;
-  shader->info.uses_gl_fragcoord = state->fs_uses_gl_fragcoord;
+  shader->uses_gl_fragcoord = state->fs_uses_gl_fragcoord;
   shader->info.pixel_center_integer = state->fs_pixel_center_integer;
   shader->info.origin_upper_left = state->fs_origin_upper_left;
   shader->ARB_fragment_coord_conventions_enable =
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index b31c055..5201005 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1826,7 +1826,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
 unsigned num_shaders)
 {
bool redeclares_gl_fragcoord = false;
-   linked_shader->info.uses_gl_fragcoord = false;
+   bool uses_gl_fragcoord = false;
linked_shader->info.origin_upper_left = false;
linked_shader->info.pixel_center_integer = false;
 
@@ -1844,9 +1844,9 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
*that have a static use gl_FragCoord."
*/
   if ((redeclares_gl_fragcoord && !shader->redeclares_gl_fragcoord &&
-   shader->info.uses_gl_fragcoord)
+   shader->uses_gl_fragcoord)
   || (shader->redeclares_gl_fragcoord && !redeclares_gl_fragcoord &&
-  linked_shader->info.uses_gl_fragcoord)) {
+  uses_gl_fragcoord)) {
  linker_error(prog, "fragment shader defined with conflicting "
  "layout qualifiers for gl_FragCoord\n");
   }
@@ -1870,11 +1870,9 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program 
*prog,
* are multiple redeclarations, all the fields except uses_gl_fragcoord
* are already known to be the same.
*/
-  if (shader->redeclares_gl_fragcoord || shader->info.uses_gl_fragcoord) {
+  if (shader->redeclares_gl_fragcoord || shader->uses_gl_fragcoord) {
  redeclares_gl_fragcoord = shader->redeclares_gl_fragcoord;
- linked_shader->info.uses_gl_fragcoord =
-linked_shader->info.uses_gl_fragcoord ||
-shader->info.uses_gl_fragcoord;
+ uses_gl_fragcoord |= shader->uses_gl_fragcoord;
  linked_shader->info.origin_upper_left =
 shader->info.origin_upper_left;
  linked_shader->info.pixel_center_integer =
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index b8d9fcf..3882571 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2234,8 +2234,6 @@ struct gl_subroutine_function
  */
 struct gl_shader_info
 {
-   bool uses_gl_fragcoord;
-
/**
 * Fragment shader state from GLSL 1.50 layout qualifiers.
 */
@@ -2421,6 +2419,7 @@ struct gl_shader
bool ARB_fragment_coord_conventions_enable;
 
bool redeclares_gl_fragcoord;
+   bool uses_gl_fragcoord;
 
struct gl_shader_info info;
 };
-- 
2.7.4

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

[Mesa-dev] [PATCH 14/14] mesa: remove unused gl_shader_info field from gl_linked_shader

[Timothy Arceri]

---
 src/mesa/main/mtypes.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index b5c4f5d..fbf58b9 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2348,8 +2348,6 @@ struct gl_linked_shader
struct exec_list *packed_varyings;
struct exec_list *fragdata_arrays;
struct glsl_symbol_table *symbols;
-
-   struct gl_shader_info info;
 };
 
 static inline GLbitfield gl_external_samplers(struct gl_program *prog)
-- 
2.7.4

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