Re: [Mesa-dev] [PATCH] intel: decoder: handle 0 sized structs

2018-08-25 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Sat, Aug 25, 2018 at 12:23 PM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> Gen7.5 has a BLEND_STATE of size 0 which includes a variable length
> group. We did not deal with that very well, leading to an endless
> loop.
>
> Signed-off-by: Lionel Landwerlin 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
> ---
>  src/intel/common/gen_decoder.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/common/gen_decoder.c
> b/src/intel/common/gen_decoder.c
> index 9e46f271633..ec22b545492 100644
> --- a/src/intel/common/gen_decoder.c
> +++ b/src/intel/common/gen_decoder.c
> @@ -997,7 +997,7 @@ gen_field_iterator_init(struct gen_field_iterator
> *iter,
> iter->p_bit = p_bit;
>
> int length = gen_group_get_length(iter->group, iter->p);
> -   iter->p_end = length > 0 ? [length] : NULL;
> +   iter->p_end = length >= 0 ? [length] : NULL;
> iter->print_colors = print_colors;
>  }
>
> @@ -1012,10 +1012,14 @@ gen_field_iterator_next(struct gen_field_iterator
> *iter)
>   iter_start_field(iter, iter->group->next->fields);
>
>bool result = iter_decode_field(iter);
> -  if (iter->p_end)
> - assert(result);
> +  if (!result && iter->p_end) {
> + /* We're dealing with a non empty struct of length=0
> (BLEND_STATE on
> +  * Gen 7.5)
> +  */
> + assert(iter->group->dw_length == 0);
> +  }
>
> -  return true;
> +  return result;
> }
>
> if (!iter_advance_field(iter))
> --
> 2.18.0
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles

2018-08-25 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=77449
Bug 77449 depends on bug 104809, which changed state.

Bug 104809 Summary: anv: DOOM 2016 and Wolfenstein II:The New Colossus crash 
due to not having depthBoundsTest
https://bugs.freedesktop.org/show_bug.cgi?id=104809

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] anv: Fill holes in the VF VUE to zero

2018-08-25 Thread Jason Ekstrand
On Sat, Aug 25, 2018 at 5:56 PM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> I don't know how you find this stuff...
>

Hours and hours of painfully reading through aub dumps looking for stuff
that's out of place and many experiments.  This hang has probably taken me
near enough to a week of debugging time all told.


> Reviewed-by: Lionel Landwerlin 
>

Thanks!

--Jason


> On 25/08/2018 23:17, Jason Ekstrand wrote:
> > Cc: mesa-sta...@lists.freedesktop.org
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809
> > ---
> >   src/intel/vulkan/genX_pipeline.c | 29 -
> >   1 file changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/intel/vulkan/genX_pipeline.c
> b/src/intel/vulkan/genX_pipeline.c
> > index 022f324606e..b531205508c 100644
> > --- a/src/intel/vulkan/genX_pipeline.c
> > +++ b/src/intel/vulkan/genX_pipeline.c
> > @@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline,
> >  GENX(3DSTATE_VERTEX_ELEMENTS));
> >  if (!p)
> > return;
> > -   memset(p + 1, 0, (num_dwords - 1) * 4);
> > +
> > +   for (uint32_t i = 0; i < total_elems; i++) {
> > +  /* The SKL docs for VERTEX_ELEMENT_STATE say:
> > +   *
> > +   *"All elements must be valid from Element[0] to the last
> valid
> > +   *element. (I.e. if Element[2] is valid then Element[1] and
> > +   *Element[0] must also be valid)."
> > +   *
> > +   * The SKL docs for 3D_Vertex_Component_Control say:
> > +   *
> > +   *"Don't store this component. (Not valid for Component 0,
> but can
> > +   *be used for Component 1-3)."
> > +   *
> > +   * So we can't just leave a vertex element blank and hope for the
> best.
> > +   * We have to tell the VF hardware to put something in it; so we
> just
> > +   * store a bunch of zero.
> > +   *
> > +   * TODO: Compact vertex elements so we never end up with holes.
> > +   */
> > +  struct GENX(VERTEX_ELEMENT_STATE) element = {
> > + .Valid = true,
> > + .Component0Control = VFCOMP_STORE_0,
> > + .Component1Control = VFCOMP_STORE_0,
> > + .Component2Control = VFCOMP_STORE_0,
> > + .Component3Control = VFCOMP_STORE_0,
> > +  };
> > +  GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], );
> > +   }
> >
> >  for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount;
> i++) {
> > const VkVertexInputAttributeDescription *desc =
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] anv: Fill holes in the VF VUE to zero

2018-08-25 Thread Lionel Landwerlin

I don't know how you find this stuff...

Reviewed-by: Lionel Landwerlin 

On 25/08/2018 23:17, Jason Ekstrand wrote:

Cc: mesa-sta...@lists.freedesktop.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809
---
  src/intel/vulkan/genX_pipeline.c | 29 -
  1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c
index 022f324606e..b531205508c 100644
--- a/src/intel/vulkan/genX_pipeline.c
+++ b/src/intel/vulkan/genX_pipeline.c
@@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline,
 GENX(3DSTATE_VERTEX_ELEMENTS));
 if (!p)
return;
-   memset(p + 1, 0, (num_dwords - 1) * 4);
+
+   for (uint32_t i = 0; i < total_elems; i++) {
+  /* The SKL docs for VERTEX_ELEMENT_STATE say:
+   *
+   *"All elements must be valid from Element[0] to the last valid
+   *element. (I.e. if Element[2] is valid then Element[1] and
+   *Element[0] must also be valid)."
+   *
+   * The SKL docs for 3D_Vertex_Component_Control say:
+   *
+   *"Don't store this component. (Not valid for Component 0, but can
+   *be used for Component 1-3)."
+   *
+   * So we can't just leave a vertex element blank and hope for the best.
+   * We have to tell the VF hardware to put something in it; so we just
+   * store a bunch of zero.
+   *
+   * TODO: Compact vertex elements so we never end up with holes.
+   */
+  struct GENX(VERTEX_ELEMENT_STATE) element = {
+ .Valid = true,
+ .Component0Control = VFCOMP_STORE_0,
+ .Component1Control = VFCOMP_STORE_0,
+ .Component2Control = VFCOMP_STORE_0,
+ .Component3Control = VFCOMP_STORE_0,
+  };
+  GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], );
+   }
  
 for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount; i++) {

const VkVertexInputAttributeDescription *desc =



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


Re: [Mesa-dev] [PATCH] intel/tools: Add 0x in front of a couple of hex values

2018-08-25 Thread Lionel Landwerlin

Reviewed-by: Lionel Landwerlin 

On 25/08/2018 23:34, Jason Ekstrand wrote:

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

diff --git a/src/intel/common/gen_batch_decoder.c 
b/src/intel/common/gen_batch_decoder.c
index 6884a999401..1a5c8c37968 100644
--- a/src/intel/common/gen_batch_decoder.c
+++ b/src/intel/common/gen_batch_decoder.c
@@ -248,11 +248,11 @@ dump_binding_table(struct gen_batch_decode_ctx *ctx, 
uint32_t offset, int count)
  
if (pointers[i] % 32 != 0 ||

addr < bo.addr || addr + size >= bo.addr + bo.size) {
- fprintf(ctx->fp, "pointer %u: %08x \n", i, pointers[i]);
+ fprintf(ctx->fp, "pointer %u: 0x%08x \n", i, pointers[i]);
   continue;
}
  
-  fprintf(ctx->fp, "pointer %u: %08x\n", i, pointers[i]);

+  fprintf(ctx->fp, "pointer %u: 0x%08x\n", i, pointers[i]);
ctx_print_group(ctx, strct, addr, bo.map + (addr - bo.addr));
 }
  }



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


