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

[Kenneth Graunke]

On Monday, August 27, 2018 6:18:21 PM PDT Marek Olšák wrote:
> On Mon, Aug 27, 2018 at 5:55 PM, Kenneth Graunke  
> wrote:
> > On Monday, August 27, 2018 11:05:19 AM PDT Marek Olšák wrote:
> >> Yeah, this will be more complicated because it's per RT.
> >>
> >> I suggest adding a PIPE_CAP for the hw capability to force DST_ALPHA
> >> to 0, and applying this workaround only if the PIPE_CAP is 0.
> >>
> >> Marek
> >
> > I was thinking of applying this hack based on PIPE_CAP_INDEP_BLEND_FUNC.
> >
> > Ilia seemed to think that it was unsupportable on older Nouveau (without
> > independent alpha blending), but harmless either way on modern Nouveau.
> > (Supposedly Freedreno does this workaround in the driver...)
> >
> > Would you prefer not to have this triggered?
> 
> Yes. I think it would be unnecessary overhead for us.
> 
> Marek

Okay, cool, thanks for the feedback.  Let's shelve this for now.

I'm thinking of reworking how I handle RGBX -> RGBA stuff anyway,
and may need to handle it in the driver in the end.  I'll see how
things look and either handle it myself or come up with a version
of this patch which makes the hack optional.


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] EGL/mesa: Initial writeup for MESA_query_renderer

[Veluri Mithun]

---
 docs/specs/EGL_MESA_query_renderer.txt | 58 ++
 1 file changed, 58 insertions(+)
 create mode 100644 docs/specs/EGL_MESA_query_renderer.txt

diff --git a/docs/specs/EGL_MESA_query_renderer.txt 
b/docs/specs/EGL_MESA_query_renderer.txt
new file mode 100644
index 00..0fa9b33d00
--- /dev/null
+++ b/docs/specs/EGL_MESA_query_renderer.txt
@@ -0,0 +1,58 @@
+Name
+
+MESA_query_renderer
+
+Name Strings
+
+EGL_MESA_query_renderer
+
+Contact
+
+Rob Clark  
+Nicolai Hähnle 
+
+Contibutors
+
+Veluri Mithun 
+
+Status
+
+XXX - Not complete yet!!! (draft)
+
+Version
+
+Version 1, 2018-08-24
+
+Number
+
+EGL Extension ###
+
+Dependencies
+
+
+
+New Procedures and Functions
+
+Bool EGLQueryRendererIntegerMESA(EGLDisplay *dpy, int screen,
+ int renderer, int attribute,
+ unsigned int *value);
+
+Overview
+
+This extension provides the applications to qurey the details
+of the all renderers available for a particular display and
+screen.
+
+IP Status
+
+
+
+New Tokens
+
+
+
+Revision History
+
+
+
+
-- 
2.17.1

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

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

[Marek Olšák]

On Mon, Aug 27, 2018 at 5:55 PM, Kenneth Graunke  wrote:
> On Monday, August 27, 2018 11:05:19 AM PDT Marek Olšák wrote:
>> Yeah, this will be more complicated because it's per RT.
>>
>> I suggest adding a PIPE_CAP for the hw capability to force DST_ALPHA
>> to 0, and applying this workaround only if the PIPE_CAP is 0.
>>
>> Marek
>
> I was thinking of applying this hack based on PIPE_CAP_INDEP_BLEND_FUNC.
>
> Ilia seemed to think that it was unsupportable on older Nouveau (without
> independent alpha blending), but harmless either way on modern Nouveau.
> (Supposedly Freedreno does this workaround in the driver...)
>
> Would you prefer not to have this triggered?

Yes. I think it would be unnecessary overhead for us.

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

Re: [Mesa-dev] [PATCH] nir: Remove outdated comment

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Mon, Aug 27, 2018 at 5:20 PM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:

> ---
>
> The move of comapre functions landed before the suggestion to remove
> the comment, so removing it now.
>
>  src/compiler/nir/nir_deref.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c
> index c8851688f9d..097ea8f1046 100644
> --- a/src/compiler/nir/nir_deref.c
> +++ b/src/compiler/nir/nir_deref.c
> @@ -271,9 +271,6 @@ nir_fixup_deref_modes(nir_shader *shader)
> }
>  }
>
> -/** Returns true if the storage referrenced to by deref completely
> contains
> - * the storage referenced by sub.
> - */
>  nir_deref_compare_result
>  nir_compare_deref_paths(nir_deref_path *a_path,
>  nir_deref_path *b_path)
> --
> 2.18.0
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl/linker: Link all out vars from a shader objects on a single stage

[Timothy Arceri]

On 27/08/18 22:20, Vadym Shovkoplias wrote:

From: "vadym.shovkoplias" 

During intra stage linking some out variables can be dropped because
it is not used in a shader with the main function. But these out vars
can be referenced on later stages which can lead to further linking
errors.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105731
Signed-off-by: Vadym Shovkoplias 
---
  src/compiler/glsl/linker.cpp | 38 
  1 file changed, 38 insertions(+)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 3ce78fe642..3b0c01c316 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -2187,6 +2187,41 @@ link_cs_input_layout_qualifiers(struct gl_shader_program 
*prog,
 }
  }
  
+/**

+ * Link all out variables on a single stage which are not
+ * directly used in a shader with the main function.
+ */
+static void
+link_output_variables(struct gl_linked_shader *linked_shader,
+  struct gl_shader **shader_list,
+  unsigned num_shaders)
+{
+   struct glsl_symbol_table *symbols = linked_shader->symbols;
+
+   for (unsigned i = 0; i < num_shaders; i++) {
+
+  /* Skip shader object with main function */
+  if (shader_list[i]->symbols->get_function("main"))
+ continue;
+
+  foreach_in_list (ir_instruction, ir, shader_list[i]->ir) {


^-- Please remove this space

Otherwise:

Reviewed-by: Timothy Arceri 



+
+ if (ir->ir_type != ir_type_variable)
+continue;
+
+ ir_variable *const var = (ir_variable *) ir;
+
+ if (var->data.mode == ir_var_shader_out &&
+   !symbols->get_variable(var->name)) {
+symbols->add_variable(var);
+linked_shader->ir->push_head(var);
+ }
+  }
+   }
+
+   return;
+}
+
  
  /**

   * Combine a group of shaders for a single stage to generate a linked shader
@@ -2352,6 +2387,9 @@ link_intrastage_shaders(void *mem_ctx,
return NULL;
 }
  
+   if (linked->Stage != MESA_SHADER_FRAGMENT)

+  link_output_variables(linked, shader_list, num_shaders);
+
 /* Make a pass over all variable declarations to ensure that arrays with
  * unspecified sizes have a size specified.  The size is inferred from the
  * max_array_access field.


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

Re: [Mesa-dev] [PATCH 3/3] anv: Claim to support depthBounds for ID games

[Jason Ekstrand]

On Mon, Aug 27, 2018 at 12:50 PM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> On 23/08/2018 16:13, Jason Ekstrand wrote:
> > ---
> >   src/intel/vulkan/anv_device.c | 9 +
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/src/intel/vulkan/anv_device.c
> b/src/intel/vulkan/anv_device.c
> > index 0357fc7c0ea..5a63592f7ca 100644
> > --- a/src/intel/vulkan/anv_device.c
> > +++ b/src/intel/vulkan/anv_device.c
> > @@ -854,6 +854,15 @@ void anv_GetPhysicalDeviceFeatures(
> >  pFeatures->vertexPipelineStoresAndAtomics =
> > pdevice->compiler->scalar_stage[MESA_SHADER_VERTEX] &&
> > pdevice->compiler->scalar_stage[MESA_SHADER_GEOMETRY];
> > +
> > +   struct anv_app_info *app_info = >instance->app_info;
> > +
> > +   /* The new DOOM and Wolfenstein games require depthBounds without
> > +* checking for it.  They seem to run fine without it so just claim
> it's
> > +* there and accept the consequences.
> > +*/
> > +   if (app_info->engine_name && strcmp(app_info->engine_name, "idTech")
> == 0)
> > +  pFeatures->depthBounds = true;
> >   }
> >
> >   void anv_GetPhysicalDeviceFeatures2(
>
> I'm struggling to understand "require depthBounds with checking for it".
> I thought one would check for the feature by reading the boolean you
> just set. So if it's not checked why would it make a difference?
>

The Vulkan spec requires that we throw VK_ERROR_INITIALIZATION_FAILED if
the client enables any features we have not advertised.  This gives us two
options: 1) advertise support for depthBounds for idTech games or 2) Skip
the check for depthBounds in CreateDevice and let it succeed even if it's
enabled for idTech games.  This patch takes the former approach since it's
easier.

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

Re: [Mesa-dev] [PATCH] i965/gen7_urb: Re-emit PUSH_CONSTANT_ALLOC on GLK

[Nanley Chery]

On Mon, Aug 27, 2018 at 03:25:37PM -0700, Kenneth Graunke wrote:
> On Monday, August 27, 2018 11:03:33 AM PDT Nanley Chery wrote:
> > On Fri, Aug 24, 2018 at 05:46:44PM -0700, Nanley Chery wrote:
> > > According to internal docs, some gen9 platforms have a pixel shader push
> > > constant synchronization issue. Although not listed among said
> > > platforms, this issue seems to be present on the GeminiLake 2x6's we've
> > > tested.
> > > 
> > > We consider the available workarounds to be too detrimental on
> > > performance. Instead, we mitigate the issue by applying part of one of
> > > the workarounds. Re-emit PUSH_CONSTANT_ALLOC at the top of every batch
> > > (as suggested by Ken).
> > > 
> > > Fixes ext_framebuffer_multisample-accuracy piglit test failures with the
> > > following options:
> > > * 6 depth_draw small depthstencil
> > > * 8 stencil_draw small depthstencil
> > > * 6 stencil_draw small depthstencil
> > > * 8 depth_resolve small
> > > * 6 stencil_resolve small depthstencil
> > > * 4 stencil_draw small depthstencil
> > > * 16 stencil_draw small depthstencil
> > > * 16 depth_draw small depthstencil
> > > * 2 stencil_resolve small depthstencil
> > > * 6 stencil_draw small
> > > * all_samples stencil_draw small
> > > * 2 depth_draw small depthstencil
> > > * all_samples depth_draw small depthstencil
> > > * all_samples stencil_resolve small
> > > * 4 depth_draw small depthstencil
> > > * all_samples depth_draw small
> > > * all_samples stencil_draw small depthstencil
> > > * 4 stencil_resolve small depthstencil
> > > * 4 depth_resolve small depthstencil
> > > * all_samples stencil_resolve small depthstencil
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106865
> > > Cc: 
> > > ---
> > >  src/mesa/drivers/dri/i965/gen7_urb.c | 23 +++
> > >  1 file changed, 23 insertions(+)
> > 
> > Ping?
> 
> So...I believe those tests are still intermittent on other platforms.
> 
> And...these platforms aren't listed as having the bug that you're trying
> to work around here.
> 
> Which makes me wonder if a GLK2x6 hack is really the right thing to do.

Right, it'd be good to know if this helps any other platforms. I'll see
if we could add a more generic patch to CI to see if stability improves
for other platforms.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965/gen7_urb: Re-emit PUSH_CONSTANT_ALLOC on GLK

[Kenneth Graunke]

On Monday, August 27, 2018 11:03:33 AM PDT Nanley Chery wrote:
> On Fri, Aug 24, 2018 at 05:46:44PM -0700, Nanley Chery wrote:
> > According to internal docs, some gen9 platforms have a pixel shader push
> > constant synchronization issue. Although not listed among said
> > platforms, this issue seems to be present on the GeminiLake 2x6's we've
> > tested.
> > 
> > We consider the available workarounds to be too detrimental on
> > performance. Instead, we mitigate the issue by applying part of one of
> > the workarounds. Re-emit PUSH_CONSTANT_ALLOC at the top of every batch
> > (as suggested by Ken).
> > 
> > Fixes ext_framebuffer_multisample-accuracy piglit test failures with the
> > following options:
> > * 6 depth_draw small depthstencil
> > * 8 stencil_draw small depthstencil
> > * 6 stencil_draw small depthstencil
> > * 8 depth_resolve small
> > * 6 stencil_resolve small depthstencil
> > * 4 stencil_draw small depthstencil
> > * 16 stencil_draw small depthstencil
> > * 16 depth_draw small depthstencil
> > * 2 stencil_resolve small depthstencil
> > * 6 stencil_draw small
> > * all_samples stencil_draw small
> > * 2 depth_draw small depthstencil
> > * all_samples depth_draw small depthstencil
> > * all_samples stencil_resolve small
> > * 4 depth_draw small depthstencil
> > * all_samples depth_draw small
> > * all_samples stencil_draw small depthstencil
> > * 4 stencil_resolve small depthstencil
> > * 4 depth_resolve small depthstencil
> > * all_samples stencil_resolve small depthstencil
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106865
> > Cc: 
> > ---
> >  src/mesa/drivers/dri/i965/gen7_urb.c | 23 +++
> >  1 file changed, 23 insertions(+)
> 
> Ping?

So...I believe those tests are still intermittent on other platforms.

And...these platforms aren't listed as having the bug that you're trying
to work around here.

Which makes me wonder if a GLK2x6 hack is really the right thing to do.


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] nir: Remove outdated comment

[Caio Marcelo de Oliveira Filho]

---

The move of comapre functions landed before the suggestion to remove
the comment, so removing it now.

 src/compiler/nir/nir_deref.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c
index c8851688f9d..097ea8f1046 100644
--- a/src/compiler/nir/nir_deref.c
+++ b/src/compiler/nir/nir_deref.c
@@ -271,9 +271,6 @@ nir_fixup_deref_modes(nir_shader *shader)
}
 }
 
-/** Returns true if the storage referrenced to by deref completely contains
- * the storage referenced by sub.
- */
 nir_deref_compare_result
 nir_compare_deref_paths(nir_deref_path *a_path,
 nir_deref_path *b_path)
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH v4 12/49] meson: don't build glx or dri by default on windows

[Dylan Baker]

Quoting Dylan Baker (2018-08-23 10:27:17)
> Quoting Eric Engestrom (2018-08-23 10:13:17)
> > On Wednesday, 2018-08-22 10:04:35 -0700, Dylan Baker wrote:
> > > Signed-off-by: Dylan Baker 
> > > Reviewed-by: Eric Anholt 
> > > ---
> > >  meson.build | 8 ++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index 1af610573d5..5102ffe0c7c 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -264,8 +264,12 @@ if with_glx == 'auto'
> > >elif with_platform_haiku
> > >  with_glx = 'disabled'
> > 
> > How about simply adding it here, before the with_gallium check?
> > 
> >   +  elif host_machine.system() == 'windows'
> >   +with_glx = 'disabled'
> > 
> > Otherwise this opens the door to a weird `(gallium + x11 + gl - vk) on
> > windows` bug here.
> 
> I'm trying to understand the bug, if you build with windows dri you'll get
> glx? It seems like then really we should have the first option be "if
> with_dri and with_dri_platform == 'drm'" (or should it be "with_dri and not
> ['windows', 'apple'].contains(with_dri_platform)"?) and instead of simply
> with_dri, or am I missing something?
> 
> Dylan

ping?

> 
> > With that:
> > Reviewed-by: Eric Engestrom 
> > 
> > >elif with_gallium
> > > -# Even when building just gallium drivers the user probably wants dri
> > > -with_glx = 'dri'
> > > +if host_machine.system() == 'windows'
> > > +  with_glx = 'disabled'
> > > +else
> > > +  # Even when building just gallium drivers the user probably wants 
> > > dri
> > > +  with_glx = 'dri'
> > > +endif
> > >elif with_platform_x11 and with_any_opengl and not with_any_vk
> > >  # The automatic behavior should not be to turn on xlib based glx when
> > >  # building only vulkan drivers
> > > -- 
> > > 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


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

Re: [Mesa-dev] [PATCH 5/9] nir: Add a local dead write vars removal pass

[Caio Marcelo de Oliveira Filho]

(Disregard the incomplete mail, still adapting to notmuch-emacs).

Jason Ekstrand  writes:

>> +static nir_deref_path *
>> +get_path(struct state *state, nir_deref_instr *deref)
>> +{
>> +   struct hash_entry *entry = _mesa_hash_table_search(state->paths,
>> deref);
>> +   if (!entry) {
>> +  nir_deref_path *path = linear_zalloc_child(state->path_lin_ctx,
>> sizeof(nir_deref_path));
>> +  nir_deref_path_init(path, deref, state->mem_ctx);
>> +  _mesa_hash_table_insert(state->paths, deref, path);
>> +  return path;
>> +   } else {
>> +  return entry->data;
>> +   }
>> +}
>>
>
> Do you have any proof that this actually helps?  The deref_path stuff was
> designed to be put on the stack and absolutely as efficient as possible.
> In the common case of a deref chain with only a couple of elements, I would
> expect to actually be more work to look it up in a hash table.

Storing those makes more sense if you read the next commit (the
"global").  Since I've created the "local" commit as an aid for
reviewing (perhaps a failure), I did not wanted to change it too much.

When I wrote there were some savings when measuring executions with
perf.

(...)

