Re: [Mesa-dev] [PATCH v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Gert Wollny
Hello Benedikt, 

Am Sonntag, den 29.04.2018, 00:06 +0200 schrieb Benedikt Schemmer:
> Hi Gert
> 
> Am 28.04.2018 um 23:51 schrieb Gert Wollny:
> > Am Samstag, den 28.04.2018, 22:43 +0200 schrieb Benedikt Schemmer:
> > > The patches apply cleanly, however I just did a shader-db test
> > > run
> > > and can't find a difference with your patch
> > > applied, am I doing something wrong?
> > 
> > AFAIK radeonsi doesn't use the register-merge optimizer in TGSI.
> > 
> 
> Ah, ok. Was wondering why your debug code doesn't output anything.
> Makes sense now ;)
Not exactly, the reason there is no output is because -DNDEBUG is set.
Without it the statistics should also be printed out on radeonsi, but
thinking of it I should probably disable it when register_merge is not
accessed, because without this the numbers will be inflated and don't
have much meaning.

> So is this useless on radeonsi?
Indeed. 

> Seemed interesting to me.
:) it certainly helps on r600 


> > > compile times went up though:
> > 
> > This is strange, because "see above". Did you compile with debug
> > information and c++11 or higher enables?
...
> >   
> > 
> not intentionally:

Then you should actually not run any code that this series adds to
mesa. I checked again, apart from the debugging output nothing will
ever be run if a drivers that report
PIPE_SHADER_CAP_TGSI_SKIP_MERGE_REGISTERS != 0 (as does radeonsi). 

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


Re: [Mesa-dev] [PATCH] nir/linker: Add nir_link_uniforms()

2018-04-29 Thread Timothy Arceri



On 29/04/18 16:48, Alejandro Piñeiro wrote:

From: Eduardo Lima Mitev 

This function will be the entry point for linking the uniforms from
the nir_shader objects associated with the gl_linked_shaders of a
program.

This patch includes initial support for linking uniforms from NIR
shaders. It is tailored for the ARB_gl_spirv needs, and it is far from
complete, but it should handle most cases of uniforms, array
uniforms, structs, samplers and images.

There are some FIXMEs related to specific features that will be
implemented in following patches, like atomic counters, UBOs and
SSBOs.

Also, note that ARB_gl_spirv makes mandatory explicit location for
normal uniforms, so this code only handles uniforms with explicit
location. But there are cases, like uniform atomic counters, that
doesn't have a location from the OpenGL point of view (they have a
binding), but that Mesa assign internally a location. That will be
handled on following patches.

A nir_linker.h file is also added. More NIR-linking related API will
be added in subsequent patches and those will include stuff from Mesa,
so reusing nir.h didn't seem a good idea.


These files should actually be src/compiler/glsl/nir_link_uniforms.c etc 
these are not general purpose nir helpers they are GLSL specific.




v2: update var->data.location with UniformStorage index when the
 storage was set up on a previous stage (Neil)

Signed-off-by: Eduardo Lima 
Signed-off-by: Neil Roberts 
---
  src/compiler/Makefile.sources|   2 +
  src/compiler/nir/meson.build |   2 +
  src/compiler/nir/nir_link_uniforms.c | 461 +++
  src/compiler/nir/nir_linker.h|  41 
  4 files changed, 506 insertions(+)
  create mode 100644 src/compiler/nir/nir_link_uniforms.c
  create mode 100644 src/compiler/nir/nir_linker.h

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index bb86972ea1a..dba70a5e5a1 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -206,6 +206,8 @@ NIR_FILES = \
nir/nir_inline_functions.c \
nir/nir_instr_set.c \
nir/nir_instr_set.h \
+   nir/nir_link_uniforms.c \
+   nir/nir_linker.h \
nir/nir_linking_helpers.c \
nir/nir_liveness.c \
nir/nir_loop_analyze.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index b28a565d0c6..baf5682d388 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -99,6 +99,8 @@ files_libnir = files(
'nir_inline_functions.c',
'nir_instr_set.c',
'nir_instr_set.h',
+  'nir_link_uniforms.c',
+  'nir_linker.h',
'nir_linking_helpers.c',
'nir_liveness.c',
'nir_loop_analyze.c',
diff --git a/src/compiler/nir/nir_link_uniforms.c 
b/src/compiler/nir/nir_link_uniforms.c
new file mode 100644
index 000..450a5ce2e21
--- /dev/null
+++ b/src/compiler/nir/nir_link_uniforms.c
@@ -0,0 +1,461 @@
+/*
+ * 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_linker.h"
+#include "compiler/glsl/ir_uniform.h" /* for gl_uniform_storage */
+#include "compiler/linker_util.h"
+#include "main/context.h"
+#include "main/mtypes.h"
+
+/* This file do the common link for GLSL uniforms, using NIR, instead of IR as
+ * the counter-part glsl/link_uniforms.cpp
+ *
+ * Also note that this is tailored for ARB_gl_spirv needs and particularities
+ * (like need to work/link without name available, explicit location for
+ * normal uniforms as mandatory, and so on).
+ */
+
+static void
+nir_setup_uniform_remap_tables(struct gl_context *ctx,
+   struct gl_shader_program *prog)
+{
+   prog->UniformRemapTable = rzalloc_array(prog,
+   struct gl_uniform_storage *,
+   prog->NumUniformRemapTable);
+   

Re: [Mesa-dev] [PATCH 17/22] nir/linker: Add nir_build_program_resource_list()

2018-04-29 Thread Timothy Arceri

Again this should be moved to src/compiler/glsl/ somewhere.

On 18/04/18 00:36, Alejandro Piñeiro wrote:

From: Eduardo Lima Mitev 

This function is equivalent to the compiler/glsl
build_program_resource_list() but will extract the resources from NIR
shaders instead.

For now, only uniforms and program inputs are implemented.

Signed-off-by: Eduardo Lima 
Signed-off-by: Alejandro Piñeiro 
---
  src/compiler/nir/nir_linker.h  |  3 ++
  src/compiler/nir/nir_linking_helpers.c | 63 ++
  2 files changed, 66 insertions(+)

diff --git a/src/compiler/nir/nir_linker.h b/src/compiler/nir/nir_linker.h
index 09f7042dffa..0e3b609a4d2 100644
--- a/src/compiler/nir/nir_linker.h
+++ b/src/compiler/nir/nir_linker.h
@@ -38,6 +38,9 @@ void
  nir_set_uniform_initializers(struct gl_context *ctx,
   struct gl_shader_program *prog);
  
+void nir_build_program_resource_list(struct gl_context *ctx,

+ struct gl_shader_program *prog);
+
  #ifdef __cplusplus
  } /* extern "C" */
  #endif
diff --git a/src/compiler/nir/nir_linking_helpers.c 
b/src/compiler/nir/nir_linking_helpers.c
index 2b0a2668a33..8733fffb4f3 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -24,6 +24,10 @@
  #include "nir.h"
  #include "util/set.h"
  #include "util/hash_table.h"
+#include "nir_linker.h"
+#include "compiler/glsl/ir_uniform.h" /* for gl_uniform_storage */
+#include "compiler/linker_util.h"
+#include "main/context.h"
  
  /* This file contains various little helpers for doing simple linking in

   * NIR.  Eventually, we'll probably want a full-blown varying packing
@@ -503,3 +507,62 @@ nir_compact_varyings(nir_shader *producer, nir_shader 
*consumer,
 compact_components(producer, consumer, comps, interp_type, interp_loc,
default_to_smooth_interp);
  }
+
+void
+nir_build_program_resource_list(struct gl_context *ctx,
+struct gl_shader_program *prog)
+{
+   /* Rebuild resource list. */
+   if (prog->data->ProgramResourceList) {
+  ralloc_free(prog->data->ProgramResourceList);
+  prog->data->ProgramResourceList = NULL;
+  prog->data->NumProgramResourceList = 0;
+   }
+
+   struct set *resource_set = _mesa_set_create(NULL,
+   _mesa_hash_pointer,
+   _mesa_key_pointer_equal);
+
+   /* Add uniforms
+*
+* Here, it is expected that nir_link_uniforms() has already been
+* called, so that UniformStorage table is already available.
+*/
+   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
+  struct gl_uniform_storage *uniform = &prog->data->UniformStorage[i];
+
+  if (!add_program_resource(prog, resource_set, GL_UNIFORM, uniform,
+uniform->active_shader_mask)) {
+ return;
+  }
+   }
+
+   /* Add inputs */
+   struct gl_linked_shader *sh = prog->_LinkedShaders[MESA_SHADER_VERTEX];
+   if (sh) {
+  nir_shader *nir = sh->Program->nir;
+  assert(nir);
+
+  nir_foreach_variable(var, &nir->inputs) {
+ struct gl_shader_variable *sh_var =
+rzalloc(prog, struct gl_shader_variable);
+
+ /* ARB_gl_spirv: names are considered optional debug info, so the 
linker
+  * needs to work without them, and returning them is optional. For
+  * simplicity we ignore names.
+  */
+ sh_var->name = NULL;
+ sh_var->type = var->type;
+ sh_var->location = var->data.location;
+
+ /* @TODO: Fill in the rest of gl_shader_variable data. */
+
+ if (!add_program_resource(prog, resource_set, GL_PROGRAM_INPUT,
+   sh_var, 1 << MESA_SHADER_VERTEX)) {
+return;
+ }
+  }
+   }
+
+   _mesa_set_destroy(resource_set, NULL);
+}


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