[Mesa-dev] [Bug 105371] r600_shader_from_tgsi - GPR limit exceeded - shader requires 360 registers

2018-08-25 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105371

--- Comment #16 from mirh  ---
(In reply to mirh from comment #5)
> (In reply to mirh from comment #1)
> > Can confirm it fixes shader 2 and 5 of GraphicsFuzz demo 
> > http://www.graphicsfuzz.com/benchmark/android-v1.html
> > 
> > Should I wait for this (or, I dunno, some day sw fp64) to land before
> > reporting of the others "gcm_sched_late_pass: unscheduled ops" errors?
> 
> Well, colour me shocked, but after building mesa-git with the last patch
> series all the tests now pass. 
> 
> Which is quite remarkable considering not even latest GCN closed drivers are
> compliant.

This with firefox though. 
Just noticed chromium 68 is still getting those errors for SETGT_DX10 and
MULADD_IEEE, and indeed fails six tests. 

Nosb fixes it.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] anv: Fill holes in the VF VUE to zero

2018-08-25 Thread Jason Ekstrand
Added to the commit message:

This fixes a GPU hang in DOOM 2016 running under wine.

On Sat, Aug 25, 2018 at 5:17 PM Jason Ekstrand  wrote:

> Cc: mesa-sta...@lists.freedesktop.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809
> ---
>  src/intel/vulkan/genX_pipeline.c | 29 -
>  1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/genX_pipeline.c
> b/src/intel/vulkan/genX_pipeline.c
> index 022f324606e..b531205508c 100644
> --- a/src/intel/vulkan/genX_pipeline.c
> +++ b/src/intel/vulkan/genX_pipeline.c
> @@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline,
> GENX(3DSTATE_VERTEX_ELEMENTS));
> if (!p)
>return;
> -   memset(p + 1, 0, (num_dwords - 1) * 4);
> +
> +   for (uint32_t i = 0; i < total_elems; i++) {
> +  /* The SKL docs for VERTEX_ELEMENT_STATE say:
> +   *
> +   *"All elements must be valid from Element[0] to the last valid
> +   *element. (I.e. if Element[2] is valid then Element[1] and
> +   *Element[0] must also be valid)."
> +   *
> +   * The SKL docs for 3D_Vertex_Component_Control say:
> +   *
> +   *"Don't store this component. (Not valid for Component 0, but
> can
> +   *be used for Component 1-3)."
> +   *
> +   * So we can't just leave a vertex element blank and hope for the
> best.
> +   * We have to tell the VF hardware to put something in it; so we
> just
> +   * store a bunch of zero.
> +   *
> +   * TODO: Compact vertex elements so we never end up with holes.
> +   */
> +  struct GENX(VERTEX_ELEMENT_STATE) element = {
> + .Valid = true,
> + .Component0Control = VFCOMP_STORE_0,
> + .Component1Control = VFCOMP_STORE_0,
> + .Component2Control = VFCOMP_STORE_0,
> + .Component3Control = VFCOMP_STORE_0,
> +  };
> +  GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], );
> +   }
>
> for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount; i++) {
>const VkVertexInputAttributeDescription *desc =
> --
> 2.17.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] intel/tools: Add 0x in front of a couple of hex values

2018-08-25 Thread Jason Ekstrand
---
 src/intel/common/gen_batch_decoder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/common/gen_batch_decoder.c 
b/src/intel/common/gen_batch_decoder.c
index 6884a999401..1a5c8c37968 100644
--- a/src/intel/common/gen_batch_decoder.c
+++ b/src/intel/common/gen_batch_decoder.c
@@ -248,11 +248,11 @@ dump_binding_table(struct gen_batch_decode_ctx *ctx, 
uint32_t offset, int count)
 
   if (pointers[i] % 32 != 0 ||
   addr < bo.addr || addr + size >= bo.addr + bo.size) {
- fprintf(ctx->fp, "pointer %u: %08x \n", i, pointers[i]);
+ fprintf(ctx->fp, "pointer %u: 0x%08x \n", i, pointers[i]);
  continue;
   }
 
-  fprintf(ctx->fp, "pointer %u: %08x\n", i, pointers[i]);
+  fprintf(ctx->fp, "pointer %u: 0x%08x\n", i, pointers[i]);
   ctx_print_group(ctx, strct, addr, bo.map + (addr - bo.addr));
}
 }
-- 
2.17.1

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


Re: [Mesa-dev] [PATCH 2/3] nir: Create sampler variables in prog_to_nir.

2018-08-25 Thread Kenneth Graunke
On Saturday, August 25, 2018 6:05:57 AM PDT Jason Ekstrand wrote:
> On Fri, Aug 24, 2018 at 8:24 PM Kenneth Graunke 
> wrote:
> 
> > This is needed for nir_gather_info to actually count the textures,
> > since it operates solely on variables.
> > ---
> >  src/mesa/program/prog_to_nir.c | 15 +--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/program/prog_to_nir.c
> > b/src/mesa/program/prog_to_nir.c
> > index 14e57b6c6a1..1f0607542e8 100644
> > --- a/src/mesa/program/prog_to_nir.c
> > +++ b/src/mesa/program/prog_to_nir.c
> > @@ -52,6 +52,7 @@ struct ptn_compile {
> > nir_variable *parameters;
> > nir_variable *input_vars[VARYING_SLOT_MAX];
> > nir_variable *output_vars[VARYING_SLOT_MAX];
> > +   nir_variable *sampler_vars[32]; /* matches number of bits in
> > TexSrcUnit */
> > nir_register **output_regs;
> > nir_register **temp_regs;
> >
> > @@ -484,9 +485,10 @@ ptn_kil(nir_builder *b, nir_ssa_def **src)
> >  }
> >
> >  static void
> > -ptn_tex(nir_builder *b, nir_alu_dest dest, nir_ssa_def **src,
> > +ptn_tex(struct ptn_compile *c, nir_alu_dest dest, nir_ssa_def **src,
> >  struct prog_instruction *prog_inst)
> >  {
> > +   nir_builder *b = >build;
> > nir_tex_instr *instr;
> > nir_texop op;
> > unsigned num_srcs;
> > @@ -568,6 +570,15 @@ ptn_tex(nir_builder *b, nir_alu_dest dest,
> > nir_ssa_def **src,
> >unreachable("can't reach");
> > }
> >
> > +   if (!c->sampler_vars[prog_inst->TexSrcUnit]) {
> > +  const struct glsl_type *type =
> > + glsl_sampler_type(instr->sampler_dim, false, false,
> > GLSL_TYPE_FLOAT);
> > +  nir_variable *var =
> > + nir_variable_create(b->shader, nir_var_uniform, type, "sampler");
> > +  var->data.binding = prog_inst->TexSrcUnit;
> > +  c->sampler_vars[prog_inst->TexSrcUnit] = var;
> >
> 
> Can samplers be indirected?  If so, we probably want an array of samplers
> instead 32 distinct samplers.

That would be a frill.  This is ARB_fragment_program.  You get an
integer constant for your texture unit number. :)

--Ken


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] anv: Fill holes in the VF VUE to zero

2018-08-25 Thread Jason Ekstrand
Cc: mesa-sta...@lists.freedesktop.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809
---
 src/intel/vulkan/genX_pipeline.c | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c
index 022f324606e..b531205508c 100644
--- a/src/intel/vulkan/genX_pipeline.c
+++ b/src/intel/vulkan/genX_pipeline.c
@@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline,
GENX(3DSTATE_VERTEX_ELEMENTS));
if (!p)
   return;