>> +static bool
>> +remove_dead_write_vars_local(struct state *state, nir_block *block)
>> +{
>> +   bool progress = false;
>> +
>> +   struct util_dynarray unused_writes;
>> +   util_dynarray_init(_writes, state->mem_ctx);
>> +
>> +   nir_foreach_instr_safe(instr, block) {
>>
>
> It wouldn't hurt to add a case for call instructions which does a barrier
> on everything I mentioned below as well as globals and locals.

Makes sense.  But I don't get locals are affect?  Is this to cover the
parameters being passed to the call?

>
>> +  if (instr->type != nir_instr_type_intrinsic)
>> + continue;
>> +
>> +  nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
>> +  switch (intrin->intrinsic) {
>> +  case nir_intrinsic_barrier:
>> +  case nir_intrinsic_memory_barrier: {
>> + nir_variable_mode modes = ~(nir_var_local | nir_var_global |
>> + nir_var_shader_in | nir_var_uniform);
>>
>
> The only thing a barrier like this affects is shared, storage, and output.
> Locals and globals can't cross between shader channels so there's no reason
> to do anything with them on a barrier.  For inputs and uniforms, they're
> never written anyway so there's no point in doing anything with them on a
> barrier.

This came from previous code, but except for "system values" it seems to
do the right thing (note the ~).  Is the suggestion to use a "positive"
enumeration, e.g.

clear_unused_for_modes(_writes, nir_var_shader_out |
   nir_var_shader_storage |
   nir_var_shared);



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

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

[Kenneth Graunke]

On Monday, August 27, 2018 11:05:19 AM PDT Marek Olšák wrote:
> Yeah, this will be more complicated because it's per RT.
> 
> I suggest adding a PIPE_CAP for the hw capability to force DST_ALPHA
> to 0, and applying this workaround only if the PIPE_CAP is 0.
> 
> Marek

I was thinking of applying this hack based on PIPE_CAP_INDEP_BLEND_FUNC.

Ilia seemed to think that it was unsupportable on older Nouveau (without
independent alpha blending), but harmless either way on modern Nouveau.
(Supposedly Freedreno does this workaround in the driver...)

Would you prefer not to have this triggered?


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 5/9] nir: Add a local dead write vars removal pass

[Caio Marcelo de Oliveira Filho]

Jason Ekstrand  writes:

> On Wed, Aug 15, 2018 at 4:57 PM Caio Marcelo de Oliveira Filho <
> caio.olive...@intel.com> wrote:
>
>> Instead of doing this as part of the existing (local) copy prop vars
>> pass.  This is an intermediate step before changing both the dead
>> write and the copy prop vars to act on the whole program instead of on
>> local blocks.  The nature of data we store and the way we iterate is
>> different enough that would be awkward keeping those together.
>> ---
>>  src/compiler/Makefile.sources  |   1 +
>>  src/compiler/nir/meson.build   |   1 +
>>  src/compiler/nir/nir.h |   2 +
>>  src/compiler/nir/nir_opt_dead_write_vars.c | 243 +
>>  4 files changed, 247 insertions(+)
>>  create mode 100644 src/compiler/nir/nir_opt_dead_write_vars.c
>>
>> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
>> index 27a54e0be09..fa93ad08a16 100644
>> --- a/src/compiler/Makefile.sources
>> +++ b/src/compiler/Makefile.sources
>> @@ -274,6 +274,7 @@ NIR_FILES = \
>> nir/nir_opt_cse.c \
>> nir/nir_opt_dce.c \
>> nir/nir_opt_dead_cf.c \
>> +   nir/nir_opt_dead_write_vars.c \
>> nir/nir_opt_gcm.c \
>> nir/nir_opt_global_to_local.c \
>> nir/nir_opt_if.c \
>> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
>> index 8708f9b069c..1c164a548a7 100644
>> --- a/src/compiler/nir/meson.build
>> +++ b/src/compiler/nir/meson.build
>> @@ -158,6 +158,7 @@ files_libnir = files(
>>'nir_opt_cse.c',
>>'nir_opt_dce.c',
>>'nir_opt_dead_cf.c',
>> +  'nir_opt_dead_write_vars.c',
>>'nir_opt_gcm.c',
>>'nir_opt_global_to_local.c',
>>'nir_opt_if.c',
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index d0fa693884b..becf6e351c3 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -2968,6 +2968,8 @@ bool nir_opt_dce(nir_shader *shader);
>>
>>  bool nir_opt_dead_cf(nir_shader *shader);
>>
>> +bool nir_opt_dead_write_vars(nir_shader *shader);
>> +
>>  bool nir_opt_gcm(nir_shader *shader, bool value_number);
>>
>>  bool nir_opt_if(nir_shader *shader);
>> diff --git a/src/compiler/nir/nir_opt_dead_write_vars.c
>> b/src/compiler/nir/nir_opt_dead_write_vars.c
>> new file mode 100644
>> index 000..822bfa5595d
>> --- /dev/null
>> +++ b/src/compiler/nir/nir_opt_dead_write_vars.c
>> @@ -0,0 +1,243 @@
>> +/*
>> + * Copyright © 2018 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 "nir.h"
>> +#include "nir_builder.h"
>> +#include "nir_deref.h"
>> +
>> +#include "util/u_dynarray.h"
>> +
>> +struct state {
>> +   void *mem_ctx;
>> +
>> +   /* Maps nir_deref_instr to a corresponding nir_deref_path.  Avoids
>> +* rebuilding the paths for the same deref. */
>> +   struct hash_table *paths;
>> +   void *path_lin_ctx;
>> +};
>> +
>> +static nir_deref_path *
>> +get_path(struct state *state, nir_deref_instr *deref)
>> +{
>> +   struct hash_entry *entry = _mesa_hash_table_search(state->paths,
>> deref);
>> +   if (!entry) {
>> +  nir_deref_path *path = linear_zalloc_child(state->path_lin_ctx,
>> sizeof(nir_deref_path));
>> +  nir_deref_path_init(path, deref, state->mem_ctx);
>> +  _mesa_hash_table_insert(state->paths, deref, path);
>> +  return path;
>> +   } else {
>> +  return entry->data;
>> +   }
>> +}
>>
>
> Do you have any proof that this actually helps?  The deref_path stuff was
> designed to be put on the stack and absolutely as efficient as possible.
> In the common case of a deref chain with only a couple of elements, I would
> expect to actually be more work to look it up in a hash table.
>
>
>> +
>> +/* Entry for unused_writes arrays. */
>> +struct write_entry {
>> +   /* If NULL indicates the 

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

[Caio Marcelo de Oliveira Filho]

Jason Ekstrand  writes:

> 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.

My assumption here was the number was small (dozens), and would be
potentially increase as we move other intrinsics to use derefs
(e.g. SSBO).


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

My assumption here is that we don't expect to compare all the derefs,
but only little groups.  This affected my choice.

I can verify both assumptions with shader-db, but I expect scenario
change as we add support for more intrinsics to use 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.

I did post this with a suboptimal deref_map that had the sufficient
interface so the rest of the code could work.  With the expected
implementation (a tree, similar to lower_vars_to_ssa) we have a better
behavior.  Assuming n derefs with at most m size (in general m is small
enough so we don't care):

- "may alias": O(m), walk tree and deref_path in parallel.

- "intersection": O(n * m), by walking both trees in parallel and marking the
  common bits.

- "union": O(n * m), by walking both trees in parallel and adding the
  missing bits to the target.

- "subtraction": O(m)
- "subtraction map": O(n * m), walking both trees in parallel

So with some simplification, these are the complexities we are looking
at.

- local pass: O(I * n * m) number of instructions times the number of
  derefs, to build all the maps.  Which can be simplified to O(n^2 * m)
  ~ O(n^2).

- data flow iteration: for each block (with strech, is proportional to
  n), perform map operations, so we end up with O(n^2 * m).

- final pass: for each block, for each "unused write", check if the OUT
  map contains it. O(n^2 * m).

In retrospect I should've been more clear on what expect from deref_map
now (and later).  And of course I might be overlooking something :-)


>>
>> 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;
>> };

I did explore something _similar_ for this pass: collecting the relevant
derefs, keep them in an array, use their indices in the bitsets
(combined with the components); plus bootstrap a "matrix" to compare
them.  Because I had the "little groups" assumption in mind, I was even
toying with lazy initialize the matrix, so only really compare if we
need to.

(...)

>> 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 iterations.
>> This data structure could easily be put in nir_derefs.c and shared between
>> this pass and copy-prop.

Yep, as-is deref_map_add_map is not good, but the plan was replace it
with something better.  Seems to me it can be implemented in O(n).

(...)

> Also, if we're going to share it between passes, it might be good to have
> nir_deref_compare_map_init take a shader and do the walk to find all derefs
> and the bake in a single step and we can avoid exposing quite as many
> details to the user.

That is a good point and might justify the work to pre 

Re: [Mesa-dev] [PATCH] intel: decoder: unify MI_BB_START field naming

[Andres Gomez]

Lionel, should we also include this in the stable queues ?

On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:
> The batch decoder looks for a field with a particular name to decide
> whether an MI_BB_START leads into a second batch buffer level. Because
> the names are different between Gen7.5/8 and the newer generation we
> fail that test and keep on reading (invalid) instructions.
> 
> Signed-off-by: Lionel Landwerlin 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
> ---
>  src/intel/genxml/gen75.xml | 6 +++---
>  src/intel/genxml/gen8.xml  | 6 +++---
>  src/intel/vulkan/anv_batch_chain.c | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
> index 5b01fd45400..dfc3d891498 100644
> --- a/src/intel/genxml/gen75.xml
> +++ b/src/intel/genxml/gen75.xml
> @@ -2314,9 +2314,9 @@
>
>  
>   default="49"/>
> -
> -  
> -  
> +
> +  
> +  
>  
>  
>  
> diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
> index 4ed41d15612..330366b7ed0 100644
> --- a/src/intel/genxml/gen8.xml
> +++ b/src/intel/genxml/gen8.xml
> @@ -2553,9 +2553,9 @@
>
>  
>   default="49"/>
> -
> -  
> -  
> +
> +  
> +  
>  
>  
>  
> diff --git a/src/intel/vulkan/anv_batch_chain.c 
> b/src/intel/vulkan/anv_batch_chain.c
> index c47a81c8a4d..0f7c8325ea4 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer *cmd_buffer,
> anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
>bbs.DWordLength   = cmd_buffer->device->info.gen < 8 ?
>gen7_length : gen8_length;
> -  bbs._2ndLevelBatchBuffer  = _1stlevelbatch;
> +  bbs.SecondLevelBatchBuffer= Firstlevelbatch;
>bbs.AddressSpaceIndicator = ASI_PPGTT;
>bbs.BatchBufferStartAddress   = (struct anv_address) { bo, offset };
> }
-- 
Br,

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

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

[Andres Gomez]

Lionel, should we also include this in the stable queues ?


On Sat, 2018-08-25 at 18:23 +0100, Lionel Landwerlin 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))
-- 
Br,

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

Re: [Mesa-dev] [PATCH] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Andres Gomez]

Vadym, should we also include this in the stable queues ?


On Mon, 2018-08-20 at 16:31 +0300, vadym.shovkoplias wrote:
> From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
> 
> "Only the input variables that are actually read need to be written
>  by the previous stage; it is allowed to have superfluous
>  declarations of input variables."
> 
> Fixes:
> * interstage-multiple-shader-objects.shader_test
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
> Signed-off-by: Vadym Shovkoplias 
> ---
>  src/compiler/glsl/link_interface_blocks.cpp | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compiler/glsl/link_interface_blocks.cpp 
> b/src/compiler/glsl/link_interface_blocks.cpp
> index e5eca9460e..801fbcd5d9 100644
> --- a/src/compiler/glsl/link_interface_blocks.cpp
> +++ b/src/compiler/glsl/link_interface_blocks.cpp
> @@ -417,9 +417,15 @@ validate_interstage_inout_blocks(struct 
> gl_shader_program *prog,
> * write to any of the pre-defined outputs (e.g. if the vertex shader
> * does not write to gl_Position, etc), which is allowed and results in
> * undefined behavior.
> +   *
> +   * From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
> +   *
> +   *"Only the input variables that are actually read need to be 
> written
> +   * by the previous stage; it is allowed to have superfluous
> +   * declarations of input variables."
> */
>if (producer_def == NULL &&
> -  !is_builtin_gl_in_block(var, consumer->Stage)) {
> +  !is_builtin_gl_in_block(var, consumer->Stage) && var->data.used) {
>   linker_error(prog, "Input block `%s' is not an output of "
>"the previous stage\n", 
> var->get_interface_type()->name);
>   return;
-- 
Br,

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

[Mesa-dev] [PATCH] gallivm/radeonsi: allow to pass two swizzles into fetches.

[Dave Airlie]

From: Dave Airlie 

This hijacks the top 16-bits of swizzle, to pass in the swizzle
for the second channel.

This fixes handling .yx swizzles of 64-bit values.

This should fixup radeonsi and llvmpipe.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107524
---
 src/gallium/auxiliary/gallivm/lp_bld_tgsi.c   |  9 ++
 .../auxiliary/gallivm/lp_bld_tgsi_soa.c   | 86 ---
 src/gallium/drivers/radeonsi/si_shader.c  |  7 +-
 .../drivers/radeonsi/si_shader_tgsi_setup.c   | 18 ++--
 4 files changed, 79 insertions(+), 41 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c 
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
index 64d2cd703be..2c3be8fb127 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
@@ -353,6 +353,15 @@ lp_build_emit_fetch_src(
  assert(0 && "invalid swizzle in emit_fetch()");
  return bld_base->base.undef;
   }
+  if (tgsi_type_is_64bit(stype)) {
+unsigned swizzle2;
+swizzle2 = tgsi_util_get_full_src_register_swizzle(reg, chan_index + 
1);
+if (swizzle2 > 3) {
+   assert(0 && "invalid swizzle in emit_fetch()");
+   return bld_base->base.undef;
+}
+swizzle |= (swizzle2 << 16);
+  }
}
 
assert(reg->Register.Index <= bld_base->info->file_max[reg->Register.File]);
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c 
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
index 83d7dbea9a2..79ece639e35 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
@@ -1190,7 +1190,7 @@ emit_fetch_constant(
struct lp_build_tgsi_context * bld_base,
const struct tgsi_full_src_register * reg,
enum tgsi_opcode_type stype,
-   unsigned swizzle)
+   unsigned swizzle_in)
 {
struct lp_build_tgsi_soa_context * bld = lp_soa_context(bld_base);
struct gallivm_state *gallivm = bld_base->base.gallivm;
@@ -1200,6 +1200,7 @@ emit_fetch_constant(
LLVMValueRef consts_ptr;
LLVMValueRef num_consts;
LLVMValueRef res;
+   unsigned swizzle = swizzle_in & 0x;
 
/* XXX: Handle fetching xyzw components as a vector */
assert(swizzle != ~0u);
@@ -1241,7 +1242,7 @@ emit_fetch_constant(
 
   if (tgsi_type_is_64bit(stype)) {
  LLVMValueRef swizzle_vec2;
- swizzle_vec2 = lp_build_const_int_vec(gallivm, uint_bld->type, 
swizzle + 1);
+ swizzle_vec2 = lp_build_const_int_vec(gallivm, uint_bld->type, 
swizzle_in >> 16);
  index_vec2 = lp_build_shl_imm(uint_bld, indirect_index, 2);
  index_vec2 = lp_build_add(uint_bld, index_vec2, swizzle_vec2);
   }
@@ -1256,21 +1257,42 @@ emit_fetch_constant(
 
   scalar_ptr = LLVMBuildGEP(builder, consts_ptr,
 , 1, "");
-  if (stype == TGSI_TYPE_DOUBLE) {
- LLVMTypeRef dptr_type = 
LLVMPointerType(LLVMDoubleTypeInContext(gallivm->context), 0);
- scalar_ptr = LLVMBuildBitCast(builder, scalar_ptr, dptr_type, "");
- bld_broad = _base->dbl_bld;
-  } else if (stype == TGSI_TYPE_UNSIGNED64) {
- LLVMTypeRef u64ptr_type = 
LLVMPointerType(LLVMInt64TypeInContext(gallivm->context), 0);
- scalar_ptr = LLVMBuildBitCast(builder, scalar_ptr, u64ptr_type, "");
- bld_broad = _base->uint64_bld;
-  } else if (stype == TGSI_TYPE_SIGNED64) {
- LLVMTypeRef i64ptr_type = 
LLVMPointerType(LLVMInt64TypeInContext(gallivm->context), 0);
- scalar_ptr = LLVMBuildBitCast(builder, scalar_ptr, i64ptr_type, "");
- bld_broad = _base->int64_bld;
+
+  if (tgsi_type_is_64bit(stype) && ((swizzle_in >> 16) != swizzle + 1)) {
+
+ LLVMValueRef scalar2, scalar2_ptr;
+ LLVMValueRef shuffles[2];
+ index = lp_build_const_int32(gallivm, reg->Register.Index * 4 + 
(swizzle_in >> 16));
+
+ scalar2_ptr = LLVMBuildGEP(builder, consts_ptr,
+, 1, "");
+
+ scalar = LLVMBuildLoad(builder, scalar_ptr, "");
+ scalar2 = LLVMBuildLoad(builder, scalar2_ptr, "");
+ shuffles[0] = lp_build_const_int32(gallivm, 0);
+ shuffles[1] = lp_build_const_int32(gallivm, 1);
+
+ res = 
LLVMGetUndef(LLVMVectorType(LLVMFloatTypeInContext(gallivm->context), 
bld_base->base.type.length * 2));
+ res = LLVMBuildInsertElement(builder, res, scalar, shuffles[0], "");
+ res = LLVMBuildInsertElement(builder, res, scalar2, shuffles[1], "");
+  } else {
+if (stype == TGSI_TYPE_DOUBLE) {
+   LLVMTypeRef dptr_type = 
LLVMPointerType(LLVMDoubleTypeInContext(gallivm->context), 0);
+   scalar_ptr = LLVMBuildBitCast(builder, scalar_ptr, dptr_type, "");
+   bld_broad = _base->dbl_bld;
+} else if (stype == TGSI_TYPE_UNSIGNED64) {
+   LLVMTypeRef u64ptr_type = 
LLVMPointerType(LLVMInt64TypeInContext(gallivm->context), 0);
+   scalar_ptr = 

[Mesa-dev] [PATCH 4/5 v2] i965/fs: Emit BRW_AOP_INC or BRW_AOP_DEC for imageAtomicAdd of +1 or -1

[Ian Romanick]

From: Ian Romanick 

v2: Refactor selection of atomic opcode to a separate function.
Suggested by Jason.

No changes on any other Intel platforms.

Skylake
total instructions in shared programs: 14304261 -> 14304241 (<.01%)
instructions in affected programs: 1625 -> 1605 (-1.23%)
helped: 4
HURT: 0
helped stats (abs) min: 1 max: 8 x̄: 5.00 x̃: 5
helped stats (rel) min: 1.01% max: 14.29% x̄: 5.86% x̃: 4.07%
95% mean confidence interval for instructions value: -10.66 0.66
95% mean confidence interval for instructions %-change: -15.91% 4.19%
Inconclusive result (value mean confidence interval includes 0).

total cycles in shared programs: 527531226 -> 527531194 (<.01%)
cycles in affected programs: 92204 -> 92172 (-0.03%)
helped: 2
HURT: 0

Haswell and Broadwell had similar results. (Broadwell shown)
total instructions in shared programs: 14615730 -> 14615710 (<.01%)
instructions in affected programs: 1838 -> 1818 (-1.09%)
helped: 4
HURT: 0
helped stats (abs) min: 1 max: 8 x̄: 5.00 x̃: 5
helped stats (rel) min: 0.89% max: 13.04% x̄: 5.37% x̃: 3.78%
95% mean confidence interval for instructions value: -10.66 0.66
95% mean confidence interval for instructions %-change: -14.59% 3.85%
Inconclusive result (value mean confidence interval includes 0).

Signed-off-by: Ian Romanick 
Reviewed-by: Caio Marcelo de Oliveira Filho 
---
 src/intel/compiler/brw_fs_nir.cpp | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/intel/compiler/brw_fs_nir.cpp 
b/src/intel/compiler/brw_fs_nir.cpp
index 467aee0393f..15f34326c58 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -3899,10 +3899,16 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , 
nir_intrinsic_instr *instr
   var->data.image.write_only ? GL_NONE : format);
   } else {
  int op;
+ unsigned num_srcs = info->num_srcs;
 
  switch (instr->intrinsic) {
  case nir_intrinsic_image_deref_atomic_add:
-op = BRW_AOP_ADD;
+assert(num_srcs == 4);
+
+op = get_op_for_atomic_add(instr, 3);
+
+if (op != BRW_AOP_ADD)
+   num_srcs = 3;
 break;
  case nir_intrinsic_image_deref_atomic_min:
 op = (get_image_base_type(type) == BRW_REGISTER_TYPE_D ?
@@ -3931,10 +3937,10 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , 
nir_intrinsic_instr *instr
 unreachable("Not reachable.");
  }
 
- const fs_reg src0 = (info->num_srcs >= 4 ?
+ const fs_reg src0 = (num_srcs >= 4 ?
   retype(get_nir_src(instr->src[3]), base_type) :
   fs_reg());
- const fs_reg src1 = (info->num_srcs >= 5 ?
+ const fs_reg src1 = (num_srcs >= 5 ?
   retype(get_nir_src(instr->src[4]), base_type) :
   fs_reg());
 
-- 
2.14.4

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

[Mesa-dev] [PATCH 2/5 v2] i965/fs: Emit BRW_AOP_INC or BRW_AOP_DEC for atomicAdd of +1 or -1

[Ian Romanick]

From: Ian Romanick 

Funny story... a single shader was hurt for instructions, spills, fills.
That same shader was also the most helped for cycles.  #GPUsAreWeird

No changes on any other Intel platform.

v2: Refactor selection of atomic opcode to a separate function.
Suggested by Jason.

Haswell, Broadwell, and Skylake had similar results. (Skylake shown)
total instructions in shared programs: 14304116 -> 14304261 (<.01%)
instructions in affected programs: 12776 -> 12921 (1.13%)
helped: 19
HURT: 1
helped stats (abs) min: 1 max: 16 x̄: 2.32 x̃: 1
helped stats (rel) min: 0.05% max: 7.27% x̄: 0.92% x̃: 0.55%
HURT stats (abs)   min: 189 max: 189 x̄: 189.00 x̃: 189
HURT stats (rel)   min: 4.87% max: 4.87% x̄: 4.87% x̃: 4.87%
95% mean confidence interval for instructions value: -12.83 27.33
95% mean confidence interval for instructions %-change: -1.57% 0.31%
Inconclusive result (value mean confidence interval includes 0).

total cycles in shared programs: 527552861 -> 527531226 (<.01%)
cycles in affected programs: 1459195 -> 1437560 (-1.48%)
helped: 16
HURT: 2
helped stats (abs) min: 2 max: 21328 x̄: 1353.69 x̃: 6
helped stats (rel) min: 0.01% max: 5.29% x̄: 0.36% x̃: 0.03%
HURT stats (abs)   min: 12 max: 12 x̄: 12.00 x̃: 12
HURT stats (rel)   min: 0.03% max: 0.03% x̄: 0.03% x̃: 0.03%
95% mean confidence interval for cycles value: -3699.81 1295.92
95% mean confidence interval for cycles %-change: -0.94% 0.30%
Inconclusive result (value mean confidence interval includes 0).

total spills in shared programs: 8025 -> 8033 (0.10%)
spills in affected programs: 208 -> 216 (3.85%)
helped: 1
HURT: 1

total fills in shared programs: 10989 -> 11040 (0.46%)
fills in affected programs: 444 -> 495 (11.49%)
helped: 1
HURT: 1

Ivy Bridge
total instructions in shared programs: 11709181 -> 11709153 (<.01%)
instructions in affected programs: 3505 -> 3477 (-0.80%)
helped: 3
HURT: 0
helped stats (abs) min: 1 max: 23 x̄: 9.33 x̃: 4
helped stats (rel) min: 0.11% max: 1.16% x̄: 0.63% x̃: 0.61%

total cycles in shared programs: 254741126 -> 254738801 (<.01%)
cycles in affected programs: 919067 -> 916742 (-0.25%)
helped: 3
HURT: 0
helped stats (abs) min: 21 max: 2144 x̄: 775.00 x̃: 160
helped stats (rel) min: 0.03% max: 0.90% x̄: 0.32% x̃: 0.03%

total spills in shared programs: 4536 -> 4533 (-0.07%)
spills in affected programs: 40 -> 37 (-7.50%)
helped: 1
HURT: 0

total fills in shared programs: 4819 -> 4813 (-0.12%)
fills in affected programs: 94 -> 88 (-6.38%)
helped: 1
HURT: 0

Signed-off-by: Ian Romanick 
Reviewed-by: Caio Marcelo de Oliveira Filho  [v1]
---
 src/intel/compiler/brw_fs_nir.cpp | 27 +++
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/intel/compiler/brw_fs_nir.cpp 
b/src/intel/compiler/brw_fs_nir.cpp
index 9c9df5ac09f..67c39a661ec 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -3604,6 +3604,21 @@ fs_visitor::nir_emit_fs_intrinsic(const fs_builder ,
}
 }
 
+static int
+get_op_for_atomic_add(nir_intrinsic_instr *instr, unsigned src)
+{
+   const nir_const_value *const val = nir_src_as_const_value(instr->src[src]);
+
+   if (val != NULL) {
+  if (val->i32[0] == 1)
+ return BRW_AOP_INC;
+  else if (val->i32[0] == -1)
+ return BRW_AOP_DEC;
+   }
+
+   return BRW_AOP_ADD;
+}
+
 void
 fs_visitor::nir_emit_cs_intrinsic(const fs_builder ,
   nir_intrinsic_instr *instr)
@@ -3660,7 +3675,7 @@ fs_visitor::nir_emit_cs_intrinsic(const fs_builder ,
}
 
case nir_intrinsic_shared_atomic_add:
-  nir_emit_shared_atomic(bld, BRW_AOP_ADD, instr);
+  nir_emit_shared_atomic(bld, get_op_for_atomic_add(instr, 1), instr);
   break;
case nir_intrinsic_shared_atomic_imin:
   nir_emit_shared_atomic(bld, BRW_AOP_IMIN, instr);
@@ -4378,7 +4393,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , 
nir_intrinsic_instr *instr
}
 
case nir_intrinsic_ssbo_atomic_add:
-  nir_emit_ssbo_atomic(bld, BRW_AOP_ADD, instr);
+  nir_emit_ssbo_atomic(bld, get_op_for_atomic_add(instr, 2), instr);
   break;
case nir_intrinsic_ssbo_atomic_imin:
   nir_emit_ssbo_atomic(bld, BRW_AOP_IMIN, instr);
@@ -4888,7 +4903,9 @@ fs_visitor::nir_emit_ssbo_atomic(const fs_builder ,
}
 
fs_reg offset = get_nir_src(instr->src[1]);
-   fs_reg data1 = get_nir_src(instr->src[2]);
+   fs_reg data1;
+   if (op != BRW_AOP_INC && op != BRW_AOP_DEC && op != BRW_AOP_PREDEC)
+  data1 = get_nir_src(instr->src[2]);
fs_reg data2;
if (op == BRW_AOP_CMPWR)
   data2 = get_nir_src(instr->src[3]);
@@ -4962,7 +4979,9 @@ fs_visitor::nir_emit_shared_atomic(const fs_builder ,
 
fs_reg surface = brw_imm_ud(GEN7_BTI_SLM);
fs_reg offset;
-   fs_reg data1 = get_nir_src(instr->src[1]);
+   fs_reg data1;
+   if (op != BRW_AOP_INC && op != BRW_AOP_DEC && op != BRW_AOP_PREDEC)
+  data1 = get_nir_src(instr->src[1]);
fs_reg data2;
if (op == BRW_AOP_CMPWR)
   data2 

Re: [Mesa-dev] [PATCH 2/5] i965/fs: Emit BRW_AOP_INC or BRW_AOP_DEC for atomicAdd of +1 or -1

[Ian Romanick]

On 08/26/2018 05:21 AM, Jason Ekstrand wrote:
> On August 24, 2018 22:49:14 "Ian Romanick"  wrote:
> 
>> From: Ian Romanick 
>>
>> Funny story... a single shader was hurt for instructions, spills, fills.
>> That same shader was also the most helped for cycles.  #GPUsAreWeird
>>
>> No changes on any other Intel platform.
>>
>> Haswell, Broadwell, and Skylake had similar results. (Skylake shown)
>> total instructions in shared programs: 14304116 -> 14304261 (<.01%)
>> instructions in affected programs: 12776 -> 12921 (1.13%)
>> helped: 19
>> HURT: 1
>> helped stats (abs) min: 1 max: 16 x̄: 2.32 x̃: 1
>> helped stats (rel) min: 0.05% max: 7.27% x̄: 0.92% x̃: 0.55%
>> HURT stats (abs)   min: 189 max: 189 x̄: 189.00 x̃: 189
>> HURT stats (rel)   min: 4.87% max: 4.87% x̄: 4.87% x̃: 4.87%
>> 95% mean confidence interval for instructions value: -12.83 27.33
>> 95% mean confidence interval for instructions %-change: -1.57% 0.31%
>> Inconclusive result (value mean confidence interval includes 0).
>>
>> total cycles in shared programs: 527552861 -> 527531226 (<.01%)
>> cycles in affected programs: 1459195 -> 1437560 (-1.48%)
>> helped: 16
>> HURT: 2
>> helped stats (abs) min: 2 max: 21328 x̄: 1353.69 x̃: 6
>> helped stats (rel) min: 0.01% max: 5.29% x̄: 0.36% x̃: 0.03%
>> HURT stats (abs)   min: 12 max: 12 x̄: 12.00 x̃: 12
>> HURT stats (rel)   min: 0.03% max: 0.03% x̄: 0.03% x̃: 0.03%
>> 95% mean confidence interval for cycles value: -3699.81 1295.92
>> 95% mean confidence interval for cycles %-change: -0.94% 0.30%
>> Inconclusive result (value mean confidence interval includes 0).
>>
>> total spills in shared programs: 8025 -> 8033 (0.10%)
>> spills in affected programs: 208 -> 216 (3.85%)
>> helped: 1
>> HURT: 1
>>
>> total fills in shared programs: 10989 -> 11040 (0.46%)
>> fills in affected programs: 444 -> 495 (11.49%)
>> helped: 1
>> HURT: 1
>>
>> Ivy Bridge
>> total instructions in shared programs: 11709181 -> 11709153 (<.01%)
>> instructions in affected programs: 3505 -> 3477 (-0.80%)
>> helped: 3
>> HURT: 0
>> helped stats (abs) min: 1 max: 23 x̄: 9.33 x̃: 4
>> helped stats (rel) min: 0.11% max: 1.16% x̄: 0.63% x̃: 0.61%
>>
>> total cycles in shared programs: 254741126 -> 254738801 (<.01%)
>> cycles in affected programs: 919067 -> 916742 (-0.25%)
>> helped: 3
>> HURT: 0
>> helped stats (abs) min: 21 max: 2144 x̄: 775.00 x̃: 160
>> helped stats (rel) min: 0.03% max: 0.90% x̄: 0.32% x̃: 0.03%
>>
>> total spills in shared programs: 4536 -> 4533 (-0.07%)
>> spills in affected programs: 40 -> 37 (-7.50%)
>> helped: 1
>> HURT: 0
>>
>> total fills in shared programs: 4819 -> 4813 (-0.12%)
>> fills in affected programs: 94 -> 88 (-6.38%)
>> helped: 1
>> HURT: 0
>>
>> Signed-off-by: Ian Romanick 
>> ---
>> src/intel/compiler/brw_fs_nir.cpp | 38
>> --
>> 1 file changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/intel/compiler/brw_fs_nir.cpp
>> b/src/intel/compiler/brw_fs_nir.cpp
>> index 9c9df5ac09f..a2c3d715380 100644
>> --- a/src/intel/compiler/brw_fs_nir.cpp
>> +++ b/src/intel/compiler/brw_fs_nir.cpp
>> @@ -3659,9 +3659,20 @@ fs_visitor::nir_emit_cs_intrinsic(const
>> fs_builder ,
>>   break;
>>    }
>>
>> -   case nir_intrinsic_shared_atomic_add:
>> -  nir_emit_shared_atomic(bld, BRW_AOP_ADD, instr);
>> +   case nir_intrinsic_shared_atomic_add: {
>> +  int op = BRW_AOP_ADD;
>> +  const nir_const_value *const val =
>> nir_src_as_const_value(instr->src[1]);
>> +
>> +  if (val != NULL) {
>> + if (val->i32[0] == 1)
>> +    op = BRW_AOP_INC;
>> + else if (val->i32[0] == -1)
>> +    op = BRW_AOP_DEC;
>> +  }
> 
> Given the repeated pattern in this and the other patches, would it make
> sense to add a little brw_atomic_add_op helper that takes a nir_src? 
> I'm not sure exactly how that would work with adjusting the number of
> surfed everywhere though.  Feel free to disregard if it doesn't make sense.

I had thought of that, then dismissed it... but that was when I was
thinking there was only one place to do this.  Now that it has grown to
3, I think this is a good idea.

> Another thought (more of an FYI, really) is that I had considered doing
> this as a NIR lowering pass at one point. Not sure if anyone else has
> hardware inc/dec operations though.  You don't need to do anything with
> that information; I'm just throwing it out there.

I thought about that too... I don't know.  It would be more work to
achieve basically the same result.  It might be possible to have it
detect cases like atomicAdd(a, -1) - 1 to generate BRW_AOP_PREDEC.

> --Jason
> 
>> +
>> +  nir_emit_shared_atomic(bld, op, instr);
>>   break;
>> +   }
>>    case nir_intrinsic_shared_atomic_imin:
>>   nir_emit_shared_atomic(bld, BRW_AOP_IMIN, instr);
>>   break;
>> @@ -4377,9 +4388,20 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
>> , nir_intrinsic_instr *instr
>>   break;
>>    }
>>
>> -   case 

[Mesa-dev] [Bug 107698] Ubuntu 18.10 kernel 4.18rc1-4.18.6 or kernel 4.19rc1

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107698

--- Comment #3 from efeli...@yahoo.com ---
When I boot ubuntu 18.10 with kernel 4.18rc1-4.18.6 or kernel 4.19rc1 , the
result is black screen.

-- 
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

[Mesa-dev] [Bug 107699] Account and Access request

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107699

--- Comment #1 from Sagar Ghuge  ---
Created attachment 141302
  --> https://bugs.freedesktop.org/attachment.cgi?id=141302=edit
SSH public Key

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

[Mesa-dev] [Bug 107699] Account and Access request

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107699

Bug ID: 107699
   Summary: Account and Access request
   Product: Mesa
   Version: unspecified
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: ghuge...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 141301
  --> https://bugs.freedesktop.org/attachment.cgi?id=141301=edit
GPG Key

Real Name: Sagar Ghuge
E-mail: sagar.gh...@intel.com
Preferred account name: sghuge


Hello,

I'm asking for an account and access rights in order to contribute to Mesa
project.

Thank you.

-- 
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 v2] intel/tools: new i965_disasm tool

[Sagar Ghuge]

Hey Matt,

Thanks for reviewing the patch. 

On 08/27/2018 11:06 AM, Matt Turner wrote:
> On Mon, Aug 20, 2018 at 3:13 PM Sagar Ghuge  wrote:
>>
>> Adds a new i965 instruction disassemble tool
>>
>> v2: 1) fix a few nits (Matt Turner)
>> 2) Remove i965_disasm header (Matt Turner)
>>
>> Signed-off-by: Sagar Ghuge 
>> ---
>>  src/intel/Makefile.tools.am   |  14 +++
>>  src/intel/tools/i965_disasm.c | 199 ++
>>  src/intel/tools/meson.build   |  11 ++
>>  3 files changed, 224 insertions(+)
>>  create mode 100644 src/intel/tools/i965_disasm.c
>>
>> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
>> index 00624084e6..385819abc2 100644
>> --- a/src/intel/Makefile.tools.am
>> +++ b/src/intel/Makefile.tools.am
>> @@ -22,6 +22,7 @@
>>  noinst_PROGRAMS += \
>> tools/aubinator \
>> tools/aubinator_error_decode \
>> +   tools/i965_disasm \
>> tools/error2aub
>>
>>
>> @@ -62,6 +63,19 @@ tools_aubinator_error_decode_CFLAGS = \
>> $(AM_CFLAGS) \
>> $(ZLIB_CFLAGS)
>>
>> +tools_i965_disasm_SOURCES = \
>> +   tools/i965_disasm.c
>> +
>> +tools_i965_disasm_LDADD = \
>> +   common/libintel_common.la \
>> +   compiler/libintel_compiler.la \
>> +   dev/libintel_dev.la \
>> +   $(top_builddir)/src/util/libmesautil.la \
>> +   $(PTHREAD_LIBS)
>> +
>> +tools_i965_disasm_CFLAGS = \
>> +   $(AM_CFLAGS)
>> +
>>
>>  tools_error2aub_SOURCES = \
>> tools/gen_context.h \
>> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
>> new file mode 100644
>> index 00..757d2c7db1
>> --- /dev/null
>> +++ b/src/intel/tools/i965_disasm.c
>> @@ -0,0 +1,199 @@
>> +/*
>> + * Copyright © 2018 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 "compiler/brw_inst.h"
>> +#include "compiler/brw_eu.h"
>> +#include "dev/gen_device_info.h"
>> +
>> +uint64_t INTEL_DEBUG;
>> +uint16_t pci_id = 0;
>> +FILE *outfile;
> 
> I expected to find code that opens a file and sets outfile, but
> outfile is only set to stderr. I'd either add a -o option or get rid
> of the outfile variable entirely. Either way works, but in both cases
> error messages should be printed to stderr instead of outfile since
> you wouldn't want errors intermixed with the disassembly.
> 
> So either:
> 
>1) add a -o out option (defaulting outfile to stdout) and change
> errors to be printed to stderr; or
> 
>2) remove the outfile variable, output errors to stderr and
> disassembly to stdout
> 
> If you choose to keep outfile, I think it can become a local variable in 
> main().
> 
>> +
>> +struct i965_disasm {
>> +struct gen_device_info devinfo;
>> +};
> 
> I think we should simplify some things by removing the i965_disasm type.
> 
>> +
>> +/* Return size of file in bytes pointed by fp */
>> +static size_t
>> +i965_disasm_get_file_size(FILE *fp)
>> +{
>> +   size_t size;
>> +
>> +   fseek(fp, 0L, SEEK_END);
>> +   size = ftell(fp);
>> +   fseek(fp, 0L, SEEK_SET);
>> +
>> +   return size;
>> +}
>> +
>> +/* Return number of bytes read */
>> +static size_t
>> +i965_disasm_read_binary(FILE *fp, void **assembly)
>> +{
>> +   size_t end = i965_disasm_get_file_size(fp);
>> +   *assembly = malloc(end + 1);
>> +   fread(*assembly, end, 1, fp);
>> +   fclose(fp);
>> +
>> +   return end;
>> +}
> 
> I think it looks more natural to return assembly and return 'end' via
> an out-parameter. I think we do that elsewhere in the compiler or i965
> driver but I can't find where at the moment.
> I agree. 
>> +
>> +/* Disassemble i965 instructions from buffer assembly
>> + * start : starting offset within buffer
>> + * end : points to last byte of buffer
>> + */
>> +static void
>> +i965_disasm_disassemble(struct i965_disasm *disasm, 

[Mesa-dev] [Bug 107655] X segfaults on startup in r300_dri.so, making system unusable

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107655

--- Comment #7 from mirh  ---
(In reply to Ilia Mirkin from comment #6)
> The intention (and original function) was that this would be safe -- swr
> would check for AVX support and load the relevant library (libAVX/libAVX2),
> or bail on load.
> 
> Something must have regressed in there and no one noticed since AVX has
> become fairly common.

Or, I mean, more easily it might be a problem with its special build root?
AVX might be common nowadays, but not *that* much (especially considering all
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=mesa-git#n48

-- 
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 1/2] meson: Be a bit more helpful when arch or OS is unknown

[Dylan Baker]

Quoting Guido Günther (2018-08-27 09:32:54)
> Hi,
> On Mon, Aug 27, 2018 at 09:23:44AM -0700, Dylan Baker wrote:
> > Quoting Guido Günther (2018-08-26 13:23:59)
> > > V2: Add one missing @0@
> > > 
> > > Signed-off-by: Guido Günther 
> > > ---
> > >  meson.build | 21 ++---
> > >  1 file changed, 14 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index 1b3dfa221c9..c3a7e8cdd74 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -102,13 +102,15 @@ if _drivers.contains('auto')
> > >  elif ['arm', 'aarch64'].contains(host_machine.cpu_family())
> > >_drivers = []
> > >  else
> > > -  error('Unknown architecture. Please pass -Ddri-drivers to set 
> > > driver options. Patches gladly accepted to fix this.')
> > > +  error('Unknown architecture @0@. Please pass -Ddri-drivers to set 
> > > driver options. Patches gladly accepted to fix this.'.format(
> > > +host_machine.cpu_family()))
> > >  endif
> > >elif ['darwin', 'windows', 'cygwin', 
> > > 'haiku'].contains(host_machine.system())
> > >  # only swrast would make sense here, but gallium swrast is a much 
> > > better default
> > >  _drivers = []
> > >else
> > > -error('Unknown OS. Please pass -Ddri-drivers to set driver options. 
> > > Patches gladly accepted to fix this.')
> > > +error('Unknown OS @0@. Please pass -Ddri-drivers to set driver 
> > > options. Patches gladly accepted to fix this.'.format(
> > > +  host_machine.system()))
> > >endif
> > >  endif
> > >  
> > > @@ -135,12 +137,14 @@ if _drivers.contains('auto')
> > >  'tegra', 'virgl', 'swrast',
> > >]
> > >  else
> > > -  error('Unknown architecture. Please pass -Dgallium-drivers to set 
> > > driver options. Patches gladly accepted to fix this.')
> > > +  error('Unknown architecture @0@. Please pass -Dgallium-drivers to 
> > > set driver options. Patches gladly accepted to fix this.'.format(
> > > +host_machine.cpu_family()))
> > >  endif
> > >elif ['darwin', 'windows', 'cygwin', 
> > > 'haiku'].contains(host_machine.system())
> > >  _drivers = ['swrast']
> > >else
> > > -error('Unknown OS. Please pass -Dgallium-drivers to set driver 
> > > options. Patches gladly accepted to fix this.')
> > > +error('Unknown OS @0@. Please pass -Dgallium-drivers to set driver 
> > > options. Patches gladly accepted to fix this.'.format(
> > > +  host_machine.system()))
> > >endif
> > >  endif
> > >  with_gallium_pl111 = _drivers.contains('pl111')
> > > @@ -176,13 +180,15 @@ if _vulkan_drivers.contains('auto')
> > >  if host_machine.cpu_family().startswith('x86')
> > >_vulkan_drivers = ['amd', 'intel']
> > >  else
> > > -  error('Unknown architecture. Please pass -Dvulkan-drivers to set 
> > > driver options. Patches gladly accepted to fix this.')
> > > +  error('Unknown architecture @0@. Please pass -Dvulkan-drivers to 
> > > set driver options. Patches gladly accepted to fix this.'.format(
> > > +host_machine.cpu_family()))
> > >  endif
> > >elif ['darwin', 'windows', 'cygwin', 
> > > 'haiku'].contains(host_machine.system())
> > >  # No vulkan driver supports windows or macOS currently
> > >  _vulkan_drivers = []
> > >else
> > > -error('Unknown OS. Please pass -Dvulkan-drivers to set driver 
> > > options. Patches gladly accepted to fix this.')
> > > +error('Unknown OS @0@. Please pass -Dvulkan-drivers to set driver 
> > > options. Patches gladly accepted to fix this.'.format(
> > > +  host_machine.system()))
> > >endif
> > >  endif
> > >  
> > > @@ -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 @0@. Please pass -Dplatforms to set platforms. 
> > > Patches gladly accepted to fix this.'.format(
> > > +  host_machine.system()))
> > >endif
> > >  endif
> > >  
> > > -- 
> > > 2.18.0
> > > 
> > 
> > for the series,
> > Reviewed-by: Dylan Baker 
> 
> Thanks!
> 
> > 
> > Do you need me to push these for you?
> 
> Yes, please.
>  -- Guido
> 

Pushed. Thanks for this!


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

Re: [Mesa-dev] [PATCH 3/4] radeonsi: add radeonsi_zerovram driconfig option

[Marek Olšák]

On Fri, Aug 24, 2018 at 10:33 AM, Michel Dänzer  wrote:
> On 2018-08-24 1:06 p.m., Timothy Arceri wrote:
>> More and more games seem to require this so lets make it a config
>> option.
>> ---
>>  src/gallium/drivers/radeonsi/driinfo_radeonsi.h |  1 +
>>  src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c   | 10 +++---
>>  src/util/xmlpool/t_options.h|  5 +
>>  3 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h 
>> b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
>> index 7f57b4ea892..8c5078c13f3 100644
>> --- a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
>> +++ b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
>> @@ -3,6 +3,7 @@ DRI_CONF_SECTION_PERFORMANCE
>>  DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
>>  DRI_CONF_RADEONSI_ASSUME_NO_Z_FIGHTS("false")
>>  DRI_CONF_RADEONSI_COMMUTATIVE_BLEND_ADD("false")
>> +DRI_CONF_RADEONSI_ZERO_ALL_VRAM_ALLOCS("false")
>>  DRI_CONF_SECTION_END
>>
>>  [...]
>>
>> @@ -414,3 +414,8 @@ DRI_CONF_OPT_END
>>  DRI_CONF_OPT_BEGIN_B(radeonsi_clear_db_cache_before_clear, def) \
>>  DRI_CONF_DESC(en,"Clear DB cache before fast depth clear") \
>>  DRI_CONF_OPT_END
>> +
>> +#define DRI_CONF_RADEONSI_ZERO_ALL_VRAM_ALLOCS(def) \
>> +DRI_CONF_OPT_BEGIN_B(radeonsi_zerovram, def) \
>> +DRI_CONF_DESC(en,"Zero all vram allocations") \
>> +DRI_CONF_OPT_END
>>
>
> I'd name the option simply "zerovram", so it could be used by other
> drivers as well.
>
>
> BTW, AFAICT, currently this only affects BOs allocated from the kernel,
> not those re-used from the BO cache. I wonder if that couldn't still
> cause trouble with some apps.

It could. Anyway:

Reviewed-by: Marek Olšák 

Marek

>
>
> --
> Earthling Michel Dänzer   |   http://www.amd.com
> Libre software enthusiast | Mesa and X developer
> ___
> 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/4] radeonsi: enable radeonsi_zerovram for No Mans Sky

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Fri, Aug 24, 2018 at 7:06 AM, Timothy Arceri  wrote:
> ---
>  src/util/00-mesa-defaults.conf | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/util/00-mesa-defaults.conf b/src/util/00-mesa-defaults.conf
> index ad59efba50b..5d15b3819fb 100644
> --- a/src/util/00-mesa-defaults.conf
> +++ b/src/util/00-mesa-defaults.conf
> @@ -322,5 +322,8 @@ TODO: document the other workarounds.
>  
>   />
>  
> +
> +
> +
>  
>  
> --
> 2.17.1
>
> ___
> 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 2/4] radeonsi: enable GL 4.5 in compat profile

[Marek Olšák]

Reviewed-by: Marek Olšák 

BTW, this is conditional on ARB_direct_state_access being implemented properly.

Marek

On Fri, Aug 24, 2018 at 7:06 AM, Timothy Arceri  wrote:
> ---
>  src/gallium/drivers/radeonsi/si_get.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_get.c 
> b/src/gallium/drivers/radeonsi/si_get.c
> index 47368fb7c91..f4c61a7e408 100644
> --- a/src/gallium/drivers/radeonsi/si_get.c
> +++ b/src/gallium/drivers/radeonsi/si_get.c
> @@ -184,8 +184,7 @@ static int si_get_param(struct pipe_screen *pscreen, enum 
> pipe_cap param)
> case PIPE_CAP_GLSL_FEATURE_LEVEL:
> case PIPE_CAP_GLSL_FEATURE_LEVEL_COMPATIBILITY:
> if (sscreen->info.has_indirect_compute_dispatch)
> -   return param == PIPE_CAP_GLSL_FEATURE_LEVEL ?
> -   450 : 440;
> +   return 450;
> return 420;
>
> case PIPE_CAP_MAX_TEXTURE_BUFFER_SIZE:
> --
> 2.17.1
>
> ___
> 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/4] mesa: enable ARB_direct_state_access in compat for GL3.1+

[Marek Olšák]

_mesa_VertexArrayVertexBuffer_no_error crashes if vaobj == 0.
_mesa_VertexArrayVertexBuffer doesn't have this issue.

Marek

On Fri, Aug 24, 2018 at 7:06 AM, Timothy Arceri  wrote:
> We could enable it for lower versions of GL but this allows us
> to just use the existing version/extension checks that are already
> used by the core profile.
> ---
>  src/mapi/glapi/gen/apiexec.py| 194 +++
>  src/mesa/main/extensions_table.h |   2 +-
>  src/mesa/main/fbobject.c |  13 ++-
>  3 files changed, 105 insertions(+), 104 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/apiexec.py b/src/mapi/glapi/gen/apiexec.py
> index b163d88549b..e2fc124be22 100644
> --- a/src/mapi/glapi/gen/apiexec.py
> +++ b/src/mapi/glapi/gen/apiexec.py
> @@ -152,103 +152,103 @@ functions = {
>
>  # OpenGL 4.5 / GL_ARB_direct_state_access.   Mesa can expose the 
> extension
>  # with core profile.
> -"CreateTransformFeedbacks": exec_info(compatibility=45, core=31),
> -"TransformFeedbackBufferBase": exec_info(compatibility=45, core=31),
> -"TransformFeedbackBufferRange": exec_info(compatibility=45, core=31),
> -"GetTransformFeedbackiv": exec_info(compatibility=45, core=31),
> -"GetTransformFeedbacki_v": exec_info(compatibility=45, core=31),
> -"GetTransformFeedbacki64_v": exec_info(compatibility=45, core=31),
> -"CreateBuffers": exec_info(compatibility=45, core=31),
> -"NamedBufferStorage": exec_info(compatibility=45, core=31),
> -"NamedBufferData": exec_info(compatibility=45, core=31),
> -"NamedBufferSubData": exec_info(compatibility=45, core=31),
> -"CopyNamedBufferSubData": exec_info(compatibility=45, core=31),
> -"ClearNamedBufferData": exec_info(compatibility=45, core=31),
> -"ClearNamedBufferSubData": exec_info(compatibility=45, core=31),
> -"MapNamedBuffer": exec_info(compatibility=45, core=31),
> -"MapNamedBufferRange": exec_info(compatibility=45, core=31),
> -"UnmapNamedBuffer": exec_info(compatibility=45, core=31),
> -"FlushMappedNamedBufferRange": exec_info(compatibility=45, core=31),
> -"GetNamedBufferParameteriv": exec_info(compatibility=45, core=31),
> -"GetNamedBufferParameteri64v": exec_info(compatibility=45, core=31),
> -"GetNamedBufferPointerv": exec_info(compatibility=45, core=31),
> -"GetNamedBufferSubData": exec_info(compatibility=45, core=31),
> -"CreateFramebuffers": exec_info(compatibility=45, core=31),
> -"NamedFramebufferRenderbuffer": exec_info(compatibility=45, core=31),
> -"NamedFramebufferParameteri": exec_info(compatibility=45, core=31),
> -"NamedFramebufferTexture": exec_info(compatibility=45, core=31),
> -"NamedFramebufferTextureLayer": exec_info(compatibility=45, core=31),
> -"NamedFramebufferDrawBuffer": exec_info(compatibility=45, core=31),
> -"NamedFramebufferDrawBuffers": exec_info(compatibility=45, core=31),
> -"NamedFramebufferReadBuffer": exec_info(compatibility=45, core=31),
> -"InvalidateNamedFramebufferData": exec_info(compatibility=45, core=31),
> -"InvalidateNamedFramebufferSubData": exec_info(compatibility=45, 
> core=31),
> -"ClearNamedFramebufferiv": exec_info(compatibility=45, core=31),
> -"ClearNamedFramebufferuiv": exec_info(compatibility=45, core=31),
> -"ClearNamedFramebufferfv": exec_info(compatibility=45, core=31),
> -"ClearNamedFramebufferfi": exec_info(compatibility=45, core=31),
> -"BlitNamedFramebuffer": exec_info(compatibility=45, core=31),
> -"CheckNamedFramebufferStatus": exec_info(compatibility=45, core=31),
> -"GetNamedFramebufferParameteriv": exec_info(compatibility=45, core=31),
> -"GetNamedFramebufferAttachmentParameteriv": exec_info(compatibility=45, 
> core=31),
> -"CreateRenderbuffers": exec_info(compatibility=45, core=31),
> -"NamedRenderbufferStorage": exec_info(compatibility=45, core=31),
> -"NamedRenderbufferStorageMultisample": exec_info(compatibility=45, 
> core=31),
> -"GetNamedRenderbufferParameteriv": exec_info(compatibility=45, core=31),
> -"CreateTextures": exec_info(compatibility=45, core=31),
> -"TextureBuffer": exec_info(compatibility=45, core=31),
> -"TextureBufferRange": exec_info(compatibility=45, core=31),
> -"TextureStorage1D": exec_info(compatibility=45, core=31),
> -"TextureStorage2D": exec_info(compatibility=45, core=31),
> -"TextureStorage3D": exec_info(compatibility=45, core=31),
> -"TextureStorage2DMultisample": exec_info(compatibility=45, core=31),
> -"TextureStorage3DMultisample": exec_info(compatibility=45, core=31),
> -"TextureSubImage1D": exec_info(compatibility=45, core=31),
> -"TextureSubImage2D": exec_info(compatibility=45, core=31),
> -"TextureSubImage3D": exec_info(compatibility=45, core=31),
> -"CompressedTextureSubImage1D": exec_info(compatibility=45, core=31),
> -"CompressedTextureSubImage2D": exec_info(compatibility=45, core=31),
> -"CompressedTextureSubImage3D": 

Re: [Mesa-dev] [Mesa-stable] [PATCH] mesa: allow GL_UNSIGNED_BYTE type for SNORM reads

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Mon, Aug 27, 2018 at 7:46 AM, Tapani Pälli  wrote:
> OpenGL ES spec states:
>"For normalized fixed-point rendering surfaces, the combination format
> RGBA and type UNSIGNED_BYTE is accepted."
>
> This fixes following failing VK-GL-CTS tests:
>
>KHR-GLES3.packed_pixels.pbo_rectangle.rgba8_snorm
>KHR-GLES3.packed_pixels.rectangle.rgba8_snorm
>KHR-GLES3.packed_pixels.varied_rectangle.rgba8_snorm
>
> Signed-off-by: Tapani Pälli 
> https://bugs.freedesktop.org/show_bug.cgi?id=107658
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>
> This is a partial fix to the bug. I believe there are 2 separate
> issues within reported bug and this fixes the first one.
>
>  src/mesa/main/readpix.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index 2cbb578a37f..556c860d393 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -958,6 +958,15 @@ read_pixels_es3_error_check(struct gl_context *ctx, 
> GLenum format, GLenum type,
> return GL_NO_ERROR;
>   }
>}
> +  if (type == GL_UNSIGNED_BYTE) {
> + switch (internalFormat) {
> + case GL_R8_SNORM:
> + case GL_RG8_SNORM:
> + case GL_RGBA8_SNORM:
> +if (_mesa_has_EXT_render_snorm(ctx))
> +   return GL_NO_ERROR;
> + }
> +  }
>break;
> case GL_BGRA:
>/* GL_EXT_read_format_bgra */
> --
> 2.14.4
>
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Sat, Aug 25, 2018 at 3:19 AM, Kenneth Graunke  wrote:
> 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] intel/tools: new i965_disasm tool

[Matt Turner]

On Mon, Aug 20, 2018 at 3:13 PM Sagar Ghuge  wrote:
>
> Adds a new i965 instruction disassemble tool
>
> v2: 1) fix a few nits (Matt Turner)
> 2) Remove i965_disasm header (Matt Turner)
>
> Signed-off-by: Sagar Ghuge 
> ---
>  src/intel/Makefile.tools.am   |  14 +++
>  src/intel/tools/i965_disasm.c | 199 ++
>  src/intel/tools/meson.build   |  11 ++
>  3 files changed, 224 insertions(+)
>  create mode 100644 src/intel/tools/i965_disasm.c
>
> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
> index 00624084e6..385819abc2 100644
> --- a/src/intel/Makefile.tools.am
> +++ b/src/intel/Makefile.tools.am
> @@ -22,6 +22,7 @@
>  noinst_PROGRAMS += \
> tools/aubinator \
> tools/aubinator_error_decode \
> +   tools/i965_disasm \
> tools/error2aub
>
>
> @@ -62,6 +63,19 @@ tools_aubinator_error_decode_CFLAGS = \
> $(AM_CFLAGS) \
> $(ZLIB_CFLAGS)
>
> +tools_i965_disasm_SOURCES = \
> +   tools/i965_disasm.c
> +
> +tools_i965_disasm_LDADD = \
> +   common/libintel_common.la \
> +   compiler/libintel_compiler.la \
> +   dev/libintel_dev.la \
> +   $(top_builddir)/src/util/libmesautil.la \
> +   $(PTHREAD_LIBS)
> +
> +tools_i965_disasm_CFLAGS = \
> +   $(AM_CFLAGS)
> +
>
>  tools_error2aub_SOURCES = \
> tools/gen_context.h \
> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
> new file mode 100644
> index 00..757d2c7db1
> --- /dev/null
> +++ b/src/intel/tools/i965_disasm.c
> @@ -0,0 +1,199 @@
> +/*
> + * Copyright © 2018 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 "compiler/brw_inst.h"
> +#include "compiler/brw_eu.h"
> +#include "dev/gen_device_info.h"
> +
> +uint64_t INTEL_DEBUG;
> +uint16_t pci_id = 0;
> +FILE *outfile;

I expected to find code that opens a file and sets outfile, but
outfile is only set to stderr. I'd either add a -o option or get rid
of the outfile variable entirely. Either way works, but in both cases
error messages should be printed to stderr instead of outfile since
you wouldn't want errors intermixed with the disassembly.

So either:

   1) add a -o out option (defaulting outfile to stdout) and change
errors to be printed to stderr; or

   2) remove the outfile variable, output errors to stderr and
disassembly to stdout

If you choose to keep outfile, I think it can become a local variable in main().

> +
> +struct i965_disasm {
> +struct gen_device_info devinfo;
> +};

I think we should simplify some things by removing the i965_disasm type.

> +
> +/* Return size of file in bytes pointed by fp */
> +static size_t
> +i965_disasm_get_file_size(FILE *fp)
> +{
> +   size_t size;
> +
> +   fseek(fp, 0L, SEEK_END);
> +   size = ftell(fp);
> +   fseek(fp, 0L, SEEK_SET);
> +
> +   return size;
> +}
> +
> +/* Return number of bytes read */
> +static size_t
> +i965_disasm_read_binary(FILE *fp, void **assembly)
> +{
> +   size_t end = i965_disasm_get_file_size(fp);
> +   *assembly = malloc(end + 1);
> +   fread(*assembly, end, 1, fp);
> +   fclose(fp);
> +
> +   return end;
> +}