Re: [Mesa-dev] [PATCH 15/22] nir/linker: Set the uniform initial values

2018-04-29 Thread Timothy Arceri

This too should be moved to src/compiler/glsl/ somewhere.

On 18/04/18 00:36, Alejandro Piñeiro wrote:

From: Neil Roberts 

This is based on link_uniform_initializers.cpp.
---
  src/compiler/Makefile.sources|   1 +
  src/compiler/nir/meson.build |   1 +
  src/compiler/nir/nir_link_uniform_initializers.c | 292 +++
  src/compiler/nir/nir_link_uniforms.c |   1 +
  src/compiler/nir/nir_linker.h|   4 +
  5 files changed, 299 insertions(+)
  create mode 100644 src/compiler/nir/nir_link_uniform_initializers.c

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index dba70a5e5a1..305ea30b2ab 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -206,6 +206,7 @@ NIR_FILES = \
nir/nir_inline_functions.c \
nir/nir_instr_set.c \
nir/nir_instr_set.h \
+   nir/nir_link_uniform_initializers.c \
nir/nir_link_uniforms.c \
nir/nir_linker.h \
nir/nir_linking_helpers.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index baf5682d388..4f7a2f853e8 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -101,6 +101,7 @@ files_libnir = files(
'nir_instr_set.h',
'nir_link_uniforms.c',
'nir_linker.h',
+  'nir_link_uniform_initializers.c',
'nir_linking_helpers.c',
'nir_liveness.c',
'nir_loop_analyze.c',
diff --git a/src/compiler/nir/nir_link_uniform_initializers.c 
b/src/compiler/nir/nir_link_uniform_initializers.c
new file mode 100644
index 000..94e213330ce
--- /dev/null
+++ b/src/compiler/nir/nir_link_uniform_initializers.c
@@ -0,0 +1,292 @@
+/*
+ * 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_linker.h"
+#include "compiler/glsl/ir_uniform.h" /* for gl_uniform_storage */
+#include "main/context.h"
+#include "main/mtypes.h"
+
+struct set_opaque_binding_closure {
+   struct gl_shader_program *shader_prog;
+   struct gl_program *prog;
+   const nir_variable *var;
+   int binding;
+   int location;
+};
+
+static void
+set_opaque_binding(struct set_opaque_binding_closure *data,
+   const struct glsl_type *type)
+{
+   if (glsl_type_is_array(type) &&
+   glsl_type_is_array(glsl_get_array_element(type))) {
+  const struct glsl_type *element_type = glsl_get_array_element(type);
+
+  for (unsigned int i = 0; i < glsl_get_length(type); i++)
+ set_opaque_binding(data, element_type);
+
+  return;
+   }
+
+   if (data->location < 0 ||
+   data->location >= data->prog->sh.data->NumUniformStorage)
+  return;
+
+   struct gl_uniform_storage *storage =
+  data->prog->sh.data->UniformStorage + data->location++;
+
+   const unsigned elements = MAX2(storage->array_elements, 1);
+
+   for (unsigned int i = 0; i < elements; i++)
+  storage->storage[i].i = data->binding++;
+
+   for (int sh = 0; sh < MESA_SHADER_STAGES; sh++) {
+  struct gl_linked_shader *shader = data->shader_prog->_LinkedShaders[sh];
+
+  if (!shader)
+ continue;
+  if (!storage->opaque[sh].active)
+ continue;
+
+  if (glsl_type_is_sampler(storage->type)) {
+ for (unsigned i = 0; i < elements; i++) {
+const unsigned index = storage->opaque[sh].index + i;
+
+if (storage->is_bindless) {
+   if (index >= shader->Program->sh.NumBindlessSamplers)
+  break;
+   shader->Program->sh.BindlessSamplers[index].unit =
+  storage->storage[i].i;
+   shader->Program->sh.BindlessSamplers[index].bound = true;
+   shader->Program->sh.HasBoundBindlessSampler = true;
+} else {
+   if (index >= ARRAY_SIZE(shader->Program->SamplerUnits))
+  break;
+   

Re: [Mesa-dev] [PATCH v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Benedikt Schemmer
Hi Gert,

couldn't resist at least to try what would happen if I enable register merge 
for radeonsi:

 PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR SpillVGPR  PrivVGPR 
  Scratch  CodeSize  MaxWaves Waits
 piglit 80732   -0.16 %   -0.02 % . .0.87 % 
   0.86 %0.04 % . .
 
--
 All affected 513  -17.58 %   -2.30 % . .4.12 % 
   5.87 %1.73 %0.10 % .
 
--
 Total  80732   -0.16 %   -0.02 % . .0.87 % 
   0.86 %0.04 % . .

I had already removed the defines around the debug code so thats also happily 
outputting data.

fails with two piglit shaders:


[require]
GLSL >= 3.30

[fragment shader]
// [config]
// expect_result: pass
// glsl_version: 3.30
// require_extensions: GL_ARB_bindless_texture GL_ARB_shader_image_load_store
// [end config]

#version 330
#extension GL_ARB_bindless_texture: require
#extension GL_ARB_shader_image_load_store: enable
#extension GL_ARB_arrays_of_arrays: enable

struct s {
writeonly image2D img[3][2];
int y;
};

void main()
{
s a[2][4];
imageStore(a[0][0].img[0][0], ivec2(0, 0), vec4(1, 2, 3, 4));
}


and


[require]
GLSL >= 3.30

[fragment shader]
// [config]
// expect_result: pass
// glsl_version: 3.30
// require_extensions: GL_ARB_bindless_texture
// [end config]

#version 330
#extension GL_ARB_bindless_texture: require
#extension GL_ARB_arrays_of_arrays: enable

struct s {
sampler2D tex[3][2];
int y;
};

out vec4 color;

void main()
{
s a[2][4];
color = texture2D(a[0][0].tex[0][0], vec2(0, 0));
}


Real world is a little different:

Max Increase:

SGPRS: 72 -> 96 (33.33 %) (in shaders/cat/1787.shader_test)
VGPRS: 64 -> 84 (31.25 %) (in 
shaders/dirtrally/0859b69789591d7046e211400b1edd9a7cfca734_742.shader_test)
Spilled SGPRs: 0 -> 16 (0.00 %) (in 
shaders/deusex_mankind/d64e2084204e29749639e8fbd9a1e507c7e5e1dd_6840.shader_test)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 24 -> 32 (33.33 %) (in 
shaders/deusex_mankind/28cac87049d8c833e72296a5a02ea6118f1144e5_5876.shader_test)
Scratch size: 28 -> 36 (28.57 %) dwords per thread (in 
shaders/deusex_mankind/28cac87049d8c833e72296a5a02ea6118f1144e5_5876.shader_test)
Code Size: 6988 -> 8036 (15.00 %) bytes (in shaders/cat/1847.shader_test)
LDS: 0 -> 0 (0.00 %) blocks
Max Waves: 5 -> 7 (40.00 %) (in 
shaders/ruiner/0967c5fce7fc456496b1cfa25fbb1d1c4dcf9bed_2958.shader_test)
Wait states: 0 -> 0 (0.00 %)

Max Decrease:

SGPRS: 104 -> 64 (-38.46 %) (in 
shaders/deusex_mankind/480ddf21b1076d36f9ffd9911389656b5d8e12cb_2878.shader_test)
VGPRS: 44 -> 36 (-18.18 %) (in 
shaders/ruiner/0967c5fce7fc456496b1cfa25fbb1d1c4dcf9bed_2958.shader_test)
Spilled SGPRs: 19 -> 0 (-100.00 %) (in 
shaders/deusex_mankind/0749c9ae23417f918c7286fe502ff5de4cb8e1a0_3276.shader_test)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 17576 -> 17276 (-1.71 %) bytes (in 
shaders/ruiner/75b96ff36f5328b9ff9366f0d0fd58a1046f51bc_3053.shader_test)
LDS: 0 -> 0 (0.00 %) blocks
Max Waves: 8 -> 5 (-37.50 %) (in 
shaders/deusex_mankind/8dabec49e5b6c3b1cbcbaee194eff69f164d72f4_3968.shader_test)
Wait states: 0 -> 0 (0.00 %)


 PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR SpillVGPR  PrivVGPR 
  Scratch  CodeSize  MaxWaves Waits
 0ad6 . . . . . 
. . . .
 aer  590 .0.26 %  -20.00 % . . 
.0.34 % . .
 alien_isolation 1414 . . . . . 
. . . .
 anholt10 . . . . . 
. . . .
 bioshock_infinite   2581   -0.02 %0.03 % . . . 
.0.13 % . .
 blackmesa584 . . . . . 
. . . .
 cat  573   -0.06 %   -0.12 % . . . 
.0.20 %0.05 % .
 csgo1392 . .   -0.88 % . . 
.   -0.03 % . .
 deadisland_definitive   17760.06 % . . . . 
.0.15 %0.01 % .
 deadisland_original11602 . . . . . 
.0.05 % . .
 deadisland_riptide_..293   -0.06 %0.06 % . . . 
.0.32 % . .
 deusex_mankind  50510.08 % 

Re: [Mesa-dev] [PATCH] nir/linker: Add nir_link_uniforms()

2018-04-29 Thread Alejandro Piñeiro
On 29/04/18 10:13, Timothy Arceri wrote:
>
>
> On 29/04/18 16:48, Alejandro Piñeiro wrote:
>> From: Eduardo Lima Mitev 
>>
>> This function will be the entry point for linking the uniforms from
>> the nir_shader objects associated with the gl_linked_shaders of a
>> program.
>>
>> This patch includes initial support for linking uniforms from NIR
>> shaders. It is tailored for the ARB_gl_spirv needs, and it is far from
>> complete, but it should handle most cases of uniforms, array
>> uniforms, structs, samplers and images.
>>
>> There are some FIXMEs related to specific features that will be
>> implemented in following patches, like atomic counters, UBOs and
>> SSBOs.
>>
>> Also, note that ARB_gl_spirv makes mandatory explicit location for
>> normal uniforms, so this code only handles uniforms with explicit
>> location. But there are cases, like uniform atomic counters, that
>> doesn't have a location from the OpenGL point of view (they have a
>> binding), but that Mesa assign internally a location. That will be
>> handled on following patches.
>>
>> A nir_linker.h file is also added. More NIR-linking related API will
>> be added in subsequent patches and those will include stuff from Mesa,
>> so reusing nir.h didn't seem a good idea.
>
> These files should actually be src/compiler/glsl/nir_link_uniforms.c
> etc these are not general purpose nir helpers they are GLSL specific.

Yes, it is true that are not general purpose nir helpers, but they are
not GLSL specific either. As mentioned on the commit message and on the
introductory comment, it is heavily tailored for ARB_gl_spirv, so they
are SPIR-V specific. In fact, they fit better at src/compiler/spirv than
on src/compiler/glsl. But as that directory is even more general
purpose, so we placed them at src/compiler/nir as it using the
nir-shader to do the linkage.

The alternative would be place them on a new directory, something like
src/compiler/gl_spirv, but that seems somewhat an overkill.

>
>>
>> v2: update var->data.location with UniformStorage index when the
>>  storage was set up on a previous stage (Neil)
>>
>> Signed-off-by: Eduardo Lima 
>> Signed-off-by: Neil Roberts > Signed-off-by: Alejandro Piñeiro 
>> ---
>>   src/compiler/Makefile.sources    |   2 +
>>   src/compiler/nir/meson.build |   2 +
>>   src/compiler/nir/nir_link_uniforms.c | 461
>> +++
>>   src/compiler/nir/nir_linker.h    |  41 
>>   4 files changed, 506 insertions(+)
>>   create mode 100644 src/compiler/nir/nir_link_uniforms.c
>>   create mode 100644 src/compiler/nir/nir_linker.h
>>
>> diff --git a/src/compiler/Makefile.sources
>> b/src/compiler/Makefile.sources
>> index bb86972ea1a..dba70a5e5a1 100644
>> --- a/src/compiler/Makefile.sources
>> +++ b/src/compiler/Makefile.sources
>> @@ -206,6 +206,8 @@ NIR_FILES = \
>>   nir/nir_inline_functions.c \
>>   nir/nir_instr_set.c \
>>   nir/nir_instr_set.h \
>> +    nir/nir_link_uniforms.c \
>> +    nir/nir_linker.h \
>>   nir/nir_linking_helpers.c \
>>   nir/nir_liveness.c \
>>   nir/nir_loop_analyze.c \
>> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
>> index b28a565d0c6..baf5682d388 100644
>> --- a/src/compiler/nir/meson.build
>> +++ b/src/compiler/nir/meson.build
>> @@ -99,6 +99,8 @@ files_libnir = files(
>>     'nir_inline_functions.c',
>>     'nir_instr_set.c',
>>     'nir_instr_set.h',
>> +  'nir_link_uniforms.c',
>> +  'nir_linker.h',
>>     'nir_linking_helpers.c',
>>     'nir_liveness.c',
>>     'nir_loop_analyze.c',
>> diff --git a/src/compiler/nir/nir_link_uniforms.c
>> b/src/compiler/nir/nir_link_uniforms.c
>> new file mode 100644
>> index 000..450a5ce2e21
>> --- /dev/null
>> +++ b/src/compiler/nir/nir_link_uniforms.c
>> @@ -0,0 +1,461 @@
>> +/*
>> + * 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 CONN

Re: [Mesa-dev] [PATCH v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Gert Wollny
Am Sonntag, den 29.04.2018, 10:43 +0200 schrieb Benedikt Schemmer:
> Hi Gert,
> 
> couldn't resist at least to try what would happen if I enable
> register merge for radeonsi:
> 
>  PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR
> SpillVGPR  PrivVGPR   Scratch  CodeSize  MaxWaves Waits
>  piglit 80732   -0.16 %   -0.02
> % . .0.87 %0.86 %0.04 % . .
>  --
> 
>  All affected 513  -17.58 %   -2.30
> % . .4.12 %5.87 %1.73 %0.10 % .
>  --
> 
>  Total  80732   -0.16 %   -0.02
> % . .0.87 %0.86 %0.04 % . .
> 
> I had already removed the defines around the debug code so thats also
> happily outputting data.
> 
> fails with two piglit shaders:
Which are the names of these test? I'd like to check this on r600,
because here I didn't see any regressions last time I checked. 


> Real world is a little different:

I guess these tests refer to enabled register_merge - without and with
this patch set, no? 

Out of curiosity, did you also look at how enabling register_merge
(before this series) impacts the result as compared to the normal
operation of radeonsi? 
 

> If theres an easy way to figure out when your code makes it worse and
> when its an improvement this would be really nice.

My insentive for this series was, that on r600 the arrays are allocated
 before the final optimization pass on the byte code that requires that
the number of registers is <= 124. When I started this no spilling was
implemented, and shaders with too many arrays and registers would
simply fail. Now spilling is impelmented, but AFAIK reducing the
numbers of registers in the final optimization pass does not result in
changed spilling, so bringing down the number of registers before tgsi-
to-bytecode is still of interest. 

For radeonsi my guess would be that the llvm optimizer works better
when the registers are not yet merged, and that would be the reason why
register_merge is disabled. 

In any case, Timothy wrote in this thread [1] (last message) that he
had similar patches for NIR. 

Best, 
Gert 

[1] https://patchwork.freedesktop.org/patch/189842/

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


Re: [Mesa-dev] [PATCH] nir/linker: Add nir_link_uniforms()

2018-04-29 Thread Timothy Arceri

On 29/04/18 19:05, Alejandro Piñeiro wrote:

On 29/04/18 10:13, Timothy Arceri wrote:

On 29/04/18 16:48, Alejandro Piñeiro wrote:

From: Eduardo Lima Mitev 

This function will be the entry point for linking the uniforms from
the nir_shader objects associated with the gl_linked_shaders of a
program.

This patch includes initial support for linking uniforms from NIR
shaders. It is tailored for the ARB_gl_spirv needs, and it is far from
complete, but it should handle most cases of uniforms, array
uniforms, structs, samplers and images.

There are some FIXMEs related to specific features that will be
implemented in following patches, like atomic counters, UBOs and
SSBOs.

Also, note that ARB_gl_spirv makes mandatory explicit location for
normal uniforms, so this code only handles uniforms with explicit
location. But there are cases, like uniform atomic counters, that
doesn't have a location from the OpenGL point of view (they have a
binding), but that Mesa assign internally a location. That will be
handled on following patches.

A nir_linker.h file is also added. More NIR-linking related API will
be added in subsequent patches and those will include stuff from Mesa,
so reusing nir.h didn't seem a good idea.


These files should actually be src/compiler/glsl/nir_link_uniforms.c
etc these are not general purpose nir helpers they are GLSL specific.


Yes, it is true that are not general purpose nir helpers, but they are
not GLSL specific either. As mentioned on the commit message and on the
introductory comment, it is heavily tailored for ARB_gl_spirv, so they
are SPIR-V specific.


But why? Why not try to make it reusable as a linker for GLSL? What 
exactly is tailored for ARB_gl_spirv? And does this really block us 
reusing it for GLSL?


I've expressed my opinion on this long ago, we are very close to being 
able to implement a NIR linker for GLSL, spending a little effort 
designing this linker code as share-able seems like a no brainer to me.


Ignoring the above issue I'm fairly certain adding a dependency on core 
mesa GL to NIR is a no no so either way these files don't belong in 
src/compiler/nir



In fact, they fit better at src/compiler/spirv than
on src/compiler/glsl. But as that directory is even more general
purpose, so we placed them at src/compiler/nir as it using the
nir-shader to do the linkage.

The alternative would be place them on a new directory, something like
src/compiler/gl_spirv, but that seems somewhat an overkill.





v2: update var->data.location with UniformStorage index when the
  storage was set up on a previous stage (Neil)

Signed-off-by: Eduardo Lima 
Signed-off-by: Neil Roberts 
---
   src/compiler/Makefile.sources    |   2 +
   src/compiler/nir/meson.build |   2 +
   src/compiler/nir/nir_link_uniforms.c | 461
+++
   src/compiler/nir/nir_linker.h    |  41 
   4 files changed, 506 insertions(+)
   create mode 100644 src/compiler/nir/nir_link_uniforms.c
   create mode 100644 src/compiler/nir/nir_linker.h

diff --git a/src/compiler/Makefile.sources
b/src/compiler/Makefile.sources
index bb86972ea1a..dba70a5e5a1 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -206,6 +206,8 @@ NIR_FILES = \
   nir/nir_inline_functions.c \
   nir/nir_instr_set.c \
   nir/nir_instr_set.h \
+    nir/nir_link_uniforms.c \
+    nir/nir_linker.h \
   nir/nir_linking_helpers.c \
   nir/nir_liveness.c \
   nir/nir_loop_analyze.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index b28a565d0c6..baf5682d388 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -99,6 +99,8 @@ files_libnir = files(
     'nir_inline_functions.c',
     'nir_instr_set.c',
     'nir_instr_set.h',
+  'nir_link_uniforms.c',
+  'nir_linker.h',
     'nir_linking_helpers.c',
     'nir_liveness.c',
     'nir_loop_analyze.c',
diff --git a/src/compiler/nir/nir_link_uniforms.c
b/src/compiler/nir/nir_link_uniforms.c
new file mode 100644
index 000..450a5ce2e21
--- /dev/null
+++ b/src/compiler/nir/nir_link_uniforms.c
@@ -0,0 +1,461 @@
+/*
+ * 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,
+ * FITNE

Re: [Mesa-dev] [PATCH v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Benedikt Schemmer


Am 29.04.2018 um 11:34 schrieb Gert Wollny:
> Am Sonntag, den 29.04.2018, 10:43 +0200 schrieb Benedikt Schemmer:
>> Hi Gert,
>>
>> couldn't resist at least to try what would happen if I enable
>> register merge for radeonsi:
>>
>>  PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR
>> SpillVGPR  PrivVGPR   Scratch  CodeSize  MaxWaves Waits
>>  piglit 80732   -0.16 %   -0.02
>> % . .0.87 %0.86 %0.04 % . .
>>  --
>> 
>>  All affected 513  -17.58 %   -2.30
>> % . .4.12 %5.87 %1.73 %0.10 % .
>>  --
>> 
>>  Total  80732   -0.16 %   -0.02
>> % . .0.87 %0.86 %0.04 % . .
>>
>> I had already removed the defines around the debug code so thats also
>> happily outputting data.
>>
>> fails with two piglit shaders:
> Which are the names of these test? I'd like to check this on r600,
> because here I didn't see any regressions last time I checked. 
> 
might of course be different on r600 (is bindless available?),
also shader-db is more sensitive to problems than piglit

1. tests/spec/arb_bindless_texture/compiler/images/arrays-of-struct.frag
2. tests/spec/arb_bindless_texture/compiler/samplers/arrays-of-struct.frag

> 
>> Real world is a little different:
> 
> I guess these tests refer to enabled register_merge - without and with
> this patch set, no? 
> 
> Out of curiosity, did you also look at how enabling register_merge
> (before this series) impacts the result as compared to the normal
> operation of radeonsi? 

no I didn't
but if I do

(old vs new)
nothing vs register merge:

 PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR SpillVGPR  PrivVGPR 
  Scratch  CodeSize  MaxWaves Waits
 piglit 80732 . . . . . 
.0.06 % . .
 
--
 All affected 4350.89 %0.36 % . . . 
.3.61 %   -0.06 % .
 
--
 Total  80732 . . . . . 
.0.06 % . .

register merge vs yours:

 PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR SpillVGPR  PrivVGPR 
  Scratch  CodeSize  MaxWaves Waits
 piglit 80732   -0.16 %   -0.02 % . .0.87 % 
   0.86 %   -0.02 % . .
 
--
 All affected  83  -56.92 %  -14.22 % . .   11.67 % 
  16.67 %   -2.86 %0.93 % .
 
--
 Total  80732   -0.16 %   -0.02 % . .0.87 % 
   0.86 %   -0.02 % . .

nothing vs yours:

 PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR SpillVGPR  PrivVGPR 
  Scratch  CodeSize  MaxWaves Waits
 piglit 80732   -0.16 %   -0.02 % . .0.87 % 
   0.86 %0.04 % . .
 
--
 All affected 513  -17.58 %   -2.30 % . .4.12 % 
   5.87 %1.73 %0.10 % .
 
--
 Total  80732   -0.16 %   -0.02 % . .0.87 % 
   0.86 %0.04 % . .


>  
> 
>> If theres an easy way to figure out when your code makes it worse and
>> when its an improvement this would be really nice.
> 
> My insentive for this series was, that on r600 the arrays are allocated
>  before the final optimization pass on the byte code that requires that
> the number of registers is <= 124. When I started this no spilling was
> implemented, and shaders with too many arrays and registers would
> simply fail. Now spilling is impelmented, but AFAIK reducing the
> numbers of registers in the final optimization pass does not result in
> changed spilling, so bringing down the number of registers before tgsi-
> to-bytecode is still of interest. 
> 
> For radeonsi my guess would be that the llvm optimizer works better
> when the registers are not yet merged, and that would be the reason why
> register_merge is disabled. 

well at least sometimes it doesn't, low hanging fruit m

Re: [Mesa-dev] [PATCH v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Benedikt Schemmer
Real world

(old vs new)
nothing vs register merge:

PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR SpillVGPR  PrivVGPR  
 Scratch  CodeSize  MaxWaves Waits
 0ad6 . . . . . 
. . . .
 aer  590 .0.26 %  -20.00 % . . 
.0.34 % . .
 alien_isolation 1414 . . . . . 
. . . .
 anholt10 . . . . . 
. . . .
 bioshock_infinite   2581   -0.02 % . . . . 
.0.12 % . .
 blackmesa584 . . . . . 
. . . .
 cat  573 .   -0.02 % . . . 
.0.11 % . .
 csgo1392 . .   -0.88 % . . 
.   -0.03 % . .
 deadisland_definitive   17760.06 % . . . . 
.0.15 %0.01 % .
 deadisland_original11602 . . . . . 
.0.05 % . .
 deadisland_riptide_..293   -0.06 %0.06 % . . . 
.0.32 % . .
 deusex_mankind  50510.14 %   -0.01 %   -1.57 % . . 
.0.18 % . .
 dirtrally787   -0.04 %0.15 %1.08 % . . 
.0.27 %   -0.07 % .
 dolphin   22 . . . . . 
. . . .
 dyinglight  4012 .0.05 % . . . 
.0.34 %   -0.01 % .
 eurotruck2   216 . . . . . 
. . . .
 f1_2015  746   -0.04 %0.02 %0.93 % . . 
.0.08 % . .
 glamor16   -2.33 % . . . . 
.3.97 % . .
 hl2ep1   294 . . . . . 
. . . .
 hl2ep2   154 . . . . . 
. . . .
 hl2lostcoast  66 . . . . . 
. . . .
 hlsl3582 . . . . . 
.   -0.14 % . .
 humus-celshading   4 . . . . . 
. . . .
 humus-domino   6 . . . . . 
. . . .
 humus-dynamicbranching24 . . . . . 
. . . .
 humus-hdr 10 . . . . . 
. . . .
 humus-portals  2 . . . . . 
. . . .
 humus-volumetricfog..  6 . . . . . 
. . . .
 kerbal  1016 .0.11 % . . . 
.0.31 % . .
 larago   664 . . . . . 
.0.01 % . .
 madmax   354 .   -0.08 % . . . 
.   -0.02 %0.04 % .
 metro2033redux  4410 .0.03 % . . . 
.0.06 %   -0.01 % .
 nexuiz80 . . . . . 
. . . .
 ruiner   685 .   -0.02 % . . . 
.0.04 % . .
 sauerbraten7 . . . . . 
. . . .
 serioussam2017   736 . .7.59 % . . 
.0.05 % . .
 soma 436 . . . . . 
. . . .
 specops 1814 . . . . . 
.0.35 % . .
 stellaris434 . . . . . 
.0.11 % . .
 supertuxkart   4 . . . . . 
. . . .
 talos762   -0.02 % .0.09 % . . 
.0.01 % . .
 tesseract430

Re: [Mesa-dev] [PATCH v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Benedikt Schemmer
sorry the last one wasnt correct:

register merge vs yours

Max Increase:

SGPRS: 80 -> 96 (20.00 %) (in 
shaders/deusex_mankind/7c39f71090a9db19ac2e1542ea12804ae6c6495b_4864.shader_test)
VGPRS: 64 -> 84 (31.25 %) (in 
shaders/dirtrally/0859b69789591d7046e211400b1edd9a7cfca734_742.shader_test)
Spilled SGPRs: 0 -> 16 (0.00 %) (in 
shaders/deusex_mankind/d64e2084204e29749639e8fbd9a1e507c7e5e1dd_6840.shader_test)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 24 -> 32 (33.33 %) (in 
shaders/deusex_mankind/28cac87049d8c833e72296a5a02ea6118f1144e5_5876.shader_test)
Scratch size: 28 -> 36 (28.57 %) dwords per thread (in 
shaders/deusex_mankind/28cac87049d8c833e72296a5a02ea6118f1144e5_5876.shader_test)
Code Size: 6988 -> 8036 (15.00 %) bytes (in shaders/cat/1847.shader_test)
LDS: 0 -> 0 (0.00 %) blocks
Max Waves: 5 -> 7 (40.00 %) (in 
shaders/ruiner/0967c5fce7fc456496b1cfa25fbb1d1c4dcf9bed_2958.shader_test)
Wait states: 0 -> 0 (0.00 %)

Max Decrease:

SGPRS: 96 -> 72 (-25.00 %) (in 
shaders/deusex_mankind/c1f098c7b14b1ba291cfa9bba4e41ba91acaba30_3630.shader_test)
VGPRS: 80 -> 68 (-15.00 %) (in 
shaders/dirtrally/710d3319bc986ea003f1a84ec6d3c01b2a8b9ded_2482.shader_test)
Spilled SGPRs: 19 -> 0 (-100.00 %) (in 
shaders/deusex_mankind/0749c9ae23417f918c7286fe502ff5de4cb8e1a0_3276.shader_test)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 9080 -> 8712 (-4.05 %) bytes (in 
shaders/dirtrally/2e1a8e04b49dee3e39b6f30a7c707f395abf_2480.shader_test)
LDS: 0 -> 0 (0.00 %) blocks
Max Waves: 8 -> 5 (-37.50 %) (in 
shaders/deusex_mankind/8dabec49e5b6c3b1cbcbaee194eff69f164d72f4_3968.shader_test)
Wait states: 0 -> 0 (0.00 %)


 PERCENTAGE DELTASShaders SGPRs VGPRs SpillSGPR SpillVGPR  PrivVGPR 
  Scratch  CodeSize  MaxWaves Waits
 0ad6 . . . . . 
. . . .
 aer  590 . . . . . 
. . . .
 alien_isolation 1414 . . . . . 
. . . .
 anholt10 . . . . . 
. . . .
 bioshock_infinite   2581 .0.04 % . . . 
.0.01 % . .
 blackmesa584 . . . . . 
. . . .
 cat  573   -0.06 %   -0.09 % . . . 
.0.08 %0.05 % .
 csgo1392 . . . . . 
. . . .
 deadisland_definitive   1776 . . . . . 
. . . .
 deadisland_original11602 . . . . . 
. . . .
 deadisland_riptide_..293 . . . . . 
. . . .
 deusex_mankind  5051   -0.06 % .   -4.64 % .   33.33 % 
  28.57 % .   -0.01 % .
 dirtrally7870.04 %0.49 %   -0.46 % . . 
.0.03 %   -0.23 % .
 dolphin   22 . . . . . 
. . . .
 dyinglight  4012 . . . . . 
. . . .
 eurotruck2   216 . . . . . 
. . . .
 f1_2015  746 .   -0.03 %1.77 % . . 
.0.05 % . .
 glamor16 . . . . . 
. . . .
 hl2ep1   294 . . . . . 
. . . .
 hl2ep2   154 . . . . . 
. . . .
 hl2lostcoast  66 . . . . . 
. . . .
 hlsl3582 . . . . . 
. . . .
 humus-celshading   4 . . . . . 
. . . .
 humus-domino   6 . . . . . 
. . . .
 humus-dynamicbranching24 . . . . . 
. . . .
 humus-hdr 10 . . . . . 
. . . .
 humus-portals  2 . . . . . 
. . .  

[Mesa-dev] [Bug 106180] [bisected] radv vulkan smoke test black screen (Add support for DRI3 v1.2)

2018-04-29 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106180

--- Comment #5 from mercuriete  ---
Hi
I tried that patch on top of mesa-18.1.0_rc2

it fixes some issues with resizing windows and with alt+tab in csgo+opengl

But it does not fix this problem.

* vulkan smoketest on intel haswell is OK (now doesnt crash when resizing
window)

* vulkan smoketest on radv is a BLACK window and crashes when maximize window.
terminate called after throwing an instance of 'std::runtime_error'
what():  VkResult -101004 returned

* dota2 on radv only shows the cursor and cpu usage is 100% on cpu0 but no
crash.
* dota2 on anv haswell same behaviour (show cursor and 100% usage on cpu0)



Perf information with radv+PRIME with dota2

# To display the perf.data header info, please use --header/--header-only
options.
#
#
# Total Lost Samples: 0
#
# Samples: 29K of event 'cycles:uppp'
# Event count (approx.): 5618911073
#
# Overhead  Command  Shared Object Symbol   
#   ...   
...
#
93.64%  VKRenderThread   [kernel.vmlinux]  [.]
syscall_return_via_sysret
 2.00%  VKRenderThread   libtier0.so   [.]
CThreadFastMutex::Lock
 1.03%  VKRenderThread   libpthread-2.25.so[.] pthread_yield
 0.58%  VKRenderThread   libc-2.25.so  [.] __sched_yield
 0.47%  VKRenderThread   libtier0.so   [.] ThreadSleep@plt
 0.26%  VKRenderThread   [kernel.vmlinux]  [.]
native_irq_return_iret
 0.25%  VKRenderThread   [unknown] [k]
0x7f2d7dd06495
 0.24%  VKRenderThread   libtier0.so   [.]
pthread_yield@plt
 0.23%  SDLAudioP2   libm-2.25.so  [.] __powf_finite
 0.17%  SDLAudioP2   [kernel.vmlinux]  [.]
syscall_return_via_sysret
 0.15%  AsyncScheduledF  [kernel.vmlinux]  [.]
syscall_return_via_sysret
 0.10%  VKRenderThread   libpthread-2.25.so[.] sched_yield@plt
 0.09%  dota2[kernel.vmlinux]  [.]
syscall_return_via_sysret
 0.08%  AsyncScheduledF  libm-2.25.so  [.] __powf_finite
 0.05%  VKRenderThread   libtier0.so   [.] ThreadSleep
 0.04%  SDLAudioP2   libm-2.25.so  [.] powf
 0.02%  SDLAudioP2   libm-2.25.so  [.] log10f
 0.02%  AsyncScheduledF  libm-2.25.so  [.] powf
 0.01%  SDLAudioP2   [kernel.vmlinux]  [.]
native_irq_return_iret
 0.01%  AsyncScheduledF  libtier0.so   [.] Plat_FloatTime
 0.01%  SDLAudioP2   libsoundsystem.so [.]
0x00169aa4
 0.01%  AsyncScheduledF  libpthread-2.25.so[.]
pthread_cond_timedwait@@GLIBC_2.3.2
 0.01%  SDLAudioP2   libm-2.25.so  [.]
0x6d91
 0.01%  AsyncScheduledF  libm-2.25.so  [.] log10f
 0.01%  SDLAudioP2   libm-2.25.so  [.]
0x81f9
 0.01%  SDLAudioP2   libc-2.25.so  [.]
0x0013e29f
 0.01%  SDLAudioP2   libc-2.25.so  [.]
0x0013e3c0

-- 
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 106283] Shader replacements works only for limited use cases

2018-04-29 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106283

--- Comment #6 from Mark Janes  ---
FrameRetrace supports gl shader replacement.  I'll enhance it to support
glProgramString.  It will be easier to debug shaders with FrameRetrace than
using the environment variables.

https://fosdem.org/2018/schedule/event/apitrace/

-- 
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 v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Gert Wollny
Hello Benedict,

thanks for all the testing!

On 29.04.2018 12:12, Benedikt Schemmer wrote:
>> Which are the names of these test? I'd like to check this on r600,
>> because here I didn't see any regressions last time I checked. 
>>
> might of course be different on r600 (is bindless available?),
> also shader-db is more sensitive to problems than piglit
>
> 1. tests/spec/arb_bindless_texture/compiler/images/arrays-of-struct.frag
> 2. tests/spec/arb_bindless_texture/compiler/samplers/arrays-of-struct.frag
Indeed, bindless testures are not available on r600, so it is quite
difficult for me to test this. I would guess that parameters related to
this might be stored in the TGSI declaration that I currently don't check.

If you have time for it, could you send me a TGSI dump of one of these
shaders?
With "ST_DEBUG=tgsi" this should be possible.
>
>> For radeonsi my guess would be that the llvm optimizer works better
>> when the registers are not yet merged, and that would be the reason why
>> register_merge is disabled. 
> well at least sometimes it doesn't, low hanging fruit maybe?
Unfortunately, I can't test on radeonsi

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


[Mesa-dev] [Bug 106315] The witness + dxvk suffers flickering garbage

2018-04-29 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106315

Bug ID: 106315
   Summary: The witness + dxvk suffers flickering garbage
   Product: Mesa
   Version: git
  Hardware: Other
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Vulkan/radeon
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: nota...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

The witness + wine + dxvk suffers from flickering garbage. It's most visible
with MSAA off, when it's on the problem is only visible in water reflections.
The problem doesn't show up under Windows + dxvk, which suggests it's a driver
issue (probably LLVM?)

Renderdoc capture with MSAA off:
https://drive.google.com/file/d/1MZ3GaxQaKgdJ-FZHRj7hnpmimv9bKodU/view?usp=sharing

Recorded using renderdoc nightly (2018_04_28) because dxvk requires
VK_KHR_descriptor_update_template now.

GPU: POLARIS10 (RX 470)
LLVM: 7.0.0 r330470

-- 
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 v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Benedikt Schemmer


Am 29.04.2018 um 21:44 schrieb Gert Wollny:
> Hello Benedict,
> 
> thanks for all the testing!

thanks for all the developing ;)

> 
> On 29.04.2018 12:12, Benedikt Schemmer wrote:
>>> Which are the names of these test? I'd like to check this on r600,
>>> because here I didn't see any regressions last time I checked. 
>>>
>> might of course be different on r600 (is bindless available?),
>> also shader-db is more sensitive to problems than piglit
>>
>> 1. tests/spec/arb_bindless_texture/compiler/images/arrays-of-struct.frag
>> 2. tests/spec/arb_bindless_texture/compiler/samplers/arrays-of-struct.frag
> Indeed, bindless testures are not available on r600, so it is quite
> difficult for me to test this. I would guess that parameters related to
> this might be stored in the TGSI declaration that I currently don't check.
> 
> If you have time for it, could you send me a TGSI dump of one of these
> shaders?
> With "ST_DEBUG=tgsi" this should be possible.

I created 'R600_DEBUG=merge' so I can switch without having to recompile.


$ST_DEBUG=tgsi ./run wollny/

ATTENTION: default value of option allow_glsl_extension_directive_midshader 
overridden by environment.
FRAG
DCL TEMP[0..55], ARRAY(1), LOCAL
IMM[0] INT32 {0, 0, 0, 0}
IMM[1] FLT32 {1., 2., 3., 4.}
  0: STORE TEMP[0], IMM[0]., IMM[1], 2D
  1: END

wollny/41d88325fd2a57cb6af40de02dc281ee0683cc40_2.shader_test - LLVM diagnostic 
(remark): :0:0: 12 instructions in function
wollny/41d88325fd2a57cb6af40de02dc281ee0683cc40_2.shader_test - Shader Stats: 
SGPRS: 16 VGPRS: 16 Code Size: 80 LDS: 0 Scratch: 0 Max Waves: 8 Spilled SGPRs: 
0 Spilled VGPRs: 0 PrivMem VGPRs: 0
FRAG
DCL OUT[0], COLOR
DCL TEMP[0..1], LOCAL
DCL TEMP[2..57], ARRAY(1), LOCAL
IMM[0] FLT32 {0., 0., 0., 0.}
  0: MOV TEMP[0].xy, IMM[0].
  1: TEX TEMP[1], TEMP[0], TEMP[2].xyxy, 2D
  2: MOV OUT[0], TEMP[1]
  3: END

wollny/a115868a349cd666b842a0e70f47451b4463903a_2.shader_test - LLVM diagnostic 
(remark): :0:0: 11 instructions in function
wollny/a115868a349cd666b842a0e70f47451b4463903a_2.shader_test - Shader Stats: 
SGPRS: 24 VGPRS: 16 Code Size: 72 LDS: 0 Scratch: 0 Max Waves: 8 Spilled SGPRs: 
0 Spilled VGPRs: 0 PrivMem VGPRs: 0
Thread 0 took 0.13 seconds and compiled 2 shaders (not including SIMD16) with 1 
GL context switches



$R600_DEBUG=merge ST_DEBUG=tgsi ./run wollny/

ATTENTION: default value of option allow_glsl_extension_directive_midshader 
overridden by environment.
run: state_tracker/st_glsl_to_tgsi.cpp:5783: ureg_dst 
dst_register(st_translate*, gl_register_file, unsigned int, unsigned int): 
Assertion `array_id && array_id <= t->num_temp_arrays' failed.

 => CRASHED <= while processing these shaders:

wollny/41d88325fd2a57cb6af40de02dc281ee0683cc40_2.shader_test

>>
>>> For radeonsi my guess would be that the llvm optimizer works better
>>> when the registers are not yet merged, and that would be the reason why
>>> register_merge is disabled. 
>> well at least sometimes it doesn't, low hanging fruit maybe?
> Unfortunately, I can't test on radeonsi

I can, if you dont mind waiting for an answer sometimes.

But maybe even easier: is there an implicit/explicit magic number I can play 
with to see if it changes anything?

ATM it seems like your code improves half the shaders its affecting a lot and 
hurting the other half bad like it hits an invisible wall.
I think one problem could be the relationship between VGPRs and SGPRs used and 
max Wavefronts achieved.

This is somewhat similar to NIR although that changes things all over the place.

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


[Mesa-dev] [Bug 106246] radv: VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT and bringing down initial pipeline compile times

2018-04-29 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106246

--- Comment #6 from Timothy Arceri  ---
(In reply to Timothy Arceri from comment #4)
> (In reply to Nicolai Hähnle from comment #3)
> > As long as scratch buffer support is robust, removing LLVM IR optimization
> > passes is probably not a problem, though you really do want mem2reg and I
> > don't think we spend much time in the others (at least radeonsi didn't, last
> > time I checked).
> > 
> > Using the -O0 settings for the codegen backend is a lot riskier. Our compute
> > folks have done some work fixing bugs there, but I really wouldn't recommend
> > it.
> 
> Yeah I've done some experimenting with the Blacksmith demo. I'm not sure we
> can get much benefit implementing
> VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT with the current state of
> things. 
> 
> Default:
> Sum of shader compile times: 325933 ms
> 
> With only LLVM DCE opt (compilation fails without this):
> Sum of shader compile times: 326451 ms
> 
> No NIR linking plus single pass over NIR opts (compilation fails without
> this):
> Sum of shader compile times: 294788 ms

I've done some playing around with the LLVM cogegen opt levels:

LLVMCodeGenLevelNone + LLVMAddEarlyCSEMemSSAPass (compilation fails without
this):
Sum of shader compile times: 211403 ms
However there are all sorts of rendering issues when running the demo.

No NIR linking plus single pass over NIR opts (compilation fails without this),
LLVMCodeGenLevelNone + LLVMAddEarlyCSEMemSSAPass(compilation fails without
this):
Sum of shader compile times: 179775 ms
With this the demo doesn't actually display the graphics it just shows a
flickering Unity logo throughout the run.

-- 
You are receiving this mail because:
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] ac/nir: expand 64-bit vec3 loads to fix shuffling.

2018-04-29 Thread Dave Airlie
From: Dave Airlie 

If loading 64-bit vec3 values, a 4 component load would be followed
by a 2 component load and the resulting shuffle would fail as it
requires 2 4 components. This just expands the second results
vector out to 4 components.

This fixes 100 CTS tests:
dEQP-VK.spirv_assembly.type.vec3.*64*
---
 src/amd/common/ac_nir_to_llvm.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index e4ae6ef49ad..b77d62a39b0 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1572,6 +1572,11 @@ static LLVMValueRef visit_load_buffer(struct 
ac_nir_context *ctx,
LLVMConstInt(ctx->ac.i32, 6, false), 
LLVMConstInt(ctx->ac.i32, 7, false)
};
 
+   if (num_components == 6) {
+   /* we end up with a v4f32 and v2f32 but shuffle fails 
on that */
+   results[1] = ac_build_expand_to_vec4(&ctx->ac, 
results[1], 4);
+   }
+
LLVMValueRef swizzle = LLVMConstVector(masks, num_components);
ret = LLVMBuildShuffleVector(ctx->ac.builder, results[0],
 results[num_components > 4 ? 1 : 
0], swizzle, "");
-- 
2.14.3

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


[Mesa-dev] [Bug 106246] radv: VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT and bringing down initial pipeline compile times

2018-04-29 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106246

--- Comment #7 from Timothy Arceri  ---
(In reply to Timothy Arceri from comment #6)
> (In reply to Timothy Arceri from comment #4)
> > (In reply to Nicolai Hähnle from comment #3)
> > > As long as scratch buffer support is robust, removing LLVM IR optimization
> > > passes is probably not a problem, though you really do want mem2reg and I
> > > don't think we spend much time in the others (at least radeonsi didn't, 
> > > last
> > > time I checked).
> > > 
> > > Using the -O0 settings for the codegen backend is a lot riskier. Our 
> > > compute
> > > folks have done some work fixing bugs there, but I really wouldn't 
> > > recommend
> > > it.
> > 
> > Yeah I've done some experimenting with the Blacksmith demo. I'm not sure we
> > can get much benefit implementing
> > VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT with the current state of
> > things. 
> > 
> > Default:
> > Sum of shader compile times: 325933 ms
> > 
> > With only LLVM DCE opt (compilation fails without this):
> > Sum of shader compile times: 326451 ms
> > 
> > No NIR linking plus single pass over NIR opts (compilation fails without
> > this):
> > Sum of shader compile times: 294788 ms
> 
> I've done some playing around with the LLVM cogegen opt levels:
> 
> LLVMCodeGenLevelNone + LLVMAddEarlyCSEMemSSAPass (compilation fails without
> this):
> Sum of shader compile times: 211403 ms
> However there are all sorts of rendering issues when running the demo.
> 
> No NIR linking plus single pass over NIR opts (compilation fails without
> this),
> LLVMCodeGenLevelNone + LLVMAddEarlyCSEMemSSAPass(compilation fails without
> this):
> Sum of shader compile times: 179775 ms
> With this the demo doesn't actually display the graphics it just shows a
> flickering Unity logo throughout the run.

Ok so it seems this speed up (and the display issues that go with it) and due
to switching from the GreedyRegisterAllocator to the FastRegisterAllocator.

-- 
You are receiving this mail because:
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 v3 00/13] TGSI: improved live range tracking, also including arrays

2018-04-29 Thread Gert Wollny

[...]
> 
> $R600_DEBUG=merge ST_DEBUG=tgsi ./run wollny/
> 
> ATTENTION: default value of option
> allow_glsl_extension_directive_midshader overridden by environment.
> run: state_tracker/st_glsl_to_tgsi.cpp:5783: ureg_dst
> dst_register(st_translate*, gl_register_file, unsigned int, unsigned
> int): Assertion `array_id && array_id <= t->num_temp_arrays' failed.

This would indicate that the array is split, but still addressed
somewhere, i.e. I don't look at all possible locations where arrays are
used. I think I debug this by letting the R600 driver advertise
bindless texture support (obvioulsy it will not pass the tests, because
it is not implemented and  the hardware may not even support it, but
the GLSL->TGSI conversion should still work. 


> 
> 
> I can, if you dont mind waiting for an answer sometimes.
> 
I will rely on this, but I also will take some time do work on it. 


> But maybe even easier: is there an implicit/explicit magic number I
> can play with to see if it changes anything?

Unfortunately, in my code there are no specific magic numbers to
change, if there is anything then it is probably in the LLVM backend
(that is not used with r600). 

The only think you could try is to disable one of the two parts in my
code: one is the split_array(), called around 

  st_glsl_to_tgsi:7018 

and the other is where the array merging and interleaving are applied,
i.e. line 

  st_glsl_to_tgsi:5537

My take is that this latter step is responsible for the regressions, 
especially the array interleaving could make it more difficult to
optimize, because if the optimizer pass doesn't look at a per-component 
access, but on the higher level per-register base, then register live-
ranges for the array elements will become longer, making it more
difficult to schedule everything without spilling.


> ATM it seems like your code improves half the shaders its affecting a
> lot and hurting the other half bad like it hits an invisible wall.
> I think one problem could be the relationship between VGPRs and SGPRs
> used and max Wavefronts achieved.
Unfortunately, on r600 such fine grained information is not available,
so that I also have no real clue. 

Cheer,
Gert

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


[Mesa-dev] [PATCH 2/3] vulkan/wsi: VK_EXT_hdr_metadata plumbing

2018-04-29 Thread Tapani Pälli
From: Ville Syrjälä 

Add the common wsi infrastructure for VK_EXT_hdr_metadata.

Signed-off-by: Ville Syrjälä 
Signed-off-by: Tapani Pälli 
---
 src/vulkan/wsi/wsi_common.c | 12 
 src/vulkan/wsi/wsi_common.h |  5 +
 src/vulkan/wsi/wsi_common_private.h |  2 ++
 3 files changed, 19 insertions(+)

diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index fe262b4968..676346f0a2 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -773,6 +773,18 @@ wsi_common_destroy_swapchain(VkDevice device,
swapchain->destroy(swapchain, pAllocator);
 }
 
+void
+wsi_common_set_hdr_metadata(uint32_t swapchainCount,
+const VkSwapchainKHR *pSwapchain,
+const VkHdrMetadataEXT *pMetadata)
+{
+   for (uint32_t i = 0; i < swapchainCount; i++) {
+  WSI_FROM_HANDLE(wsi_swapchain, swapchain, pSwapchain[i]);
+  if (swapchain->set_hdr_metadata)
+ swapchain->set_hdr_metadata(swapchain, &pMetadata[i]);
+   }
+}
+
 VkResult
 wsi_common_get_images(VkSwapchainKHR _swapchain,
   uint32_t *pSwapchainImageCount,
diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
index 6cf729ba02..b41a633e02 100644
--- a/src/vulkan/wsi/wsi_common.h
+++ b/src/vulkan/wsi/wsi_common.h
@@ -202,6 +202,11 @@ wsi_common_destroy_swapchain(VkDevice device,
  VkSwapchainKHR swapchain,
  const VkAllocationCallbacks *pAllocator);
 
+void
+wsi_common_set_hdr_metadata(uint32_t swapchainCount,
+const VkSwapchainKHR* pSwapchains,
+const VkHdrMetadataEXT* pMetadata);
+
 VkResult
 wsi_common_queue_present(const struct wsi_device *wsi,
  VkDevice device_h,
diff --git a/src/vulkan/wsi/wsi_common_private.h 
b/src/vulkan/wsi/wsi_common_private.h
index b608119b96..c0210f5588 100644
--- a/src/vulkan/wsi/wsi_common_private.h
+++ b/src/vulkan/wsi/wsi_common_private.h
@@ -67,6 +67,8 @@ struct wsi_swapchain {
VkResult (*queue_present)(struct wsi_swapchain *swap_chain,
  uint32_t image_index,
  const VkPresentRegionKHR *damage);
+   void (*set_hdr_metadata)(struct wsi_swapchain *swapchain,
+const VkHdrMetadataEXT *metadata);
 };
 
 VkResult
-- 
2.13.6

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


[Mesa-dev] [PATCH 0/3] common bits from HDR POC

2018-04-29 Thread Tapani Pälli
Hi;

I took Ville's HDR POC Mesa patches, fixed some issues and removed all
Wayland specific parts. Here are the bits that could be already integrated,
rest will be window system specific parts.

// Tapani


Tapani Pälli (1):
  egl: implement EXT_surface_SMPTE2086_metadata and
EXT_surface_CTA861_3_metadata

Ville Syrjälä (2):
  vulkan/wsi: VK_EXT_hdr_metadata plumbing
  anv: Plug in VK_EXT_hdr_metadata

 src/egl/drivers/dri2/egl_dri2.c |   3 +
 src/egl/main/eglapi.c   |   2 +
 src/egl/main/egldisplay.h   |   2 +
 src/egl/main/eglsurface.c   | 169 
 src/egl/main/eglsurface.h   |  19 
 src/intel/vulkan/anv_extensions.py  |   1 +
 src/intel/vulkan/anv_wsi.c  |   9 ++
 src/vulkan/wsi/wsi_common.c |  12 +++
 src/vulkan/wsi/wsi_common.h |   5 ++
 src/vulkan/wsi/wsi_common_private.h |   2 +
 10 files changed, 224 insertions(+)

-- 
2.13.6

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


[Mesa-dev] [PATCH 1/3] egl: implement EXT_surface_SMPTE2086_metadata and EXT_surface_CTA861_3_metadata

2018-04-29 Thread Tapani Pälli
Patch implements common bits for EXT_surface_SMPTE2086_metadata
and EXT_surface_CTA861_3_metadata extensions by adding new required
attributes and eglQuerySurface + eglSurfaceAttrib changes.

Currently none of the drivers are utilizing this data but this patch
is enabler in getting there.

Following dEQP cases pass with these changes:
   dEQP-EGL.functional.hdr_metadata.smpte2086
   dEQP-EGL.functional.hdr_metadata.cta861_3

Signed-off-by: Ville Syrjälä 
Signed-off-by: Tapani Pälli 
---
 src/egl/drivers/dri2/egl_dri2.c |   3 +
 src/egl/main/eglapi.c   |   2 +
 src/egl/main/egldisplay.h   |   2 +
 src/egl/main/eglsurface.c   | 169 
 src/egl/main/eglsurface.h   |  19 +
 5 files changed, 195 insertions(+)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 45d0c7275c..fcfd3852d9 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -712,6 +712,9 @@ dri2_setup_screen(_EGLDisplay *disp)
  disp->Extensions.EXT_create_context_robustness = EGL_TRUE;
}
 
+   disp->Extensions.EXT_surface_SMPTE2086_metadata = EGL_TRUE;
+   disp->Extensions.EXT_surface_CTA861_3_metadata = EGL_TRUE;
+
if (dri2_dpy->no_error)
   disp->Extensions.KHR_create_context_no_error = EGL_TRUE;
 
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index c110349119..493fd6d1b9 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -488,6 +488,8 @@ _eglCreateExtensionsString(_EGLDisplay *dpy)
_EGL_CHECK_EXTENSION(EXT_create_context_robustness);
_EGL_CHECK_EXTENSION(EXT_image_dma_buf_import);
_EGL_CHECK_EXTENSION(EXT_image_dma_buf_import_modifiers);
+   _EGL_CHECK_EXTENSION(EXT_surface_CTA861_3_metadata);
+   _EGL_CHECK_EXTENSION(EXT_surface_SMPTE2086_metadata);
_EGL_CHECK_EXTENSION(EXT_swap_buffers_with_damage);
 
_EGL_CHECK_EXTENSION(IMG_context_priority);
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index d7e51991a4..6dc77ebe3d 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -105,6 +105,8 @@ struct _egl_extensions
EGLBoolean EXT_image_dma_buf_import;
EGLBoolean EXT_image_dma_buf_import_modifiers;
EGLBoolean EXT_pixel_format_float;
+   EGLBoolean EXT_surface_CTA861_3_metadata;
+   EGLBoolean EXT_surface_SMPTE2086_metadata;
EGLBoolean EXT_swap_buffers_with_damage;
 
unsigned int IMG_context_priority;
diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
index 3bd14a8cd0..1bf4723243 100644
--- a/src/egl/main/eglsurface.c
+++ b/src/egl/main/eglsurface.c
@@ -86,6 +86,90 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint 
*attrib_list)
 break;
  surf->GLColorspace = val;
  break;
+  case EGL_SMPTE2086_DISPLAY_PRIMARY_RX_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.display_primary_r.x = val;
+ break;
+  case EGL_SMPTE2086_DISPLAY_PRIMARY_RY_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.display_primary_r.y = val;
+ break;
+  case EGL_SMPTE2086_DISPLAY_PRIMARY_GX_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.display_primary_g.x = val;
+ break;
+  case EGL_SMPTE2086_DISPLAY_PRIMARY_GY_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.display_primary_g.y = val;
+ break;
+  case EGL_SMPTE2086_DISPLAY_PRIMARY_BX_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.display_primary_b.x = val;
+ break;
+  case EGL_SMPTE2086_DISPLAY_PRIMARY_BY_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.display_primary_b.y = val;
+ break;
+  case EGL_SMPTE2086_WHITE_POINT_X_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.white_point.x = val;
+ break;
+  case EGL_SMPTE2086_WHITE_POINT_Y_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }
+ surf->HdrMetadata.white_point.y = val;
+ break;
+  case EGL_SMPTE2086_MAX_LUMINANCE_EXT:
+ if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
+err = EGL_BAD_ATTRIBUTE;
+break;
+ }

[Mesa-dev] [PATCH 3/3] anv: Plug in VK_EXT_hdr_metadata

2018-04-29 Thread Tapani Pälli
From: Ville Syrjälä 

The common wsi stuff for VK_EXT_hdr_metadata is in place, and
we shouldn't need any hardware specifics, so let's expose the
extension.

Signed-off-by: Ville Syrjälä 
---
 src/intel/vulkan/anv_extensions.py | 1 +
 src/intel/vulkan/anv_wsi.c | 9 +
 2 files changed, 10 insertions(+)

diff --git a/src/intel/vulkan/anv_extensions.py 
b/src/intel/vulkan/anv_extensions.py
index b5bee0881c..a3416b3e31 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -112,6 +112,7 @@ EXTENSIONS = [
 Extension('VK_EXT_global_priority',   1,
   'device->has_context_priority'),
 Extension('VK_EXT_shader_viewport_index_layer',   1, True),
+Extension('VK_EXT_hdr_metadata',  1, True),
 ]
 
 class VkVersion:
diff --git a/src/intel/vulkan/anv_wsi.c b/src/intel/vulkan/anv_wsi.c
index 20094f93e1..61a4f42535 100644
--- a/src/intel/vulkan/anv_wsi.c
+++ b/src/intel/vulkan/anv_wsi.c
@@ -191,6 +191,15 @@ void anv_DestroySwapchainKHR(
wsi_common_destroy_swapchain(_device, swapchain, alloc);
 }
 
+void anv_SetHdrMetadataEXT(
+   VkDevice  device,
+   uint32_t  swapchainCount,
+   const VkSwapchainKHR* pSwapchains,
+   const VkHdrMetadataEXT*   pMetadata)
+{
+   wsi_common_set_hdr_metadata(swapchainCount, pSwapchains, pMetadata);
+}
+
 VkResult anv_GetSwapchainImagesKHR(
 VkDevice device,
 VkSwapchainKHR   swapchain,
-- 
2.13.6

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


Re: [Mesa-dev] [PATCH] dri3: Only update number of back buffers in loader_dri3_get_buffers

2018-04-29 Thread Sergii Romantsov
Hello,
with my simple tests it works, but have some remarks (but don't have much
experience with dri3):

1. It looks little dangerous to move logic of destroying buffers and seems
in your version we are loosing some logic with 'busy' and 'num_back'
2. And also looks like buffers just will not be destroyed on time for some
cases.
3. From my opinion it will be safer to use in function 'dri3_get_buffer'
call (with my patch):
 'dri3_fence_await(draw->conn, NULL, buffer)' instead of
'dri3_fence_await(draw->conn, draw, buffer)'

On Fri, Apr 27, 2018 at 6:56 PM, Michel Dänzer  wrote:

> From: Michel Dänzer 
>
> And only free no longer needed back buffers there as well.
>
> We want to stick to the same back buffer throughout a frame, otherwise
> we can run into various issues.
>
> Bugzilla: https://bugs.freedesktop.org/105906
> Fixes: 3160cb86aa92 "egl/x11: Re-allocate buffers if format is suboptimal"
> Signed-off-by: Michel Dänzer 
> ---
>
> Sergii, Eero, here's an alternative fix which I think is cleaner, can
> you test it?
>
>  src/loader/loader_dri3_helper.c | 19 +++
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_
> helper.c
> index 23729f7ecb2..6db8303d26d 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -420,13 +420,6 @@ dri3_handle_present_event(struct
> loader_dri3_drawable *draw,
>
>   if (buf && buf->pixmap == ie->pixmap)
>  buf->busy = 0;
> -
> - if (buf && draw->cur_blit_source != b && !buf->busy &&
> - (buf->reallocate ||
> - (draw->num_back <= b && b < LOADER_DRI3_MAX_BACK))) {
> -dri3_free_render_buffer(draw, buf);
> -draw->buffers[b] = NULL;
> - }
>}
>break;
> }
> @@ -559,7 +552,6 @@ dri3_find_back(struct loader_dri3_drawable *draw)
> /* Check whether we need to reuse the current back buffer as new back.
>  * In that case, wait until it's not busy anymore.
>  */
> -   dri3_update_num_back(draw);
> num_to_consider = draw->num_back;
> if (!loader_dri3_have_image_blit(draw) && draw->cur_blit_source !=
> -1) {
>num_to_consider = 1;
> @@ -1815,6 +1807,7 @@ loader_dri3_get_buffers(__DRIdrawable *driDrawable,
>  {
> struct loader_dri3_drawable *draw = loaderPrivate;
> struct loader_dri3_buffer   *front, *back;
> +   int buf_id;
>
> buffers->image_mask = 0;
> buffers->front = NULL;
> @@ -1826,6 +1819,16 @@ loader_dri3_get_buffers(__DRIdrawable *driDrawable,
> if (!dri3_update_drawable(driDrawable, draw))
>return false;
>
> +   dri3_update_num_back(draw);
> +
> +   /* Free no longer needed back buffers */
> +   for (buf_id = draw->num_back; buf_id < LOADER_DRI3_MAX_BACK; buf_id++)
> {
> +  if (draw->cur_blit_source != buf_id && draw->buffers[buf_id]) {
> + dri3_free_render_buffer(draw, draw->buffers[buf_id]);
> + draw->buffers[buf_id] = NULL;
> +  }
> +   }
> +
> /* pixmaps always have front buffers.
>  * Exchange swaps also mandate fake front buffers.
>  */
> --
> 2.17.0
>
>


-- 
Sergii Romantsov
GlobalLogic Inc.
www.globallogic.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] nir/linker: Add nir_link_uniforms()

2018-04-29 Thread Alejandro Piñeiro
On 29/04/18 12:09, Timothy Arceri wrote:
> On 29/04/18 19:05, Alejandro Piñeiro wrote:
>> On 29/04/18 10:13, Timothy Arceri wrote:
>>> On 29/04/18 16:48, Alejandro Piñeiro wrote:
 From: Eduardo Lima Mitev 

 This function will be the entry point for linking the uniforms from
 the nir_shader objects associated with the gl_linked_shaders of a
 program.

 This patch includes initial support for linking uniforms from NIR
 shaders. It is tailored for the ARB_gl_spirv needs, and it is far from
 complete, but it should handle most cases of uniforms, array
 uniforms, structs, samplers and images.

 There are some FIXMEs related to specific features that will be
 implemented in following patches, like atomic counters, UBOs and
 SSBOs.

 Also, note that ARB_gl_spirv makes mandatory explicit location for
 normal uniforms, so this code only handles uniforms with explicit
 location. But there are cases, like uniform atomic counters, that
 doesn't have a location from the OpenGL point of view (they have a
 binding), but that Mesa assign internally a location. That will be
 handled on following patches.

 A nir_linker.h file is also added. More NIR-linking related API will
 be added in subsequent patches and those will include stuff from Mesa,
 so reusing nir.h didn't seem a good idea.
>>>
>>> These files should actually be src/compiler/glsl/nir_link_uniforms.c
>>> etc these are not general purpose nir helpers they are GLSL specific.
>>
>> Yes, it is true that are not general purpose nir helpers, but they are
>> not GLSL specific either. As mentioned on the commit message and on the
>> introductory comment, it is heavily tailored for ARB_gl_spirv, so they
>> are SPIR-V specific.
>
> But why? Why not try to make it reusable as a linker for GLSL? What
> exactly is tailored for ARB_gl_spirv? And does this really block us
> reusing it for GLSL?
>
> I've expressed my opinion on this long ago, we are very close to being
> able to implement a NIR linker for GLSL, spending a little effort
> designing this linker code as share-able seems like a no brainer to me.

Yes, it is true that we didn't explain in detail that decision on the
mailing list. We briefly mentioned that on the patches that we were
sending to the list, and explained on my presentation on FOSDEM [1],
where Nicolai mentioned during the Q&A section that they agreed to us
(at least with going for a nir-based linking). In fact, at the beginning
we also hoped that this work would be more aligned with a more general
nir-linker, and more easily reused for a nir-based GLSL linker, but our
opinion changed as we started to code the needs for this extension.

In summary the main reason are the names. Right now GLSL linking is
based on the names. Uniform name, ubo name, and so on. So for example,
from link_uniform_blocks.cpp:

   /* This hash table will track all of the uniform blocks that have been
    * encountered.  Since blocks with the same block-name must be the same,
    * the hash is organized by block-name.
    */

And most the validation rules on GLSL are based, or include somehow, the
name of the variables.

But on ARB_gl_spirv, everything should work without names. Read as
example issues 12, 21 and 22 of the spec [2] as a reference. In fact
names are optional even if the SPIR-V include them (see issue 19). So
the linking for nir shaders coming from ARB_gl_spirv should work based
on the location, binding, offsets, etc. With the previous example, ubos
are linked using the binding. So explicit bindings are now mandatory (so
the validation rule here change, as not having a explicit binding can be
raised as an error).

So in order to work for ARB_gl_spirv it should work without names. In
order to reuse it for GLSL it should work with names, in fact based on
them. Validation rules for both are different. And getting both working
at the same time would just add a complexity that IMHO we don't need
right now. We already have a GLSL linker, so it is not really worth
right now to get this new NIR linker to work there too. Right now we are
focusing on getting ARB_gl_spirv linkage working.

Another reason is that ARB_gl_spirv GLSL supported features are not
exactly any existing GLSL. It is based on GL_KHR_vulkan_glsl, but at the
same time, slightly tweaked. See "Differences Relative to Other
Specifications" on the ARB_gl_spirv spec [2].

So perhaps in the future some of this work could be reused for nir-base
linker for GLSL. But, IMHO, that shouldn't be one of the features
driving the development. I think that we should work first on what it is
not supported.

>
> Ignoring the above issue I'm fairly certain adding a dependency on
> core mesa GL to NIR is a no no so either way these files don't belong
> in src/compiler/nir

But we already have that dependency, right? I mean gl_program have a
reference to nir_shader for some time now.

And in any case, the implementation of ARB_g