-   memset(p + 1, 0, (num_dwords - 1) * 4);
+
+   for (uint32_t i = 0; i < total_elems; i++) {
+  /* The SKL docs for VERTEX_ELEMENT_STATE say:
+   *
+   *"All elements must be valid from Element[0] to the last valid
+   *element. (I.e. if Element[2] is valid then Element[1] and
+   *Element[0] must also be valid)."
+   *
+   * The SKL docs for 3D_Vertex_Component_Control say:
+   *
+   *"Don't store this component. (Not valid for Component 0, but can
+   *be used for Component 1-3)."
+   *
+   * So we can't just leave a vertex element blank and hope for the best.
+   * We have to tell the VF hardware to put something in it; so we just
+   * store a bunch of zero.
+   *
+   * TODO: Compact vertex elements so we never end up with holes.
+   */
+  struct GENX(VERTEX_ELEMENT_STATE) element = {
+ .Valid = true,
+ .Component0Control = VFCOMP_STORE_0,
+ .Component1Control = VFCOMP_STORE_0,
+ .Component2Control = VFCOMP_STORE_0,
+ .Component3Control = VFCOMP_STORE_0,
+  };
+  GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], );
+   }
 
for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount; i++) {
   const VkVertexInputAttributeDescription *desc =
-- 
2.17.1

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


[Mesa-dev] [PATCH] intel: decoder: handle 0 sized structs

2018-08-25 Thread Lionel Landwerlin
Gen7.5 has a BLEND_STATE of size 0 which includes a variable length
group. We did not deal with that very well, leading to an endless
loop.

Signed-off-by: Lionel Landwerlin 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
---
 src/intel/common/gen_decoder.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index 9e46f271633..ec22b545492 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -997,7 +997,7 @@ gen_field_iterator_init(struct gen_field_iterator *iter,
iter->p_bit = p_bit;
 
int length = gen_group_get_length(iter->group, iter->p);
-   iter->p_end = length > 0 ? [length] : NULL;
+   iter->p_end = length >= 0 ? [length] : NULL;
iter->print_colors = print_colors;
 }
 
@@ -1012,10 +1012,14 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
  iter_start_field(iter, iter->group->next->fields);
 
   bool result = iter_decode_field(iter);
-  if (iter->p_end)
- assert(result);
+  if (!result && iter->p_end) {
+ /* We're dealing with a non empty struct of length=0 (BLEND_STATE on
+  * Gen 7.5)
+  */
+ assert(iter->group->dw_length == 0);
+  }
 
-  return true;
+  return result;
}
 
if (!iter_advance_field(iter))
-- 
2.18.0

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


Re: [Mesa-dev] [PATCH] intel: tools: Fix aubinator_error's fprintf call (format-security)

2018-08-25 Thread Kai Wasserbäch
Hey Lionel,
Lionel Landwerlin wrote on 8/25/18 5:50 PM:
> On 25/08/2018 11:00, Kai Wasserbäch wrote:
>> The recent commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be introduced
>> the new function aubinator_error() which is a trivial wrapper around
>> fprintf() to STDERR. The call to fprintf() however is passed the message
>> msg directly:
>>    fprintf(stderr, msg);
>>
>> This is a format-security violation and leads to an FTBFS with
>> -Werror=format-security (GCC 8):
>>    ../../../src/intel/tools/aubinator.c: In function 'aubinator_error':
>>    ../../../src/intel/tools/aubinator.c:74:4: error: format not a string
>> literal and no format arguments [-Werror=format-security]
>>    fprintf(stderr, msg);
>>    ^~~
>>
>> This patch fixes this trivially by introducing a catch-all "%s" format
>> argument.
>>
>> Fixes: 4616639b49b ("intel: tools: split aub parsing from aubinator")
>> Cc: Lionel Landwerlin 
>> Signed-off-by: Kai Wasserbäch 
> 
> 
> Reviewed-by: Lionel Landwerlin 

thank you for the review and pushing the patch for me!

Cheers,
Kai



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


Re: [Mesa-dev] [PATCH] intel: tools: Fix aubinator_error's fprintf call (format-security)

2018-08-25 Thread Lionel Landwerlin

On 25/08/2018 11:00, Kai Wasserbäch wrote:

The recent commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be introduced
the new function aubinator_error() which is a trivial wrapper around
fprintf() to STDERR. The call to fprintf() however is passed the message
msg directly:
   fprintf(stderr, msg);

This is a format-security violation and leads to an FTBFS with
-Werror=format-security (GCC 8):
   ../../../src/intel/tools/aubinator.c: In function 'aubinator_error':
   ../../../src/intel/tools/aubinator.c:74:4: error: format not a string 
literal and no format arguments [-Werror=format-security]
   fprintf(stderr, msg);
   ^~~

This patch fixes this trivially by introducing a catch-all "%s" format
argument.

Fixes: 4616639b49b ("intel: tools: split aub parsing from aubinator")
Cc: Lionel Landwerlin 
Signed-off-by: Kai Wasserbäch 



Reviewed-by: Lionel Landwerlin 



---

Hey,
just tried compiling the latest version of Mesa but due to commit
4616639b49b4bbc91e503c1c27632dccc1c2b5be by Lionel I got a FTBFS. Please
commit this patch for me, if you accept it. I do not have commit access
for Mesa.

The pull request for this would be:
  The following changes since commit 1281608849a058fcdbe2f64481a159d58af3d888:

gallium: Split out PIPE_CAP_TEXTURE_MIRROR_CLAMP_TO_EDGE. (2018-08-24 
17:25:36 -0700)

  are available in the Git repository at:

https://gitlab.freedesktop.org/curan/mesa.git aub-format-security

  for you to fetch changes up to 24957b0db3c12a097d70033ec8596da5bafedd55:

intel: tools: Fix aubinator_error's fprintf call (format-security) 
(2018-08-25 11:43:25 +0200)

Cheers,
Kai

  src/intel/tools/aubinator.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index 374ed46f86..c22d191f14 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -71,7 +71,7 @@ struct brw_instruction;
  static void
  aubinator_error(void *user_data, const void *aub_data, const char *msg)
  {
-   fprintf(stderr, msg);
+   fprintf(stderr, "%s", msg);
  }
  
  static void



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


Re: [Mesa-dev] [PATCH 6/9] nir: Make dead_write_vars pass global