I think it looks more natural to return assembly and return 'end' via
an out-parameter. I think we do that elsewhere in the compiler or i965
driver but I can't find where at the moment.

> +
> +/* Disassemble i965 instructions from buffer assembly
> + * start : starting offset within buffer
> + * end : points to last byte of buffer
> + */
> +static void
> +i965_disasm_disassemble(struct i965_disasm *disasm, void *assembly,
> +int start, int end, FILE *out)
> +{
> +   brw_disassemble(>devinfo, assembly, start, end, out);
> +}

I would remove this function and call brw_disassemble directly.

> +
> +static struct i965_disasm *
> +i965_disasm_init(void)

I would pass 

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

[Marek Olšák]

Yeah, this will be more complicated because it's per RT.

I suggest adding a PIPE_CAP for the hw capability to force DST_ALPHA
to 0, and applying this workaround only if the PIPE_CAP is 0.

Marek


On Sat, Aug 25, 2018 at 10:46 AM, Ilia Mirkin  wrote:
> 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965/gen7_urb: Re-emit PUSH_CONSTANT_ALLOC on GLK

[Nanley Chery]

On Fri, Aug 24, 2018 at 05:46:44PM -0700, Nanley Chery wrote:
> According to internal docs, some gen9 platforms have a pixel shader push
> constant synchronization issue. Although not listed among said
> platforms, this issue seems to be present on the GeminiLake 2x6's we've
> tested.
> 
> We consider the available workarounds to be too detrimental on
> performance. Instead, we mitigate the issue by applying part of one of
> the workarounds. Re-emit PUSH_CONSTANT_ALLOC at the top of every batch
> (as suggested by Ken).
> 
> Fixes ext_framebuffer_multisample-accuracy piglit test failures with the
> following options:
> * 6 depth_draw small depthstencil
> * 8 stencil_draw small depthstencil
> * 6 stencil_draw small depthstencil
> * 8 depth_resolve small
> * 6 stencil_resolve small depthstencil
> * 4 stencil_draw small depthstencil
> * 16 stencil_draw small depthstencil
> * 16 depth_draw small depthstencil
> * 2 stencil_resolve small depthstencil
> * 6 stencil_draw small
> * all_samples stencil_draw small
> * 2 depth_draw small depthstencil
> * all_samples depth_draw small depthstencil
> * all_samples stencil_resolve small
> * 4 depth_draw small depthstencil
> * all_samples depth_draw small
> * all_samples stencil_draw small depthstencil
> * 4 stencil_resolve small depthstencil
> * 4 depth_resolve small depthstencil
> * all_samples stencil_resolve small depthstencil
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106865
> Cc: 
> ---
>  src/mesa/drivers/dri/i965/gen7_urb.c | 23 +++
>  1 file changed, 23 insertions(+)

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

Re: [Mesa-dev] [PATCH 3/3] anv: Claim to support depthBounds for ID games

[Lionel Landwerlin]

On 23/08/2018 16:13, Jason Ekstrand wrote:

---
  src/intel/vulkan/anv_device.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 0357fc7c0ea..5a63592f7ca 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -854,6 +854,15 @@ void anv_GetPhysicalDeviceFeatures(
 pFeatures->vertexPipelineStoresAndAtomics =
pdevice->compiler->scalar_stage[MESA_SHADER_VERTEX] &&
pdevice->compiler->scalar_stage[MESA_SHADER_GEOMETRY];
+
+   struct anv_app_info *app_info = >instance->app_info;
+
+   /* The new DOOM and Wolfenstein games require depthBounds without
+* checking for it.  They seem to run fine without it so just claim it's
+* there and accept the consequences.
+*/
+   if (app_info->engine_name && strcmp(app_info->engine_name, "idTech") == 0)
+  pFeatures->depthBounds = true;
  }
  
  void anv_GetPhysicalDeviceFeatures2(


I'm struggling to understand "require depthBounds with checking for it".
I thought one would check for the feature by reading the boolean you 
just set. So if it's not checked why would it make a difference?


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

Re: [Mesa-dev] [PATCH v3] intel/eu: print bytes instead of 32 bit hex value

[Matt Turner]

Looks great!

Reviewed-by: Matt Turner 

I'll push it shortly.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3] intel/eu: print bytes instead of 32 bit hex value

[Sagar Ghuge]

INTEL_DEBUG=hex prints 32 bit hex value and due to endianness of CPU
byte order is reversed. In order to disassemble binary files, print
each byte instead of 32 bit hex value.

v2: Print blank spaces in order to vertically align output of compacted
instructions hex value with uncompacted instructions hex value.
(Matt Turner)

v3: Fix line wrap at correct length

Signed-off-by: Sagar Ghuge 
---
 src/intel/compiler/brw_eu.c | 47 +++--
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c
index 6ef0a6a577..3fb4e40507 100644
--- a/src/intel/compiler/brw_eu.c
+++ b/src/intel/compiler/brw_eu.c
@@ -364,24 +364,37 @@ brw_disassemble(const struct gen_device_info *devinfo,
 
   if (compacted) {
  brw_compact_inst *compacted = (void *)insn;
-if (dump_hex) {
-   fprintf(out, "0x%08x 0x%08x   ",
-   ((uint32_t *)insn)[1],
-   ((uint32_t *)insn)[0]);
-}
-
-brw_uncompact_instruction(devinfo, , compacted);
-insn = 
-offset += 8;
+ if (dump_hex) {
+unsigned char * insn_ptr = ((unsigned char *)[0]);
+const unsigned int blank_spaces = 24;
+for (int i = 0 ; i < 8; i = i + 4) {
+   fprintf(out, "%02x %02x %02x %02x ",
+   insn_ptr[i],
+   insn_ptr[i + 1],
+   insn_ptr[i + 2],
+   insn_ptr[i + 3]);
+}
+/* Make compacted instructions hex value output vertically aligned
+ * with uncompacted instructions hex value
+ */
+fprintf(out, "%*c", blank_spaces, ' ');
+ }
+
+ brw_uncompact_instruction(devinfo, , compacted);
+ insn = 
+ offset += 8;
   } else {
-if (dump_hex) {
-   fprintf(out, "0x%08x 0x%08x 0x%08x 0x%08x ",
-   ((uint32_t *)insn)[3],
-   ((uint32_t *)insn)[2],
-   ((uint32_t *)insn)[1],
-   ((uint32_t *)insn)[0]);
-}
-offset += 16;
+ if (dump_hex) {
+unsigned char * insn_ptr = ((unsigned char *)[0]);
+for (int i = 0 ; i < 16; i = i + 4) {
+   fprintf(out, "%02x %02x %02x %02x ",
+   insn_ptr[i],
+   insn_ptr[i + 1],
+   insn_ptr[i + 2],
+   insn_ptr[i + 3]);
+}
+ }
+ offset += 16;
   }
 
   brw_disassemble_inst(out, devinfo, insn, compacted);
-- 
2.17.1

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

[Mesa-dev] [Bug 107698] Ubuntu 18.10 kernel 4.18rc1-4.18.6 or kernel 4.19rc1

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107698

--- Comment #2 from Michel Dänzer  ---
This report is missing a description of the problem in prose.

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

[Mesa-dev] [Bug 107698] Ubuntu 18.10 kernel 4.18rc1-4.18.6 or kernel 4.19rc1

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107698

efeli...@yahoo.com changed:

   What|Removed |Added

 CC||efeli...@yahoo.com

--- Comment #1 from efeli...@yahoo.com ---
Created attachment 141298
  --> https://bugs.freedesktop.org/attachment.cgi?id=141298=edit
logs

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

[Mesa-dev] [Bug 107698] Ubuntu 18.10 kernel 4.18rc1-4.18.6 or kernel 4.19rc1

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107698

Bug ID: 107698
   Summary: Ubuntu 18.10 kernel 4.18rc1-4.18.6 or kernel 4.19rc1
   Product: Mesa
   Version: 18.2
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: Drivers/OSMesa
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: efeli...@yahoo.com
QA Contact: mesa-dev@lists.freedesktop.org

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
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

[Guido Günther]

Hi,
On Mon, Aug 27, 2018 at 09:23:44AM -0700, Dylan Baker wrote:
> Quoting Guido Günther (2018-08-26 13:23:59)
> > V2: Add one missing @0@
> > 
> > Signed-off-by: Guido Günther 
> > ---
> >  meson.build | 21 ++---
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index 1b3dfa221c9..c3a7e8cdd74 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -102,13 +102,15 @@ if _drivers.contains('auto')
> >  elif ['arm', 'aarch64'].contains(host_machine.cpu_family())
> >_drivers = []
> >  else
> > -  error('Unknown architecture. Please pass -Ddri-drivers to set driver 
> > options. Patches gladly accepted to fix this.')
> > +  error('Unknown architecture @0@. Please pass -Ddri-drivers to set 
> > driver options. Patches gladly accepted to fix this.'.format(
> > +host_machine.cpu_family()))
> >  endif
> >elif ['darwin', 'windows', 'cygwin', 
> > 'haiku'].contains(host_machine.system())
> >  # only swrast would make sense here, but gallium swrast is a much 
> > better default
> >  _drivers = []
> >else
> > -error('Unknown OS. Please pass -Ddri-drivers to set driver options. 
> > Patches gladly accepted to fix this.')
> > +error('Unknown OS @0@. Please pass -Ddri-drivers to set driver 
> > options. Patches gladly accepted to fix this.'.format(
> > +  host_machine.system()))
> >endif
> >  endif
> >  
> > @@ -135,12 +137,14 @@ if _drivers.contains('auto')
> >  'tegra', 'virgl', 'swrast',
> >]
> >  else
> > -  error('Unknown architecture. Please pass -Dgallium-drivers to set 
> > driver options. Patches gladly accepted to fix this.')
> > +  error('Unknown architecture @0@. Please pass -Dgallium-drivers to 
> > set driver options. Patches gladly accepted to fix this.'.format(
> > +host_machine.cpu_family()))
> >  endif
> >elif ['darwin', 'windows', 'cygwin', 
> > 'haiku'].contains(host_machine.system())
> >  _drivers = ['swrast']
> >else
> > -error('Unknown OS. Please pass -Dgallium-drivers to set driver 
> > options. Patches gladly accepted to fix this.')
> > +error('Unknown OS @0@. Please pass -Dgallium-drivers to set driver 
> > options. Patches gladly accepted to fix this.'.format(
> > +  host_machine.system()))
> >endif
> >  endif
> >  with_gallium_pl111 = _drivers.contains('pl111')
> > @@ -176,13 +180,15 @@ if _vulkan_drivers.contains('auto')
> >  if host_machine.cpu_family().startswith('x86')
> >_vulkan_drivers = ['amd', 'intel']
> >  else
> > -  error('Unknown architecture. Please pass -Dvulkan-drivers to set 
> > driver options. Patches gladly accepted to fix this.')
> > +  error('Unknown architecture @0@. Please pass -Dvulkan-drivers to set 
> > driver options. Patches gladly accepted to fix this.'.format(
> > +host_machine.cpu_family()))
> >  endif
> >elif ['darwin', 'windows', 'cygwin', 
> > 'haiku'].contains(host_machine.system())
> >  # No vulkan driver supports windows or macOS currently
> >  _vulkan_drivers = []
> >else
> > -error('Unknown OS. Please pass -Dvulkan-drivers to set driver options. 
> > Patches gladly accepted to fix this.')
> > +error('Unknown OS @0@. Please pass -Dvulkan-drivers to set driver 
> > options. Patches gladly accepted to fix this.'.format(
> > +  host_machine.system()))
> >endif
> >  endif
> >  
> > @@ -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 @0@. Please pass -Dplatforms to set platforms. 
> > Patches gladly accepted to fix this.'.format(
> > +  host_machine.system()))
> >endif
> >  endif
> >  
> > -- 
> > 2.18.0
> > 
> 
> for the series,
> Reviewed-by: Dylan Baker 