2018-08-25 Thread Jason Ekstrand
On Sat, Aug 25, 2018 at 9:40 AM Jason Ekstrand  wrote:

> Sorry I haven't given you any in-line review yet.  I've been letting this
> pass simmer in my brain a bit and thinking about complexity and data
> structures.  Here's a few questions for which I do not expect you to have
> actual answers:
>
>  1) How many unique derefs should we expect to see in a shader?  Dozens?
> Hundreds?  Thousands?  My guess is that, after vars_to_ssa has picked off
> all the easy ones, it should be on the order of a couple hundred at most
> but I don't actually know what a reasonable upper limit to assume is.
>
>  2) Is every deref going to be eventually compared to every other deref or
> are we going to have separate little groups of derefs?
>
> I'm asking these questions because I have some concern about how expensive
> this pass (and copy prop) is going to be.  If you have a shader with a huge
> number of loops each of which has a very self-contained set of derefs, the
> current pass as-is probably has an efficiency that's around O(n) in the
> number of instructions.  However, in the case where most derefs are in play
> all the time, then I supsect this pass as-is looks more like O(n^3) in the
> number of derefs or worse.
>
> If the total number of unique derefs is reasonable AND the graph of deref
> comparisons isn't too sparse, then I wonder if it wouldn't be better to
> have some intermediate data structure like this:
>
> struct deref_compare_map {
>void *mem_ctx;
>
>struct hash_table *id_map;
>
>nir_deref_path *paths;
>BITSET_WORD **alias;
>BITSET_WORD **contains;
>BITSET_WORD **contained_in;
> };
>
> void deref_compare_map_init(struct deref_compare_map *map, void *mem_ctx)
> {
>memset(map, 0, sizeof(*map));
>map->mem_ctx = ralloc_context(mem_ctx);
>
>/* I'm pretty sure we have deref hashing and equality functions
> somewhere already, we just have to pull them out and use them.  It is,
> however, possible that they were in nir_instr_set.c and got deleted as part
> of the move to deref instructions. */
>map->id_map = _mesa_hash_table_create(map->mem_ctx, deref_hash,
> deref_equal);
> }
>
> void deref_compare_map_finish(struct deref_compare_map *map)
> {
>ralloc_free(map->mem_ctx);
> }
>
> uint32_t deref_compare_map_get_id(struct deref_compare_map *map,
> nir_deref_instr *deref)
> {
>struct hash_entry *entry = _mesa_hash_set_lookup(map->info_map, deref);
>if (entry)
>   return (uint32_t)(uintptr_t)entry->data;
>
>/* We can't be adding anything after we've baked it */
>assert(map->alias == NULL);
>
>uint32_t id = map->id_map->entries;
>_mesa_hash_set_add(map->id_map, deref, id);
>assert(map->id_map->entries == id + 1);
> }
>
> static BITSET_WORDS **
> alloc_set_of_sets(void *mem_ctx, unsigned entries)
> {
>BITSET_WORDS **sets = ralloc_array(mem_ctx, BITSET_WORD *, entries);
>BITSET_WORDS *data = rzalloc_array(mem_ctx, BITSET_WORD, entries *
> BITSET_WORDS(entries));
>for (unsigned i = 0; i < entries; i++)
>   sets[i] = data + i * BITSET_WORDS(entries);
>return sets;
> }
>
> void deref_compare_map_bake(struct deref_compare_map *map)
> {
>const unsigned num_derefs = map->id_map->entries;
>
>map->paths = ralloc_array(map->mem_ctx, nir_deref_path, num_derefs);
>struct hash_entry *entry;
>hash_table_foreach(map->id_map, entry) {
>   uint32_t id = (uintptr_t)entry->data;
>   nir_deref_path_init(>paths[id], (void *)entry->key,
> map->mem_ctx);
>}
>
>map->alias = alloc_set_of_sets(map->mem_ctx, map->id_map->entries);
>map->contains = alloc_set_of_sets(map->mem_ctx, map->id_map->entries);
>map->contained_in = alloc_set_of_sets(map->mem_ctx,
> map->id_map->entries);
>
>for (unsigned a = 0; a < num_derefs; a++) {
>   /* Handle comparing a with itself */
>   BITSET_SET(map->alias[a], a);
>   BITSET_SET(map->contains[a], a);
>   BITSET_SET(map->contained_in[a], a);
>
>   for (unsigned b = 0; b < a; b++) {
>  enum nir_deref_compare_result comp =
> nir_compare_deref_paths(>paths[a], >paths[b]);
>  if (comp & derefs_may_alias_bit) {
> BITSET_SET(map->alias[a], b);
> BITSET_SET(map->alias[b], a);
>  }
>
>  if (comp & derefs_a_contains_b_bit) {
> BITSET_SET(map->contains[a], b);
> BITSET_SET(map->contained_in[b], a);
>  }
>
>  if (comp & derefs_b_contains_a_bit) {
> BITSET_SET(map->contains[b], a);
> BITSET_SET(map->contained_in[a], b);
>  }
>   }
>}
> }
>
> The idea would be to make it a 5-step pass:
>
>  1) find all derefs
>  2) Bake the deref compare map
>  3) Local elimination and building kill etc. sets.
>  4) Data-flow
>  5) global elimination
>
> Doing that would give us some better run-time guarantees:
>
>  1) Set operations on bitsets are O(n) (where n is actually
> DIV_ROUND_UP(n, 32) thanks to bitsets)
>  2) We call 

Re: [Mesa-dev] [PATCH] st/mesa: Override blend factors involving alpha if it doesn't exist.

2018-08-25 Thread Ilia Mirkin
You have to make sure to flip blend->independent_blend_enable to 1 in
that case, and that will only work on some (well, most, but not all)
hardware. Pre-something Tesla-era and I assume r600-era gpu's must
have all rt's configured the same way.

I'm also not 100% sure that it's OK to access the draw buffers in
here, but Marek would know for sure -- he restructured the atoms logic
a while back, and I haven't re-learned the new scheme.

  -ilia