Thanks!

> 
> Do you need me to push these for you?

Yes, please.
 -- Guido


___
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

[Dylan Baker]

Quoting Guido Günther (2018-08-26 13:23:59)
> V2: Add one missing @0@
> 
> Signed-off-by: Guido Günther 
> ---
>  meson.build | 21 ++---
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 1b3dfa221c9..c3a7e8cdd74 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -102,13 +102,15 @@ if _drivers.contains('auto')
>  elif ['arm', 'aarch64'].contains(host_machine.cpu_family())
>_drivers = []
>  else
> -  error('Unknown architecture. Please pass -Ddri-drivers to set driver 
> options. Patches gladly accepted to fix this.')
> +  error('Unknown architecture @0@. Please pass -Ddri-drivers to set 
> driver options. Patches gladly accepted to fix this.'.format(
> +host_machine.cpu_family()))
>  endif
>elif ['darwin', 'windows', 'cygwin', 
> 'haiku'].contains(host_machine.system())
>  # only swrast would make sense here, but gallium swrast is a much better 
> default
>  _drivers = []
>else
> -error('Unknown OS. Please pass -Ddri-drivers to set driver options. 
> Patches gladly accepted to fix this.')
> +error('Unknown OS @0@. Please pass -Ddri-drivers to set driver options. 
> Patches gladly accepted to fix this.'.format(
> +  host_machine.system()))
>endif
>  endif
>  
> @@ -135,12 +137,14 @@ if _drivers.contains('auto')
>  'tegra', 'virgl', 'swrast',
>]
>  else
> -  error('Unknown architecture. Please pass -Dgallium-drivers to set 
> driver options. Patches gladly accepted to fix this.')
> +  error('Unknown architecture @0@. Please pass -Dgallium-drivers to set 
> driver options. Patches gladly accepted to fix this.'.format(
> +host_machine.cpu_family()))
>  endif
>elif ['darwin', 'windows', 'cygwin', 
> 'haiku'].contains(host_machine.system())
>  _drivers = ['swrast']
>else
> -error('Unknown OS. Please pass -Dgallium-drivers to set driver options. 
> Patches gladly accepted to fix this.')
> +error('Unknown OS @0@. Please pass -Dgallium-drivers to set driver 
> options. Patches gladly accepted to fix this.'.format(
> +  host_machine.system()))
>endif
>  endif
>  with_gallium_pl111 = _drivers.contains('pl111')
> @@ -176,13 +180,15 @@ if _vulkan_drivers.contains('auto')
>  if host_machine.cpu_family().startswith('x86')
>_vulkan_drivers = ['amd', 'intel']
>  else
> -  error('Unknown architecture. Please pass -Dvulkan-drivers to set 
> driver options. Patches gladly accepted to fix this.')
> +  error('Unknown architecture @0@. Please pass -Dvulkan-drivers to set 
> driver options. Patches gladly accepted to fix this.'.format(
> +host_machine.cpu_family()))
>  endif
>elif ['darwin', 'windows', 'cygwin', 
> 'haiku'].contains(host_machine.system())
>  # No vulkan driver supports windows or macOS currently
>  _vulkan_drivers = []
>else
> -error('Unknown OS. Please pass -Dvulkan-drivers to set driver options. 
> Patches gladly accepted to fix this.')
> +error('Unknown OS @0@. Please pass -Dvulkan-drivers to set driver 
> options. Patches gladly accepted to fix this.'.format(
> +  host_machine.system()))
>endif
>  endif
>  
> @@ -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 @0@. Please pass -Dplatforms to set platforms. Patches 
> gladly accepted to fix this.'.format(
> +  host_machine.system()))
>endif
>  endif
>  
> -- 
> 2.18.0
> 

for the series,
Reviewed-by: Dylan Baker 

Do you need me to push these for you?


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

Re: [Mesa-dev] Meson-windows v4: Potential path too long during zlib subproject build when using MsBuild backend

[Dylan Baker]

Quoting Liviu Prodea (2018-08-26 13:36:47)
> See https://gitlab.freedesktop.org/dbaker/mesa/issues/1 for details.

I think I saw something recently about this in the meson bug tracker, I'm
following up with meson upstream and I'll get back to you.

Dylan


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

[Mesa-dev] [Bug 107655] X segfaults on startup in r300_dri.so, making system unusable

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107655

--- Comment #6 from Ilia Mirkin  ---
The intention (and original function) was that this would be safe -- swr would
check for AVX support and load the relevant library (libAVX/libAVX2), or bail
on load.

Something must have regressed in there and no one noticed since AVX has become
fairly common.

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

[Mesa-dev] [Bug 107655] X segfaults on startup in r300_dri.so, making system unusable

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107655

Michel Dänzer  changed:

   What|Removed |Added

   Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org
 Status|RESOLVED|REOPENED
 QA Contact|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org
  Component|Drivers/Gallium/r300|Drivers/Gallium/swr
 Resolution|NOTOURBUG   |---

--- Comment #5 from Michel Dänzer  ---
(In reply to Sergey Kondakov from comment #4)
> I don't know the actual reason of the crash but the guys there figured out 
> that
> the crash was coming from AVX instruction in Mesa's SWR code. The affected
> machine does not support any kind of AVX, so it threw out the error. But it's
> unclear why SWR even been trying to initialize during the load of r300_dri.

I think it's the combination of two things:

* All Gallium drivers are linked into a single binary (so-called mega-driver)

* SWR is compiled with AVX support and has initializers which are automatically
executed when the above binary is dlopen()ed.

Until there's a solution for this, SWR cannot be enabled in a build which has
to run on non-AVX capable CPUs.

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

[Mesa-dev] [PATCH] glsl/linker: Link all out vars from a shader objects on a single stage

[Vadym Shovkoplias]

From: "vadym.shovkoplias" 

During intra stage linking some out variables can be dropped because
it is not used in a shader with the main function. But these out vars
can be referenced on later stages which can lead to further linking
errors.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105731
Signed-off-by: Vadym Shovkoplias 
---
 src/compiler/glsl/linker.cpp | 38 
 1 file changed, 38 insertions(+)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 3ce78fe642..3b0c01c316 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -2187,6 +2187,41 @@ link_cs_input_layout_qualifiers(struct gl_shader_program 
*prog,
}
 }
 
+/**
+ * Link all out variables on a single stage which are not
+ * directly used in a shader with the main function.
+ */
+static void
+link_output_variables(struct gl_linked_shader *linked_shader,
+  struct gl_shader **shader_list,
+  unsigned num_shaders)
+{
+   struct glsl_symbol_table *symbols = linked_shader->symbols;
+
+   for (unsigned i = 0; i < num_shaders; i++) {
+
+  /* Skip shader object with main function */
+  if (shader_list[i]->symbols->get_function("main"))
+ continue;
+
+  foreach_in_list (ir_instruction, ir, shader_list[i]->ir) {
+
+ if (ir->ir_type != ir_type_variable)
+continue;
+
+ ir_variable *const var = (ir_variable *) ir;
+
+ if (var->data.mode == ir_var_shader_out &&
+   !symbols->get_variable(var->name)) {
+symbols->add_variable(var);
+linked_shader->ir->push_head(var);
+ }
+  }
+   }
+
+   return;
+}
+
 
 /**
  * Combine a group of shaders for a single stage to generate a linked shader
@@ -2352,6 +2387,9 @@ link_intrastage_shaders(void *mem_ctx,
   return NULL;
}
 
+   if (linked->Stage != MESA_SHADER_FRAGMENT)
+  link_output_variables(linked, shader_list, num_shaders);
+
/* Make a pass over all variable declarations to ensure that arrays with
 * unspecified sizes have a size specified.  The size is inferred from the
 * max_array_access field.
-- 
2.18.0

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

[Mesa-dev] [PATCH] mesa: allow GL_UNSIGNED_BYTE type for SNORM reads

[Tapani Pälli]

OpenGL ES spec states:
   "For normalized fixed-point rendering surfaces, the combination format
RGBA and type UNSIGNED_BYTE is accepted."

This fixes following failing VK-GL-CTS tests:

   KHR-GLES3.packed_pixels.pbo_rectangle.rgba8_snorm
   KHR-GLES3.packed_pixels.rectangle.rgba8_snorm
   KHR-GLES3.packed_pixels.varied_rectangle.rgba8_snorm

Signed-off-by: Tapani Pälli 
https://bugs.freedesktop.org/show_bug.cgi?id=107658
Cc: mesa-sta...@lists.freedesktop.org
---

This is a partial fix to the bug. I believe there are 2 separate
issues within reported bug and this fixes the first one.

 src/mesa/main/readpix.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 2cbb578a37f..556c860d393 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -958,6 +958,15 @@ read_pixels_es3_error_check(struct gl_context *ctx, GLenum 
format, GLenum type,
return GL_NO_ERROR;
  }
   }
+  if (type == GL_UNSIGNED_BYTE) {
+ switch (internalFormat) {
+ case GL_R8_SNORM:
+ case GL_RG8_SNORM:
+ case GL_RGBA8_SNORM:
+if (_mesa_has_EXT_render_snorm(ctx))
+   return GL_NO_ERROR;
+ }
+  }
   break;
case GL_BGRA:
   /* GL_EXT_read_format_bgra */
-- 
2.14.4

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

Re: [Mesa-dev] [PATCH 4/4] virgl: add debug-switch to output TGSI

[Gert Wollny]

Am Montag, den 27.08.2018, 12:17 +0200 schrieb Erik Faye-Lund:
> 
> On ma., aug. 27, 2018 at 9:49 AM, Gert Wollny  com> wrote:
> > Am Montag, den 20.08.2018, 14:10 +0200 schrieb Erik Faye-Lund:
> > >  This is quite useful for debugging shader-transpiling issues in
> > >  virglrenderer.
> > Isn't this coverted by ST_DEBUG=tgsi? 
> > 
> 
> Ah, I wasn't even aware of this. 
> 
> There's one minor difference here, though: virgl does some
> transformations on the TGSI before passing it to the host. It can
> sometimes be useful to see the difference.
> 
> > Also, virglrenderer has a variable vrend_dump_shaders in
> > vrend_renderer.c that enables dumping all the TGSI + the created
> > GLSL
> > shaders. 
> 
> Yeah, and that's also useful. But there's two differences here:
> - vrend_dump_shaders dumps the TGSI *after* parsing, comparing the
> over-the-wire TGSI and the parsed TGSI has helped me in the past to
> find flags being culled during parsing etc in the past.
> - vrend_dump_shaders needs a recompile of virglrenderer, which makes
> it a lot more inconvenient from a turn-around point of view if
> running on qemu for instance.
> 
> I don't know, perhaps this is a bit too many similar features? I can
> certainly drop this patch for now.
Given these differences it actually makes sense to have this output
available, so you have my R-b for this patch.

> Another (maybe more useful) option could be to allow a per-client
> override of the vrend_dump_shaders-functionality that could be
> enabled from the host?
I think adding the same kind of debug capability to virglrenderer that
can be enabled via environment variables would be very useful. 

Best, 
Gert 



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

[Mesa-dev] [Bug 101247] Mesa fails to link GLSL programs with unused output blocks

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=101247

--- Comment #3 from vadym  ---
Patch merged to master:

commit 4a8444d5bc865119218eca8674e5614535f4829e (HEAD -> new_master,
origin/master, origin/HEAD)
Author: vadym.shovkoplias 
Date:   Thu Aug 23 13:12:16 2018 +0300

glsl/linker: Allow unused in blocks which are not declated on previous
stage

>From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:

"Only the input variables that are actually read need to be written
 by the previous stage; it is allowed to have superfluous
 declarations of input variables."

Fixes:
* interstage-multiple-shader-objects.shader_test

v2:
  Update comment in ir.h since the usage of "used" field
  has been extended.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
Signed-off-by: Vadym Shovkoplias 
Reviewed-by: Alejandro Piñeiro 
Reviewed-by: Timothy Arceri 

-- 
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 4/4] virgl: add debug-switch to output TGSI

[Erik Faye-Lund]

On ma., aug. 27, 2018 at 9:49 AM, Gert Wollny 
 wrote:

Am Montag, den 20.08.2018, 14:10 +0200 schrieb Erik Faye-Lund:

 This is quite useful for debugging shader-transpiling issues in
 virglrenderer.

Isn't this coverted by ST_DEBUG=tgsi?



Ah, I wasn't even aware of this.

There's one minor difference here, though: virgl does some 
transformations on the TGSI before passing it to the host. It can 
sometimes be useful to see the difference.



Also, virglrenderer has a variable vrend_dump_shaders in
vrend_renderer.c that enables dumping all the TGSI + the created GLSL
shaders.


Yeah, and that's also useful. But there's two differences here:
- vrend_dump_shaders dumps the TGSI *after* parsing, comparing the 
over-the-wire TGSI and the parsed TGSI has helped me in the past to 
find flags being culled during parsing etc in the past.
- vrend_dump_shaders needs a recompile of virglrenderer, which makes it 
a lot more inconvenient from a turn-around point of view if running on 
qemu for instance.


I don't know, perhaps this is a bit too many similar features? I can 
certainly drop this patch for now.


Another (maybe more useful) option could be to allow a per-client 
override of the vrend_dump_shaders-functionality that could be enabled 
from the host?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Alejandro Piñeiro]

On 27/08/18 12:08, Vadym Shovkoplias wrote:
> I haven't got write permissions, so I'll appreciate if someone can
> push it.
> Thanks!

Just done:
https://gitlab.freedesktop.org/mesa/mesa/commit/4a8444d5bc865119218eca8674e5614535f4829e