On Sat, Aug 25, 2018 at 3:48 AM, Kenneth Graunke  wrote:
> When faking an RGB format with an RGBA format, there may be a channel
> of data containing garbage.  st/mesa already overrides texture swizzles
> to replace the A channel with ONE.  This patch makes it override blend
> factors to achieve a similar effect.
>
> It appears that st_update_blend is already called when the framebuffer
> is changed, so it should be safe to look at the render target formats
> without adding additional state dependencies.
> ---
>  src/mesa/state_tracker/st_atom_blend.c | 35 ++
>  1 file changed, 35 insertions(+)
>
> diff --git a/src/mesa/state_tracker/st_atom_blend.c 
> b/src/mesa/state_tracker/st_atom_blend.c
> index 804de2f154f..3e6e4411107 100644
> --- a/src/mesa/state_tracker/st_atom_blend.c
> +++ b/src/mesa/state_tracker/st_atom_blend.c
> @@ -41,6 +41,7 @@
>
>  #include "framebuffer.h"
>  #include "main/blend.h"
> +#include "main/glformats.h"
>  #include "main/macros.h"
>
>  /**
> @@ -142,6 +143,28 @@ blend_per_rt(const struct gl_context *ctx, unsigned 
> num_cb)
> return GL_FALSE;
>  }
>
> +/**
> + * Modify blend function to force destination alpha to 1.0
> + *
> + * If \c function specifies a blend function that uses destination alpha,
> + * replace it with a function that hard-wires destination alpha to 1.0.  This
> + * is used when rendering to xRGB targets.
> + */
> +static enum pipe_blendfactor
> +fix_xrgb_alpha(enum pipe_blendfactor factor)
> +{
> +   switch (factor) {
> +   case PIPE_BLENDFACTOR_DST_ALPHA:
> +  return PIPE_BLENDFACTOR_ONE;
> +
> +   case PIPE_BLENDFACTOR_INV_DST_ALPHA:
> +   case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
> +  return PIPE_BLENDFACTOR_ZERO;
> +   default:
> +  return factor;
> +   }
> +}
> +
>  void
>  st_update_blend( struct st_context *st )
>  {
> @@ -210,6 +233,18 @@ st_update_blend( struct st_context *st )
>  blend->rt[i].alpha_dst_factor =
> translate_blend(ctx->Color.Blend[j].DstA);
>   }
> +
> + const struct gl_renderbuffer *rb =
> +ctx->DrawBuffer->_ColorDrawBuffers[i];
> +
> + if (rb && !_mesa_base_format_has_channel(rb->_BaseFormat,
> +  GL_TEXTURE_ALPHA_TYPE)) {
> +struct pipe_rt_blend_state *rt = >rt[i];
> +rt->rgb_src_factor = fix_xrgb_alpha(rt->rgb_src_factor);
> +rt->rgb_dst_factor = fix_xrgb_alpha(rt->rgb_dst_factor);
> +rt->alpha_src_factor = fix_xrgb_alpha(rt->alpha_src_factor);
> +rt->alpha_dst_factor = fix_xrgb_alpha(rt->alpha_dst_factor);
> + }
>}
> }
> else {
> --
> 2.18.0
>
> ___
> 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 6/9] nir: Make dead_write_vars pass global

2018-08-25 Thread Jason Ekstrand
Sorry I haven't given you any in-line review yet.  I've been letting this
pass simmer in my brain a bit and thinking about complexity and data
structures.  Here's a few questions for which I do not expect you to have
actual answers:

 1) How many unique derefs should we expect to see in a shader?  Dozens?
Hundreds?  Thousands?  My guess is that, after vars_to_ssa has picked off
all the easy ones, it should be on the order of a couple hundred at most
but I don't actually know what a reasonable upper limit to assume is.

 2) Is every deref going to be eventually compared to every other deref or
are we going to have separate little groups of derefs?

I'm asking these questions because I have some concern about how expensive
this pass (and copy prop) is going to be.  If you have a shader with a huge
number of loops each of which has a very self-contained set of derefs, the
current pass as-is probably has an efficiency that's around O(n) in the
number of instructions.  However, in the case where most derefs are in play
all the time, then I supsect this pass as-is looks more like O(n^3) in the
number of derefs or worse.

If the total number of unique derefs is reasonable AND the graph of deref
comparisons isn't too sparse, then I wonder if it wouldn't be better to
have some intermediate data structure like this:

struct deref_compare_map {
   void *mem_ctx;

   struct hash_table *id_map;

   nir_deref_path *paths;
   BITSET_WORD **alias;
   BITSET_WORD **contains;
   BITSET_WORD **contained_in;
};

void deref_compare_map_init(struct deref_compare_map *map, void *mem_ctx)
{
   memset(map, 0, sizeof(*map));
   map->mem_ctx = ralloc_context(mem_ctx);

   /* I'm pretty sure we have deref hashing and equality functions
somewhere already, we just have to pull them out and use them.  It is,
however, possible that they were in nir_instr_set.c and got deleted as part
of the move to deref instructions. */
   map->id_map = _mesa_hash_table_create(map->mem_ctx, deref_hash,
deref_equal);
}

void deref_compare_map_finish(struct deref_compare_map *map)
{
   ralloc_free(map->mem_ctx);
}

uint32_t deref_compare_map_get_id(struct deref_compare_map *map,
nir_deref_instr *deref)
{
   struct hash_entry *entry = _mesa_hash_set_lookup(map->info_map, deref);
   if (entry)
  return (uint32_t)(uintptr_t)entry->data;

   /* We can't be adding anything after we've baked it */
   assert(map->alias == NULL);

   uint32_t id = map->id_map->entries;
   _mesa_hash_set_add(map->id_map, deref, id);
   assert(map->id_map->entries == id + 1);
}

static BITSET_WORDS **
alloc_set_of_sets(void *mem_ctx, unsigned entries)
{
   BITSET_WORDS **sets = ralloc_array(mem_ctx, BITSET_WORD *, entries);
   BITSET_WORDS *data = rzalloc_array(mem_ctx, BITSET_WORD, entries *
BITSET_WORDS(entries));
   for (unsigned i = 0; i < entries; i++)
  sets[i] = data + i * BITSET_WORDS(entries);
   return sets;
}

void deref_compare_map_bake(struct deref_compare_map *map)
{
   const unsigned num_derefs = map->id_map->entries;

   map->paths = ralloc_array(map->mem_ctx, nir_deref_path, num_derefs);
   struct hash_entry *entry;
   hash_table_foreach(map->id_map, entry) {
  uint32_t id = (uintptr_t)entry->data;
  nir_deref_path_init(>paths[id], (void *)entry->key,
map->mem_ctx);
   }

   map->alias = alloc_set_of_sets(map->mem_ctx, map->id_map->entries);
   map->contains = alloc_set_of_sets(map->mem_ctx, map->id_map->entries);
   map->contained_in = alloc_set_of_sets(map->mem_ctx,
map->id_map->entries);

   for (unsigned a = 0; a < num_derefs; a++) {
  /* Handle comparing a with itself */
  BITSET_SET(map->alias[a], a);
  BITSET_SET(map->contains[a], a);
  BITSET_SET(map->contained_in[a], a);

  for (unsigned b = 0; b < a; b++) {
 enum nir_deref_compare_result comp =
nir_compare_deref_paths(>paths[a], >paths[b]);
 if (comp & derefs_may_alias_bit) {
BITSET_SET(map->alias[a], b);
BITSET_SET(map->alias[b], a);
 }

 if (comp & derefs_a_contains_b_bit) {
BITSET_SET(map->contains[a], b);
BITSET_SET(map->contained_in[b], a);
 }

 if (comp & derefs_b_contains_a_bit) {
BITSET_SET(map->contains[b], a);
BITSET_SET(map->contained_in[a], b);
 }
  }
   }
}

The idea would be to make it a 5-step pass:

 1) find all derefs
 2) Bake the deref compare map
 3) Local elimination and building kill etc. sets.
 4) Data-flow
 5) global elimination

Doing that would give us some better run-time guarantees:

 1) Set operations on bitsets are O(n) (where n is actually DIV_ROUND_UP(n,
32) thanks to bitsets)
 2) We call nir_deref_path_init exactly num_derefs times for the entire pass
 3) We call nir_compare_deref_paths exactly (num_derefs * (num_derefs - 1))
/ 2 times for the entire pass.

As it is, deref_map_add_map is O(n^2) comparisons and we do that operation
inside our fixed-point loop which has an unknown number of 

Re: [Mesa-dev] [PATCH 2/9] nir: Export deref comparison functions

2018-08-25 Thread Jason Ekstrand
On Wed, Aug 15, 2018 at 4:57 PM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:

> Reviewed-by: Timothy Arceri 
> ---
>  src/compiler/nir/nir_deref.c  | 109 
>  src/compiler/nir/nir_deref.h  |  10 ++
>  src/compiler/nir/nir_opt_copy_prop_vars.c | 145 ++
>  3 files changed, 132 insertions(+), 132 deletions(-)
>
> diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c
> index c03acf83597..d013b423a8b 100644
> --- a/src/compiler/nir/nir_deref.c
> +++ b/src/compiler/nir/nir_deref.c
> @@ -270,3 +270,112 @@ nir_fixup_deref_modes(nir_shader *shader)
>}
> }
>  }
> +
> +/** Returns true if the storage referrenced to by deref completely
> contains
> + * the storage referenced by sub.
>

Please fix this comment while we're here.  It's so out of date as to be
laughable.


> + */
> +nir_deref_compare_result
> +nir_compare_deref_paths(nir_deref_path *a_path,
> +nir_deref_path *b_path)
> +{
> +   if (a_path->path[0]->var != b_path->path[0]->var)
> +  return 0;
> +
> +   /* Start off assuming they fully compare.  We ignore equality for
> now.  In
> +* the end, we'll determine that by containment.
> +*/
> +   nir_deref_compare_result result = nir_derefs_may_alias_bit |
> + nir_derefs_a_contains_b_bit |
> + nir_derefs_b_contains_a_bit;
> +
> +   nir_deref_instr **a_p = _path->path[1];
> +   nir_deref_instr **b_p = _path->path[1];
> +   while (*a_p != NULL && *b_p != NULL) {
> +  nir_deref_instr *a_tail = *(a_p++);
> +  nir_deref_instr *b_tail = *(b_p++);
> +
> +  switch (a_tail->deref_type) {
> +  case nir_deref_type_array:
> +  case nir_deref_type_array_wildcard: {
> + assert(b_tail->deref_type == nir_deref_type_array ||
> +b_tail->deref_type == nir_deref_type_array_wildcard);
> +
> + if (a_tail->deref_type == nir_deref_type_array_wildcard) {
> +if (b_tail->deref_type != nir_deref_type_array_wildcard)
> +   result &= ~nir_derefs_b_contains_a_bit;
> + } else if (b_tail->deref_type == nir_deref_type_array_wildcard) {
> +if (a_tail->deref_type != nir_deref_type_array_wildcard)
> +   result &= ~nir_derefs_a_contains_b_bit;
> + } else {
> +assert(a_tail->deref_type == nir_deref_type_array &&
> +   b_tail->deref_type == nir_deref_type_array);
> +assert(a_tail->arr.index.is_ssa && b_tail->arr.index.is_ssa);
> +
> +nir_const_value *a_index_const =
> +   nir_src_as_const_value(a_tail->arr.index);
> +nir_const_value *b_index_const =
> +   nir_src_as_const_value(b_tail->arr.index);
> +if (a_index_const && b_index_const) {
> +   /* If they're both direct and have different offsets, they
> +* don't even alias much less anything else.
> +*/
> +   if (a_index_const->u32[0] != b_index_const->u32[0])
> +  return 0;
> +} else if (a_tail->arr.index.ssa == b_tail->arr.index.ssa) {
> +   /* They're the same indirect, continue on */
> +} else {
> +   /* They're not the same index so we can't prove anything
> about
> +* containment.
> +*/
> +   result &= ~(nir_derefs_a_contains_b_bit |
> nir_derefs_b_contains_a_bit);
> +}
> + }
> + break;
> +  }
> +
> +  case nir_deref_type_struct: {
> + /* If they're different struct members, they don't even alias */
> + if (a_tail->strct.index != b_tail->strct.index)
> +return 0;
> + break;
> +  }
> +
> +  default:
> + unreachable("Invalid deref type");
> +  }
> +   }
> +
> +   /* If a is longer than b, then it can't contain b */
> +   if (*a_p != NULL)
> +  result &= ~nir_derefs_a_contains_b_bit;
> +   if (*b_p != NULL)
> +  result &= ~nir_derefs_b_contains_a_bit;
> +
> +   /* If a contains b and b contains a they must be equal. */
> +   if ((result & nir_derefs_a_contains_b_bit) && (result &
> nir_derefs_b_contains_a_bit))
> +  result |= nir_derefs_equal_bit;
> +
> +   return result;
> +}
> +
> +nir_deref_compare_result
> +nir_compare_derefs(nir_deref_instr *a, nir_deref_instr *b)
> +{
> +   if (a == b) {
> +  return nir_derefs_equal_bit | nir_derefs_may_alias_bit |
> + nir_derefs_a_contains_b_bit | nir_derefs_b_contains_a_bit;
> +   }
> +
> +   nir_deref_path a_path, b_path;
> +   nir_deref_path_init(_path, a, NULL);
> +   nir_deref_path_init(_path, b, NULL);
> +   assert(a_path.path[0]->deref_type == nir_deref_type_var);
> +   assert(b_path.path[0]->deref_type == nir_deref_type_var);
> +
> +   nir_deref_compare_result result = nir_compare_deref_paths(_path,
> _path);
> +
> +   

Re: [Mesa-dev] [PATCH 2/3] nir: Create sampler variables in prog_to_nir.

2018-08-25 Thread Jason Ekstrand
On Fri, Aug 24, 2018 at 8:24 PM Kenneth Graunke 
wrote:

> This is needed for nir_gather_info to actually count the textures,
> since it operates solely on variables.
> ---
>  src/mesa/program/prog_to_nir.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/program/prog_to_nir.c
> b/src/mesa/program/prog_to_nir.c
> index 14e57b6c6a1..1f0607542e8 100644
> --- a/src/mesa/program/prog_to_nir.c
> +++ b/src/mesa/program/prog_to_nir.c
> @@ -52,6 +52,7 @@ struct ptn_compile {
> nir_variable *parameters;
> nir_variable *input_vars[VARYING_SLOT_MAX];
> nir_variable *output_vars[VARYING_SLOT_MAX];
> +   nir_variable *sampler_vars[32]; /* matches number of bits in
> TexSrcUnit */
> nir_register **output_regs;
> nir_register **temp_regs;
>
> @@ -484,9 +485,10 @@ ptn_kil(nir_builder *b, nir_ssa_def **src)
>  }
>
>  static void
> -ptn_tex(nir_builder *b, nir_alu_dest dest, nir_ssa_def **src,
> +ptn_tex(struct ptn_compile *c, nir_alu_dest dest, nir_ssa_def **src,
>  struct prog_instruction *prog_inst)
>  {
> +   nir_builder *b = >build;
> nir_tex_instr *instr;
> nir_texop op;
> unsigned num_srcs;
> @@ -568,6 +570,15 @@ ptn_tex(nir_builder *b, nir_alu_dest dest,
> nir_ssa_def **src,
>unreachable("can't reach");
> }
>
> +   if (!c->sampler_vars[prog_inst->TexSrcUnit]) {
> +  const struct glsl_type *type =
> + glsl_sampler_type(instr->sampler_dim, false, false,
> GLSL_TYPE_FLOAT);
> +  nir_variable *var =
> + nir_variable_create(b->shader, nir_var_uniform, type, "sampler");
> +  var->data.binding = prog_inst->TexSrcUnit;
> +  c->sampler_vars[prog_inst->TexSrcUnit] = var;
>

Can samplers be indirected?  If so, we probably want an array of samplers
instead 32 distinct samplers.


> +   }
> +
> unsigned src_number = 0;
>
> instr->src[src_number].src =
> @@ -784,7 +795,7 @@ ptn_emit_instruction(struct ptn_compile *c, struct
> prog_instruction *prog_inst)
> case OPCODE_TXD:
> case OPCODE_TXL:
> case OPCODE_TXP:
> -  ptn_tex(b, dest, src, prog_inst);
> +  ptn_tex(c, dest, src, prog_inst);
>break;
>
> case OPCODE_SWZ:
> --
> 2.18.0
>
> ___
> 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] intel: tools: Fix aubinator_error's fprintf call (format-security)

2018-08-25 Thread Kai Wasserbäch
The recent commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be introduced
the new function aubinator_error() which is a trivial wrapper around
fprintf() to STDERR. The call to fprintf() however is passed the message
msg directly:
  fprintf(stderr, msg);

This is a format-security violation and leads to an FTBFS with
-Werror=format-security (GCC 8):
  ../../../src/intel/tools/aubinator.c: In function 'aubinator_error':
  ../../../src/intel/tools/aubinator.c:74:4: error: format not a string literal 
and no format arguments [-Werror=format-security]
  fprintf(stderr, msg);
  ^~~

This patch fixes this trivially by introducing a catch-all "%s" format
argument.

Fixes: 4616639b49b ("intel: tools: split aub parsing from aubinator")
Cc: Lionel Landwerlin 
Signed-off-by: Kai Wasserbäch 
---

Hey,
just tried compiling the latest version of Mesa but due to commit
4616639b49b4bbc91e503c1c27632dccc1c2b5be by Lionel I got a FTBFS. Please
commit this patch for me, if you accept it. I do not have commit access
for Mesa.

The pull request for this would be:
 The following changes since commit 1281608849a058fcdbe2f64481a159d58af3d888:

   gallium: Split out PIPE_CAP_TEXTURE_MIRROR_CLAMP_TO_EDGE. (2018-08-24 
17:25:36 -0700)

 are available in the Git repository at:

   https://gitlab.freedesktop.org/curan/mesa.git aub-format-security

 for you to fetch changes up to 24957b0db3c12a097d70033ec8596da5bafedd55:

   intel: tools: Fix aubinator_error's fprintf call (format-security) 
(2018-08-25 11:43:25 +0200)

Cheers,
Kai

 src/intel/tools/aubinator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index 374ed46f86..c22d191f14 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -71,7 +71,7 @@ struct brw_instruction;
 static void
 aubinator_error(void *user_data, const void *aub_data, const char *msg)
 {
-   fprintf(stderr, msg);
+   fprintf(stderr, "%s", msg);
 }
 
 static void
-- 
2.18.0

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


[Mesa-dev] [PATCH] st/mesa: Override blend factors involving alpha if it doesn't exist.

2018-08-25 Thread Kenneth Graunke
When faking an RGB format with an RGBA format, there may be a channel
of data containing garbage.  st/mesa already overrides texture swizzles
to replace the A channel with ONE.  This patch makes it override blend
factors to achieve a similar effect.

It appears that st_update_blend is already called when the framebuffer
is changed, so it should be safe to look at the render target formats
without adding additional state dependencies.
---
 src/mesa/state_tracker/st_atom_blend.c | 35 ++
 1 file changed, 35 insertions(+)

diff --git a/src/mesa/state_tracker/st_atom_blend.c 
b/src/mesa/state_tracker/st_atom_blend.c
index 804de2f154f..3e6e4411107 100644
--- a/src/mesa/state_tracker/st_atom_blend.c
+++ b/src/mesa/state_tracker/st_atom_blend.c
@@ -41,6 +41,7 @@
 
 #include "framebuffer.h"
 #include "main/blend.h"
+#include "main/glformats.h"
 #include "main/macros.h"
 
 /**
@@ -142,6 +143,28 @@ blend_per_rt(const struct gl_context *ctx, unsigned num_cb)
return GL_FALSE;
 }
 
+/**
+ * Modify blend function to force destination alpha to 1.0
+ *
+ * If \c function specifies a blend function that uses destination alpha,
+ * replace it with a function that hard-wires destination alpha to 1.0.  This
+ * is used when rendering to xRGB targets.
+ */
+static enum pipe_blendfactor
+fix_xrgb_alpha(enum pipe_blendfactor factor)
+{
+   switch (factor) {
+   case PIPE_BLENDFACTOR_DST_ALPHA:
+  return PIPE_BLENDFACTOR_ONE;
+
+   case PIPE_BLENDFACTOR_INV_DST_ALPHA:
+   case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
+  return PIPE_BLENDFACTOR_ZERO;
+   default:
+  return factor;
+   }
+}
+
 void
 st_update_blend( struct st_context *st )
 {
@@ -210,6 +233,18 @@ st_update_blend( struct st_context *st )
 blend->rt[i].alpha_dst_factor =
translate_blend(ctx->Color.Blend[j].DstA);
  }
+
+ const struct gl_renderbuffer *rb =
+ctx->DrawBuffer->_ColorDrawBuffers[i];
+
+ if (rb && !_mesa_base_format_has_channel(rb->_BaseFormat,
+  GL_TEXTURE_ALPHA_TYPE)) {
+struct pipe_rt_blend_state *rt = >rt[i];
+rt->rgb_src_factor = fix_xrgb_alpha(rt->rgb_src_factor);
+rt->rgb_dst_factor = fix_xrgb_alpha(rt->rgb_dst_factor);
+rt->alpha_src_factor = fix_xrgb_alpha(rt->alpha_src_factor);
+rt->alpha_dst_factor = fix_xrgb_alpha(rt->alpha_dst_factor);
+ }
   }
}
else {
-- 
2.18.0

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


Re: [Mesa-dev] [PATCH 1/2] meson: Be a bit more helpful when arch or OS is unknown

2018-08-25 Thread Stuart Young
On Fri, 24 Aug 2018 at 22:23, Guido Günther  wrote:

>
> @@ -230,7 +236,8 @@ if _platforms.contains('auto')
>elif ['haiku'].contains(host_machine.system())
>  _platforms = ['haiku']
>else
> -error('Unknown OS. Please pass -Dplatforms to set platforms. Patches
> gladly accepted to fix this.')
> +error('Unknown OS. Please pass -Dplatforms to set platforms. Patches
> gladly accepted to fix this.'.format(
> +  host_machine.system()))
>endif
>  endif
>

Did you mean that last "Unknown OS." to be "Unknown OS @0@." like all the
others?

-- 
Stuart Young (aka Cefiar)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] st/mesa: Disable blending for integer formats.