>
> On Mon, Aug 27, 2018 at 12:52 PM, Alejandro Piñeiro
> mailto:apinhe...@igalia.com>> wrote:
>
> On 27/08/18 11:12, Vadym Shovkoplias wrote:
>> Hi Timothy, Alejandro, Marek,
>>
>> Thanks for review! Can we merge the patch now ?
>
> Yes, as soon as a patch gets a Rb, it can be pushed to master. Do
> you have write permissions to do it by yourself, or do you need
> someone of do that in your behalf?
>
>
>>
>> On Mon, Aug 27, 2018 at 6:08 AM, Timothy Arceri
>> mailto:tarc...@itsqueeze.com>> wrote:
>>
>> Reviewed-by: Timothy Arceri > >
>>
>>
>> On 24/08/18 18:25, Alejandro Piñeiro wrote:
>>
>> CCing Timothy just in case he still thinks that the
>> original comment
>> should remain as it is. In any case, it looks to me, so:
>>
>> Reviewed-by: Alejandro Piñeiro > >
>>
>>
>> On 23/08/18 12:12, vadym.shovkoplias wrote:
>>
>>  From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
>>
>>      "Only the input variables that are actually read
>> need to be written
>>       by the previous stage; it is allowed to have
>> superfluous
>>       declarations of input variables."
>>
>> Fixes:
>>      * interstage-multiple-shader-objects.shader_test
>>
>> v2:
>>    Update comment in ir.h since the usage of "used" field
>>    has been extended.
>>
>> Bugzilla:
>> https://bugs.freedesktop.org/show_bug.cgi?id=101247
>> 
>> Signed-off-by: Vadym Shovkoplias
>> > >
>> ---
>>   src/compiler/glsl/ir.h                      | 4 ++--
>>   src/compiler/glsl/link_interface_blocks.cpp | 8
>> +++-
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ir.h
>> b/src/compiler/glsl/ir.h
>> index 67b38f48ef..d05d1998a5 100644
>> --- a/src/compiler/glsl/ir.h
>> +++ b/src/compiler/glsl/ir.h
>> @@ -667,8 +667,8 @@ public:
>>          * variable has been used.  For example, it
>> is an error to redeclare a
>>          * variable as invariant after it has been used.
>>          *
>> -       * This is only maintained in the
>> ast_to_hir.cpp path, not in
>> -       * Mesa's fixed function or ARB program paths.
>> +       * This is maintained in the ast_to_hir.cpp
>> path and during linking,
>> +       * but not in Mesa's fixed function or ARB
>> program paths.
>>          */
>>         unsigned used:1;
>>   diff --git
>> a/src/compiler/glsl/link_interface_blocks.cpp
>> b/src/compiler/glsl/link_interface_blocks.cpp
>> index e5eca9460e..801fbcd5d9 100644
>> --- a/src/compiler/glsl/link_interface_blocks.cpp
>> +++ b/src/compiler/glsl/link_interface_blocks.cpp
>> @@ -417,9 +417,15 @@
>> validate_interstage_inout_blocks(struct
>> gl_shader_program *prog,
>>          * write to any of the pre-defined outputs
>> (e.g. if the vertex shader
>>          * does not write to gl_Position, etc), which
>> is allowed and results in
>>          * undefined behavior.
>> +       *
>> +       * From Section 4.3.4 (Inputs) of the GLSL
>> 1.50 spec:
>> +       *
>> +       *    "Only the input variables that are
>> actually read need to be written
>> +       *     by the previous stage; it is allowed to
>> have superfluous
>> +       *     declarations of input variables."
>>          */
>>         if (producer_def == NULL &&
>> -          !is_builtin_gl_in_block(var,
>> consumer->Stage)) {
>> +          !is_builtin_gl_in_block(var,
>> consumer->Stage) && var->data.used) {
>>            linker_error(prog, 

Re: [Mesa-dev] [PATCH] egl/android: rework device probing

[Robert Foss]

Hey,

On 27/08/2018 11.47, Emil Velikov wrote:

On 24 August 2018 at 18:55, Robert Foss  wrote:

Hey Emil,


On 24/08/2018 14.21, Emil Velikov wrote:


From: Emil Velikov 

Unlike the other platforms, here we aim do guess if the device that we
somewhat arbitrarily picked, is supported or not.

In particular: when a vendor is _not_ requested we loop through all
devices, picking the first one which can create a DRI screen.

When a vendor is requested - we use that and do _not_ fall-back to any
other device.

The former seems a bit fiddly, but considering EGL_EXT_explicit_device and
EGL_MESA_query_renderer are MIA, this is the best we can do for the
moment.

With those (proposed) extensions userspace will be able to create a
separate EGL display for each device, query device details and make the
conscious decision which one to use.

Cc: Robert Foss 
Cc: Tomasz Figa 
Signed-off-by: Emil Velikov 
---
Thanks for the clarification Tomasz. The original code was using a
fall-back even a vendor was explicitly requested, confusing me a bit ;-)
---
   src/egl/drivers/dri2/platform_android.c | 71 +++--
   1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_android.c
b/src/egl/drivers/dri2/platform_android.c
index 1f9fe27ab85..5bf627dec7d 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -1420,13 +1420,32 @@ droid_filter_device(_EGLDisplay *disp, int fd,
const char *vendor)
  return 0;
   }
   +static int
+droid_probe_device(_EGLDisplay *disp)
+{
+  /* Check that the device is supported, by attempting to:
+   * - load the dri module
+   * - and, create a screen
+   */
+   if (!droid_load_driver(disp)) {
+  _eglLog(_EGL_WARNING, "DRI2: failed to load driver");
+  return -1;
+   }
+
+   if (!dri2_create_screen(disp)) {
+  _eglLog(_EGL_WARNING, "DRI2: failed to create screen");
+  return -1;
+   }
+   return 0;
+}
+
   static int
   droid_open_device(_EGLDisplay *disp)
   {
   #define MAX_DRM_DEVICES 32
  drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL };
  int prop_set, num_devices;
-   int fd = -1, fallback_fd = -1;
+   int fd = -1;
char *vendor_name = NULL;
  char vendor_buf[PROPERTY_VALUE_MAX];
@@ -1451,33 +1470,39 @@ droid_open_device(_EGLDisplay *disp)
continue;
 }
   -  if (vendor_name && droid_filter_device(disp, fd, vendor_name)) {
- /* Match requested, but not found - set as fallback */
- if (fallback_fd == -1) {
-fallback_fd = fd;
- } else {
+  /* If a vendor is explicitly provided, we use only that.
+   * Otherwise we fall-back the first device that is supported.
+   */
+  if (vendor_name) {
+ if (droid_filter_device(disp, fd, vendor_name)) {
+/* Device does not match - try next device */
   close(fd);
   fd = -1;
+continue;
}
-
+ /* If the requested device matches use it, regardless if
+  * init fails. Do not fall-back to any other device.
+  */
+ if (droid_probbe_device(disp)) {



Typo in function name.


Thanks for having a look Rob. Will fix (plus second instance below).


+close(fd);
+fd = -1;
+ }



Isn't the above comment saying that the if statement just below it shouldn't
be there? Or am I misparsing something?


I think it matches with the comment - note the function returns an int
0 on success.

We could change that esp. since others - droid_load_driver return an EGLBoolean.
Alternatively we could use if (droid_probbe_device(disp) != 0)

I'm fine either way.


Upon re-reading this, I'm happy with the way it is.

Feel free to add my r-b with the above typo fixed.



-Emil


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

Re: [Mesa-dev] [PATCH v2] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Vadym Shovkoplias]

I haven't got write permissions, so I'll appreciate if someone can push it.
Thanks!

On Mon, Aug 27, 2018 at 12:52 PM, Alejandro Piñeiro 
wrote:

> On 27/08/18 11:12, Vadym Shovkoplias wrote:
>
> Hi Timothy, Alejandro, Marek,
>
> Thanks for review! Can we merge the patch now ?
>
>
> Yes, as soon as a patch gets a Rb, it can be pushed to master. Do you have
> write permissions to do it by yourself, or do you need someone of do that
> in your behalf?
>
>
>
> On Mon, Aug 27, 2018 at 6:08 AM, Timothy Arceri 
> wrote:
>
>> Reviewed-by: Timothy Arceri 
>>
>>
>> On 24/08/18 18:25, Alejandro Piñeiro wrote:
>>
>>> CCing Timothy just in case he still thinks that the original comment
>>> should remain as it is. In any case, it looks to me, so:
>>>
>>> Reviewed-by: Alejandro Piñeiro 
>>>
>>>
>>> On 23/08/18 12:12, vadym.shovkoplias wrote:
>>>
  From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:

  "Only the input variables that are actually read need to be written
   by the previous stage; it is allowed to have superfluous
   declarations of input variables."

 Fixes:
  * interstage-multiple-shader-objects.shader_test

 v2:
Update comment in ir.h since the usage of "used" field
has been extended.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
 Signed-off-by: Vadym Shovkoplias 
 ---
   src/compiler/glsl/ir.h  | 4 ++--
   src/compiler/glsl/link_interface_blocks.cpp | 8 +++-
   2 files changed, 9 insertions(+), 3 deletions(-)

 diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
 index 67b38f48ef..d05d1998a5 100644
 --- a/src/compiler/glsl/ir.h
 +++ b/src/compiler/glsl/ir.h
 @@ -667,8 +667,8 @@ public:
  * variable has been used.  For example, it is an error to
 redeclare a
  * variable as invariant after it has been used.
  *
 -   * This is only maintained in the ast_to_hir.cpp path, not in
 -   * Mesa's fixed function or ARB program paths.
 +   * This is maintained in the ast_to_hir.cpp path and during
 linking,
 +   * but not in Mesa's fixed function or ARB program paths.
  */
 unsigned used:1;
   diff --git a/src/compiler/glsl/link_interface_blocks.cpp
 b/src/compiler/glsl/link_interface_blocks.cpp
 index e5eca9460e..801fbcd5d9 100644
 --- a/src/compiler/glsl/link_interface_blocks.cpp
 +++ b/src/compiler/glsl/link_interface_blocks.cpp
 @@ -417,9 +417,15 @@ validate_interstage_inout_blocks(struct
 gl_shader_program *prog,
  * write to any of the pre-defined outputs (e.g. if the vertex
 shader
  * does not write to gl_Position, etc), which is allowed and
 results in
  * undefined behavior.
 +   *
 +   * From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
 +   *
 +   *"Only the input variables that are actually read need to
 be written
 +   * by the previous stage; it is allowed to have superfluous
 +   * declarations of input variables."
  */
 if (producer_def == NULL &&
 -  !is_builtin_gl_in_block(var, consumer->Stage)) {
 +  !is_builtin_gl_in_block(var, consumer->Stage) &&
 var->data.used) {
linker_error(prog, "Input block `%s' is not an output of "
 "the previous stage\n",
 var->get_interface_type()->name);
return;

>>>
>>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
>
> --
>
> Vadym Shovkoplias | Senior Software Engineer
> GlobalLogic
> P +380.57.766.7667  M +3.8050.931.7304  S vadym.shovkoplias
> www.globallogic.com
> 
> http://www.globallogic.com/email_disclaimer.txt
>
>
>


-- 

Vadym Shovkoplias | Senior Software Engineer
GlobalLogic
P +380.57.766.7667  M +3.8050.931.7304  S vadym.shovkoplias
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Alejandro Piñeiro]

On 27/08/18 11:12, Vadym Shovkoplias wrote:
> Hi Timothy, Alejandro, Marek,
>
> Thanks for review! Can we merge the patch now ?

Yes, as soon as a patch gets a Rb, it can be pushed to master. Do you
have write permissions to do it by yourself, or do you need someone of
do that in your behalf?

>
> On Mon, Aug 27, 2018 at 6:08 AM, Timothy Arceri  > wrote:
>
> Reviewed-by: Timothy Arceri  >
>
>
> On 24/08/18 18:25, Alejandro Piñeiro wrote:
>
> CCing Timothy just in case he still thinks that the original
> comment
> should remain as it is. In any case, it looks to me, so:
>
> Reviewed-by: Alejandro Piñeiro  >
>
>
> On 23/08/18 12:12, vadym.shovkoplias wrote:
>
>  From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
>
>      "Only the input variables that are actually read need
> to be written
>       by the previous stage; it is allowed to have superfluous
>       declarations of input variables."
>
> Fixes:
>      * interstage-multiple-shader-objects.shader_test
>
> v2:
>    Update comment in ir.h since the usage of "used" field
>    has been extended.
>
> Bugzilla:
> https://bugs.freedesktop.org/show_bug.cgi?id=101247
> 
> Signed-off-by: Vadym Shovkoplias
>  >
> ---
>   src/compiler/glsl/ir.h                      | 4 ++--
>   src/compiler/glsl/link_interface_blocks.cpp | 8 +++-
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index 67b38f48ef..d05d1998a5 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -667,8 +667,8 @@ public:
>          * variable has been used.  For example, it is an
> error to redeclare a
>          * variable as invariant after it has been used.
>          *
> -       * This is only maintained in the ast_to_hir.cpp
> path, not in
> -       * Mesa's fixed function or ARB program paths.
> +       * This is maintained in the ast_to_hir.cpp path
> and during linking,
> +       * but not in Mesa's fixed function or ARB program
> paths.
>          */
>         unsigned used:1;
>   diff --git a/src/compiler/glsl/link_interface_blocks.cpp
> b/src/compiler/glsl/link_interface_blocks.cpp
> index e5eca9460e..801fbcd5d9 100644
> --- a/src/compiler/glsl/link_interface_blocks.cpp
> +++ b/src/compiler/glsl/link_interface_blocks.cpp
> @@ -417,9 +417,15 @@
> validate_interstage_inout_blocks(struct gl_shader_program
> *prog,
>          * write to any of the pre-defined outputs (e.g.
> if the vertex shader
>          * does not write to gl_Position, etc), which is
> allowed and results in
>          * undefined behavior.
> +       *
> +       * From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
> +       *
> +       *    "Only the input variables that are actually
> read need to be written
> +       *     by the previous stage; it is allowed to have
> superfluous
> +       *     declarations of input variables."
>          */
>         if (producer_def == NULL &&
> -          !is_builtin_gl_in_block(var, consumer->Stage)) {
> +          !is_builtin_gl_in_block(var, consumer->Stage)
> && var->data.used) {
>            linker_error(prog, "Input block `%s' is not an
> output of "
>                         "the previous stage\n",
> var->get_interface_type()->name);
>            return;
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>
>
>
>
> -- 
>
> Vadym Shovkoplias | Senior Software Engineer
> GlobalLogic
> P +380.57.766.7667  M +3.8050.931.7304  S vadym.shovkoplias
> www.globallogic.com 
> 
> http://www.globallogic.com/email_disclaimer.txt

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

Re: [Mesa-dev] [PATCH] egl/android: do not indent HAVE_DRM_GRALLOC preprocessor directive

[Emil Velikov]

On 24 August 2018 at 20:54, Mauro Rossi  wrote:
> Hi,
>
> Il giorno mer 15 ago 2018 alle ore 15:13 Mauro Rossi
>  ha scritto:
>>
>> Fixes: 3f7bca44d9 ("egl/android: #ifdef out flink name support")
>> Fixes: c7bb82136b ("egl/android: Add DRM node probing and filtering")
>> Signed-off-by: Mauro Rossi 
>> ---
>>  src/egl/drivers/dri2/platform_android.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_android.c 
>> b/src/egl/drivers/dri2/platform_android.c
>> index 834bbd258e..f8c85f97cf 100644
>> --- a/src/egl/drivers/dri2/platform_android.c
>> +++ b/src/egl/drivers/dri2/platform_android.c
>> @@ -1226,7 +1226,7 @@ droid_load_driver(_EGLDisplay *disp)
>> dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == 
>> DRM_NODE_RENDER;
>>
>> if (!dri2_dpy->is_render_node) {
>> -   #ifdef HAVE_DRM_GRALLOC
>> +#ifdef HAVE_DRM_GRALLOC
>> /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM 
>> names
>>  * for backwards compatibility with drm_gralloc. (Do not use on new
>>  * systems.) */
>> @@ -1235,10 +1235,10 @@ droid_load_driver(_EGLDisplay *disp)
>>err = "DRI2: failed to load driver";
>>goto error;
>> }
>> -   #else
>> +#else
>> err = "DRI2: handle is not for a render node";
>> goto error;
>> -   #endif
>> +#endif
>> } else {
>> dri2_dpy->loader_extensions = droid_image_loader_extensions;
>> if (!dri2_load_driver_dri3(disp)) {
>> --
>> 2.17.1
>>
>
> Please provide one R-b ,
> in order to proceed in commit to gitlab master
> and propose as candidate for mesa 18.2.0 release
>
Reviewed-by: Emil Velikov 

I wouldn't worry too much about landing this in 18.2.0 since it's
mostly a cosmetic change.

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

Re: [Mesa-dev] [PATCH] egl/android: rework device probing

[Emil Velikov]

On 24 August 2018 at 18:55, Robert Foss  wrote:
> Hey Emil,
>
>
> On 24/08/2018 14.21, Emil Velikov wrote:
>>
>> From: Emil Velikov 
>>
>> Unlike the other platforms, here we aim do guess if the device that we
>> somewhat arbitrarily picked, is supported or not.
>>
>> In particular: when a vendor is _not_ requested we loop through all
>> devices, picking the first one which can create a DRI screen.
>>
>> When a vendor is requested - we use that and do _not_ fall-back to any
>> other device.
>>
>> The former seems a bit fiddly, but considering EGL_EXT_explicit_device and
>> EGL_MESA_query_renderer are MIA, this is the best we can do for the
>> moment.
>>
>> With those (proposed) extensions userspace will be able to create a
>> separate EGL display for each device, query device details and make the
>> conscious decision which one to use.
>>
>> Cc: Robert Foss 
>> Cc: Tomasz Figa 
>> Signed-off-by: Emil Velikov 
>> ---
>> Thanks for the clarification Tomasz. The original code was using a
>> fall-back even a vendor was explicitly requested, confusing me a bit ;-)
>> ---
>>   src/egl/drivers/dri2/platform_android.c | 71 +++--
>>   1 file changed, 43 insertions(+), 28 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_android.c
>> b/src/egl/drivers/dri2/platform_android.c
>> index 1f9fe27ab85..5bf627dec7d 100644
>> --- a/src/egl/drivers/dri2/platform_android.c
>> +++ b/src/egl/drivers/dri2/platform_android.c
>> @@ -1420,13 +1420,32 @@ droid_filter_device(_EGLDisplay *disp, int fd,
>> const char *vendor)
>>  return 0;
>>   }
>>   +static int
>> +droid_probe_device(_EGLDisplay *disp)
>> +{
>> +  /* Check that the device is supported, by attempting to:
>> +   * - load the dri module
>> +   * - and, create a screen
>> +   */
>> +   if (!droid_load_driver(disp)) {
>> +  _eglLog(_EGL_WARNING, "DRI2: failed to load driver");
>> +  return -1;
>> +   }
>> +
>> +   if (!dri2_create_screen(disp)) {
>> +  _eglLog(_EGL_WARNING, "DRI2: failed to create screen");
>> +  return -1;
>> +   }
>> +   return 0;
>> +}
>> +
>>   static int
>>   droid_open_device(_EGLDisplay *disp)
>>   {
>>   #define MAX_DRM_DEVICES 32
>>  drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL };
>>  int prop_set, num_devices;
>> -   int fd = -1, fallback_fd = -1;
>> +   int fd = -1;
>>char *vendor_name = NULL;
>>  char vendor_buf[PROPERTY_VALUE_MAX];
>> @@ -1451,33 +1470,39 @@ droid_open_device(_EGLDisplay *disp)
>>continue;
>> }
>>   -  if (vendor_name && droid_filter_device(disp, fd, vendor_name)) {
>> - /* Match requested, but not found - set as fallback */
>> - if (fallback_fd == -1) {
>> -fallback_fd = fd;
>> - } else {
>> +  /* If a vendor is explicitly provided, we use only that.
>> +   * Otherwise we fall-back the first device that is supported.
>> +   */
>> +  if (vendor_name) {
>> + if (droid_filter_device(disp, fd, vendor_name)) {
>> +/* Device does not match - try next device */
>>   close(fd);
>>   fd = -1;
>> +continue;
>>}
>> -
>> + /* If the requested device matches use it, regardless if
>> +  * init fails. Do not fall-back to any other device.
>> +  */
>> + if (droid_probbe_device(disp)) {
>
>
> Typo in function name.
>
Thanks for having a look Rob. Will fix (plus second instance below).

>> +close(fd);
>> +fd = -1;
>> + }
>
>
> Isn't the above comment saying that the if statement just below it shouldn't
> be there? Or am I misparsing something?
>
I think it matches with the comment - note the function returns an int
0 on success.

We could change that esp. since others - droid_load_driver return an EGLBoolean.
Alternatively we could use if (droid_probbe_device(disp) != 0)

I'm fine either way.

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

Re: [Mesa-dev] [PATCH] i965/batch: don't ignore the 'brw_new_batch' call for a 'new batch'

[andrey simiklit]

Hi all,

It would be great if somebody look at this.
I guess that this issue can affect every place where we use
'intel_batchbuffer_save_state/intel_batchbuffer_reset_to_saved' but only
when the 'brw_batch_has_aperture_space' function returns false several
times in a row.
Pay attention that the last batch
 of log has an
aperture with negative value "(-1023.8Mb aperture)".
Please correct me if I am incorrect.

Regards,
Andrii.
On Tue, Aug 21, 2018 at 3:00 PM andrey simiklit 
wrote:

> Hi all,
>
> The bug for this issue was created:
> https://bugs.freedesktop.org/show_bug.cgi?id=107626
>
> Regards,
> Andrii.
>
>
> On Mon, Aug 20, 2018 at 5:43 PM,  wrote:
>
>> From: Andrii Simiklit 
>>
>> If we restore the 'new batch' using 'intel_batchbuffer_reset_to_saved'
>> function we must restore the default state of the batch using
>> 'brw_new_batch' function because the 'intel_batchbuffer_flush'
>> function will not do it for the 'new batch' again.
>> At least the following fields of the batch
>> 'state_base_address_emitted','aperture_space', 'state_used'
>> should be restored to default values to avoid:
>> 1. the aperture_space overflow
>> 2. the missed STATE_BASE_ADDRESS commad in the batch
>> 3. the memory overconsumption of the 'statebuffer'
>>due to uncleared 'state_used' field.
>> etc.
>>
>> Signed-off-by: Andrii Simiklit 
>> ---
>>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 104
>> +++---
>>  1 file changed, 59 insertions(+), 45 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> index 65d2c64..b8c5fb4 100644
>> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> @@ -219,7 +219,7 @@ add_exec_bo(struct intel_batchbuffer *batch, struct
>> brw_bo *bo)
>> bo->index = batch->exec_count;
>> batch->exec_bos[batch->exec_count] = bo;
>> batch->aperture_space += bo->size;
>> -
>> +   assert((batch->aperture_space >= 0) && "error 'batch->aperture_space'
>> field is overflown!");
>> return batch->exec_count++;
>>  }
>>
>> @@ -290,6 +290,51 @@
>> intel_batchbuffer_reset_and_clear_render_cache(struct brw_context *brw)
>> brw_cache_sets_clear(brw);
>>  }
>>
>> +/**
>> + * Called when starting a new batch buffer.
>> + */
>> +static void
>> +brw_new_batch(struct brw_context *brw)
>> +{
>> +   /* Unreference any BOs held by the previous batch, and reset counts.
>> */
>> +   for (int i = 0; i < brw->batch.exec_count; i++) {
>> +  brw_bo_unreference(brw->batch.exec_bos[i]);
>> +  brw->batch.exec_bos[i] = NULL;
>> +   }
>> +   brw->batch.batch_relocs.reloc_count = 0;
>> +   brw->batch.state_relocs.reloc_count = 0;
>> +   brw->batch.exec_count = 0;
>> +   brw->batch.aperture_space = 0;
>> +
>> +   brw_bo_unreference(brw->batch.state.bo);
>> +
>> +   /* Create a new batchbuffer and reset the associated state: */
>> +   intel_batchbuffer_reset_and_clear_render_cache(brw);
>> +
>> +   /* If the kernel supports hardware contexts, then most hardware state
>> is
>> +* preserved between batches; we only need to re-emit state that is
>> required
>> +* to be in every batch.  Otherwise we need to re-emit all the state
>> that
>> +* would otherwise be stored in the context (which for all intents and
>> +* purposes means everything).
>> +*/
>> +   if (brw->hw_ctx == 0) {
>> +  brw->ctx.NewDriverState |= BRW_NEW_CONTEXT;
>> +  brw_upload_invariant_state(brw);
>> +   }
>> +
>> +   brw->ctx.NewDriverState |= BRW_NEW_BATCH;
>> +
>> +   brw->ib.index_size = -1;
>> +
>> +   /* We need to periodically reap the shader time results, because
>> rollover
>> +* happens every few seconds.  We also want to see results every once
>> in a
>> +* while, because many programs won't cleanly destroy our context, so
>> the
>> +* end-of-run printout may not happen.
>> +*/
>> +   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
>> +  brw_collect_and_report_shader_time(brw);
>> +}
>> +
>>  void
>>  intel_batchbuffer_save_state(struct brw_context *brw)
>>  {
>> @@ -311,6 +356,19 @@ intel_batchbuffer_reset_to_saved(struct brw_context
>> *brw)
>> brw->batch.exec_count = brw->batch.saved.exec_count;
>>
>> brw->batch.map_next = brw->batch.saved.map_next;
>> +   if (USED_BATCH(brw->batch) == 0)
>> +   {
>> +  /**
>> +   * The 'intel_batchbuffer_flush' function will not call
>> +   * the 'brw_new_batch' function when the USED_BATCH returns 0.
>> +   * It may leads to the few following issue:
>> +   * The 'aperture_space' field can be overflown
>> +   * The 'statebuffer' buffer contains the big unused space
>> +   * The STATE_BASE_ADDRESS message is missed
>> +   * etc
>> +   **/
>> +  brw_new_batch(brw);
>> +   }
>>  }
>>
>>  void
>> @@ -529,50 +587,6 @@ intel_batchbuffer_require_space(struct brw_context
>> *brw, GLuint sz)
>> }
>>  }
>>
>> 

Re: [Mesa-dev] [PATCH v2] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Vadym Shovkoplias]

Hi Timothy, Alejandro, Marek,

Thanks for review! Can we merge the patch now ?

On Mon, Aug 27, 2018 at 6:08 AM, Timothy Arceri 
wrote:

> Reviewed-by: Timothy Arceri 
>
>
> On 24/08/18 18:25, Alejandro Piñeiro wrote:
>
>> CCing Timothy just in case he still thinks that the original comment
>> should remain as it is. In any case, it looks to me, so:
>>
>> Reviewed-by: Alejandro Piñeiro 
>>
>>
>> On 23/08/18 12:12, vadym.shovkoplias wrote:
>>
>>>  From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
>>>
>>>  "Only the input variables that are actually read need to be written
>>>   by the previous stage; it is allowed to have superfluous
>>>   declarations of input variables."
>>>
>>> Fixes:
>>>  * interstage-multiple-shader-objects.shader_test
>>>
>>> v2:
>>>Update comment in ir.h since the usage of "used" field
>>>has been extended.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
>>> Signed-off-by: Vadym Shovkoplias 
>>> ---
>>>   src/compiler/glsl/ir.h  | 4 ++--
>>>   src/compiler/glsl/link_interface_blocks.cpp | 8 +++-
>>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
>>> index 67b38f48ef..d05d1998a5 100644
>>> --- a/src/compiler/glsl/ir.h
>>> +++ b/src/compiler/glsl/ir.h
>>> @@ -667,8 +667,8 @@ public:
>>>  * variable has been used.  For example, it is an error to
>>> redeclare a
>>>  * variable as invariant after it has been used.
>>>  *
>>> -   * This is only maintained in the ast_to_hir.cpp path, not in
>>> -   * Mesa's fixed function or ARB program paths.
>>> +   * This is maintained in the ast_to_hir.cpp path and during
>>> linking,
>>> +   * but not in Mesa's fixed function or ARB program paths.
>>>  */
>>> unsigned used:1;
>>>   diff --git a/src/compiler/glsl/link_interface_blocks.cpp
>>> b/src/compiler/glsl/link_interface_blocks.cpp
>>> index e5eca9460e..801fbcd5d9 100644
>>> --- a/src/compiler/glsl/link_interface_blocks.cpp
>>> +++ b/src/compiler/glsl/link_interface_blocks.cpp
>>> @@ -417,9 +417,15 @@ validate_interstage_inout_blocks(struct
>>> gl_shader_program *prog,
>>>  * write to any of the pre-defined outputs (e.g. if the vertex
>>> shader
>>>  * does not write to gl_Position, etc), which is allowed and
>>> results in
>>>  * undefined behavior.
>>> +   *
>>> +   * From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
>>> +   *
>>> +   *"Only the input variables that are actually read need to be
>>> written
>>> +   * by the previous stage; it is allowed to have superfluous
>>> +   * declarations of input variables."
>>>  */
>>> if (producer_def == NULL &&
>>> -  !is_builtin_gl_in_block(var, consumer->Stage)) {
>>> +  !is_builtin_gl_in_block(var, consumer->Stage) &&
>>> var->data.used) {
>>>linker_error(prog, "Input block `%s' is not an output of "
>>> "the previous stage\n",
>>> var->get_interface_type()->name);
>>>return;
>>>
>>
>> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



-- 

Vadym Shovkoplias | Senior Software Engineer
GlobalLogic
P +380.57.766.7667  M +3.8050.931.7304  S vadym.shovkoplias
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v4 7/7] nir: add loop unroll support for complex wrapper loops

[Timothy Arceri]

In GLSL IR we cheat with switch statements and simply convert them
into loops with a single iteration. This allowed us to make use of
the existing jump instruction handling provided by the loop handing
code, it also allows dead code to be cleaned up once we have
wrapped the code in a loop.

However using loops in this way created previously unrollable loops
which limits further optimisations. Here we provide a way to unroll
loops that end in a break and have multiple other exits.

All shader-db changes are from the dolphin uber shaders. There is a
small amount of HURT shaders but in general the improvements far
exceed the HURT.

shader-db results IVB:

total instructions in shared programs: 10018187 -> 10016468 (-0.02%)
instructions in affected programs: 104080 -> 102361 (-1.65%)
helped: 36
HURT: 15

total cycles in shared programs: 220065064 -> 154529655 (-29.78%)
cycles in affected programs: 126063017 -> 60527608 (-51.99%)
helped: 51
HURT: 0

total loops in shared programs: 2515 -> 2308 (-8.23%)
loops in affected programs: 903 -> 696 (-22.92%)
helped: 51
HURT: 0

total spills in shared programs: 4370 -> 4124 (-5.63%)
spills in affected programs: 1397 -> 1151 (-17.61%)
helped: 9
HURT: 12

total fills in shared programs: 4581 -> 4419 (-3.54%)
fills in affected programs: 2201 -> 2039 (-7.36%)
helped: 9
HURT: 15
---
 src/compiler/nir/nir_opt_loop_unroll.c | 113 +
 1 file changed, 76 insertions(+), 37 deletions(-)

diff --git a/src/compiler/nir/nir_opt_loop_unroll.c 
b/src/compiler/nir/nir_opt_loop_unroll.c
index 9c33267cb72..fa60523aac7 100644
--- a/src/compiler/nir/nir_opt_loop_unroll.c
+++ b/src/compiler/nir/nir_opt_loop_unroll.c
@@ -67,7 +67,6 @@ loop_prepare_for_unroll(nir_loop *loop)
/* Remove continue if its the last instruction in the loop */
nir_instr *last_instr = nir_block_last_instr(nir_loop_last_block(loop));
if (last_instr && last_instr->type == nir_instr_type_jump) {
-  assert(nir_instr_as_jump(last_instr)->type == nir_jump_continue);
   nir_instr_remove(last_instr);
}
 }
@@ -474,54 +473,91 @@ complex_unroll(nir_loop *loop, nir_loop_terminator 
*unlimit_term,
 static bool
 wrapper_unroll(nir_loop *loop)
 {
-   bool progress = false;
-
-   nir_block *blk_after_loop =
-  nir_cursor_current_block(nir_after_cf_node(>cf_node));
-
-   /* There may still be some single src phis following the loop that
-* have not yet been cleaned up by another pass. Tidy those up before
-* unrolling the loop.
-*/
-   nir_foreach_instr_safe(instr, blk_after_loop) {
-  if (instr->type != nir_instr_type_phi)
- break;
+   if (!list_empty(>info->loop_terminator_list)) {
 
-  nir_phi_instr *phi = nir_instr_as_phi(instr);
-  assert(exec_list_length(>srcs) == 1);
+  /* Unrolling a loop with a large number of exits can result in a
+   * large inrease in register pressure. For now we just skip
+   * unrolling if we have more than 3 exits (not including the break
+   * at the end of the loop).
+   *
+   * TODO: Most loops that fit this pattern are simply switch
+   * statements that are converted to a loop to take advantage of
+   * exiting jump instruction handling. In this case we could make
+   * use of a binary seach pattern like we do in
+   * nir_lower_indirect_derefs(), this should allow us to unroll the
+   * loops in an optimal way and should also avoid some of the
+   * register pressure that comes from simply nesting the
+   * terminators one after the other.
+   */
+  if (list_length(>info->loop_terminator_list) > 3)
+ return false;
+
+  loop_prepare_for_unroll(loop);
+
+  nir_cursor cursor = nir_after_block(nir_loop_last_block(loop));
+  list_for_each_entry(nir_loop_terminator, terminator,
+  >info->loop_terminator_list,
+  loop_terminator_link) {
+
+ /* Remove break from the terminator */
+ nir_instr *break_instr =
+nir_block_last_instr(terminator->break_block);
+ nir_instr_remove(break_instr);
+
+ /* Pluck out the loop body. */
+ nir_cf_list loop_body;
+ nir_cf_extract(_body,
+nir_after_cf_node(>nif->cf_node),
+cursor);
+
+ /* Reinsert loop body into continue from block */
+ nir_cf_reinsert(_body,
+ nir_after_block(terminator->continue_from_block));
+
+ cursor = terminator->continue_from_then ?
+   nir_after_block(nir_if_last_then_block(terminator->nif)) :
+   nir_after_block(nir_if_last_else_block(terminator->nif));
+  }
+   } else {
+  nir_block *blk_after_loop =
+ nir_cursor_current_block(nir_after_cf_node(>cf_node));
 
-  nir_phi_src *phi_src = exec_node_data(nir_phi_src,
-exec_list_get_head(>srcs),
-node);
+  /* There may 

[Mesa-dev] [PATCH v4 3/7] nir: always attempt to find loop terminators

[Timothy Arceri]

This will help later patches with unrolling loops that end with a
break i.e. loops the always exit on their first interation.

Reviewed-by: Jason Ekstrand 
---
 src/compiler/nir/nir_loop_analyze.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/compiler/nir/nir_loop_analyze.c 
b/src/compiler/nir/nir_loop_analyze.c
index d564296aa67..5454b7691ba 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -717,13 +717,6 @@ get_loop_info(loop_info_state *state, nir_function_impl 
*impl)
   }
}
 
-   /* Induction analysis needs invariance information so get that first */
-   compute_invariance_information(state);
-
-   /* We have invariance information so try to find induction variables */
-   if (!compute_induction_information(state))
-  return;
-
/* Try to find all simple terminators of the loop. If we can't find any,
 * or we find possible terminators that have side effects then bail.
 */
@@ -737,6 +730,13 @@ get_loop_info(loop_info_state *state, nir_function_impl 
*impl)
   return;
}
 
+   /* Induction analysis needs invariance information so get that first */
+   compute_invariance_information(state);
+
+   /* We have invariance information so try to find induction variables */
+   if (!compute_induction_information(state))
+  return;
+
/* Run through each of the terminators and try to compute a trip-count */
find_trip_count(state);
 
-- 
2.17.1

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

[Mesa-dev] [PATCH v4 5/7] nir/opt_loop_unroll: Remove unneeded phis if we make progress

[Timothy Arceri]

Now that SSA values can be derefs and they have special rules, we have
to be a bit more careful about our LCSSA phis.  In particular, we need
to clean up in case LCSSA ended up creating a phi node for a deref.
This avoids validation issues with some CTS tests with the following
patch, but its possible this we could also see the same problem with
the existing unrolling passes.
---
 src/compiler/nir/nir_opt_loop_unroll.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_opt_loop_unroll.c 
b/src/compiler/nir/nir_opt_loop_unroll.c
index a1ad0e59814..e0e0b754716 100644
--- a/src/compiler/nir/nir_opt_loop_unroll.c
+++ b/src/compiler/nir/nir_opt_loop_unroll.c
@@ -575,9 +575,17 @@ nir_opt_loop_unroll_impl(nir_function_impl *impl,
 _nested_loop);
}
 
-   if (progress)
+   if (progress) {
   nir_lower_regs_to_ssa_impl(impl);
 
+  /* Calling nir_convert_loop_to_lcssa() adds extra phi nodes which may
+   * not be valid if they're used for something such as a deref.
+   *  Remove any unneeded phis.
+   */
+  nir_copy_prop(impl->function->shader);
+  nir_opt_remove_phis_impl(impl);
+   }
+
return progress;
 }
 
-- 
2.17.1

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

[Mesa-dev] [PATCH v4 4/7] nir: add complex_loop bool to loop info

[Timothy Arceri]

In order to be sure loop_terminator_list is an accurate
representation of all the jumps in the loop we need to be sure we
didn't encounter any other complex behaviour such as continues,
nested breaks, etc during analysis.

This will be used in the following patch.

Reviewed-by: Jason Ekstrand 
---
 src/compiler/nir/nir.h  | 6 ++
 src/compiler/nir/nir_loop_analyze.c | 8 ++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 0caacd30173..20e8b501460 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -1801,6 +1801,12 @@ typedef struct {
/* Unroll the loop regardless of its size */
bool force_unroll;
 
+   /* Does the loop contain complex loop terminators, continues or other
+* complex behaviours? If this is true we can't rely on
+* loop_terminator_list to be complete or accurate.
+*/
+   bool complex_loop;
+
nir_loop_terminator *limiting_terminator;
 
/* A list of loop_terminators terminating this loop. */
diff --git a/src/compiler/nir/nir_loop_analyze.c 
b/src/compiler/nir/nir_loop_analyze.c
index 5454b7691ba..9c3fd2f286f 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -317,15 +317,19 @@ find_loop_terminators(loop_info_state *state)
   * not find a loop terminator, but there is a break-statement then
   * we should return false so that we do not try to find trip-count
   */
- if (!nir_is_trivial_loop_if(nif, break_blk))
+ if (!nir_is_trivial_loop_if(nif, break_blk)) {
+state->loop->info->complex_loop = true;
 return false;
+ }
 
  /* Continue if the if contained no jumps at all */
  if (!break_blk)
 continue;
 
- if (nif->condition.ssa->parent_instr->type == nir_instr_type_phi)
+ if (nif->condition.ssa->parent_instr->type == nir_instr_type_phi) {
+state->loop->info->complex_loop = true;
 return false;
+ }
 
  nir_loop_terminator *terminator =
 rzalloc(state->loop->info, nir_loop_terminator);
-- 
2.17.1

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

[Mesa-dev] [PATCH v4 2/7] nir: propagates if condition evaluation down some alu chains

[Timothy Arceri]

v2:
 - only allow nir_op_inot or nir_op_b2i when alu input is 1.
 - use some helpers as suggested by Jason.

shader-db IVB results:

total instructions in shared programs: 9993483 -> 9993472 (-0.00%)
instructions in affected programs: 1300 -> 1289 (-0.85%)
helped: 11
HURT: 0

total cycles in shared programs: 219476091 -> 219476059 (-0.00%)
cycles in affected programs: 7675 -> 7643 (-0.42%)
helped: 10
HURT: 1
---
 src/compiler/nir/nir_opt_if.c | 135 --
 1 file changed, 129 insertions(+), 6 deletions(-)

diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index 11c6693d302..6d81705f6b7 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -403,9 +403,113 @@ evaluate_if_condition(nir_if *nif, nir_cursor cursor, 
uint32_t *value)
}
 }
 
+/*
+ * This propagates if condition evaluation down the chain of some alu
+ * instructions. For example by checking the use of some of the following alu
+ * instruction we can eventually replace ssa_107 with NIR_TRUE.
+ *
+ *   loop {
+ *  block block_1:
+ *  vec1 32 ssa_85 = load_const (0x0002)
+ *  vec1 32 ssa_86 = ieq ssa_48, ssa_85
+ *  vec1 32 ssa_87 = load_const (0x0001)
+ *  vec1 32 ssa_88 = ieq ssa_48, ssa_87
+ *  vec1 32 ssa_89 = ior ssa_86, ssa_88
+ *  vec1 32 ssa_90 = ieq ssa_48, ssa_0
+ *  vec1 32 ssa_91 = ior ssa_89, ssa_90
+ *  if ssa_86 {
+ * block block_2:
+ * ...
+ *break
+ *  } else {
+ *block block_3:
+ *  }
+ *  block block_4:
+ *  if ssa_88 {
+ *block block_5:
+ * ...
+ *break
+ *  } else {
+ *block block_6:
+ *  }
+ *  block block_7:
+ *  if ssa_90 {
+ *block block_8:
+ * ...
+ *break
+ *  } else {
+ *block block_9:
+ *  }
+ *  block block_10:
+ *  vec1 32 ssa_107 = inot ssa_91
+ *  if ssa_107 {
+ *block block_11:
+ *break
+ *  } else {
+ *block block_12:
+ *  }
+ *   }
+ */
 static bool
-evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx,
-   bool is_if_condition)
+propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,
+ nir_src *alu_use, nir_alu_instr *alu, void *mem_ctx,
+ bool is_if_condition)
+{
+   bool progress = false;
+
+   uint32_t bool_value;
+   nir_cursor cursor = nir_before_src(alu_use, is_if_condition);
+   if (nir_op_infos[alu->op].num_inputs == 1) {
+  if ((alu->op == nir_op_inot || alu->op == nir_op_b2i) &&
+  evaluate_if_condition(nif, cursor, _value)) {
+ replace_if_condition_use_with_const(alu_use, mem_ctx, cursor,
+ bool_value, is_if_condition);
+ progress = true;
+  }
+   } else {
+  assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
+
+  if (evaluate_if_condition(nif, cursor, _value)) {
+ nir_ssa_def *def[2];
+ for (unsigned i = 0; i < 2; i++) {
+if (alu->src[i].src.ssa == use_src->ssa) {
+   if (is_if_condition) {
+  b->cursor =
+ nir_before_cf_node(_use->parent_if->cf_node);
+   } else {
+  b->cursor = nir_before_instr(alu_use->parent_instr);
+   }
+
+   nir_const_value const_value;
+   const_value.u32[0] = bool_value;
+
+   def[i] = nir_build_imm(b, 1, 32, const_value);
+} else {
+   def[i] = alu->src[i].src.ssa;
+}
+ }
+
+ nir_ssa_def *nalu =
+nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
+
+ /* Rewrite use to use new alu instruction */
+ nir_src new_src = nir_src_for_ssa(nalu);
+
+ if (is_if_condition)
+nir_if_rewrite_condition(alu_use->parent_if, new_src);
+ else
+nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src);
+
+ progress = true;
+  }
+   }
+
+   return progress;
+}
+
+static bool
+evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src,
+   void *mem_ctx, bool is_if_condition)
 {
bool progress = false;
 
@@ -417,23 +521,42 @@ evaluate_condition_use(nir_if *nif, nir_src *use_src, 
void *mem_ctx,
   progress = true;
}
 
+   if (!is_if_condition &&
+   use_src->parent_instr->type == nir_instr_type_alu &&
+   (nir_instr_as_alu(use_src->parent_instr)->op == nir_op_ior ||
+nir_instr_as_alu(use_src->parent_instr)->op == nir_op_iand ||
+nir_op_infos[nir_instr_as_alu(use_src->parent_instr)->op].num_inputs 
== 1)) {
+
+ nir_alu_instr *alu = nir_instr_as_alu(use_src->parent_instr);
+
+ nir_foreach_use_safe(alu_use, >dest.dest.ssa) {
+progress |= propagate_condition_eval(b, nif, use_src, 

[Mesa-dev] [PATCH v4 6/7] nir: add loop unroll support for wrapper loops

[Timothy Arceri]

This adds support for unrolling the classic

do {
// ...
} while (false)

that is used to wrap multi-line macros. GLSL IR also wraps switch
statements in a loop like this.

shader-db results IVB:

total loops in shared programs: 2515 -> 2512 (-0.12%)
loops in affected programs: 33 -> 30 (-9.09%)
helped: 3
HURT: 0
---
 src/compiler/nir/nir_opt_loop_unroll.c | 77 ++
 1 file changed, 77 insertions(+)

diff --git a/src/compiler/nir/nir_opt_loop_unroll.c 
b/src/compiler/nir/nir_opt_loop_unroll.c
index e0e0b754716..9c33267cb72 100644
--- a/src/compiler/nir/nir_opt_loop_unroll.c
+++ b/src/compiler/nir/nir_opt_loop_unroll.c
@@ -465,6 +465,65 @@ complex_unroll(nir_loop *loop, nir_loop_terminator 
*unlimit_term,
_mesa_hash_table_destroy(remap_table, NULL);
 }
 
+/* Unrolls the classic wrapper loops e.g
+ *
+ *do {
+ *// ...
+ *} while (false)
+ */
+static bool
+wrapper_unroll(nir_loop *loop)
+{
+   bool progress = false;
+
+   nir_block *blk_after_loop =
+  nir_cursor_current_block(nir_after_cf_node(>cf_node));
+
+   /* There may still be some single src phis following the loop that
+* have not yet been cleaned up by another pass. Tidy those up before
+* unrolling the loop.
+*/
+   nir_foreach_instr_safe(instr, blk_after_loop) {
+  if (instr->type != nir_instr_type_phi)
+ break;
+
+  nir_phi_instr *phi = nir_instr_as_phi(instr);
+  assert(exec_list_length(>srcs) == 1);
+
+  nir_phi_src *phi_src = exec_node_data(nir_phi_src,
+exec_list_get_head(>srcs),
+node);
+
+  nir_ssa_def_rewrite_uses(>dest.ssa, phi_src->src);
+  nir_instr_remove(instr);
+
+  progress = true;
+   }
+
+   nir_block *last_loop_blk = nir_loop_last_block(loop);
+   if (nir_block_ends_in_break(last_loop_blk)) {
+
+  /* Remove break at end of the loop */
+  nir_instr *break_instr = nir_block_last_instr(last_loop_blk);
+  nir_instr_remove(break_instr);
+
+  /* Pluck out the loop body. */
+  nir_cf_list loop_body;
+  nir_cf_extract(_body, nir_before_block(nir_loop_first_block(loop)),
+ nir_after_block(nir_loop_last_block(loop)));
+
+  /* Reinsert loop body after the loop */
+  nir_cf_reinsert(_body, nir_after_cf_node(>cf_node));
+
+  /* The loop has been unrolled so remove it. */
+  nir_cf_node_remove(>cf_node);
+
+  progress = true;
+   }
+
+   return progress;
+}
+
 static bool
 is_loop_small_enough_to_unroll(nir_shader *shader, nir_loop_info *li)
 {
@@ -516,6 +575,24 @@ process_loops(nir_shader *sh, nir_cf_node *cf_node, bool 
*has_nested_loop_out)
 */
if (!progress) {
 
+  /* Check for the classic
+   *
+   *do {
+   *// ...
+   *} while (false)
+   *
+   * that is used to wrap multi-line macros. GLSL IR also wraps switch
+   * statements in a loop like this.
+   */
+  if (loop->info->limiting_terminator == NULL &&
+  list_empty(>info->loop_terminator_list) &&
+  !loop->info->complex_loop) {
+
+ progress = wrapper_unroll(loop);
+
+ goto exit;
+  }
+
   if (has_nested_loop || loop->info->limiting_terminator == NULL)
  goto exit;
 
-- 
2.17.1

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

[Mesa-dev] [PATCH v4 1/7] nir: evaluate if condition uses inside the if branches

[Timothy Arceri]

Since we know what side of the branch we ended up on we can just
replace the use with a constant.

All the spill changes in shader-db are from Dolphin uber shaders,
despite some small regressions the change is clearly positive.

V2: insert new constant after any phis in the
use->parent_instr->type == nir_instr_type_phi path.

v3:
 - use nir_after_block_before_jump() for inserting const
 - check dominance of phi uses correctly

v4:
 - create some helpers as suggested by Jason.

shader-db results IVB:

total instructions in shared programs: 201 -> 9993483 (-0.06%)
instructions in affected programs: 163235 -> 157517 (-3.50%)
helped: 132
HURT: 2

total cycles in shared programs: 231670754 -> 219476091 (-5.26%)
cycles in affected programs: 143424120 -> 131229457 (-8.50%)
helped: 115
HURT: 24

total spills in shared programs: 4383 -> 4370 (-0.30%)
spills in affected programs: 1656 -> 1643 (-0.79%)
helped: 9
HURT: 18

total fills in shared programs: 4610 -> 4581 (-0.63%)
fills in affected programs: 374 -> 345 (-7.75%)
helped: 6
HURT: 0
---
 src/compiler/nir/nir.h|  22 +++
 src/compiler/nir/nir_opt_if.c | 113 ++
 2 files changed, 135 insertions(+)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 009a6d60371..0caacd30173 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2331,6 +2331,28 @@ nir_after_block_before_jump(nir_block *block)
}
 }
 
+static inline nir_cursor
+nir_before_src(nir_src *src, bool is_if_condition)
+{
+   if (is_if_condition) {
+  nir_block *prev_block =
+ nir_cf_node_as_block(nir_cf_node_prev(>parent_if->cf_node));
+  assert(!nir_block_ends_in_jump(prev_block));
+  return nir_after_block(prev_block);
+   } else if (src->parent_instr->type == nir_instr_type_phi) {
+  nir_phi_instr *cond_phi = nir_instr_as_phi(src->parent_instr);
+  nir_foreach_phi_src(phi_src, cond_phi) {
+ if (phi_src->src.ssa == src->ssa) {
+return nir_after_block_before_jump(phi_src->pred);
+ }
+  }
+
+  unreachable("failed to find phi src");
+   } else {
+  return nir_before_instr(src->parent_instr);
+   }
+}
+
 static inline nir_cursor
 nir_before_cf_node(nir_cf_node *node)
 {
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index dacf2d6c667..11c6693d302 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -369,6 +369,76 @@ opt_if_loop_terminator(nir_if *nif)
return true;
 }
 
+static void
+replace_if_condition_use_with_const(nir_src *use, void *mem_ctx,
+nir_cursor cursor, unsigned nir_boolean,
+bool if_condition)
+{
+   /* Create const */
+   nir_load_const_instr *load = nir_load_const_instr_create(mem_ctx, 1, 32);
+   load->value.u32[0] = nir_boolean;
+   nir_instr_insert(cursor, >instr);
+
+   /* Rewrite use to use const */
+   nir_src new_src = nir_src_for_ssa(>def);
+
+   if (if_condition)
+  nir_if_rewrite_condition(use->parent_if, new_src);
+   else
+  nir_instr_rewrite_src(use->parent_instr, use, new_src);
+}
+
+static bool
+evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t *value)
+{
+   nir_block *use_block = nir_cursor_current_block(cursor);
+   if (nir_block_dominates(nir_if_first_then_block(nif), use_block)) {
+  *value = NIR_TRUE;
+  return true;
+   } else if (nir_block_dominates(nir_if_first_else_block(nif), use_block)) {
+  *value = NIR_FALSE;
+  return true;
+   } else {
+  return false;
+   }
+}
+
+static bool
+evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx,
+   bool is_if_condition)
+{
+   bool progress = false;
+
+   uint32_t value;
+   nir_cursor cursor = nir_before_src(use_src, is_if_condition);
+   if (evaluate_if_condition(nif, cursor, )) {
+  replace_if_condition_use_with_const(use_src, mem_ctx, cursor, value,
+  is_if_condition);
+  progress = true;
+   }
+
+   return progress;
+}
+
+static bool
+opt_if_evaluate_condition_use(nir_if *nif, void *mem_ctx)
+{
+   bool progress = false;
+
+   /* Evaluate any uses of the if condition inside the if branches */
+   assert(nif->condition.is_ssa);
+   nir_foreach_use_safe(use_src, nif->condition.ssa) {
+  progress |= evaluate_condition_use(nif, use_src, mem_ctx, false);
+   }
+
+   nir_foreach_if_use_safe(use_src, nif->condition.ssa) {
+  if (use_src->parent_if != nif)
+ progress |= evaluate_condition_use(nif, use_src, mem_ctx, true);
+   }
+
+   return progress;
+}
+
 static bool
 opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
 {
@@ -402,6 +472,41 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
return progress;
 }
 
+/**
+ * These optimisations depend on nir_metadata_block_index and therefore must
+ * not do anything to cause the metadata to become invalid.
+ */
+static bool

[Mesa-dev] [Bug 107524] Broken packDouble2x32 at llvmpipe

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107524

--- Comment #8 from Matwey V. Kornilov  ---
(In reply to Dave Airlie from comment #6)
> Created attachment 141293 [details] [review]
> pass two swizzles into fetches.

I've checked that the patch fixes the issue for llvm at my side.

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

Re: [Mesa-dev] [PATCH 3/4] virgl: introduce $VIRGL_DEBUG=verbose

[Gert Wollny]

Am Montag, den 20.08.2018, 14:10 +0200 schrieb Erik Faye-Lund:
> This adds an environment-varaible that can be used for driver-

variable 

With that patches 1-3 are 
Reviewed-By: Gert Wollny 

> specific flags, as well as a flag for it to enable verbose output.

> 
> While we're at it, quiet some overly chatty debug-output by default.
> 
> Signed-off-by: Erik Faye-Lund 



> ---
>  src/gallium/drivers/virgl/virgl_context.c | 6 --
>  src/gallium/drivers/virgl/virgl_encode.c  | 3 ++-
>  src/gallium/drivers/virgl/virgl_screen.c  | 9 +
>  src/gallium/drivers/virgl/virgl_screen.h  | 3 +++
>  4 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_context.c
> b/src/gallium/drivers/virgl/virgl_context.c
> index dc1dd2d3c2..edc03f5dcf 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -1115,8 +1115,10 @@ static void virgl_get_sample_position(struct
> pipe_context *ctx,
> }
> out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
> out_value[1] = (bits & 0xf) / 16.0f;
> -   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
> - index, sample_count, out_value[0], out_value[1]);
> +
> +   if (virgl_debug & VIRGL_DEBUG_VERBOSE)
> +  debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
> +   index, sample_count, out_value[0], out_value[1]);
>  }
>  
>  struct pipe_context *virgl_context_create(struct pipe_screen
> *pscreen,
> diff --git a/src/gallium/drivers/virgl/virgl_encode.c
> b/src/gallium/drivers/virgl/virgl_encode.c
> index 6b0077ac0c..ea544fe5cd 100644
> --- a/src/gallium/drivers/virgl/virgl_encode.c
> +++ b/src/gallium/drivers/virgl/virgl_encode.c
> @@ -261,7 +261,8 @@ int virgl_encode_shader_state(struct
> virgl_context *ctx,
>  
>bret = tgsi_dump_str(tokens, TGSI_DUMP_FLOAT_AS_HEX, str,
> str_total_size);
>if (bret == false) {
> - debug_printf("Failed to translate shader in available space
> - trying again\n");
> + if (virgl_debug & VIRGL_DEBUG_VERBOSE)
> +debug_printf("Failed to translate shader in available
> space - trying again\n");
>   old_size = str_total_size;
>   str_total_size = 65536 * ++retry_size;
>   str = REALLOC(str, old_size, str_total_size);
> diff --git a/src/gallium/drivers/virgl/virgl_screen.c
> b/src/gallium/drivers/virgl/virgl_screen.c
> index 86063c66aa..61147c423c 100644
> --- a/src/gallium/drivers/virgl/virgl_screen.c
> +++ b/src/gallium/drivers/virgl/virgl_screen.c
> @@ -36,6 +36,13 @@
>  #include "virgl_public.h"
>  #include "virgl_context.h"
>  
> +int virgl_debug = 0;
> +static const struct debug_named_value debug_options[] = {
> +   { "verbose", VIRGL_DEBUG_VERBOSE, NULL },
> +   DEBUG_NAMED_VALUE_END
> +};
> +DEBUG_GET_ONCE_FLAGS_OPTION(virgl_debug, "VIRGL_DEBUG",
> debug_options, 0)
> +
>  static const char *
>  virgl_get_vendor(struct pipe_screen *screen)
>  {
> @@ -724,6 +731,8 @@ virgl_create_screen(struct virgl_winsys *vws)
> if (!screen)
>return NULL;
>  
> +   virgl_debug = debug_get_option_virgl_debug();
> +
> screen->vws = vws;
> screen->base.get_name = virgl_get_name;
> screen->base.get_vendor = virgl_get_vendor;
> diff --git a/src/gallium/drivers/virgl/virgl_screen.h
> b/src/gallium/drivers/virgl/virgl_screen.h
> index dcf5816d60..8334d16242 100644
> --- a/src/gallium/drivers/virgl/virgl_screen.h
> +++ b/src/gallium/drivers/virgl/virgl_screen.h
> @@ -27,6 +27,9 @@
>  #include "util/slab.h"
>  #include "virgl_winsys.h"
>  
> +#define VIRGL_DEBUG_VERBOSE 1
> +extern int virgl_debug;
> +
>  struct virgl_screen {
> struct pipe_screen base;
>  
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] virgl: add debug-switch to output TGSI

[Gert Wollny]

Am Montag, den 20.08.2018, 14:10 +0200 schrieb Erik Faye-Lund:
> This is quite useful for debugging shader-transpiling issues in
> virglrenderer.
Isn't this coverted by ST_DEBUG=tgsi? 

Also, virglrenderer has a variable vrend_dump_shaders in
vrend_renderer.c that enables dumping all the TGSI + the created GLSL
shaders. 

Best, 
Gert


> 
> Signed-off-by: Erik Faye-Lund 
> ---
>  src/gallium/drivers/virgl/virgl_encode.c | 3 +++
>  src/gallium/drivers/virgl/virgl_screen.c | 1 +
>  src/gallium/drivers/virgl/virgl_screen.h | 1 +
>  3 files changed, 5 insertions(+)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_encode.c
> b/src/gallium/drivers/virgl/virgl_encode.c
> index ea544fe5cd..4e99dda0f7 100644
> --- a/src/gallium/drivers/virgl/virgl_encode.c
> +++ b/src/gallium/drivers/virgl/virgl_encode.c
> @@ -274,6 +274,9 @@ int virgl_encode_shader_state(struct
> virgl_context *ctx,
> if (bret == false)
>return -1;
>  
> +   if (virgl_debug & VIRGL_DEBUG_TGSI)
> +  debug_printf("TGSI:\n---8<---\n%s\n---8<---\n", str);
> +
> shader_len = strlen(str) + 1;
>  
> left_bytes = shader_len;
> diff --git a/src/gallium/drivers/virgl/virgl_screen.c
> b/src/gallium/drivers/virgl/virgl_screen.c
> index 61147c423c..d9b50c9cca 100644
> --- a/src/gallium/drivers/virgl/virgl_screen.c
> +++ b/src/gallium/drivers/virgl/virgl_screen.c
> @@ -39,6 +39,7 @@
>  int virgl_debug = 0;
>  static const struct debug_named_value debug_options[] = {
> { "verbose", VIRGL_DEBUG_VERBOSE, NULL },
> +   { "tgsi", VIRGL_DEBUG_TGSI, NULL },
> DEBUG_NAMED_VALUE_END
>  };
>  DEBUG_GET_ONCE_FLAGS_OPTION(virgl_debug, "VIRGL_DEBUG",
> debug_options, 0)
> diff --git a/src/gallium/drivers/virgl/virgl_screen.h
> b/src/gallium/drivers/virgl/virgl_screen.h
> index 8334d16242..719f5166d7 100644
> --- a/src/gallium/drivers/virgl/virgl_screen.h
> +++ b/src/gallium/drivers/virgl/virgl_screen.h
> @@ -28,6 +28,7 @@
>  #include "virgl_winsys.h"
>  
>  #define VIRGL_DEBUG_VERBOSE 1
> +#define VIRGL_DEBUG_TGSI2
>  extern int virgl_debug;
>  
>  struct virgl_screen {
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] anv/android: add GetAndroidHardwareBufferPropertiesANDROID

[Jason Ekstrand]

On Sun, Aug 26, 2018 at 11:54 PM Tapani Pälli 
wrote:

>
>
> On 08/24/2018 05:04 PM, Jason Ekstrand wrote:
> > On Fri, Aug 24, 2018 at 12:08 AM Tapani Pälli  > > wrote:
> >
> >
> >
> > On 08/22/2018 05:18 PM, Jason Ekstrand wrote:
> >  > On Tue, Aug 21, 2018 at 3:27 AM Tapani Pälli
> > mailto:tapani.pa...@intel.com>
> >  > >>
> > wrote:
> >  >
> >  > When adding YUV support, we need to figure out
> > implementation-defined
> >  > external format identifier.
> >  >
> >  > Signed-off-by: Tapani Pälli  > 
> >  >  >>>
> >  > ---
> >  >   src/intel/vulkan/anv_android.c | 99
> >  > ++
> >  >   1 file changed, 99 insertions(+)
> >  >
> >  > diff --git a/src/intel/vulkan/anv_android.c
> >  > b/src/intel/vulkan/anv_android.c
> >  > index 46c41d57861..7d0eb588e2b 100644
> >  > --- a/src/intel/vulkan/anv_android.c
> >  > +++ b/src/intel/vulkan/anv_android.c
> >  > @@ -29,6 +29,8 @@
> >  >   #include 
> >  >
> >  >   #include "anv_private.h"
> >  > +#include "vk_format_info.h"
> >  > +#include "vk_util.h"
> >  >
> >  >   static int anv_hal_open(const struct hw_module_t* mod,
> > const char*
> >  > id, struct hw_device_t** dev);
> >  >   static int anv_hal_close(struct hw_device_t *dev);
> >  > @@ -96,6 +98,103 @@ anv_hal_close(struct hw_device_t *dev)
> >  >  return -1;
> >  >   }
> >  >
> >  > +static VkResult
> >  > +get_ahw_buffer_format_properties(
> >  > +   VkDevice device_h,
> >  > +   const struct AHardwareBuffer *buffer,
> >  > +   VkAndroidHardwareBufferFormatPropertiesANDROID
> *pProperties)
> >  > +{
> >  > +   ANV_FROM_HANDLE(anv_device, device, device_h);
> >  > +
> >  > +   /* Get a description of buffer contents . */
> >  > +   AHardwareBuffer_Desc desc;
> >  > +   AHardwareBuffer_describe(buffer, );
> >  > +
> >  > +   /* Verify description. */
> >  > +   uint64_t gpu_usage =
> >  > +  AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE |
> >  > +  AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT |
> >  > +  AHARDWAREBUFFER_USAGE_GPU_DATA_BUFFER;
> >  > +
> >  > +   /* "Buffer must be a valid Android hardware buffer object
> > with
> >  > at least
> >  > +* one of the AHARDWAREBUFFER_USAGE_GPU_* usage flags."
> >  > +*/
> >  > +   if (!(desc.usage & (gpu_usage)))
> >  > +  return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR;
> >  > +
> >  > +   /* Fill properties fields based on description. */
> >  > +   VkAndroidHardwareBufferFormatPropertiesANDROID *p =
> > pProperties;
> >  > +
> >  > +   p->pNext = NULL;
> >  >
> >  >
> >  > You shouldn't be overwriting pNext.  That's used by the client to
> > let
> >  > them chain in multiple structs to fill out in case Google ever
> > extends
> >  > this extension.  Also, while we're here, it'd be good to throw in
> an
> >  > assert that p->sType is the right thing.
> >
> > Yes of course, will remove.
> >
> >  > +   p->format = vk_format_from_android(desc.format);
> >  > +   p->externalFormat = 1; /* XXX */
> >  > +
> >  > +   const struct anv_format *anv_format =
> > anv_get_format(p->format);
> >  > +   struct anv_physical_device *physical_device =
> >  > +  >instance->physicalDevice;
> >  > +   const struct gen_device_info *devinfo =
> > _device->info;
> >  >
> >  >
> >  > If all you need is devinfo, that's avilable in the device; you
> don't
> >  > need to get the physical device for it.  Should be device->info.
> >
> > OK
> >
> >  > +
> >  > +   p->formatFeatures =
> >  > +  anv_get_image_format_features(devinfo, p->format,
> >  > +anv_format,
> >  > VK_IMAGE_TILING_OPTIMAL);
> >  > +
> >  > +   /* "The formatFeatures member *must* include
> >  > +*  VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT and at least one
> of
> >  > +*  VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT or
> >  > +*  VK_FORMAT_FEATURE_COSITED_CHROMA_SAMPLES_BIT"
> >  > +*/
> >  > +   p->formatFeatures |=
> >  > +  VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT;
> >  >
> >  >
> >  > Uh... Why not just throw in SAMPLED_BIT?  For that matter, all of
> > the
> >  > formats you have in your 

[Mesa-dev] [PATCH 2/2] i965: Add INTEL_fragment_shader_ordering support.

[kevin . rogovin]

From: Kevin Rogovin 

Adds suppport for INTEL_fragment_shader_ordering. We achieve
the fragment ordering by using the same instruction as for
beginInvocationInterlockARB() which is by issuing a memory
fence via sendc.

Signed-off-by: Kevin Rogovin 
---
 docs/relnotes/18.3.0.html| 1 +
 src/intel/compiler/brw_fs_nir.cpp| 1 +
 src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html
index afcb044817..71fb41ca86 100644
--- a/docs/relnotes/18.3.0.html
+++ b/docs/relnotes/18.3.0.html
@@ -59,6 +59,7 @@ Note: some of the new features are only available with 
certain drivers.
 GL_EXT_vertex_attrib_64bit on i965, nvc0, radeonsi.
 GL_EXT_window_rectangles on radeonsi.
 GL_KHR_texture_compression_astc_sliced_3d on radeonsi.
+GL_INTEL_fragment_shader_ordering on i965.
 GL_NV_fragment_shader_interlock on i965.
 
 
diff --git a/src/intel/compiler/brw_fs_nir.cpp 
b/src/intel/compiler/brw_fs_nir.cpp
index 9c9df5ac09..62bff2a323 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -4836,6 +4836,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , 
nir_intrinsic_instr *instr
   break;
}
 
+   case nir_intrinsic_begin_fragment_shader_ordering:
case nir_intrinsic_begin_invocation_interlock: {
   const fs_builder ubld = bld.group(8, 0);
   const fs_reg tmp = ubld.vgrf(BRW_REGISTER_TYPE_UD, 2);
diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 0b137664b0..1ea8594c34 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -247,6 +247,7 @@ intelInitExtensions(struct gl_context *ctx)
   ctx->Extensions.OES_primitive_bounding_box = true;
   ctx->Extensions.OES_texture_buffer = true;
   ctx->Extensions.ARB_fragment_shader_interlock = true;
+  ctx->Extensions.INTEL_fragment_shader_ordering = true;
 
   if (can_do_pipelined_register_writes(brw->screen)) {
  ctx->Extensions.ARB_draw_indirect = true;
-- 
2.17.1

___
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/GLSL plumbing for INTEL_fragment_shader_ordering.

[kevin . rogovin]

From: Kevin Rogovin 

   This extension provides new GLSL built-in function
   beginFragmentShaderOrderingIntel() that guarantees
   (taking wording of GL_INTEL_fragment_shader_ordering
   extension) that any memory transactions issued by
   shader invocations from previous primitives mapped to
   same xy window coordinates (and same sample when
   per-sample shading is active), complete and are visible
   to the shader invocation that called
   beginFragmentShaderOrderingINTEL().

   One advantage of INTEL_fragment_shader_ordering over
   ARB_fragment_shader_interlock is that it provides a
   function that operates as a memory barrie (instead
   of a defining a critcial section) that can be called
   under arbitary control flow from any function (in
   contrast the begin/end of ARB_fragment_shader_interlock
   may only be called once, from main(), under no control
   flow.

Signed-off-by: Kevin Rogovin 
---
 src/compiler/glsl/builtin_functions.cpp  | 17 +
 src/compiler/glsl/glsl_parser_extras.cpp |  1 +
 src/compiler/glsl/glsl_parser_extras.h   |  2 ++
 src/compiler/glsl/glsl_to_nir.cpp|  6 ++
 src/compiler/glsl/ir.h   |  1 +
 src/compiler/nir/nir_intrinsics.py   |  1 +
 src/mesa/main/extensions_table.h |  1 +
 src/mesa/main/mtypes.h   |  1 +
 8 files changed, 30 insertions(+)

diff --git a/src/compiler/glsl/builtin_functions.cpp 
b/src/compiler/glsl/builtin_functions.cpp
index b601880686..5650365d1d 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -525,6 +525,12 @@ supports_nv_fragment_shader_interlock(const 
_mesa_glsl_parse_state *state)
return state->NV_fragment_shader_interlock_enable;
 }
 
+static bool
+supports_intel_fragment_shader_ordering(const _mesa_glsl_parse_state *state)
+{
+   return state->INTEL_fragment_shader_ordering_enable;
+}
+
 static bool
 shader_clock(const _mesa_glsl_parse_state *state)
 {
@@ -1305,6 +1311,11 @@ builtin_builder::create_intrinsics()
supports_arb_fragment_shader_interlock,
ir_intrinsic_end_invocation_interlock), NULL);
 
+   add_function("__intrinsic_begin_fragment_shader_ordering",
+_invocation_interlock_intrinsic(
+   supports_intel_fragment_shader_ordering,
+   ir_intrinsic_begin_fragment_shader_ordering), NULL);
+
add_function("__intrinsic_shader_clock",
 _shader_clock_intrinsic(shader_clock,
 glsl_type::uvec2_type),
@@ -3419,6 +3430,12 @@ builtin_builder::create_builtins()
supports_nv_fragment_shader_interlock),
 NULL);
 
+   add_function("beginFragmentShaderOrderingINTEL",
+_invocation_interlock(
+   "__intrinsic_begin_fragment_shader_ordering",
+   supports_intel_fragment_shader_ordering),
+NULL);
+
add_function("anyInvocationARB",
 _vote("__intrinsic_vote_any", vote),
 NULL);
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 0a7d0d78b1..21d4122444 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -725,6 +725,7 @@ static const _mesa_glsl_extension 
_mesa_glsl_supported_extensions[] = {
EXT_AEP(EXT_texture_buffer),
EXT_AEP(EXT_texture_cube_map_array),
EXT(INTEL_conservative_rasterization),
+   EXT(INTEL_fragment_shader_ordering),
EXT(INTEL_shader_atomic_float_minmax),
EXT(MESA_shader_integer_functions),
EXT(NV_fragment_shader_interlock),
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index 2c8353214a..e03b34d7d6 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -812,6 +812,8 @@ struct _mesa_glsl_parse_state {
bool EXT_texture_cube_map_array_warn;
bool INTEL_conservative_rasterization_enable;
bool INTEL_conservative_rasterization_warn;
+   bool INTEL_fragment_shader_ordering_enable;
+   bool INTEL_fragment_shader_ordering_warn;
bool INTEL_shader_atomic_float_minmax_enable;
bool INTEL_shader_atomic_float_minmax_warn;
bool MESA_shader_integer_functions_enable;
diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
b/src/compiler/glsl/glsl_to_nir.cpp
index a53000f47e..efbb2317ac 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -742,6 +742,9 @@ nir_visitor::visit(ir_call *ir)
   case ir_intrinsic_end_invocation_interlock:
  op = nir_intrinsic_end_invocation_interlock;
  break;
+  case ir_intrinsic_begin_fragment_shader_ordering:
+ op = nir_intrinsic_begin_fragment_shader_ordering;
+ break;
   case ir_intrinsic_group_memory_barrier:
  op = nir_intrinsic_group_memory_barrier;
  break;
@@ -975,6 +978,9 @@ 

[Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[kevin . rogovin]

From: Kevin Rogovin 

INTEL_fragment_shader_ordering provides the ability for shaders
to issue a call to gaurnantee memory write operation ordering
of overlapping pixels or samples. In contrast to
ARB_fragment_shader_interlock, INTEL_fragment_shader_ordering
instead of defining a critical region (which must be in main() and
under no flow control) provides a single function that acts like
a memory barrier that can be called under any control flow.

Kevin Rogovin (2):
  mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.
  i965: Add INTEL_fragment_shader_ordering support.

 docs/relnotes/18.3.0.html|  1 +
 src/compiler/glsl/builtin_functions.cpp  | 17 +
 src/compiler/glsl/glsl_parser_extras.cpp |  1 +
 src/compiler/glsl/glsl_parser_extras.h   |  2 ++
 src/compiler/glsl/glsl_to_nir.cpp|  6 ++
 src/compiler/glsl/ir.h   |  1 +
 src/compiler/nir/nir_intrinsics.py   |  1 +
 src/intel/compiler/brw_fs_nir.cpp|  1 +
 src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
 src/mesa/main/extensions_table.h |  1 +
 src/mesa/main/mtypes.h   |  1 +
 11 files changed, 33 insertions(+)

-- 
2.17.1

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