2018-08-25 Thread Kenneth Graunke
Blending isn't valid for integer formats.  Rather than having drivers
worry about this, just disable blending in this case.  This hopefully
will increase hits in the CSO cache as well, by eliminating most of the
meaningless fields in this case.
---
 src/mesa/state_tracker/st_atom_blend.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/state_tracker/st_atom_blend.c 
b/src/mesa/state_tracker/st_atom_blend.c
index 57400e2e791..804de2f154f 100644
--- a/src/mesa/state_tracker/st_atom_blend.c
+++ b/src/mesa/state_tracker/st_atom_blend.c
@@ -171,6 +171,7 @@ st_update_blend( struct st_context *st )
   /* blending enabled */
   for (i = 0, j = 0; i < num_state; i++) {
  if (!(ctx->Color.BlendEnabled & (1 << i)) ||
+ (ctx->DrawBuffer->_IntegerBuffers & (1 << i)) ||
  !blend->rt[i].colormask)
 continue;
 
-- 
2.18.0

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


Re: [Mesa-dev] [PATCH 0/5] Emit BRW_AOP_INC or BRW_AOP_DEC

2018-08-25 Thread Caio Marcelo de Oliveira Filho
Ian Romanick  writes:

> I don't know why we never did this.  Almost every shader in shader-db
> that uses atomicAdd or imageAtomicAdd uses it with a constant of 1 or
> -1.

Nice. The series is

Reviewed-by: Caio Marcelo de Oliveira Filho 



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


[Mesa-dev] [PATCH 2/2] run: Add Cannonlake and Ice Lake as platforms

2018-08-25 Thread Ian Romanick
From: Ian Romanick 

PCI IDs directly from Mesa's include/pci_ids/i965_pci_ids.h.

Signed-off-by: Ian Romanick 
---
 run.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/run.c b/run.c
index 200c0f4..c70e367 100644
--- a/run.c
+++ b/run.c
@@ -418,6 +418,8 @@ const struct platform platforms[] = {
 "byt",  "0x0F33",
 "bdw",  "0x162E",
 "skl",  "0x191D",
+"cnl",  "0x5A54",
+"icl",  "0x8A50",
 };
 
 void print_usage(const char *prog_name)
-- 
2.14.4

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


[Mesa-dev] [PATCH 1/2] report.py: Gather and log some statistics about the helped / hurt data

2018-08-25 Thread Ian Romanick
From: Ian Romanick 

A previous method that I tried was treating the helped and hurt data
as samples from separate populations, and these were compared using a
T-test.  Since we're applying a common change to "both" sample sets, I
don't think this is a valid analysis.

Instead, I think it is more valid to look at the entire change set as a
sample of a single population and compare the mean of that sample to
zero.  Only the changed samples are examined because the vast majority
of the sample in unaffected.  If the mean of the entire sample was used,
the mean confidence interval would always include zero.  It would be
more valid, I believe, include shaders that were affected but had no
change in instruction or cycle count.  I don't know of a way to
determine this using the existing shader-db infrastructure.

These two different methods communicate two different things.  The first
tries to determine whether the shaders hurt are affected more or less
than the shaders helped.  This doesn't capture any information about the
number of shaders affected.  There might be 1,000 shaders helped and 3
hurt, and the conclusion could still be negative.  The second methods
trieds to determine whether the sample set is overall helped or hurt.
This allows the magnitued of hurt (or help) to be overwhelmed by the
number of helped (or hurt) shaders.  There could be 1,000 shaders helped
by 1 instruction and 3 shaders hurt by 50 instructions, and the
conclusion would be positive.

Comparing the declared result with the mean and median, I feel like the
second method matches my intuitive interpretation of the data.  Here is
a result of the T-test:

total cycles in shared programs: 559379982 -> 559342256 (<.01%)
cycles in affected programs: 10791218 -> 10753492 (-0.35%)
helped: 1952
HURT: 908
helped stats (abs) min: 1 max: 5762 x̄: 37.71 x̃: 16
helped stats (rel) min: <.01% max: 28.57% x̄: 3.54% x̃: 2.09%
HURT stats (abs)   min: 1 max: 573 x̄: 39.51 x̃: 10
HURT stats (rel)   min: <.01% max: 27.78% x̄: 1.93% x̃: 0.66%
abs t: -0.34, p: 73.70%
rel t: 9.88, p: <.01%
Inconclusive result (cannot disprove both null hypothoses).

And here is the result of the mean confidence interval tests on the
same data:

total cycles in shared programs: 559378112 -> 559340386 (<.01%)
cycles in affected programs: 10791218 -> 10753492 (-0.35%)
helped: 1952
HURT: 908
helped stats (abs) min: 1 max: 5762 x̄: 37.71 x̃: 16
helped stats (rel) min: <.01% max: 28.57% x̄: 3.54% x̃: 2.09%
HURT stats (abs)   min: 1 max: 573 x̄: 39.51 x̃: 10
HURT stats (rel)   min: <.01% max: 27.78% x̄: 1.93% x̃: 0.66%
95% mean confidence interval for cycles value: -18.27 -8.11
95% mean confidence interval for cycles %-change: -1.98% -1.63%
Cycles are helped.

Since the confidence interval is calculated based on the sample mean and
the sample standard deviation, it can include values out side the sample
minimum and maximum.  This can lead to unexpected conclusions.  In this
case all of the affected shaders were helped, but the result is
inconclusive.

total instructions in shared programs: 7886959 -> 7886925 (<.01%)
instructions in affected programs: 1340 -> 1306 (-2.54%)
helped: 4
HURT: 0
helped stats (abs) min: 2 max: 15 x̄: 8.50 x̃: 8
helped stats (rel) min: 0.63% max: 4.30% x̄: 2.45% x̃: 2.43%
95% mean confidence interval for instructions value: -20.44 3.44
95% mean confidence interval for instructions %-change: -5.78% 0.89%
Inconclusive result (value mean confidence interval includes 0).

v2: Don't log statistics for spill or fills.  Simplify T-test logging.

v3: Use confidence interval instead.

Signed-off-by: Ian Romanick 
---
 report.py | 101 +-
 1 file changed, 100 insertions(+), 1 deletion(-)

diff --git a/report.py b/report.py
index b306220..6d53052 100755
--- a/report.py
+++ b/report.py
@@ -2,6 +2,9 @@
 
 import re
 import argparse
+import statistics
+from scipy import stats
+import numpy as np
 
 
 def get_results(filename):
@@ -63,6 +66,35 @@ def get_result_string(p, b, a):
 def split_list(string):
 return string.split(",")
 
+
+def gather_statistics(changes, before, after, m):
+stats = (0.0, 0, 0.0, 0, 0, 0.0, 0.0, 0.0, 0.0, 0.0)
+
+num = len(changes)
+if num > 0:
+absolute = [abs(before[p][m] - after[p][m]) for p in changes]
+relative = [0 if before[p][m] == 0 else abs(before[p][m] - 
after[p][m]) / before[p][m] for p in changes]
+
+stats = (statistics.mean(absolute),
+ statistics.median(absolute),
+ min(absolute),
+ max(absolute),
+ statistics.mean(relative),
+ statistics.median(relative),
+ min(relative),
+ max(relative))
+
+return stats
+
+
+def mean_confidence_interval(data, confidence=0.95):
+a = 1.0 * np.array(data)
+n