Re: [Mesa-dev] [PATCH v2] glsl: fix array assignments of a swizzled vector

2018-10-05 Thread Ilia Mirkin
On Sat, Oct 6, 2018 at 1:24 AM Ilia Mirkin  wrote:
>
> This happens in situations where we might do
>
>   vec.wzyx[i] = ...
>
> The swizzle would get effectively ignored because of the interaction
> between how ir_assignment->set_lhs works and overwriting the write_mask.
> There are two cases, one where i is a constant, and another where i is
> variable. We have to be extra-careful in both cases.
>
> Fixes the following WebGL test:
>
>   
> https://www.khronos.org/registry/webgl/sdk/tests/conformance2/glsl3/vector-dynamic-indexing-swizzled-lvalue.html
>
> And the new piglit tests:
>
>   swizzled-writemask-indexing-nonconst.shader_test
>   swizzled-writemask-indexing.shader_test
>
> Signed-off-by: Ilia Mirkin 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>
> v1 -> v2:
>  - include info about piglit tests
>  - add a fix for the nonconst case -- doing set_lhs last instead of
>first.
>
> Ian Romanick reviewed v1, but given the subtlety of the additional
> change for v2, going to leave that off, prefering another review.
>
> No regressions running this through Intel CI.
>
>  src/compiler/glsl/lower_vector_derefs.cpp | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/glsl/lower_vector_derefs.cpp 
> b/src/compiler/glsl/lower_vector_derefs.cpp
> index 7583d1fdd3e..223e420550b 100644
> --- a/src/compiler/glsl/lower_vector_derefs.cpp
> +++ b/src/compiler/glsl/lower_vector_derefs.cpp
> @@ -59,8 +59,7 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
> if (!deref->array->type->is_vector())
>return ir_rvalue_enter_visitor::visit_enter(ir);
>
> -   ir_dereference *const new_lhs = (ir_dereference *) deref->array;
> -   ir->set_lhs(new_lhs);
> +   ir_rvalue *const new_lhs = deref->array;
>
> void *mem_ctx = ralloc_parent(ir);
> ir_constant *old_index_constant =
> @@ -72,8 +71,15 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
> ir->rhs,
> deref->array_index);
>ir->write_mask = (1 << new_lhs->type->vector_elements) - 1;
> +  ir->set_lhs(new_lhs);
> +   } else if (new_lhs->ir_type != ir_type_swizzle) {
> +  ir->set_lhs(new_lhs);
> +  ir->write_mask = 1 << old_index_constant->get_uint_component(0);
> } else {
> -  ir->write_mask = 1 << old_index_constant->get_int_component(0);
> +  // If there "new" lhs is a swizzle, use the set_lhs helper to instead
> +  // swizzle the rhs.

Urgh, forgot to fix this based on Ian's feedback last time. This now reads

  /* If there "new" LHS is a swizzle, use the set_lhs helper to instead
   * swizzle the RHS.
   */

locally.

> +  unsigned component[1] = { old_index_constant->get_uint_component(0) };
> +  ir->set_lhs(new(mem_ctx) ir_swizzle(new_lhs, component, 1));
> }
>
> return ir_rvalue_enter_visitor::visit_enter(ir);
> --
> 2.16.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2] glsl: fix array assignments of a swizzled vector

2018-10-05 Thread Ilia Mirkin
This happens in situations where we might do

  vec.wzyx[i] = ...

The swizzle would get effectively ignored because of the interaction
between how ir_assignment->set_lhs works and overwriting the write_mask.
There are two cases, one where i is a constant, and another where i is
variable. We have to be extra-careful in both cases.

Fixes the following WebGL test:

  
https://www.khronos.org/registry/webgl/sdk/tests/conformance2/glsl3/vector-dynamic-indexing-swizzled-lvalue.html

And the new piglit tests:

  swizzled-writemask-indexing-nonconst.shader_test
  swizzled-writemask-indexing.shader_test

Signed-off-by: Ilia Mirkin 
Cc: mesa-sta...@lists.freedesktop.org
---

v1 -> v2:
 - include info about piglit tests
 - add a fix for the nonconst case -- doing set_lhs last instead of
   first.

Ian Romanick reviewed v1, but given the subtlety of the additional
change for v2, going to leave that off, prefering another review.

No regressions running this through Intel CI.

 src/compiler/glsl/lower_vector_derefs.cpp | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/lower_vector_derefs.cpp 
b/src/compiler/glsl/lower_vector_derefs.cpp
index 7583d1fdd3e..223e420550b 100644
--- a/src/compiler/glsl/lower_vector_derefs.cpp
+++ b/src/compiler/glsl/lower_vector_derefs.cpp
@@ -59,8 +59,7 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
if (!deref->array->type->is_vector())
   return ir_rvalue_enter_visitor::visit_enter(ir);
 
-   ir_dereference *const new_lhs = (ir_dereference *) deref->array;
-   ir->set_lhs(new_lhs);
+   ir_rvalue *const new_lhs = deref->array;
 
void *mem_ctx = ralloc_parent(ir);
ir_constant *old_index_constant =
@@ -72,8 +71,15 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
ir->rhs,
deref->array_index);
   ir->write_mask = (1 << new_lhs->type->vector_elements) - 1;
+  ir->set_lhs(new_lhs);
+   } else if (new_lhs->ir_type != ir_type_swizzle) {
+  ir->set_lhs(new_lhs);
+  ir->write_mask = 1 << old_index_constant->get_uint_component(0);
} else {
-  ir->write_mask = 1 << old_index_constant->get_int_component(0);
+  // If there "new" lhs is a swizzle, use the set_lhs helper to instead
+  // swizzle the rhs.
+  unsigned component[1] = { old_index_constant->get_uint_component(0) };
+  ir->set_lhs(new(mem_ctx) ir_swizzle(new_lhs, component, 1));
}
 
return ir_rvalue_enter_visitor::visit_enter(ir);
-- 
2.16.4

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


[Mesa-dev] [PATCH 12/12] i965: Enable nir_opt_idiv_const for all bit sizes

2018-10-05 Thread Jason Ekstrand
Shader-db results on Sky Lake:

total instructions in shared programs: 15105795 -> 15111403 (0.04%)
instructions in affected programs: 72774 -> 78382 (7.71%)
helped: 0
HURT: 265

Note that hurt here actually means helped because we're getting rid of
integer quotient operations (which are a send on some platforms!) and
replacing them with fairly cheap ALU ops.
---
 src/intel/compiler/brw_nir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index f61baee230a..48380039cd5 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -569,6 +569,7 @@ brw_nir_optimize(nir_shader *nir, const struct brw_compiler 
*compiler,
   OPT(nir_opt_cse);
   OPT(nir_opt_peephole_select, 0);
   OPT(nir_opt_intrinsics);
+  OPT(nir_opt_idiv_const, 0);
   OPT(nir_opt_algebraic);
   OPT(nir_opt_constant_folding);
   OPT(nir_opt_dead_cf);
@@ -675,7 +676,8 @@ brw_preprocess_nir(const struct brw_compiler *compiler, 
nir_shader *nir)
 */
nir_lower_int64(nir, nir_lower_imul64 |
 nir_lower_isign64 |
-nir_lower_divmod64);
+nir_lower_divmod64 |
+nir_lower_imul_high64);
 
nir = brw_nir_optimize(nir, compiler, is_scalar, true);
 
-- 
2.19.0

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


[Mesa-dev] [PATCH 09/12] nir: Add a saturated unsigned integer add opcode

2018-10-05 Thread Jason Ekstrand
From: Ian Romanick 

---
 src/compiler/nir/nir_opcodes.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
index 209f0c5509b..1a52b3b4228 100644
--- a/src/compiler/nir/nir_opcodes.py
+++ b/src/compiler/nir/nir_opcodes.py
@@ -437,6 +437,8 @@ def binop_reduce(name, output_size, output_type, src_type, 
prereduce_expr,
 
 binop("fadd", tfloat, commutative + associative, "src0 + src1")
 binop("iadd", tint, commutative + associative, "src0 + src1")
+binop("uadd_sat", tuint, commutative,
+  "(src0 + src1) < src0 ? UINT64_MAX : (src0 + src1)")
 binop("fsub", tfloat, "", "src0 - src1")
 binop("isub", tint, "", "src0 - src1")
 
-- 
2.19.0

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


[Mesa-dev] [PATCH 10/12] nir: Add a pass for lowering integer division by constants

2018-10-05 Thread Jason Ekstrand
It's a reasonably well-known fact in the world of compilers that integer
divisions by constants can be replaced by a multiply, an add, and some
shifts.  This commit adds such an optimization to NIR for easiest case
of udiv.  Other division operations will be added in following commits.
In order to provide some additional driver control, the pass takes a
minimum bit size to optimize.
---
 src/compiler/Makefile.sources |   1 +
 src/compiler/nir/meson.build  |   1 +
 src/compiler/nir/nir.h|   2 +
 src/compiler/nir/nir_opt_idiv_const.c | 215 ++
 4 files changed, 219 insertions(+)
 create mode 100644 src/compiler/nir/nir_opt_idiv_const.c

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index d3b06564832..5833d9b1b66 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -277,6 +277,7 @@ NIR_FILES = \
nir/nir_opt_find_array_copies.c \
nir/nir_opt_gcm.c \
nir/nir_opt_global_to_local.c \
+   nir/nir_opt_idiv_const.c \
nir/nir_opt_if.c \
nir/nir_opt_intrinsics.c \
nir/nir_opt_loop_unroll.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index 090aa7a628f..7333ec7131b 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -161,6 +161,7 @@ files_libnir = files(
   'nir_opt_find_array_copies.c',
   'nir_opt_gcm.c',
   'nir_opt_global_to_local.c',
+  'nir_opt_idiv_const.c',
   'nir_opt_if.c',
   'nir_opt_intrinsics.c',
   'nir_opt_large_constants.c',
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 9bb9f50b9b4..9eb3cc8bc00 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -3036,6 +3036,8 @@ bool nir_opt_find_array_copies(nir_shader *shader);
 
 bool nir_opt_gcm(nir_shader *shader, bool value_number);
 
+bool nir_opt_idiv_const(nir_shader *shader, unsigned min_bit_size);
+
 bool nir_opt_if(nir_shader *shader);
 
 bool nir_opt_intrinsics(nir_shader *shader);
diff --git a/src/compiler/nir/nir_opt_idiv_const.c 
b/src/compiler/nir/nir_opt_idiv_const.c
new file mode 100644
index 000..7fa739161ba
--- /dev/null
+++ b/src/compiler/nir/nir_opt_idiv_const.c
@@ -0,0 +1,215 @@
+/*
+ * 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 "util/fast_idiv_by_const.h"
+#include "util/u_math.h"
+
+static nir_ssa_def *
+build_udiv(nir_builder *b, nir_ssa_def *n, uint64_t d)
+{
+   if (d == 0) {
+  return nir_imm_intN_t(b, 0, n->bit_size);
+   } else if (util_is_power_of_two_or_zero64(d)) {
+  return nir_ushr(b, n, nir_imm_int(b, util_logbase2_64(d)));
+   } else {
+  struct util_fast_udiv_info m =
+ util_compute_fast_udiv_info(d, n->bit_size, n->bit_size);
+
+  if (m.pre_shift)
+ n = nir_ushr(b, n, nir_imm_int(b, m.pre_shift));
+  if (m.increment)
+ n = nir_uadd_sat(b, n, nir_imm_intN_t(b, m.increment, n->bit_size));
+  n = nir_umul_high(b, n, nir_imm_intN_t(b, m.multiplier, n->bit_size));
+  if (m.post_shift)
+ n = nir_ushr(b, n, nir_imm_int(b, m.post_shift));
+
+  return n;
+   }
+}
+
+static nir_ssa_def *
+build_umod(nir_builder *b, nir_ssa_def *n, uint64_t d)
+{
+   if (d == 0) {
+  return nir_imm_intN_t(b, 0, n->bit_size);
+   } else if (util_is_power_of_two_or_zero64(d)) {
+  return nir_iand(b, n, nir_imm_intN_t(b, d - 1, n->bit_size));
+   } else {
+  return nir_isub(b, n, nir_imul(b, build_udiv(b, n, d),
+nir_imm_intN_t(b, d, n->bit_size)));
+   }
+}
+
+static nir_ssa_def *
+build_idiv(nir_builder *b, nir_ssa_def *n, int64_t d)
+{
+   if (d == 0) {
+  return nir_imm_intN_t(b, 0, n->bit_size);
+   } else if (d == 1) {
+  return n;
+   } else if (d == -1) {
+  return nir_ineg(b, n);
+   } else if 

[Mesa-dev] [PATCH 11/12] i965/fs: Implement nir_op_uadd_sat

2018-10-05 Thread Jason Ekstrand
From: Ian Romanick 

---
 src/intel/compiler/brw_fs_nir.cpp | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/intel/compiler/brw_fs_nir.cpp 
b/src/intel/compiler/brw_fs_nir.cpp
index 7f453d75b64..f042eac432d 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -989,6 +989,11 @@ fs_visitor::nir_emit_alu(const fs_builder , 
nir_alu_instr *instr)
   inst->saturate = instr->dest.saturate;
   break;
 
+   case nir_op_uadd_sat:
+  inst = bld.ADD(result, op[0], op[1]);
+  inst->saturate = true;
+  break;
+
case nir_op_fmul:
   inst = bld.MUL(result, op[0], op[1]);
   inst->saturate = instr->dest.saturate;
-- 
2.19.0

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


[Mesa-dev] [PATCH 07/12] nir: Allow [iu]mul_high on non-32-bit types

2018-10-05 Thread Jason Ekstrand
---
 src/compiler/nir/nir_constant_expressions.py |  1 +
 src/compiler/nir/nir_opcodes.py  | 43 ++--
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/src/compiler/nir/nir_constant_expressions.py 
b/src/compiler/nir/nir_constant_expressions.py
index 118af9f7818..afc0739e8b2 100644
--- a/src/compiler/nir/nir_constant_expressions.py
+++ b/src/compiler/nir/nir_constant_expressions.py
@@ -79,6 +79,7 @@ template = """\
 #include 
 #include "util/rounding.h" /* for _mesa_roundeven */
 #include "util/half_float.h"
+#include "util/bigmath.h"
 #include "nir_constant_expressions.h"
 
 /**
diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
index 4ef4ecc6f22..209f0c5509b 100644
--- a/src/compiler/nir/nir_opcodes.py
+++ b/src/compiler/nir/nir_opcodes.py
@@ -443,12 +443,47 @@ binop("isub", tint, "", "src0 - src1")
 binop("fmul", tfloat, commutative + associative, "src0 * src1")
 # low 32-bits of signed/unsigned integer multiply
 binop("imul", tint, commutative + associative, "src0 * src1")
+
 # high 32-bits of signed integer multiply
-binop("imul_high", tint32, commutative,
-  "(int32_t)(((int64_t) src0 * (int64_t) src1) >> 32)")
+binop("imul_high", tint, commutative, """
+if (bit_size == 64) {
+   /* We need to do a full 128-bit x 128-bit multiply in order for the sign
+* extension to work properly.  The casts are kind-of annoying but needed
+* to prevent compiler warnings.
+*/
+   uint32_t src0_u32[4] = {
+  src0,
+  (int64_t)src0 >> 32,
+  (int64_t)src0 >> 63,
+  (int64_t)src0 >> 63,
+   };
+   uint32_t src1_u32[4] = {
+  src1,
+  (int64_t)src1 >> 32,
+  (int64_t)src1 >> 63,
+  (int64_t)src1 >> 63,
+   };
+   uint32_t prod_u32[4];
+   ubm_mul_u32arr(prod_u32, src0_u32, src1_u32);
+   dst = (uint64_t)prod_u32[2] | ((uint64_t)prod_u32[3] << 32);
+} else {
+   dst = ((int64_t)src0 * (int64_t)src1) >> bit_size;
+}
+""")
+
 # high 32-bits of unsigned integer multiply
-binop("umul_high", tuint32, commutative,
-  "(uint32_t)(((uint64_t) src0 * (uint64_t) src1) >> 32)")
+binop("umul_high", tuint, commutative, """
+if (bit_size == 64) {
+   /* The casts are kind-of annoying but needed to prevent compiler warnings. 
*/
+   uint32_t src0_u32[2] = { src0, (uint64_t)src0 >> 32 };
+   uint32_t src1_u32[2] = { src1, (uint64_t)src1 >> 32 };
+   uint32_t prod_u32[4];
+   ubm_mul_u32arr(prod_u32, src0_u32, src1_u32);
+   dst = (uint64_t)prod_u32[2] | ((uint64_t)prod_u32[3] << 32);
+} else {
+   dst = ((uint64_t)src0 * (uint64_t)src1) >> bit_size;
+}
+""")
 
 binop("fdiv", tfloat, "", "src0 / src1")
 binop("idiv", tint, "", "src1 == 0 ? 0 : (src0 / src1)")
-- 
2.19.0

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


[Mesa-dev] [PATCH 08/12] nir/lower_int64: Add support for [iu]mul_high

2018-10-05 Thread Jason Ekstrand
---
 src/compiler/nir/nir.h |  1 +
 src/compiler/nir/nir_lower_int64.c | 54 ++
 2 files changed, 55 insertions(+)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index e0df95c391c..9bb9f50b9b4 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2971,6 +2971,7 @@ typedef enum {
nir_lower_isign64 = (1 << 1),
/** Lower all int64 modulus and division opcodes */
nir_lower_divmod64 = (1 << 2),
+   nir_lower_imul_high64 = (1 << 3),
 } nir_lower_int64_options;
 
 bool nir_lower_int64(nir_shader *shader, nir_lower_int64_options options);
diff --git a/src/compiler/nir/nir_lower_int64.c 
b/src/compiler/nir/nir_lower_int64.c
index 0d7f165b406..044f0fb2e12 100644
--- a/src/compiler/nir/nir_lower_int64.c
+++ b/src/compiler/nir/nir_lower_int64.c
@@ -40,6 +40,53 @@ lower_imul64(nir_builder *b, nir_ssa_def *x, nir_ssa_def *y)
return nir_pack_64_2x32_split(b, res_lo, res_hi);
 }
 
+static nir_ssa_def *
+lower_mul_high64(nir_builder *b, nir_ssa_def *x, nir_ssa_def *y,
+ bool sign_extend)
+{
+   nir_ssa_def *x32[4], *y32[4];
+   x32[0] = nir_unpack_64_2x32_split_x(b, x);
+   x32[1] = nir_unpack_64_2x32_split_y(b, x);
+   if (sign_extend) {
+  x32[2] = x32[3] = nir_ishr(b, x32[1], nir_imm_int(b, 31));
+   } else {
+  x32[2] = x32[3] = nir_imm_int(b, 0);
+   }
+
+   y32[0] = nir_unpack_64_2x32_split_x(b, y);
+   y32[1] = nir_unpack_64_2x32_split_y(b, y);
+   if (sign_extend) {
+  y32[2] = y32[3] = nir_ishr(b, y32[1], nir_imm_int(b, 31));
+   } else {
+  y32[2] = y32[3] = nir_imm_int(b, 0);
+   }
+
+   nir_ssa_def *res[8] = { NULL, };
+
+   /* Yes, the following generates a pile of code.  However, we throw res[0]
+* and res[1] away in the end and, if we're in the umul case, four of our
+* eight dword operands will be constant zero and opt_algebraic will clean
+* this up nicely.
+*/
+   for (unsigned i = 0; i < 4; i++) {
+  nir_ssa_def *carry = NULL;
+  for (unsigned j = 0; j < 4; j++) {
+ nir_ssa_def *tmp =
+nir_pack_64_2x32_split(b, nir_imul(b, x32[i], y32[j]),
+  nir_umul_high(b, x32[i], y32[j]));
+ if (res[i + j])
+tmp = nir_iadd(b, tmp, nir_u2u64(b, res[i + j]));
+ if (carry)
+tmp = nir_iadd(b, tmp, carry);
+ res[i + j] = nir_u2u32(b, tmp);
+ carry = nir_ushr(b, tmp, nir_imm_int(b, 32));
+  }
+  res[i + 4] = nir_u2u32(b, carry);
+   }
+
+   return nir_pack_64_2x32_split(b, res[2], res[3]);
+}
+
 static nir_ssa_def *
 lower_isign64(nir_builder *b, nir_ssa_def *x)
 {
@@ -209,6 +256,9 @@ opcode_to_options_mask(nir_op opcode)
switch (opcode) {
case nir_op_imul:
   return nir_lower_imul64;
+   case nir_op_imul_high:
+   case nir_op_umul_high:
+  return nir_lower_imul_high64;
case nir_op_isign:
   return nir_lower_isign64;
case nir_op_udiv:
@@ -232,6 +282,10 @@ lower_int64_alu_instr(nir_builder *b, nir_alu_instr *alu)
switch (alu->op) {
case nir_op_imul:
   return lower_imul64(b, src[0], src[1]);
+   case nir_op_imul_high:
+  return lower_mul_high64(b, src[0], src[1], true);
+   case nir_op_umul_high:
+  return lower_mul_high64(b, src[0], src[1], false);
case nir_op_isign:
   return lower_isign64(b, src[0]);
case nir_op_udiv:
-- 
2.19.0

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


[Mesa-dev] [PATCH 03/12] util: Add fast division helpers

2018-10-05 Thread Jason Ekstrand
From: Marek Olšák 

---
 src/util/fast_idiv_by_const.h | 44 +++
 1 file changed, 44 insertions(+)

diff --git a/src/util/fast_idiv_by_const.h b/src/util/fast_idiv_by_const.h
index ac10cf79ba8..1ba9f9a20b8 100644
--- a/src/util/fast_idiv_by_const.h
+++ b/src/util/fast_idiv_by_const.h
@@ -130,6 +130,50 @@ struct util_fast_udiv_info {
 struct util_fast_udiv_info
 util_compute_fast_udiv_info(uint_t D, unsigned num_bits);
 
+/* Below are possible options for dividing by a uniform in a shader where
+ * the divisor is constant but not known at compile time.
+ */
+
+/* Full version. */
+static inline uint32_t
+util_fast_udiv32(uint32_t n, struct util_fast_udiv_info info)
+{
+   n = n >> info.pre_shift;
+   /* For non-power-of-two divisors, use a 32-bit ADD that clamps to UINT_MAX. 
*/
+   n = (((uint64_t)n + info.increment) * info.multiplier) >> 32;
+   n = n >> info.post_shift;
+   return n;
+}
+
+/* A little more efficient version if n != UINT_MAX, i.e. no unsigned
+ * wraparound in the computation.
+ */
+static inline uint32_t
+util_fast_udiv32_nuw(uint32_t n, struct util_fast_udiv_info info)
+{
+   assert(n != UINT32_MAX);
+   n = n >> info.pre_shift;
+   n = n + info.increment;
+   n = ((uint64_t)n * info.multiplier) >> 32;
+   n = n >> info.post_shift;
+   return n;
+}
+
+/* Even faster version but both operands must be 31-bit unsigned integers
+ * and the divisor must be greater than 1.
+ *
+ * info must be computed with num_bits == 31.
+ */
+static inline uint32_t
+util_fast_udiv32_u31_d_not_one(uint32_t n, struct util_fast_udiv_info info)
+{
+   assert(info.pre_shift == 0);
+   assert(info.increment == 0);
+   n = ((uint64_t)n * info.multiplier) >> 32;
+   n = n >> info.post_shift;
+   return n;
+}
+
 #ifdef __cplusplus
 } /* extern C */
 #endif
-- 
2.19.0

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


[Mesa-dev] [PATCH 01/12] util: Add a simple big math library

2018-10-05 Thread Jason Ekstrand
---
 src/util/Makefile.am  |   1 +
 src/util/Makefile.sources |   1 +
 src/util/bigmath.h| 112 ++
 src/util/meson.build  |   1 +
 4 files changed, 115 insertions(+)
 create mode 100644 src/util/bigmath.h

diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index efb94caff71..d79f2b320be 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -21,6 +21,7 @@
 
 SUBDIRS = . \
xmlpool \
+   tests/fast_idiv_by_const \
tests/hash_table \
tests/string_buffer \
tests/set
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index b562d6cd6f4..5b1548c733c 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -1,4 +1,5 @@
 MESA_UTIL_FILES := \
+   bigmath.h \
bitscan.c \
bitscan.h \
bitset.h \
diff --git a/src/util/bigmath.h b/src/util/bigmath.h
new file mode 100644
index 000..6339bb6f6ca
--- /dev/null
+++ b/src/util/bigmath.h
@@ -0,0 +1,112 @@
+/*
+ * 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.
+ */
+
+#ifndef UTIL_BIGMATH_H
+#define UTIL_BIGMATH_H
+
+#include "macros.h"
+
+#include 
+#include 
+#include 
+
+static inline bool
+_ubm_add_u32arr(uint32_t *dst, unsigned dst_len,
+uint32_t *a, unsigned a_len,
+uint32_t *b, unsigned b_len)
+{
+   uint32_t carry = 0;
+   for (unsigned i = 0; i < dst_len; i++) {
+  uint64_t sum = carry;
+  if (i < a_len)
+ sum += a[i];
+  if (i < b_len)
+ sum += b[i];
+  dst[i] = sum;
+  carry = sum >> 32;
+   }
+
+   /* Now compute overflow */
+
+   for (unsigned i = dst_len; i < a_len; i++) {
+  if (a[i])
+ return true;
+   }
+
+   for (unsigned i = dst_len; i < b_len; i++) {
+  if (b[i])
+ return true;
+   }
+
+   return carry;
+}
+#define ubm_add_u32arr(dst, a, b) \
+   _ubm_add_u32arr(dst, ARRAY_SIZE(dst), a, ARRAY_SIZE(a), b, ARRAY_SIZE(b))
+
+static inline bool
+_ubm_mul_u32arr(uint32_t *dst, unsigned dst_len,
+uint32_t *a, unsigned a_len,
+uint32_t *b, unsigned b_len)
+{
+   memset(dst, 0, dst_len * sizeof(*dst));
+
+   bool overflow = false;
+
+   for (unsigned i = 0; i < a_len; i++) {
+  uint32_t carry = 0;
+  for (unsigned j = 0; j < b_len; j++) {
+ /* The maximum values of a[i] and b[i] are UINT32_MAX so the maximum
+  * value of tmp is UINT32_MAX * UINT32_MAX.  The maximum value that
+  * will fit in tmp is
+  *
+  *UINT64_MAX = UINT32_MAX << 32 + UINT32_MAX
+  *   = UINT32_MAX * (UINT32_MAX + 1) + UINT32_MAX
+  *   = UINT32_MAX * UINT32_MAX + 2 * UINT32_MAX
+  *
+  * so we're guaranteed that we can add in two more 32-bit values
+  * without overflowing tmp.
+  */
+ uint64_t tmp = (uint64_t)a[i] * (uint64_t)b[j];
+ tmp += carry;
+ if (i + j < dst_len) {
+tmp += dst[i + j];
+dst[i + j] = tmp;
+carry = tmp >> 32;
+ } else {
+/* We're trying to write a value that doesn't fit */
+overflow = overflow || tmp > 0;
+break;
+ }
+  }
+  if (i + b_len < dst_len)
+ dst[i + b_len] = carry;
+  else
+ overflow = overflow || carry > 0;
+   }
+
+   return overflow;
+}
+#define ubm_mul_u32arr(dst, a, b) \
+   _ubm_mul_u32arr(dst, ARRAY_SIZE(dst), a, ARRAY_SIZE(a), b, ARRAY_SIZE(b))
+
+#endif /* UTIL_BIGMATH_H */
diff --git a/src/util/meson.build b/src/util/meson.build
index 027bc5b9d0d..9a99d60c158 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -23,6 +23,7 @@ inc_util = include_directories('.')
 subdir('xmlpool')
 
 files_mesa_util = files(
+  'bigmath.h',
   'bitscan.c',
   'bitscan.h',
   'bitset.h',
-- 
2.19.0


[Mesa-dev] [PATCH 06/12] util: Add tests for fast integer division by constants

2018-10-05 Thread Jason Ekstrand
While I generally trust rediculousfish to have done his homework, we've
made some adjustments to suite the needs of mesa and it'd be good to
test those.  Also, there's no better place than unit tests to clearly
document the different edge cases of the different methods.
---
 configure.ac  |   1 +
 src/util/Makefile.am  |   3 +-
 src/util/meson.build  |   1 +
 src/util/tests/fast_idiv_by_const/Makefile.am |  43 ++
 .../fast_idiv_by_const_test.cpp   | 472 ++
 src/util/tests/fast_idiv_by_const/meson.build |  30 ++
 6 files changed, 549 insertions(+), 1 deletion(-)
 create mode 100644 src/util/tests/fast_idiv_by_const/Makefile.am
 create mode 100644 
src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
 create mode 100644 src/util/tests/fast_idiv_by_const/meson.build

diff --git a/configure.ac b/configure.ac
index 34689826c98..7b0b2b20ba2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3198,6 +3198,7 @@ AC_CONFIG_FILES([Makefile
  src/util/tests/hash_table/Makefile
  src/util/tests/set/Makefile
  src/util/tests/string_buffer/Makefile
+ src/util/tests/uint_inverse/Makefile
  src/util/tests/vma/Makefile
  src/util/xmlpool/Makefile
  src/vulkan/Makefile])
diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index d79f2b320be..9e633bf65d5 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -24,7 +24,8 @@ SUBDIRS = . \
tests/fast_idiv_by_const \
tests/hash_table \
tests/string_buffer \
-   tests/set
+   tests/set \
+   tests/uint_inverse
 
 if HAVE_STD_CXX11
 SUBDIRS += tests/vma
diff --git a/src/util/meson.build b/src/util/meson.build
index cdbad98e7cb..49d84c16ebe 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -170,6 +170,7 @@ if with_tests
 )
   )
 
+  subdir('tests/fast_idiv_by_const')
   subdir('tests/hash_table')
   subdir('tests/string_buffer')
   subdir('tests/vma')
diff --git a/src/util/tests/fast_idiv_by_const/Makefile.am 
b/src/util/tests/fast_idiv_by_const/Makefile.am
new file mode 100644
index 000..1ebee09f59b
--- /dev/null
+++ b/src/util/tests/fast_idiv_by_const/Makefile.am
@@ -0,0 +1,43 @@
+# Copyright © 2018 Intel
+#
+#  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.
+
+AM_CPPFLAGS = \
+   -I$(top_srcdir)/src \
+   -I$(top_srcdir)/include \
+   -I$(top_srcdir)/src/gallium/include \
+   -I$(top_srcdir)/src/gtest/include \
+   $(PTHREAD_CFLAGS) \
+   $(DEFINES)
+
+TESTS = fast_idiv_by_const_test
+
+check_PROGRAMS = $(TESTS)
+
+fast_idiv_by_const_test_SOURCES = \
+   fast_idiv_by_const_test.cpp
+
+fast_idiv_by_const_test_LDADD = \
+   $(top_builddir)/src/gtest/libgtest.la \
+   $(top_builddir)/src/util/libmesautil.la \
+   $(PTHREAD_LIBS) \
+   $(DLOPEN_LIBS)
+
+EXTRA_DIST = meson.build
diff --git a/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp 
b/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
new file mode 100644
index 000..34b149e1c6f
--- /dev/null
+++ b/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
@@ -0,0 +1,472 @@
+/*
+ * 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 

[Mesa-dev] [PATCH 00/12] nir: Add an integer divide by constant optimization

2018-10-05 Thread Jason Ekstrand
It's reasonably well-known that you can replace an unsigned integer
division by a constant with a sequence of multiply (by constant), adds, and
shifts.  This little series implements such an optimization for NIR.  This
second version of the series is based on the integer division by constant
code by rediculousfish.  I've tried to keep authorship of the various bits
as accurate as I can hence the rather eclectic collection of patches.

Ian Romanick (2):
  nir: Add a saturated unsigned integer add opcode
  i965/fs: Implement nir_op_uadd_sat

Jason Ekstrand (7):
  util: Add a simple big math library
  util: Generalize fast integer division to be variable bit-width
  util: Add tests for fast integer division by constants
  nir: Allow [iu]mul_high on non-32-bit types
  nir/lower_int64: Add support for [iu]mul_high
  nir: Add a pass for lowering integer division by constants
  i965: Enable nir_opt_idiv_const for all bit sizes

Marek Olšák (3):
  util: import public domain code for integer division by a constant
  util: Add fast division helpers
  util: Add power-of-two divisor support to compute_fast_udiv_info

 configure.ac  |   1 +
 src/compiler/Makefile.sources |   1 +
 src/compiler/nir/meson.build  |   1 +
 src/compiler/nir/nir.h|   3 +
 src/compiler/nir/nir_constant_expressions.py  |   1 +
 src/compiler/nir/nir_lower_int64.c|  54 ++
 src/compiler/nir/nir_opcodes.py   |  45 +-
 src/compiler/nir/nir_opt_idiv_const.c | 215 
 src/intel/compiler/brw_fs_nir.cpp |   5 +
 src/intel/compiler/brw_nir.c  |   4 +-
 src/util/Makefile.am  |   4 +-
 src/util/Makefile.sources |   3 +
 src/util/bigmath.h| 112 +
 src/util/fast_idiv_by_const.c | 243 +
 src/util/fast_idiv_by_const.h | 178 +++
 src/util/meson.build  |   4 +
 src/util/tests/fast_idiv_by_const/Makefile.am |  43 ++
 .../fast_idiv_by_const_test.cpp   | 472 ++
 src/util/tests/fast_idiv_by_const/meson.build |  30 ++
 19 files changed, 1413 insertions(+), 6 deletions(-)
 create mode 100644 src/compiler/nir/nir_opt_idiv_const.c
 create mode 100644 src/util/bigmath.h
 create mode 100644 src/util/fast_idiv_by_const.c
 create mode 100644 src/util/fast_idiv_by_const.h
 create mode 100644 src/util/tests/fast_idiv_by_const/Makefile.am
 create mode 100644 
src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
 create mode 100644 src/util/tests/fast_idiv_by_const/meson.build

-- 
2.19.0

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


[Mesa-dev] [PATCH 05/12] util: Add power-of-two divisor support to compute_fast_udiv_info

2018-10-05 Thread Jason Ekstrand
From: Marek Olšák 

---
 src/util/fast_idiv_by_const.c | 21 +
 src/util/fast_idiv_by_const.h |  5 +++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/util/fast_idiv_by_const.c b/src/util/fast_idiv_by_const.c
index 65a9e640789..7b93316268c 100644
--- a/src/util/fast_idiv_by_const.c
+++ b/src/util/fast_idiv_by_const.c
@@ -52,6 +52,27 @@ util_compute_fast_udiv_info(uint64_t D, unsigned num_bits, 
unsigned UINT_BITS)
/* The eventual result */
struct util_fast_udiv_info result;
 
+   if (util_is_power_of_two_or_zero64(D)) {
+  unsigned div_shift = util_logbase2_64(D);
+
+  if (div_shift) {
+ /* Dividing by a power of two. */
+ result.multiplier = 1ull << (UINT_BITS - div_shift);
+ result.pre_shift = 0;
+ result.post_shift = 0;
+ result.increment = 0;
+ return result;
+  } else {
+ /* Dividing by 1. */
+ /* Assuming: floor((num + 1) * (2^32 - 1) / 2^32) = num */
+ result.multiplier = UINT_BITS == 64 ? UINT64_MAX :
+   (1ull << UINT_BITS) - 1;
+ result.pre_shift = 0;
+ result.post_shift = 0;
+ result.increment = 1;
+ return result;
+  }
+   }
 
/* The extra shift implicit in the difference between UINT_BITS and num_bits
 */
diff --git a/src/util/fast_idiv_by_const.h b/src/util/fast_idiv_by_const.h
index 231311f84be..3363fb9ee71 100644
--- a/src/util/fast_idiv_by_const.h
+++ b/src/util/fast_idiv_by_const.h
@@ -98,8 +98,9 @@ util_compute_fast_sdiv_info(int64_t D, unsigned SINT_BITS);
  *   emit("result >>>= UINT_BITS")
  *   if m.post_shift > 0: emit("result >>>= m.post_shift")
  *
- * The shifts by UINT_BITS may be "free" if the high half of the full multiply
- * is put in a separate register.
+ * This second version works even if D is a power of two.  The shifts by
+ * UINT_BITS may be "free" if the high half of the full multiply is put in a
+ * separate register.
  *
  * saturated_increment(n) means "increment n unless it would wrap to 0," i.e.
  *   if n == (1 << UINT_BITS)-1: result = n
-- 
2.19.0

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


[Mesa-dev] [PATCH 04/12] util: Generalize fast integer division to be variable bit-width

2018-10-05 Thread Jason Ekstrand
There's nothing inherently fixed-width in the code.  All that's required
to generalize it is to make everything internally 64-bit and pass
UINT_BITS in as a parameter to util_compute_fast_[us]div_info.  With
that, it can now handle 8, 16, 32, and 64-bit integer division by a
constant.

We also add support for division by 1 and by other powers of 2.  This is
useful if you want to divide by a uniform value in a shader where you
have the opportunity to adjust the uniform on the CPU before passing it
in.
---
 src/util/fast_idiv_by_const.c | 62 +--
 src/util/fast_idiv_by_const.h | 22 +
 2 files changed, 39 insertions(+), 45 deletions(-)

diff --git a/src/util/fast_idiv_by_const.c b/src/util/fast_idiv_by_const.c
index 0bc9b60878b..65a9e640789 100644
--- a/src/util/fast_idiv_by_const.c
+++ b/src/util/fast_idiv_by_const.c
@@ -42,22 +42,16 @@
 #include 
 #include 
 
-/* uint_t and sint_t can be replaced by different integer types and the code
- * will work as-is. The only requirement is that sizeof(uintN) == sizeof(intN).
- */
-
 struct util_fast_udiv_info
-util_compute_fast_udiv_info(uint_t D, unsigned num_bits)
+util_compute_fast_udiv_info(uint64_t D, unsigned num_bits, unsigned UINT_BITS)
 {
-   /* The numerator must fit in a uint_t */
-   assert(num_bits > 0 && num_bits <= sizeof(uint_t) * CHAR_BIT);
+   /* The numerator must fit in a uint64_t */
+   assert(num_bits > 0 && num_bits <= UINT_BITS);
assert(D != 0);
 
/* The eventual result */
struct util_fast_udiv_info result;
 
-   /* Bits in a uint_t */
-   const unsigned UINT_BITS = sizeof(uint_t) * CHAR_BIT;
 
/* The extra shift implicit in the difference between UINT_BITS and num_bits
 */
@@ -66,23 +60,23 @@ util_compute_fast_udiv_info(uint_t D, unsigned num_bits)
/* The initial power of 2 is one less than the first one that can possibly
 * work.
 */
-   const uint_t initial_power_of_2 = (uint_t)1 << (UINT_BITS-1);
+   const uint64_t initial_power_of_2 = (uint64_t)1 << (UINT_BITS-1);
 
/* The remainder and quotient of our power of 2 divided by d */
-   uint_t quotient = initial_power_of_2 / D;
-   uint_t remainder = initial_power_of_2 % D;
+   uint64_t quotient = initial_power_of_2 / D;
+   uint64_t remainder = initial_power_of_2 % D;
 
/* ceil(log_2 D) */
unsigned ceil_log_2_D;
 
/* The magic info for the variant "round down" algorithm */
-   uint_t down_multiplier = 0;
+   uint64_t down_multiplier = 0;
unsigned down_exponent = 0;
int has_magic_down = 0;
 
/* Compute ceil(log_2 D) */
ceil_log_2_D = 0;
-   uint_t tmp;
+   uint64_t tmp;
for (tmp = D; tmp > 0; tmp >>= 1)
   ceil_log_2_D += 1;
 
@@ -110,14 +104,14 @@ util_compute_fast_udiv_info(uint_t D, unsigned num_bits)
* so the check for >= ceil_log_2_D is critical.
*/
   if ((exponent + extra_shift >= ceil_log_2_D) ||
-  (D - remainder) <= ((uint_t)1 << (exponent + extra_shift)))
+  (D - remainder) <= ((uint64_t)1 << (exponent + extra_shift)))
  break;
 
   /* Set magic_down if we have not set it yet and this exponent works for
* the round_down algorithm
*/
   if (!has_magic_down &&
-  remainder <= ((uint_t)1 << (exponent + extra_shift))) {
+  remainder <= ((uint64_t)1 << (exponent + extra_shift))) {
  has_magic_down = 1;
  down_multiplier = quotient;
  down_exponent = exponent;
@@ -140,12 +134,13 @@ util_compute_fast_udiv_info(uint_t D, unsigned num_bits)
} else {
   /* Even divisor, so use a prefix-shifted dividend */
   unsigned pre_shift = 0;
-  uint_t shifted_D = D;
+  uint64_t shifted_D = D;
   while ((shifted_D & 1) == 0) {
  shifted_D >>= 1;
  pre_shift += 1;
   }
-  result = util_compute_fast_udiv_info(shifted_D, num_bits - pre_shift);
+  result = util_compute_fast_udiv_info(shifted_D, num_bits - pre_shift,
+   UINT_BITS);
   /* expect no increment or pre_shift in this path */
   assert(result.increment == 0 && result.pre_shift == 0);
   result.pre_shift = pre_shift;
@@ -153,8 +148,14 @@ util_compute_fast_udiv_info(uint_t D, unsigned num_bits)
return result;
 }
 
+static inline int64_t
+sign_extend(int64_t x, unsigned SINT_BITS)
+{
+   return (x << (64 - SINT_BITS)) >> (64 - SINT_BITS);
+}
+
 struct util_fast_sdiv_info
-util_compute_fast_sdiv_info(sint_t D)
+util_compute_fast_sdiv_info(int64_t D, unsigned SINT_BITS)
 {
/* D must not be zero. */
assert(D != 0);
@@ -164,33 +165,30 @@ util_compute_fast_sdiv_info(sint_t D)
/* Our result */
struct util_fast_sdiv_info result;
 
-   /* Bits in an sint_t */
-   const unsigned SINT_BITS = sizeof(sint_t) * CHAR_BIT;
-
/* Absolute value of D (we know D is not the most negative value since
 * that's a power of 2)
 */
-   const uint_t abs_d = (D < 0 ? -D : D);
+   const uint64_t abs_d = (D < 0 ? -D : D);
 
 

[Mesa-dev] [PATCH 02/12] util: import public domain code for integer division by a constant

2018-10-05 Thread Jason Ekstrand
From: Marek Olšák 

Compilers can use this to generate optimal code for integer division
by a constant.

Additionally, an unsigned division by a uniform that is constant but not
known at compile time can still be optimized by passing 2-4 division
factors to the shader as uniforms and executing one of the fast_udiv*
variants. The signed division algorithm doesn't have this capability.
---
 src/util/Makefile.sources |   2 +
 src/util/fast_idiv_by_const.c | 224 ++
 src/util/fast_idiv_by_const.h | 137 +
 src/util/meson.build  |   2 +
 4 files changed, 365 insertions(+)
 create mode 100644 src/util/fast_idiv_by_const.c
 create mode 100644 src/util/fast_idiv_by_const.h

diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index 5b1548c733c..e8558f561f8 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -11,6 +11,8 @@ MESA_UTIL_FILES := \
debug.h \
disk_cache.c \
disk_cache.h \
+   fast_idiv_by_const.c \
+   fast_idiv_by_const.h \
format_r11g11b10f.h \
format_rgb9e5.h \
format_srgb.h \
diff --git a/src/util/fast_idiv_by_const.c b/src/util/fast_idiv_by_const.c
new file mode 100644
index 000..0bc9b60878b
--- /dev/null
+++ b/src/util/fast_idiv_by_const.c
@@ -0,0 +1,224 @@
+/*
+ * Copyright © 2018 Advanced Micro Devices, Inc.
+ *
+ * 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.
+ */
+
+/* Imported from:
+ *   
https://raw.githubusercontent.com/ridiculousfish/libdivide/master/divide_by_constants_codegen_reference.c
+ * Paper:
+ *   http://ridiculousfish.com/files/faster_unsigned_division_by_constants.pdf
+ *
+ * The author, ridiculous_fish, wrote:
+ *
+ *  ''Reference implementations of computing and using the "magic number"
+ *approach to dividing by constants, including codegen instructions.
+ *The unsigned division incorporates the "round down" optimization per
+ *ridiculous_fish.
+ *
+ *This is free and unencumbered software. Any copyright is dedicated
+ *to the Public Domain.''
+ */
+
+#include "fast_idiv_by_const.h"
+#include "u_math.h"
+#include 
+#include 
+
+/* uint_t and sint_t can be replaced by different integer types and the code
+ * will work as-is. The only requirement is that sizeof(uintN) == sizeof(intN).
+ */
+
+struct util_fast_udiv_info
+util_compute_fast_udiv_info(uint_t D, unsigned num_bits)
+{
+   /* The numerator must fit in a uint_t */
+   assert(num_bits > 0 && num_bits <= sizeof(uint_t) * CHAR_BIT);
+   assert(D != 0);
+
+   /* The eventual result */
+   struct util_fast_udiv_info result;
+
+   /* Bits in a uint_t */
+   const unsigned UINT_BITS = sizeof(uint_t) * CHAR_BIT;
+
+   /* The extra shift implicit in the difference between UINT_BITS and num_bits
+*/
+   const unsigned extra_shift = UINT_BITS - num_bits;
+
+   /* The initial power of 2 is one less than the first one that can possibly
+* work.
+*/
+   const uint_t initial_power_of_2 = (uint_t)1 << (UINT_BITS-1);
+
+   /* The remainder and quotient of our power of 2 divided by d */
+   uint_t quotient = initial_power_of_2 / D;
+   uint_t remainder = initial_power_of_2 % D;
+
+   /* ceil(log_2 D) */
+   unsigned ceil_log_2_D;
+
+   /* The magic info for the variant "round down" algorithm */
+   uint_t down_multiplier = 0;
+   unsigned down_exponent = 0;
+   int has_magic_down = 0;
+
+   /* Compute ceil(log_2 D) */
+   ceil_log_2_D = 0;
+   uint_t tmp;
+   for (tmp = D; tmp > 0; tmp >>= 1)
+  ceil_log_2_D += 1;
+
+
+   /* Begin a loop that increments the exponent, until we find a power of 2
+* that works.
+*/
+   unsigned exponent;
+   for (exponent = 0; ; exponent++) {
+  /* Quotient and remainder is from previous exponent; compute it for this
+   * exponent.
+   */
+  if (remainder >= D - remainder) {
+ /* Doubling remainder will wrap around D */
+   

Re: [Mesa-dev] [PATCH 6/9] i965: Use 'available' fence for polling query results

2018-10-05 Thread Ian Romanick
On 10/02/2018 11:06 AM, Chris Wilson wrote:
> If we always write the 'available' flag after writing the final result
> of the query, we can probe that predicate to quickly query whether the
> result is ready from userspace. The primary advantage of checking the
> predicate is that it allows for more fine-grained queries, we do not
> have to wait for the batch to finish before the query is marked as
> ready.
> 
> We still do check the status of the batch after probing the query so
> that if the worst happens and the batch did hang without completing the
> query, we do not spin forever (although it is not as nice as completely
> eliminating the ioctl, the busy-ioctl is lightweight!).
> 
> Signed-off-by: Chris Wilson 
> Cc: Kenneth Graunke 
> Cc: Matt Turner 
> ---
>  src/mesa/drivers/dri/i965/brw_context.h   |  4 +-
>  src/mesa/drivers/dri/i965/gen6_queryobj.c | 54 ++-
>  2 files changed, 25 insertions(+), 33 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 840332294e6..418941c9194 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -468,8 +468,8 @@ struct brw_query_object {
> bool flushed;
>  };
>  
> -#define GEN6_QUERY_PREDICATE (2)
> -#define GEN6_QUERY_RESULTS (0)
> +#define GEN6_QUERY_PREDICATE (0)
> +#define GEN6_QUERY_RESULTS (1)
>  
>  static inline unsigned
>  gen6_query_predicate_offset(const struct brw_query_object *query)
> diff --git a/src/mesa/drivers/dri/i965/gen6_queryobj.c 
> b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> index dc70e2a568a..b6832588333 100644
> --- a/src/mesa/drivers/dri/i965/gen6_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> @@ -40,8 +40,7 @@
>  #include "intel_buffer_objects.h"
>  
>  static inline void
> -set_query_availability(struct brw_context *brw, struct brw_query_object 
> *query,
> -   bool available)
> +set_query_available(struct brw_context *brw, struct brw_query_object *query)
>  {
> /* For platforms that support ARB_query_buffer_object, we write the
>  * query availability for "pipelined" queries.
> @@ -58,22 +57,12 @@ set_query_availability(struct brw_context *brw, struct 
> brw_query_object *query,
>  * PIPE_CONTROL with an immediate write will synchronize with
>  * those earlier writes, so we write 1 when the value has landed.
>  */
> -   if (brw->ctx.Extensions.ARB_query_buffer_object &&
> -   brw_is_query_pipelined(query)) {
> -  unsigned flags = PIPE_CONTROL_WRITE_IMMEDIATE;
>  
> -  if (available) {
> - /* Order available *after* the query results. */
> - flags |= PIPE_CONTROL_FLUSH_ENABLE;
> -  } else {
> - /* Make it unavailable *before* any pipelined reads. */
> - flags |= PIPE_CONTROL_CS_STALL;
> -  }
> -
> -  brw_emit_pipe_control_write(brw, flags,
> -  query->bo, 
> gen6_query_predicate_offset(query),
> -  available);
> -   }
> +   brw_emit_pipe_control_write(brw,
> +   PIPE_CONTROL_WRITE_IMMEDIATE |
> +   PIPE_CONTROL_FLUSH_ENABLE,
> +   query->bo, gen6_query_predicate_offset(query),
> +   true);
>  }
>  
>  static void
> @@ -144,12 +133,12 @@ write_xfb_overflow_streams(struct gl_context *ctx,
>  }
>  
>  static bool
> -check_xfb_overflow_streams(uint64_t *results, int count)
> +check_xfb_overflow_streams(const uint64_t *results, int count)
>  {
> bool overflow = false;
>  
> for (int i = 0; i < count; i++) {
> -  uint64_t *result_i = [4 * i];
> +  const uint64_t *result_i = [4 * i];
>  
>if ((result_i[3] - result_i[2]) != (result_i[1] - result_i[0])) {
>   overflow = true;

This hunk seems unrelated, but I'm almost always in favor of more const.

> @@ -221,7 +210,8 @@ emit_pipeline_stat(struct brw_context *brw, struct brw_bo 
> *bo,
>   */
>  static void
>  gen6_queryobj_get_results(struct gl_context *ctx,
> -  struct brw_query_object *query)
> +  struct brw_query_object *query,
> +  uint64_t *results)
>  {
> struct brw_context *brw = brw_context(ctx);
> const struct gen_device_info *devinfo = >screen->devinfo;
> @@ -229,9 +219,6 @@ gen6_queryobj_get_results(struct gl_context *ctx,
> if (query->bo == NULL)
>return;
>  
> -   brw_bo_wait_rendering(query->bo);
> -   uint64_t *results = query->results;
> -
> switch (query->Base.Target) {
> case GL_TIME_ELAPSED:
>/* The query BO contains the starting and ending timestamps.
> @@ -329,10 +316,10 @@ gen6_alloc_query(struct brw_context *brw, struct 
> brw_query_object *query)
>  
> query->results = brw_bo_map(brw, query->bo,
> MAP_COHERENT | MAP_PERSISTENT |
> -   

Re: [Mesa-dev] [PATCH 3/9] i965: Replace open-coded gen6 queryobj offsets with simple helpers

2018-10-05 Thread Ian Romanick
Patches 2 and 3 are

Reviewed-by: Ian Romanick 

On 10/02/2018 11:06 AM, Chris Wilson wrote:
> Lots of places open-coded the assumed layout of the predicate/results
> within the query object, replace those with simple helpers.
> 
> v2: Fix function decl style.
> 
> Signed-off-by: Chris Wilson 
> Cc: Kenneth Graunke 
> Cc: Matt Turner 
> ---
>  .../drivers/dri/i965/brw_conditional_render.c  | 10 --
>  src/mesa/drivers/dri/i965/brw_context.h| 15 +++
>  src/mesa/drivers/dri/i965/gen6_queryobj.c  |  6 +++---
>  src/mesa/drivers/dri/i965/hsw_queryobj.c   | 18 +-
>  4 files changed, 35 insertions(+), 14 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_conditional_render.c 
> b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> index e33e79fb6ce..0177a7f80b4 100644
> --- a/src/mesa/drivers/dri/i965/brw_conditional_render.c
> +++ b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> @@ -87,8 +87,14 @@ set_predicate_for_occlusion_query(struct brw_context *brw,
>  */
> brw_emit_pipe_control_flush(brw, PIPE_CONTROL_FLUSH_ENABLE);
>  
> -   brw_load_register_mem64(brw, MI_PREDICATE_SRC0, query->bo, 0 /* offset 
> */);
> -   brw_load_register_mem64(brw, MI_PREDICATE_SRC1, query->bo, 8 /* offset 
> */);
> +   brw_load_register_mem64(brw,
> +   MI_PREDICATE_SRC0,
> +   query->bo,
> +   gen6_query_results_offset(query, 0));
> +   brw_load_register_mem64(brw,
> +   MI_PREDICATE_SRC1,
> +   query->bo,
> +   gen6_query_results_offset(query, 1));
>  }
>  
>  static void
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 7fd15669eb9..3014fa68aff 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -467,6 +467,21 @@ struct brw_query_object {
> bool flushed;
>  };
>  
> +#define GEN6_QUERY_PREDICATE (2)
> +#define GEN6_QUERY_RESULTS (0)
> +
> +static inline unsigned
> +gen6_query_predicate_offset(const struct brw_query_object *query)
> +{
> +   return GEN6_QUERY_PREDICATE * sizeof(uint64_t);
> +}
> +
> +static inline unsigned
> +gen6_query_results_offset(const struct brw_query_object *query, unsigned idx)
> +{
> +   return (GEN6_QUERY_RESULTS + idx) * sizeof(uint64_t);
> +}
> +
>  struct brw_reloc_list {
> struct drm_i915_gem_relocation_entry *relocs;
> int reloc_count;
> diff --git a/src/mesa/drivers/dri/i965/gen6_queryobj.c 
> b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> index e3097e878aa..ffdee4040fc 100644
> --- a/src/mesa/drivers/dri/i965/gen6_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> @@ -71,7 +71,7 @@ set_query_availability(struct brw_context *brw, struct 
> brw_query_object *query,
>}
>  
>brw_emit_pipe_control_write(brw, flags,
> -  query->bo, 2 * sizeof(uint64_t),
> +  query->bo, 
> gen6_query_predicate_offset(query),
>available);
> }
>  }
> @@ -326,7 +326,7 @@ gen6_begin_query(struct gl_context *ctx, struct 
> gl_query_object *q)
>  {
> struct brw_context *brw = brw_context(ctx);
> struct brw_query_object *query = (struct brw_query_object *)q;
> -   const int idx = 0;
> +   const int idx = GEN6_QUERY_RESULTS;
>  
> /* Since we're starting a new query, we need to throw away old results. */
> brw_bo_unreference(query->bo);
> @@ -416,7 +416,7 @@ gen6_end_query(struct gl_context *ctx, struct 
> gl_query_object *q)
>  {
> struct brw_context *brw = brw_context(ctx);
> struct brw_query_object *query = (struct brw_query_object *)q;
> -   const int idx = 1;
> +   const int idx = GEN6_QUERY_RESULTS + 1;
>  
> switch (query->Base.Target) {
> case GL_TIME_ELAPSED:
> diff --git a/src/mesa/drivers/dri/i965/hsw_queryobj.c 
> b/src/mesa/drivers/dri/i965/hsw_queryobj.c
> index 24f52a7d752..120733c759a 100644
> --- a/src/mesa/drivers/dri/i965/hsw_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/hsw_queryobj.c
> @@ -191,7 +191,7 @@ load_overflow_data_to_cs_gprs(struct brw_context *brw,
>struct brw_query_object *query,
>int idx)
>  {
> -   int offset = idx * sizeof(uint64_t) * 4;
> +   int offset = gen6_query_results_offset(query, 0) + idx * sizeof(uint64_t) 
> * 4;
>  
> brw_load_register_mem64(brw, HSW_CS_GPR(1), query->bo, offset);
>  
> @@ -283,7 +283,7 @@ hsw_result_to_gpr0(struct gl_context *ctx, struct 
> brw_query_object *query,
>brw_load_register_mem64(brw,
>HSW_CS_GPR(0),
>query->bo,
> -  2 * sizeof(uint64_t));
> +  gen6_query_predicate_offset(query));
>return;
> }
>  
> @@ -300,7 +300,7 @@ hsw_result_to_gpr0(struct 

Re: [Mesa-dev] [PATCH 1/9] i965: Check last known busy status on bo before asking the kernel

2018-10-05 Thread Ian Romanick
On 10/02/2018 11:06 AM, Chris Wilson wrote:
> If we know the bo is idle (that is we have no submitted a command buffer
> referencing this bo since the last query) we can skip asking the kernel.
> Note this may report a false negative if the target is being shared
> between processes (exported via dmabuf or flink). To allow the caller
> control over using the last known flag, the query is split into two.
> 
> v2: Check against external bo before trusting our own tracking.
> 
> Signed-off-by: Chris Wilson 
> Cc: Kenneth Graunke 
> Cc: Matt Turner 
> ---
>  src/mesa/drivers/dri/i965/brw_bufmgr.c | 40 --
>  src/mesa/drivers/dri/i965/brw_bufmgr.h | 11 +--
>  2 files changed, 40 insertions(+), 11 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c 
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index f1675b191c1..d9e8453787c 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -444,18 +444,40 @@ vma_free(struct brw_bufmgr *bufmgr,
> }
>  }
>  
> -int
> +static int
> +__brw_bo_busy(struct brw_bo *bo)
> +{
> +   struct drm_i915_gem_busy busy = { bo->gem_handle };

I think it makes the code more readable to keep ".handle = ..."

> +
> +   if (bo->idle && !bo->external)
> +  return 0;
> +
> +   /* If we hit an error here, it means that bo->gem_handle is invalid.
> +* Treat it as being idle (busy.busy is left as 0) and move along.
> +*/
> +   drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, );
> +
> +   bo->idle = !busy.busy;
> +   return busy.busy;
> +}
> +
> +bool
>  brw_bo_busy(struct brw_bo *bo)
>  {
> -   struct brw_bufmgr *bufmgr = bo->bufmgr;
> -   struct drm_i915_gem_busy busy = { .handle = bo->gem_handle };
> +   return __brw_bo_busy(bo);
> +}
>  
> -   int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, );
> -   if (ret == 0) {
> -  bo->idle = !busy.busy;
> -  return busy.busy;
> -   }
> -   return false;
> +bool
> +brw_bo_map_busy(struct brw_bo *bo, unsigned flags)
> +{
> +   unsigned mask;
> +
> +   if (flags & MAP_WRITE)
> +  mask = ~0u;
> +   else
> +  mask = 0x;

In other places we would do this as

   const unsigned mask = (flags & MAP_WRITE) ? ~0u : 0x;

Unless, of course, a later patch adds more choices.

> +
> +   return __brw_bo_busy(bo) & mask;
>  }
>  
>  int
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h 
> b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> index 32fc7a553c9..e1f46b091ce 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> @@ -323,10 +323,17 @@ int brw_bo_get_tiling(struct brw_bo *bo, uint32_t 
> *tiling_mode,
>  int brw_bo_flink(struct brw_bo *bo, uint32_t *name);
>  
>  /**
> - * Returns 1 if mapping the buffer for write could cause the process
> + * Returns false if mapping the buffer is not in active use by the gpu.
> + * If it returns true, any mapping for for write could cause the process

  for for^W

With those nits fixed, this patch is

Reviewed-by: Ian Romanick 

>   * to block, due to the object being active in the GPU.
>   */
> -int brw_bo_busy(struct brw_bo *bo);
> +bool brw_bo_busy(struct brw_bo *bo);
> +
> +/**
> + * Returns true if mapping the buffer for the set of flags (i.e. MAP_READ or
> + * MAP_WRITE) will cause the process to block.
> + */
> +bool brw_bo_map_busy(struct brw_bo *bo, unsigned flags);
>  
>  /**
>   * Specify the volatility of the buffer.
> 

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


Re: [Mesa-dev] [PATCH 7/9] i965: Pack simple pipelined query objects into the same buffer

2018-10-05 Thread Kenneth Graunke
On Tuesday, October 2, 2018 11:06:23 AM PDT Chris Wilson wrote:
> Reuse the same query object buffer for multiple queries within the same
> batch.
> 
> A task for the future is propagating the GL_NO_MEMORY errors.
> 
> Signed-off-by: Chris Wilson 
> Cc: Kenneth Graunke 
> Cc: Matt Turner 
> ---
>  src/mesa/drivers/dri/i965/brw_context.c   |  3 ++
>  src/mesa/drivers/dri/i965/brw_context.h   | 10 +++--
>  src/mesa/drivers/dri/i965/brw_queryobj.c  | 16 +++
>  src/mesa/drivers/dri/i965/gen6_queryobj.c | 51 ++-
>  4 files changed, 59 insertions(+), 21 deletions(-)

Don't want to do this.  This means that WaitQuery will wait on the whole
group of packed queries instead of just the one the app asked about.

Vulkan has to pack queries by design, and it turns out this was a real
issue there.  See b2c97bc789198427043cd902bc76e194e7e81c7d.  Jason ended
up making it busy-wait to avoid bo_waiting on the entire pool, and it
improved Serious Engine performance by 20%.

We could busy-wait in GL too, for lower latency but more CPU waste,
but I think I prefer separate BOs + bo_wait.


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 v2] glsl: do not attempt assignment if operand type not parsed correctly

2018-10-05 Thread Ian Romanick
Reviewed-by: Ian Romanick 

On 09/25/2018 07:04 AM, Tapani Pälli wrote:
> v2: check types of both operands (Ian)
> 
> Cc: mesa-sta...@lists.freedesktop.org
> Signed-off-by: Tapani Pälli 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108012
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index 93e7c8ec334..1082d6c91cf 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -1685,6 +1685,12 @@ ast_expression::do_hir(exec_list *instructions,
>op[1] = this->subexpressions[1]->hir(instructions, state);
>  
>orig_type = op[0]->type;
> +
> +  /* Break out if operand types were not parsed successfully. */
> +  if ((op[0]->type == glsl_type::error_type ||
> +   op[1]->type == glsl_type::error_type))
> + break;
> +
>type = arithmetic_result_type(op[0], op[1],
>  (this->oper == ast_mul_assign),
>  state, & loc);
> 

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


Re: [Mesa-dev] [PATCH 2/2] glx: be explicit about when mapping X <> GLX visuals

2018-10-05 Thread Ian Romanick
Series is

Reviewed-by: Ian Romanick 

On 10/03/2018 06:12 AM, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Write down both X and GLX visual types when mapping from one to the
> other. Makes grepping through the code a tiny bit easier.
> 
> Signed-off-by: Emil Velikov 
> ---
>  src/glx/glxext.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/glx/glxext.c b/src/glx/glxext.c
> index ca3bf9d027f..cef81920356 100644
> --- a/src/glx/glxext.c
> +++ b/src/glx/glxext.c
> @@ -343,9 +343,12 @@ static GLint
>  convert_from_x_visual_type(int visualType)
>  {
> static const int glx_visual_types[] = {
> -  GLX_STATIC_GRAY, GLX_GRAY_SCALE,
> -  GLX_STATIC_COLOR, GLX_PSEUDO_COLOR,
> -  GLX_TRUE_COLOR, GLX_DIRECT_COLOR
> +  [StaticGray]  = GLX_STATIC_GRAY,
> +  [GrayScale]   = GLX_GRAY_SCALE,
> +  [StaticColor] = GLX_STATIC_COLOR,
> +  [PseudoColor] = GLX_PSEUDO_COLOR,
> +  [TrueColor]   = GLX_TRUE_COLOR,
> +  [DirectColor] = GLX_DIRECT_COLOR,
> };
>  
> if (visualType < ARRAY_SIZE(glx_visual_types))
> 

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


Re: [Mesa-dev] [PATCH v2 6/6] radeonsi:optimizing SET_CONTEXT_REG for shaders vgt_vertex_reuse

2018-10-05 Thread Marek Olšák
Thanks. I pushed the series.

Please see also this commit:
https://cgit.freedesktop.org/mesa/mesa/commit/?id=86f004bdfcc1c14ace99f5fccf540c0d813d2254

I added it because the logic was broken for 64-bit reg_saved.

Marek
On Wed, Oct 3, 2018 at 11:55 AM Sonny Jiang  wrote:
>
> Signed-off-by: Sonny Jiang 
> ---
>  src/gallium/drivers/radeonsi/si_gfx_cs.c|  1 +
>  src/gallium/drivers/radeonsi/si_shader.h|  1 +
>  src/gallium/drivers/radeonsi/si_state.h |  1 +
>  src/gallium/drivers/radeonsi/si_state_shaders.c | 17 +++--
>  4 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_gfx_cs.c 
> b/src/gallium/drivers/radeonsi/si_gfx_cs.c
> index 532a6365bf..3ddd7864d1 100644
> --- a/src/gallium/drivers/radeonsi/si_gfx_cs.c
> +++ b/src/gallium/drivers/radeonsi/si_gfx_cs.c
> @@ -377,6 +377,7 @@ void si_begin_new_gfx_cs(struct si_context *ctx)
> ctx->tracked_regs.reg_value[SI_TRACKED_SPI_SHADER_COL_FORMAT] 
>  = 0x;
> ctx->tracked_regs.reg_value[SI_TRACKED_CB_SHADER_MASK]  = 
> 0x;
> ctx->tracked_regs.reg_value[SI_TRACKED_VGT_TF_PARAM]  = 
> 0x;
> +   
> ctx->tracked_regs.reg_value[SI_TRACKED_VGT_VERTEX_REUSE_BLOCK_CNTL]  = 
> 0x001e; /* From VI */
>
> /* Set all saved registers state to saved. */
> ctx->tracked_regs.reg_saved = 0x;
> diff --git a/src/gallium/drivers/radeonsi/si_shader.h 
> b/src/gallium/drivers/radeonsi/si_shader.h
> index 49b1ccd582..09dd558d78 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.h
> +++ b/src/gallium/drivers/radeonsi/si_shader.h
> @@ -689,6 +689,7 @@ struct si_shader {
>
> /*For save precompute registers value */
> unsigned vgt_tf_param; /* VGT_TF_PARAM */
> +   unsigned vgt_vertex_reuse_block_cntl; /* VGT_VERTEX_REUSE_BLOCK_CNTL 
> */
>  };
>
>  struct si_shader_part {
> diff --git a/src/gallium/drivers/radeonsi/si_state.h 
> b/src/gallium/drivers/radeonsi/si_state.h
> index 54b03e0992..fffc63680d 100644
> --- a/src/gallium/drivers/radeonsi/si_state.h
> +++ b/src/gallium/drivers/radeonsi/si_state.h
> @@ -313,6 +313,7 @@ enum si_tracked_reg {
>
> SI_TRACKED_CB_SHADER_MASK,
> SI_TRACKED_VGT_TF_PARAM,
> +   SI_TRACKED_VGT_VERTEX_REUSE_BLOCK_CNTL,
>
> SI_NUM_TRACKED_REGS,
>  };
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
> b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index 217ef4471c..50ce50cfb0 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -440,8 +440,8 @@ static void polaris_set_vgt_vertex_reuse(struct si_screen 
> *sscreen,
> PIPE_TESS_SPACING_FRACTIONAL_ODD)
> vtx_reuse_depth = 14;
>
> -   si_pm4_set_reg(pm4, R_028C58_VGT_VERTEX_REUSE_BLOCK_CNTL,
> -  vtx_reuse_depth);
> +   assert(pm4->shader);
> +   pm4->shader->vgt_vertex_reuse_block_cntl = vtx_reuse_depth;
> }
>  }
>
> @@ -574,6 +574,10 @@ static void si_emit_shader_es(struct si_context *sctx)
>SI_TRACKED_VGT_TF_PARAM,
>shader->vgt_tf_param);
>
> +   if (shader->vgt_vertex_reuse_block_cntl)
> +   radeon_opt_set_context_reg(sctx, 
> R_028C58_VGT_VERTEX_REUSE_BLOCK_CNTL,
> +  
> SI_TRACKED_VGT_VERTEX_REUSE_BLOCK_CNTL,
> +  
> shader->vgt_vertex_reuse_block_cntl);
>  }
>
>  static void si_shader_es(struct si_screen *sscreen, struct si_shader *shader)
> @@ -813,6 +817,10 @@ static void si_emit_shader_gs(struct si_context *sctx)
> radeon_opt_set_context_reg(sctx, 
> R_028B6C_VGT_TF_PARAM,
>SI_TRACKED_VGT_TF_PARAM,
>shader->vgt_tf_param);
> +   if (shader->vgt_vertex_reuse_block_cntl)
> +   radeon_opt_set_context_reg(sctx, 
> R_028C58_VGT_VERTEX_REUSE_BLOCK_CNTL,
> +  
> SI_TRACKED_VGT_VERTEX_REUSE_BLOCK_CNTL,
> +  
> shader->vgt_vertex_reuse_block_cntl);
> }
>  }
>
> @@ -981,6 +989,11 @@ static void si_emit_shader_vs(struct si_context *sctx)
> radeon_opt_set_context_reg(sctx, R_028B6C_VGT_TF_PARAM,
>SI_TRACKED_VGT_TF_PARAM,
>shader->vgt_tf_param);
> +
> +   if (shader->vgt_vertex_reuse_block_cntl)
> +   radeon_opt_set_context_reg(sctx, 
> R_028C58_VGT_VERTEX_REUSE_BLOCK_CNTL,
> +  
> SI_TRACKED_VGT_VERTEX_REUSE_BLOCK_CNTL,
> +   

[Mesa-dev] [PATCH:mesa] util: Make xmlconfig.c build on Solaris without d_type in dirent (v2)

2018-10-05 Thread Alan Coopersmith
v2: check for lstat() failing

Fixes: 04bdbbcab3c "xmlconfig: read more config files from drirc.d/"
Signed-off-by: Alan Coopersmith 
Reviewed-by: Roland Mainz 
Reviewed-by: Ian Romanick 
---
 src/util/xmlconfig.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/util/xmlconfig.c b/src/util/xmlconfig.c
index 5264f2598b..5907c68012 100644
--- a/src/util/xmlconfig.c
+++ b/src/util/xmlconfig.c
@@ -938,8 +938,16 @@ parseOneConfigFile(struct OptConfData *data, const char 
*filename)
 static int
 scandir_filter(const struct dirent *ent)
 {
+#ifndef DT_REG /* systems without d_type in dirent results */
+struct stat st;
+
+if ((lstat(ent->d_name, ) != 0) ||
+(!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode)))
+   return 0;
+#else
 if (ent->d_type != DT_REG && ent->d_type != DT_LNK)
return 0;
+#endif
 
 if (fnmatch("*.conf", ent->d_name, 0))
return 0;
-- 
2.15.2

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


Re: [Mesa-dev] [PATCH] glsl: prevent qualifiers modification of predeclared variables

2018-10-05 Thread Ian Romanick
On 10/05/2018 03:02 PM, Ian Romanick wrote:
> On 10/04/2018 07:08 AM, asimiklit.w...@gmail.com wrote:
>> From: Andrii Simiklit 
>>
>> GLSL 3.7 (Identifiers):
>> "However, as noted in the specification, there are some cases where
>> previously declared variables can be redeclared to change or add some
>> property, and predeclared "gl_" names are allowed to be redeclared in a
>> shader only for these specific purposes.  More generally, it is an error to
>> redeclare a variable, including those starting "gl_"."
>>
>> This patch should fix piglit tests:
>> 'clip-distance-redeclare-without-inout.frag'
>> 'clip-distance-redeclare-without-inout.vert'
>> and leads to regression in clip-distance-out-values.shader_test
>> but probably a fix should be made in the test.
> 
> I believe clip-distance-out-values.shader_test is incorrect.  The 
> redeclaration of gl_ClipDistance should have "out".  glslang rejects it with:
> 
> ERROR: 0:5: 'redeclaration' : cannot change qualification of gl_ClipDistance
> ERROR: 1 compilation errors.  No code generated.
> 
> I think Mark and Clayton would prefer it if the fix to the test landed first. 
>  That way the test never shows up as "fail" to them.
> 
> I'll send the fix for this with the new test cases that I mention below.
> 
>> As far as I understood following mailing thread:
>> https://lists.freedesktop.org/archives/piglit/2013-October/007935.html
>> looks like we have accepted to remove an ability to change qualifiers
>> but have not done it yet. Unless I missed something)
>>
>> Fixes: 8e6cb9fe51a2 "glsl: Refactor AST-to-HIR code handling variable
>>   redeclarations"
> 
> While this is technically correct, I don't think we want to add a new compile 
> error to the stable branches.  On the outside chance this breaks some 
> application, I'd rather have it sit on master for a bit first.
> 
>> Signed-off-by: Andrii Simiklit 
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 13 +
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index 93e7c8ec33..e26ae6b92a 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -4240,10 +4240,15 @@ get_variable_being_redeclared(ir_variable **var_ptr, 
>> YYLTYPE loc,
>>  */
>> if (earlier->type->is_unsized_array() && var->type->is_array()
>> && (var->type->fields.array == earlier->type->fields.array)) {
>> -  /* FINISHME: This doesn't match the qualifiers on the two
>> -   * FINISHME: declarations.  It's not 100% clear whether this is
>> -   * FINISHME: required or not.
>> -   */
>> +
>> +  if ((strcmp("gl_ClipDistance", var->name) == 0) &&
> 
> Hmm... this fixes the specific case mentioned in the old e-mail thread, but 
> this could occur with any variable.  For example, I don't think this shader 
> should compile, but it does:
> 
> #version 110
> varying float x[];
> uniform float x[3];
> uniform int i;
> 
> void main()
> {
> gl_Position = vec4(x[i]);
> }
> 
> Much to my surprise, glslang does not reject this shader.  It looks like 
> glslang rejects anything that tries to change the storage qualifier on any 
> built-in variable, but it seems to allow such changes on user-defined 
> variables.  I think that's a bug in glslang.
> 
> Let's do this for now.
> 
> 1. Change Mesa to reject any storage qualifier change on a built-in variable. 
>  This will match glslang.  We can do this by bringing the code by the comment 
> "Allow verbatim redeclarations of built-in variables. Not explicitly valid, 
> but some applications do it." up before the big if-then-else tree.  Around 
> line 4235 add something like:
> 
>if (earlier->data.how_declared == ir_var_declared_implicitly) {
>   /* Allow verbatim redeclarations of built-in variables. Not explicitly
>* valid, but some applications do it.
>*/
>   if (earlier->data.mode != var->data.mode &&
>   !(earlier->data.mode == ir_var_system_value &&
> var->data.mode == ir_var_shader_in)) {
>  _mesa_glsl_error(, state,
>   "redeclaration cannot change qualification of `%s'",
>   var->name);

Oops...

  } else if (earlier->type != var->type) {
 _mesa_glsl_error(, state,
  "redeclaration of `%s' has incorrect type",
  var->name);

>   }
>}
> 
> I changed the error message to be more similar to glslang.  I like that 
> wording, but I'm flexible.
> 
> We'll also want to remove the other tests for this (which will be redundant 
> after the above change).
> 
> 2. Add a bunch of piglit tests to make sure we match glslang.  I've got a 
> start on this, and I'll CC you on the patches when I send them.
> 
> 3. I'll submit a GLSL spec bug to make sure cases like my example above are 
> intended to be 

Re: [Mesa-dev] [PATCH] glsl: prevent qualifiers modification of predeclared variables

2018-10-05 Thread Ian Romanick
On 10/04/2018 07:08 AM, asimiklit.w...@gmail.com wrote:
> From: Andrii Simiklit 
> 
> GLSL 3.7 (Identifiers):
> "However, as noted in the specification, there are some cases where
> previously declared variables can be redeclared to change or add some
> property, and predeclared "gl_" names are allowed to be redeclared in a
> shader only for these specific purposes.  More generally, it is an error to
> redeclare a variable, including those starting "gl_"."
> 
> This patch should fix piglit tests:
> 'clip-distance-redeclare-without-inout.frag'
> 'clip-distance-redeclare-without-inout.vert'
> and leads to regression in clip-distance-out-values.shader_test
> but probably a fix should be made in the test.

I believe clip-distance-out-values.shader_test is incorrect.  The redeclaration 
of gl_ClipDistance should have "out".  glslang rejects it with:

ERROR: 0:5: 'redeclaration' : cannot change qualification of gl_ClipDistance
ERROR: 1 compilation errors.  No code generated.

I think Mark and Clayton would prefer it if the fix to the test landed first.  
That way the test never shows up as "fail" to them.

I'll send the fix for this with the new test cases that I mention below.

> As far as I understood following mailing thread:
> https://lists.freedesktop.org/archives/piglit/2013-October/007935.html
> looks like we have accepted to remove an ability to change qualifiers
> but have not done it yet. Unless I missed something)
> 
> Fixes: 8e6cb9fe51a2 "glsl: Refactor AST-to-HIR code handling variable
>redeclarations"

While this is technically correct, I don't think we want to add a new compile 
error to the stable branches.  On the outside chance this breaks some 
application, I'd rather have it sit on master for a bit first.

> Signed-off-by: Andrii Simiklit 
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index 93e7c8ec33..e26ae6b92a 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -4240,10 +4240,15 @@ get_variable_being_redeclared(ir_variable **var_ptr, 
> YYLTYPE loc,
>  */
> if (earlier->type->is_unsized_array() && var->type->is_array()
> && (var->type->fields.array == earlier->type->fields.array)) {
> -  /* FINISHME: This doesn't match the qualifiers on the two
> -   * FINISHME: declarations.  It's not 100% clear whether this is
> -   * FINISHME: required or not.
> -   */
> +
> +  if ((strcmp("gl_ClipDistance", var->name) == 0) &&

Hmm... this fixes the specific case mentioned in the old e-mail thread, but 
this could occur with any variable.  For example, I don't think this shader 
should compile, but it does:

#version 110
varying float x[];
uniform float x[3];
uniform int i;

void main()
{
gl_Position = vec4(x[i]);
}

Much to my surprise, glslang does not reject this shader.  It looks like 
glslang rejects anything that tries to change the storage qualifier on any 
built-in variable, but it seems to allow such changes on user-defined 
variables.  I think that's a bug in glslang.

Let's do this for now.

1. Change Mesa to reject any storage qualifier change on a built-in variable.  
This will match glslang.  We can do this by bringing the code by the comment 
"Allow verbatim redeclarations of built-in variables. Not explicitly valid, but 
some applications do it." up before the big if-then-else tree.  Around line 
4235 add something like:

   if (earlier->data.how_declared == ir_var_declared_implicitly) {
  /* Allow verbatim redeclarations of built-in variables. Not explicitly
   * valid, but some applications do it.
   */
  if (earlier->data.mode != var->data.mode &&
  !(earlier->data.mode == ir_var_system_value &&
var->data.mode == ir_var_shader_in)) {
 _mesa_glsl_error(, state,
  "redeclaration cannot change qualification of `%s'",
  var->name);
  }
   }

I changed the error message to be more similar to glslang.  I like that 
wording, but I'm flexible.

We'll also want to remove the other tests for this (which will be redundant 
after the above change).

2. Add a bunch of piglit tests to make sure we match glslang.  I've got a start 
on this, and I'll CC you on the patches when I send them.

3. I'll submit a GLSL spec bug to make sure cases like my example above are 
intended to be illegal.

4. Depending on the outcome of #3, update Mesa to match.

> +  earlier->data.mode != var->data.mode) {
> + _mesa_glsl_error(, state,
> +  "redeclaration of '%s %s' with incorrect 
> qualifiers '%s'.",
> +  mode_string(earlier),
> +  var->name,
> +  mode_string(var));
> +  }
>  
>const int 

Re: [Mesa-dev] [PATCH v2] gallivm: Make it possible to disable some optimization shortcuts in release builds

2018-10-05 Thread Roland Scheidegger
Looks alright to me. I'm not quite sold on the "safemath" name though, 
since "safe math" is usually associated with floating point 
optimizations, and this here is just filtering hacks. Maybe something 
like disable_filtering_hacks would be more fitting?

Reviewed-by: Roland Scheidegger 

On 10/05/2018 06:08 AM, Gert Wollny wrote:
> From: Gert Wollny 
> 
> For testing it is of interest that all tests of dEQP pass, e.g. to test
> virglrenderer on a host only providing software rendering like in a CI.
> Hence make it possible to disable certain optimizations that make tests fail.
> 
> While we are there also add some documentation to the flags to make it clear
> that this is opt-out.
> 
> Setting the environment variable "GALLIVM_PERF=disable_all" can be used to 
> make
> the following tests pass in release mode:
> 
>dEQP-GLES2.functional.texture.mipmap.2d.affine.*_linear_*
>dEQP-GLES2.functional.texture.mipmap.cube.generate.*
>dEQP-GLES2.functional.texture.vertex.2d.filtering.*_mipmap_linear_*
>dEQP-GLES2.functional.texture.vertex.2d.wrap.*
> 
> Related:
>
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.freedesktop.org%2Fshow_bug.cgi%3Fid%3D94957data=02%7C01%7Csroland%40vmware.com%7Cca786b57a0ab40daeddd08d62ac3e86d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636743418250432350sdata=czSAaylr1oCdY5lsPRxB7EsVb6mPhMU2e1t1ZuhYnYk%3Dreserved=0
> 
> v2: rename optimization disabling flag to 'safemath' and also move the
>  nopt flag to the perf flags.
> 
> Signed-off-by: Gert Wollny 
> ---
>   src/gallium/auxiliary/gallivm/lp_bld_debug.h  | 16 ---
>   src/gallium/auxiliary/gallivm/lp_bld_init.c   | 25 
> +++
>   src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c |  6 +++---
>   src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c   |  6 +++---
>   4 files changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_debug.h 
> b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
> index f96a1afa7a..eeef0d6ba6 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_debug.h
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
> @@ -39,20 +39,22 @@
>   #define GALLIVM_DEBUG_TGSI  (1 << 0)
>   #define GALLIVM_DEBUG_IR(1 << 1)
>   #define GALLIVM_DEBUG_ASM   (1 << 2)
> -#define GALLIVM_DEBUG_NO_OPT(1 << 3)
> -#define GALLIVM_DEBUG_PERF  (1 << 4)
> -#define GALLIVM_DEBUG_NO_BRILINEAR  (1 << 5)
> -#define GALLIVM_DEBUG_NO_RHO_APPROX (1 << 6)
> -#define GALLIVM_DEBUG_NO_QUAD_LOD   (1 << 7)
> -#define GALLIVM_DEBUG_GC(1 << 8)
> -#define GALLIVM_DEBUG_DUMP_BC   (1 << 9)
> +#define GALLIVM_DEBUG_PERF  (1 << 3)
> +#define GALLIVM_DEBUG_GC(1 << 4)
> +#define GALLIVM_DEBUG_DUMP_BC   (1 << 5)
>   
> +#define GALLIVM_PERF_NO_BRILINEAR  (1 << 0)
> +#define GALLIVM_PERF_NO_RHO_APPROX (1 << 1)
> +#define GALLIVM_PERF_NO_QUAD_LOD   (1 << 2)
> +#define GALLIVM_PERF_NO_OPT(1 << 3)
>   
>   #ifdef __cplusplus
>   extern "C" {
>   #endif
>   
>   
> +extern unsigned gallivm_perf;
> +
>   #ifdef DEBUG
>   extern unsigned gallivm_debug;
>   #else
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_init.c 
> b/src/gallium/auxiliary/gallivm/lp_bld_init.c
> index 1f0a01cde6..3f7c4d3154 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_init.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_init.c
> @@ -59,6 +59,17 @@ static const bool use_mcjit = USE_MCJIT;
>   static bool use_mcjit = FALSE;
>   #endif
>   
> +unsigned gallivm_perf = 0;
> +
> +static const struct debug_named_value lp_bld_perf_flags[] = {
> +   { "no_brilinear", GALLIVM_PERF_NO_BRILINEAR, "disable brilinear 
> optimization" },
> +   { "no_rho_approx", GALLIVM_PERF_NO_RHO_APPROX, "disable rho_approx 
> optimization" },
> +   { "no_quad_lod", GALLIVM_PERF_NO_QUAD_LOD, "disable quad_lod 
> optimization" },
> +   { "nopt",   GALLIVM_PERF_NO_OPT, "disable optimization passes to speed up 
> shader compilation" },
> +   { "safemath", GALLIVM_PERF_NO_BRILINEAR | GALLIVM_PERF_NO_RHO_APPROX |
> + GALLIVM_PERF_NO_QUAD_LOD, "disable unsafe optimizations" },
> +   DEBUG_NAMED_VALUE_END
> +};
>   
>   #ifdef DEBUG
>   unsigned gallivm_debug = 0;
> @@ -67,11 +78,7 @@ static const struct debug_named_value lp_bld_debug_flags[] 
> = {
>  { "tgsi",   GALLIVM_DEBUG_TGSI, NULL },
>  { "ir", GALLIVM_DEBUG_IR, NULL },
>  { "asm",GALLIVM_DEBUG_ASM, NULL },
> -   { "nopt",   GALLIVM_DEBUG_NO_OPT, NULL },
>  { "perf",   GALLIVM_DEBUG_PERF, NULL },
> -   { "no_brilinear", GALLIVM_DEBUG_NO_BRILINEAR, NULL },
> -   { "no_rho_approx", GALLIVM_DEBUG_NO_RHO_APPROX, NULL },
> -   { "no_quad_lod", GALLIVM_DEBUG_NO_QUAD_LOD, NULL },
>  { "gc", GALLIVM_DEBUG_GC, NULL },
>  { "dumpbc", GALLIVM_DEBUG_DUMP_BC, NULL },
>  DEBUG_NAMED_VALUE_END
> @@ -136,7 +143,7 @@ create_pass_manager(struct gallivm_state *gallivm)
> free(td_str);
>  }
>   
> -   if ((gallivm_debug 

Re: [Mesa-dev] [PATCH 2/2] gbm: Add GBM_FORMAT_ARGB1555 support

2018-10-05 Thread Marek Olšák
Reviewed-by: Marek Olšák 

Marek
On Fri, Oct 5, 2018 at 6:23 AM Michel Dänzer  wrote:
>
> From: Michel Dänzer 
>
> Signed-off-by: Michel Dänzer 
> ---
>  src/gbm/backends/dri/gbm_dri.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index b3d6ceb15a3..35ec3a1c3a2 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -552,6 +552,10 @@ static const struct gbm_dri_visual 
> gbm_dri_visuals_table[] = {
>   GBM_FORMAT_GR88, __DRI_IMAGE_FORMAT_GR88,
>   { 0x00ff, 0xff00, 0x, 0x },
> },
> +   {
> + GBM_FORMAT_ARGB1555, __DRI_IMAGE_FORMAT_ARGB1555,
> + { 0x7c00, 0x03e0, 0x001f, 0x8000 },
> +   },
> {
>   GBM_FORMAT_RGB565, __DRI_IMAGE_FORMAT_RGB565,
>   { 0xf800, 0x07e0, 0x001f, 0x },
> --
> 2.19.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 1/2] st/dri: Handle BGRA5551 format

2018-10-05 Thread Marek Olšák
Reviewed-by: Marek Olšák 

Marek
On Fri, Oct 5, 2018 at 6:22 AM Michel Dänzer  wrote:
>
> From: Michel Dänzer 
>
> Signed-off-by: Michel Dänzer 
> ---
>  src/gallium/state_trackers/dri/dri2.c | 13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c 
> b/src/gallium/state_trackers/dri/dri2.c
> index b17c5e16ede..4efc4334b65 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -101,6 +101,10 @@ static int convert_fourcc(int format, int 
> *dri_components_p)
>  {
> int dri_components;
> switch(format) {
> +   case __DRI_IMAGE_FOURCC_ARGB1555:
> +  format = __DRI_IMAGE_FORMAT_ARGB1555;
> +  dri_components = __DRI_IMAGE_COMPONENTS_RGBA;
> +  break;
> case __DRI_IMAGE_FOURCC_RGB565:
>format = __DRI_IMAGE_FORMAT_RGB565;
>dri_components = __DRI_IMAGE_COMPONENTS_RGB;
> @@ -187,6 +191,9 @@ static int convert_fourcc(int format, int 
> *dri_components_p)
>  static int convert_to_fourcc(int format)
>  {
> switch(format) {
> +   case __DRI_IMAGE_FORMAT_ARGB1555:
> +  format = __DRI_IMAGE_FOURCC_ARGB1555;
> +  break;
> case __DRI_IMAGE_FORMAT_RGB565:
>format = __DRI_IMAGE_FOURCC_RGB565;
>break;
> @@ -231,6 +238,9 @@ static enum pipe_format dri2_format_to_pipe_format (int 
> format)
> enum pipe_format pf;
>
> switch (format) {
> +   case __DRI_IMAGE_FORMAT_ARGB1555:
> +  pf = PIPE_FORMAT_B5G5R5A1_UNORM;
> +  break;
> case __DRI_IMAGE_FORMAT_RGB565:
>pf = PIPE_FORMAT_B5G6R5_UNORM;
>break;
> @@ -523,6 +533,9 @@ dri_image_drawable_get_buffers(struct dri_drawable 
> *drawable,
>}
>
>switch (pf) {
> +  case PIPE_FORMAT_B5G5R5A1_UNORM:
> + image_format = __DRI_IMAGE_FORMAT_ARGB1555;
> + break;
>case PIPE_FORMAT_B5G6R5_UNORM:
>   image_format = __DRI_IMAGE_FORMAT_RGB565;
>   break;
> --
> 2.19.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] st/glsl_to_tgsi: initialise need_uarl in contructor

2018-10-05 Thread Marek Olšák
Reviewed-by: Marek Olšák 

Marek
On Thu, Oct 4, 2018 at 8:55 PM Dave Airlie  wrote:
>
> From: Dave Airlie 
>
> Found by coverity
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 7ada3476961..dea91c7a189 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -4656,6 +4656,7 @@ glsl_to_tgsi_visitor::glsl_to_tgsi_visitor()
> ctx = NULL;
> prog = NULL;
> precise = 0;
> +   need_uarl = false;
> shader_program = NULL;
> shader = NULL;
> options = NULL;
> --
> 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


[Mesa-dev] [PATCH] intel: Introducing Whiskey Lake platform

2018-10-05 Thread Rodrigo Vivi
Whiskey Lake uses the same gen graphics as Coffe Lake, including some
ids that were previously marked as reserved on Coffe Lake, but that
now are moved to WHL page.

This follows the ids and approach used on kernel's commit
b9be78531d27 ("drm/i915/whl: Introducing Whiskey Lake platform")
and commit c1c8f6fa731b ("drm/i915: Redefine some Whiskey Lake SKUs")

v2: Lionel noticed that GT{1,2,3} on kernel wasn't following
spec when looking to number of EUs, so kernel has been updated.

Cc: Lionel Landwerlin 
Cc: José Roberto de Souza 
Cc: Anuj Phogat 
Signed-off-by: Rodrigo Vivi 
---
 include/pci_ids/i965_pci_ids.h  | 10 +-
 src/intel/compiler/test_eu_validate.cpp |  1 +
 src/intel/dev/gen_device_info.c |  1 +
 src/intel/tools/aubinator.c |  2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_ids.h
index 4efac638e9..cb33bea7d4 100644
--- a/include/pci_ids/i965_pci_ids.h
+++ b/include/pci_ids/i965_pci_ids.h
@@ -170,8 +170,6 @@ CHIPSET(0x3185, glk_2x6, "Intel(R) UHD Graphics 600 
(Geminilake 2x6)")
 CHIPSET(0x3E90, cfl_gt1, "Intel(R) UHD Graphics 610 (Coffeelake 2x6 GT1)")
 CHIPSET(0x3E93, cfl_gt1, "Intel(R) UHD Graphics 610 (Coffeelake 2x6 GT1)")
 CHIPSET(0x3E99, cfl_gt1, "Intel(R) HD Graphics (Coffeelake 2x6 GT1)")
-CHIPSET(0x3EA1, cfl_gt1, "Intel(R) HD Graphics (Coffeelake 2x6 GT1)")
-CHIPSET(0x3EA4, cfl_gt1, "Intel(R) HD Graphics (Coffeelake 2x6 GT1)")
 CHIPSET(0x3E91, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
 CHIPSET(0x3E92, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
 CHIPSET(0x3E96, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
@@ -179,14 +177,16 @@ CHIPSET(0x3E98, cfl_gt2, "Intel(R) HD Graphics 
(Coffeelake 3x8 GT2)")
 CHIPSET(0x3E9A, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
 CHIPSET(0x3E9B, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
 CHIPSET(0x3E94, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
-CHIPSET(0x3EA0, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
-CHIPSET(0x3EA3, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
 CHIPSET(0x3EA9, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
-CHIPSET(0x3EA2, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
 CHIPSET(0x3EA5, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
 CHIPSET(0x3EA6, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
 CHIPSET(0x3EA7, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
 CHIPSET(0x3EA8, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
+CHIPSET(0x3EA1, cfl_gt1, "Intel(R) HD Graphics (Whiskey Lake 2x6 GT1)")
+CHIPSET(0x3EA4, cfl_gt1, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT1)")
+CHIPSET(0x3EA0, cfl_gt2, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT2)")
+CHIPSET(0x3EA3, cfl_gt2, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT2)")
+CHIPSET(0x3EA2, cfl_gt3, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT3)")
 CHIPSET(0x5A49, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
 CHIPSET(0x5A4A, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
 CHIPSET(0x5A41, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
diff --git a/src/intel/compiler/test_eu_validate.cpp 
b/src/intel/compiler/test_eu_validate.cpp
index 744ae5806d..73300b2312 100644
--- a/src/intel/compiler/test_eu_validate.cpp
+++ b/src/intel/compiler/test_eu_validate.cpp
@@ -43,6 +43,7 @@ static const struct gen_info {
{ "aml", },
{ "glk", },
{ "cfl", },
+   { "whl", },
{ "cnl", },
{ "icl", },
 };
diff --git a/src/intel/dev/gen_device_info.c b/src/intel/dev/gen_device_info.c
index e2c6cbc710..5dbd060757 100644
--- a/src/intel/dev/gen_device_info.c
+++ b/src/intel/dev/gen_device_info.c
@@ -60,6 +60,7 @@ gen_device_name_to_pci_device_id(const char *name)
   { "aml", 0x591C },
   { "glk", 0x3185 },
   { "cfl", 0x3E9B },
+  { "whl", 0x3EA1 },
   { "cnl", 0x5a52 },
   { "icl", 0x8a52 },
};
diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index ef0f7650b1..1458875a31 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -300,7 +300,7 @@ int main(int argc, char *argv[])
  if (id < 0) {
 fprintf(stderr, "can't parse gen: '%s', expected brw, g4x, ilk, "
 "snb, ivb, hsw, byt, bdw, chv, skl, bxt, kbl, "
-"aml, glk, cfl, cnl, icl", optarg);
+"aml, glk, cfl, whl, cnl, icl", optarg);
 exit(EXIT_FAILURE);
  } else {
 pci_id = id;
-- 
2.17.1

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


[Mesa-dev] [PATCH v3 4/5] radeonsi: Enable adapative sync by default for radeon

2018-10-05 Thread Nicholas Kazlauskas
It's better to let most applications make use of adaptive sync
by default. Problematic applications can be placed on the blacklist
or the user can manually disable the feature.

Signed-off-by: Nicholas Kazlauskas 
---
 src/gallium/drivers/radeonsi/driinfo_radeonsi.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h 
b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
index 8c5078c13f..edf5eea874 100644
--- a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
+++ b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
@@ -1,4 +1,8 @@
 // DriConf options specific to radeonsi
+DRI_CONF_SECTION_QUALITY
+DRI_CONF_ADAPTIVE_SYNC_ENABLE("true")
+DRI_CONF_SECTION_END
+
 DRI_CONF_SECTION_PERFORMANCE
 DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
 DRI_CONF_RADEONSI_ASSUME_NO_Z_FIGHTS("false")
-- 
2.19.0

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


[Mesa-dev] [PATCH v3 5/5] loader/dri3: Enable adaptive sync via _VARIABLE_REFRESH property

2018-10-05 Thread Nicholas Kazlauskas
The DDX driver can be notified of adaptive sync suitability by
flagging the application's window with the _VARIABLE_REFRESH property.

This property is set on the first swap the application performs
when adaptive_sync_enable is set to true in the drirc.

It's performed here instead of when the loader is initialized for
two reasons:

(1) The window's drawable can be missing during loader init.
This can be observed during the Unigine Superposition benchmark.

(2) Adaptive sync will only be enabled closer to when the application
actually begins rendering. For some displays this means there is
less time spent watching flickering occur during application load.

For now this property is only set on the glx DRI3 backend. This should
cover most common applications and games on modern hardware.

Vulkan support can be implemented in a similar manner but would likely
require splitting the function out into a common helper function.

Signed-off-by: Nicholas Kazlauskas 
---
 src/loader/loader_dri3_helper.c | 46 -
 src/loader/loader_dri3_helper.h |  2 ++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index f641a34e6d..a485e241ae 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -101,6 +101,34 @@ get_xcb_visualtype_for_depth(struct loader_dri3_drawable 
*draw, int depth)
return NULL;
 }
 
+/* Sets the adaptive sync window property state. */
+static void
+set_adaptive_sync_property(xcb_connection_t *conn,
+   xcb_window_t window,
+   uint32_t state)
+{
+   static char const name[] = "_VARIABLE_REFRESH";
+   xcb_intern_atom_cookie_t cookie;
+   xcb_intern_atom_reply_t* reply;
+
+   cookie = xcb_intern_atom(conn, 0, sizeof(name), name);
+   reply = xcb_intern_atom_reply(conn, cookie, NULL);
+   if (reply == NULL)
+  return;
+
+   xcb_change_property(conn,
+   XCB_PROP_MODE_REPLACE,
+   window,
+   reply->atom,
+   XCB_ATOM_CARDINAL,
+   32,
+   1,
+   );
+
+   xcb_flush(conn);
+   free(reply);
+}
+
 /* Get red channel mask for given drawable at given depth. */
 static unsigned int
 dri3_get_red_mask_for_depth(struct loader_dri3_drawable *draw, int depth)
@@ -318,6 +346,7 @@ loader_dri3_drawable_init(xcb_connection_t *conn,
xcb_get_geometry_reply_t *reply;
xcb_generic_error_t *error;
GLint vblank_mode = DRI_CONF_VBLANK_DEF_INTERVAL_1;
+   unsigned char adaptive_sync_enable;
int swap_interval;
 
draw->conn = conn;
@@ -331,16 +360,25 @@ loader_dri3_drawable_init(xcb_connection_t *conn,
draw->have_back = 0;
draw->have_fake_front = 0;
draw->first_init = true;
+   draw->adaptive_sync_enable = false;
+   draw->adaptive_sync_active = false;
 
draw->cur_blit_source = -1;
draw->back_format = __DRI_IMAGE_FORMAT_NONE;
mtx_init(>mtx, mtx_plain);
cnd_init(>event_cnd);
 
-   if (draw->ext->config)
+   if (draw->ext->config) {
   draw->ext->config->configQueryi(draw->dri_screen,
   "vblank_mode", _mode);
 
+  draw->ext->config->configQueryb(draw->dri_screen,
+  "adaptive_sync_enable",
+  _sync_enable);
+
+  draw->adaptive_sync_enable = adaptive_sync_enable;
+   }
+
switch (vblank_mode) {
case DRI_CONF_VBLANK_NEVER:
case DRI_CONF_VBLANK_DEF_INTERVAL_0:
@@ -879,6 +917,12 @@ loader_dri3_swap_buffers_msc(struct loader_dri3_drawable 
*draw,
back = dri3_find_back_alloc(draw);
 
mtx_lock(>mtx);
+
+   if (draw->adaptive_sync_enable && !draw->adaptive_sync_active) {
+  set_adaptive_sync_property(draw->conn, (xcb_window_t)draw->drawable, 
true);
+  draw->adaptive_sync_active = true;
+   }
+
if (draw->is_different_gpu && back) {
   /* Update the linear buffer before presenting the pixmap */
   (void) loader_dri3_blit_image(draw,
diff --git a/src/loader/loader_dri3_helper.h b/src/loader/loader_dri3_helper.h
index 0d18181312..86f994cb2a 100644
--- a/src/loader/loader_dri3_helper.h
+++ b/src/loader/loader_dri3_helper.h
@@ -156,6 +156,8 @@ struct loader_dri3_drawable {
xcb_special_event_t *special_event;
 
bool first_init;
+   bool adaptive_sync_enable;
+   bool adaptive_sync_active;
int swap_interval;
 
struct loader_dri3_extensions *ext;
-- 
2.19.0

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


[Mesa-dev] [PATCH v3 2/5] util: Get program name based on path when possible

2018-10-05 Thread Nicholas Kazlauskas
Some programs start with the path and command line arguments in
argv[0] (program_invocation_name). Chromium is an example of
an application using mesa that does this.

This tries to query the real path for the symbolic link /proc/self/exe
to find the program name instead. It only uses the realpath if it
was a prefix of the invocation to avoid breaking wine programs.

Signed-off-by: Nicholas Kazlauskas 
---
 src/util/u_process.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/util/u_process.c b/src/util/u_process.c
index 5e5927678d..cd16521ab3 100644
--- a/src/util/u_process.c
+++ b/src/util/u_process.c
@@ -41,8 +41,24 @@ static const char *
 __getProgramName()
 {
char * arg = strrchr(program_invocation_name, '/');
-   if (arg)
+   if (arg) {
+  /* If the / character was found this is likely a linux path or
+   * an invocation path for a 64-bit wine program.
+   *
+   * However, some programs pass command line arguments into argv[0].
+   * Strip these arguments out by using the realpath only if it was
+   * a prefix of the invocation name.
+   */
+  static char *path;
+
+  if (!path)
+ path = realpath("/proc/self/exe", NULL);
+
+  if (path && strncmp(path, program_invocation_name, strlen(path)) == 0)
+ return path + (arg - program_invocation_name + 1);
+
   return arg+1;
+   }
 
/* If there was no '/' at all we likely have a windows like path from
 * a wine application.
-- 
2.19.0

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


[Mesa-dev] [PATCH v3 3/5] drirc: Initial blacklist for adaptive sync

2018-10-05 Thread Nicholas Kazlauskas
Applications that don't present at a predictable rate (ie. not games)
shouldn't have adapative sync enabled. This list covers some of the
common desktop compositors, web browsers and video players.

Signed-off-by: Nicholas Kazlauskas 
---
 src/util/00-mesa-defaults.conf | 79 ++
 1 file changed, 79 insertions(+)

diff --git a/src/util/00-mesa-defaults.conf b/src/util/00-mesa-defaults.conf
index a937c46d05..b5e07d70c8 100644
--- a/src/util/00-mesa-defaults.conf
+++ b/src/util/00-mesa-defaults.conf
@@ -21,6 +21,8 @@ Application bugs worked around in this file:
   built-ins (specifically gl_VertexID), which causes the vertex shaders to fail
   to compile.
 
+* Applications that are not suitable for adapative sync are blacklisted here.
+
 TODO: document the other workarounds.
 
 -->
@@ -314,6 +316,83 @@ TODO: document the other workarounds.
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
-- 
2.19.0

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


[Mesa-dev] [PATCH v3 0/5] Mesa integration for DRM variable refresh rate API

2018-10-05 Thread Nicholas Kazlauskas
These patches are part of a proposed new interface for supporting variable 
refresh rate via DRM properties.

It adds a new option for supporting adaptive sync to drirc along with the 
implementation of notifying the window manager/DDX of the support via a window 
property.

The option is enabled by default for radeonsi so included is an initial 
blacklist of applications that shouldn't have this option enabled.

In order to catch some of these applications a patch to the program name 
detection was needed to ignore arguments for some Linux applications.

=== Changes from v2 ===

* A patch to add the option to the drirc was missing from the v2 patchset. That 
patch is now included in v3.
* The method to fix program name detection for Chromium based applications for 
the drirc was modified to not break detection for 32-bit/64-bit WINE 
applications.

=== Adaptive sync and variable refresh rate ===

Adaptive sync is part of the DisplayPort specification and allows for graphics 
adapters to drive displays with varying frame timings.

Variable refresh rate (VRR) is essentially the same, but defined for HDMI.

=== Use cases for variable refresh rate ===

Variable frame (flip) timings don't align well with fixed refresh rate 
displays. This results in stuttering, tearing and/or input lag. By adjusting 
the display refresh rate dynamically these issues can be reduced or eliminated.

However, not all content is suitable for dynamic refresh adaptation. The user 
may experience "flickering" from differences in panel luminance at different 
refresh rates. Content that flips at an infrequent rate or demand is more 
likely to cause large changes in refresh rate that result in flickering.

Userland needs a way to let the driver know when the screen content is suitable 
for variable refresh rates.

=== DRM API to support variable refresh rates ===

This patch introduces a new API via atomic properties on the DRM connector and 
CRTC.

The drm_connector has one new optional boolean property:

* bool vrr_capable - set by the driver if the hardware is capable of supporting 
variable refresh rates

The drm_crtc has one new default boolean property:

* bool vrr_enabled - set by userspace indicating that variable refresh rate 
should be enabled

== Overview for DRM driver developers ===

Driver developers can attach the "vrr_capable" property by calling 
drm_connector_attach_vrr_capable_property(). This should be done on connectors 
that could be capable of supporting variable refresh rates (such as DP or HDMI).

The "vrr_capable" can then be updated accordingly by calling 
drm_connector_set_vrr_capable_property().

The "vrr_enabled" property can be checked from the drm_crtc_state object.

=== Overview for Userland developers ==

The "vrr_enabled" property on the CRTC should be set to true when the CRTC is 
suitable for variable refresh rates.

To demonstrate the suitability of the API for variable refresh and dynamic 
adaptation there are additional patches using this API that implement adaptive 
variable refresh across kernel and userland projects:

* DRM (dri-devel)
* amdgpu DRM kernel driver (amd-gfx)
* xf86-video-amdgpu 
(https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu)
* mesa (mesa-dev)

These patches enable adaptive variable refresh on X for AMD hardware. Support 
for variable refresh is disabled by default in xf86-video-amdgpu and will 
require additional user configuration.

The patches have been tested as working on upstream userland with the GNOME 
desktop environment under a single monitor setup. They also work on KDE in a 
single monitor setup with the compositor disabled.

The patches require that suitable applications flip via the Present extension 
to xf86-video-amdgpu for the entire Screen. Some compositors may interfere with 
this if they are unable to unredirect the window.

Full implementation details for these changes can be reviewed in their 
respective mailing lists and the xf86-video-amdgpu GitLab.

=== Previous discussions ===

These patches are based upon feedback from previous threads on the subject. 
These are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/190910.html
https://lists.freedesktop.org/archives/mesa-dev/2018-September/205001.html
https://lists.freedesktop.org/archives/mesa-dev/2018-September/205883.html

Nicholas Kazlauskas (5):
  util: Add adaptive_sync_enable driconf option
  util: Get program name based on path when possible
  drirc: Initial blacklist for adaptive sync
  radeonsi: Enable adapative sync by default for radeon
  loader/dri3: Enable adaptive sync via _VARIABLE_REFRESH property

 .../drivers/radeonsi/driinfo_radeonsi.h   |  4 +
 src/loader/loader_dri3_helper.c   | 46 ++-
 

[Mesa-dev] [PATCH v3 1/5] util: Add adaptive_sync_enable driconf option

2018-10-05 Thread Nicholas Kazlauskas
This option lets the user decide whether mesa should notify the
window manager / DDX driver that the current application is adaptive
sync capable.

It's off by default.

Signed-off-by: Nicholas Kazlauskas 
---
 src/util/xmlpool/t_options.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/util/xmlpool/t_options.h b/src/util/xmlpool/t_options.h
index e0a30f5fd1..789f6cc56d 100644
--- a/src/util/xmlpool/t_options.h
+++ b/src/util/xmlpool/t_options.h
@@ -210,6 +210,11 @@ DRI_CONF_OPT_BEGIN_V(vblank_mode,enum,def,"0:3") \
 DRI_CONF_DESC_END \
 DRI_CONF_OPT_END
 
+#define DRI_CONF_ADAPTIVE_SYNC_ENABLE(def) \
+DRI_CONF_OPT_BEGIN_B(adaptive_sync_enable,def) \
+DRI_CONF_DESC(en,gettext("Adapt the monitor sync to the application 
performance (when possible)")) \
+DRI_CONF_OPT_END
+
 #define DRI_CONF_MESA_GLTHREAD(def) \
 DRI_CONF_OPT_BEGIN_B(mesa_glthread, def) \
 DRI_CONF_DESC(en,gettext("Enable offloading GL driver work to a 
separate thread")) \
-- 
2.19.0

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


Re: [Mesa-dev] [PATCH:mesa] util: Make xmlconfig.c build on Solaris without d_type in dirent

2018-10-05 Thread Alan Coopersmith

On 10/ 5/18 10:04 AM, Ian Romanick wrote:

On 10/03/2018 08:21 PM, Alan Coopersmith wrote:

On 10/ 3/18 03:15 PM, Roland Mainz wrote:

On Wed, Oct 3, 2018 at 11:51 PM Alan Coopersmith
 wrote:


Introduced-by: commit 04bdbbcab3c4862bf3f54ce60fcc1d2007776f80


Is this formatting something you need for your internal systems?  The
format we've been using to signal that a change is a fix to a bug
introduced by a specific commit is:

Fixes: 04bdbbcab3c "xmlconfig: read more config files from drirc.d/"

That ensures the fix gets bundled in the correct stable release branches.


No, this is me being disconnected enough from Mesa development that I
didn't know the right way to do it, and overlooked the note about this
on https://mesa3d.org/submittingpatches.html - I'll fix that.

Thanks,

-alan-

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


Re: [Mesa-dev] [PATCH 0/9] i965: Re-implement the gen9 void-extent ASTC WA with BLORP

2018-10-05 Thread Nanley Chery
On Tue, Oct 02, 2018 at 04:59:17PM -0700, Nanley Chery wrote:
> On Wed, Sep 26, 2018 at 04:31:02PM -0700, Nanley Chery wrote:
> > The current workaround has two issues. It causes significant slow-downs [1] 
> > in
> > application startup times and uses the modified ASTC blocks for non-sampling
> > operations. This can result in incorrect texture downloads.
> > 
> > This series addresses the latter issue by keeping two copies of an ASTC
> > miptree: one that's been modified for the sampler bug (the shadow) and 
> > another
> > that hasn't (the main). The main copy is used for pixel transfer operations 
> > and
> > the shadow is used for sampling within a shader. The former issue is 
> > addressed
> > by exchanging multiple GTT-mapped memory accesses at texture upload time 
> > with a
> > render engine read and write at sampling time.
> > 
> > At the moment, I don't have any empirical data on the performance
> > implications nor on the bug fixes.

This series reduces the startup time of an internal benchmark from 7s to
0.6s. It doesn't seem to have any impact on the FPS numbers (if I'm
reading them correctly). The benchmark was run five times on a release
build of mesa.

-Nanley

> 
> I just sent out a piglit test to demonstrate the fixed texture download
> issue: https://patchwork.freedesktop.org/series/50474/
> 
> -Nanley
> 
> > I'm trying to get my hands on one of
> > the affected benchmarks. This series does pass our CI system.
> > 
> > 1. 17 seconds were saved by avoiding it in commit:
> >3e56e4642fb5875b3f5c4eb34798ba9f3d827705
> > 
> > Nanley Chery (9):
> >   i965: Rename intel_mipmap_tree::r8stencil_* -> ::shadow_*
> >   i965/miptree: Allocate a shadow_mt for an ASTC WA
> >   i965/miptree: Track the staleness of the ASTC shadow
> >   intel/blorp_blit: Fix ptr deref in convert_to_uncompressed
> >   intel/blorp_blit: Add blorp_copy_astc_wa
> >   i965/blorp: Drop tmp_surfs from surf_for_miptree
> >   i965: Do a WA blit between ASTC main and shadow
> >   i965/surface_state: Use the ASTC shadow_mt if present
> >   i965/tex_image: Drop intelCompressedTexSubImage
> > 
> >  src/intel/blorp/blorp.h   |   6 +
> >  src/intel/blorp/blorp_blit.c  | 158 +-
> >  src/intel/blorp/blorp_priv.h  |   1 +
> >  src/mesa/drivers/dri/i965/brw_blorp.c |  56 ---
> >  src/mesa/drivers/dri/i965/brw_blorp.h |   6 +
> >  src/mesa/drivers/dri/i965/brw_draw.c  |  16 ++
> >  .../drivers/dri/i965/brw_wm_surface_state.c   |  11 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  46 -
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  21 ++-
> >  src/mesa/drivers/dri/i965/intel_tex_image.c   |  87 --
> >  10 files changed, 276 insertions(+), 132 deletions(-)
> > 
> > -- 
> > 2.19.0
> > 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage

2018-10-05 Thread Liu, Leo
Reviewed-by: Leo Liu 

-Original Message-
From: Zhang, Boyuan 
Sent: Friday, October 05, 2018 12:01 PM
To: mesa-dev@lists.freedesktop.org
Cc: Liu, Leo ; imir...@alum.mit.edu; 
ckoenig.leichtzumer...@gmail.com; Zhang, Boyuan 
Subject: [PATCH] st/va: use provided sizes and coords for getimage

From: Boyuan Zhang 

vlVaGetImage should respect the width, height, and coordinates x and y that 
passed in. Therefore, pipe_box should be created with the passed in values 
instead of surface width/height. 

Signed-off-by: Boyuan Zhang 
---
 src/gallium/state_trackers/va/image.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/image.c 
b/src/gallium/state_trackers/va/image.c
index 3f892c9..c9f6f18 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -400,11 +400,14 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, 
int x, int y,
}
 
for (i = 0; i < vaimage->num_planes; i++) {
-  unsigned width, height;
+  unsigned w = align(width, 2);
+  unsigned h = align(height, 2);
   if (!views[i]) continue;
-  vlVaVideoSurfaceSize(surf, i, , );
+  vl_video_buffer_adjust_size(, , i,
+  surf->templat.chroma_format,
+  surf->templat.interlaced);
   for (j = 0; j < views[i]->texture->array_size; ++j) {
- struct pipe_box box = {0, 0, j, width, height, 1};
+ struct pipe_box box = {x, y, j, w, h, 1};
  struct pipe_transfer *transfer;
  uint8_t *map;
  map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
--
2.7.4

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


[Mesa-dev] [PATCH] intel/compiler: Print floating point values upto precision 8

2018-10-05 Thread Sagar Ghuge
avoid misinterpretation of encoded immediate values in instruction and
disassembled output.

Signed-off-by: Sagar Ghuge 
---
While encoding the immediate floating point values in instruction we use
values upto precision 8, but while disassembling, we print precision to
6 places, which round up the value and gives wrong interpretation for
encoded immediate constant. Printing it upto precision 8 will help in
reassembling it again.
 src/intel/compiler/brw_disasm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c
index 322f4544df..7cbbc080b3 100644
--- a/src/intel/compiler/brw_disasm.c
+++ b/src/intel/compiler/brw_disasm.c
@@ -1293,7 +1293,7 @@ imm(FILE *file, const struct gen_device_info *devinfo, 
enum brw_reg_type type,
   format(file, "0x%08xV", brw_inst_imm_ud(devinfo, inst));
   break;
case BRW_REGISTER_TYPE_F:
-  format(file, "%-gF", brw_inst_imm_f(devinfo, inst));
+  format(file, "%.8fF", brw_inst_imm_f(devinfo, inst));
   break;
case BRW_REGISTER_TYPE_DF:
   format(file, "%-gDF", brw_inst_imm_df(devinfo, inst));
-- 
2.17.1

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


[Mesa-dev] [PATCH 2/3] meson: Only build gallium state tracker tests with shared_glapi

2018-10-05 Thread Dylan Baker
This has always been a requirement, it's just somehow been missed in the
meson build.
---
 src/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/meson.build b/src/meson.build
index af881cff70b..73146d37143 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -82,7 +82,7 @@ if with_gallium
   subdir('gallium')
   # This has to be here since it requires libgallium, and subdir cannot
   # contain ..
-  if with_tests
+  if with_tests and with_shared_glapi
 subdir('mesa/state_tracker/tests')
   endif
 endif
-- 
2.19.0

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


[Mesa-dev] [PATCH 1/3] meson: only build clapi tests when OpenGL is being built

2018-10-05 Thread Dylan Baker
Otherwise building just vulkan (among other things) will build these
tests, pull in a bunch of stuff they shouldn't, and potentially fail to
compile.
---
 src/mapi/glapi/meson.build| 2 +-
 src/mapi/shared-glapi/meson.build | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mapi/glapi/meson.build b/src/mapi/glapi/meson.build
index 2509e19eaa3..048bee8a1ad 100644
--- a/src/mapi/glapi/meson.build
+++ b/src/mapi/glapi/meson.build
@@ -78,7 +78,7 @@ libglapi_static = static_library(
   build_by_default : false,
 )
 
-if not with_shared_glapi and with_tests
+if with_any_opengl and not with_shared_glapi and with_tests
   test(
 'glapi_static_check_table',
 executable(
diff --git a/src/mapi/shared-glapi/meson.build 
b/src/mapi/shared-glapi/meson.build
index 44e86f845f6..dcc6079af3d 100644
--- a/src/mapi/shared-glapi/meson.build
+++ b/src/mapi/shared-glapi/meson.build
@@ -50,7 +50,7 @@ libglapi = shared_library(
   install : true,
 )
 
-if with_tests
+if with_any_opengl and with_tests
   test(
 'shared-glapi-test',
 executable(
-- 
2.19.0

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


[Mesa-dev] [PATCH 3/3] meson: Don't build glsl compiler tests unless OpenGL is enabled

2018-10-05 Thread Dylan Baker
Since there are no other users of the glsl compiler.
---
 src/compiler/glsl/glcpp/meson.build | 2 +-
 src/compiler/glsl/meson.build   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/glcpp/meson.build 
b/src/compiler/glsl/glcpp/meson.build
index 769406f5331..a03d589b370 100644
--- a/src/compiler/glsl/glcpp/meson.build
+++ b/src/compiler/glsl/glcpp/meson.build
@@ -55,7 +55,7 @@ glcpp = executable(
   build_by_default : false,
 )
 
-if with_tests
+if with_any_opengl and with_tests
   modes = ['unix', 'windows', 'oldmac', 'bizarro']
   if dep_valgrind.found()
 modes += ['valgrind']
diff --git a/src/compiler/glsl/meson.build b/src/compiler/glsl/meson.build
index 7e4dd2929a3..71b4c42e4d7 100644
--- a/src/compiler/glsl/meson.build
+++ b/src/compiler/glsl/meson.build
@@ -258,6 +258,6 @@ glsl_test = executable(
   install : with_tools.contains('glsl'),
 )
 
-if with_tests
+if with_any_opengl and with_tests
   subdir('tests')
 endif
-- 
2.19.0

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


Re: [Mesa-dev] [PATCH v5 2/2] virgl: Pass resource size and transfer offsets

2018-10-05 Thread Gurchetan Singh
On Fri, Oct 5, 2018 at 1:04 AM Gert Wollny  wrote:
>
> Am Donnerstag, den 04.10.2018, 10:48 -0700 schrieb Gurchetan Singh:
> >
> > The idea is to get rid of any adjustments on both the Mesa /
> > virglrenderer sides -- so transfer size is just what's needed to
> > transfer the box size, and the offset is the just offset into the
> > iovec from which the transfer will start.  For v1, we can just
> > specify an offset of zero on the virglrenderer side.
>
> I certainly see you point, the problem is (as Tomeu also pointed out)
> we would like to enable the CI *now*, and all what is missing is this
> patch. If we want to correct the protocol it will likely take another
> couple of weeks to get it right and do all the testing, and this while
> facing the more challanging changes regarding memory handling, adding
> more GL extensions etc.
>
> > vtest isn't used in production but it will be used in the CI.  Also,
> > it's a simpler model of virgl3d and is useful for experimenting with
> > new protocol additions (hostmap, vulkan).  We should fix this now --
> > it could take a while to disentangle the workarounds should anyone
> > look at this again ...
> My proposal to unblock this would be open an issue against
> virglrenderer (because the problem is on both sides) and doeument there
> where the problems are. Whether this gets fixed by correcting this
> code, or completely replacing things (like Tomeu proposed) is another
> story.
>
> What do you think?

Opening a new issue on the virglrenderer gitlab sounds fine with me.
We shouldn't need to modify the protocol, so maybe that's why I think
it shouldn't take too long, but we can discuss on the bug.

Reviewed-by: Gurchetan Singh 


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


[Mesa-dev] [Bug 108250] [GLSL] layout-location-struct.shader_test fails to link

2018-10-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108250

vadym  changed:

   What|Removed |Added

 CC||vadym.shovkoplias@globallog
   ||ic.com

-- 
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:mesa] util: Make xmlconfig.c build on Solaris without d_type in dirent

2018-10-05 Thread Ian Romanick
On 10/03/2018 08:21 PM, Alan Coopersmith wrote:
> On 10/ 3/18 03:15 PM, Roland Mainz wrote:
>> On Wed, Oct 3, 2018 at 11:51 PM Alan Coopersmith
>>  wrote:
>>>
>>> Introduced-by: commit 04bdbbcab3c4862bf3f54ce60fcc1d2007776f80

Is this formatting something you need for your internal systems?  The
format we've been using to signal that a change is a fix to a bug
introduced by a specific commit is:

Fixes: 04bdbbcab3c "xmlconfig: read more config files from drirc.d/"

That ensures the fix gets bundled in the correct stable release branches.

Aside from that, v2 is

Reviewed-by: Ian Romanick 

>>> Signed-off-by: Alan Coopersmith 
>>> ---
>>>   src/util/xmlconfig.c | 8 
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/src/util/xmlconfig.c b/src/util/xmlconfig.c
>>> index 5264f2598b..608972f812 100644
>>> --- a/src/util/xmlconfig.c
>>> +++ b/src/util/xmlconfig.c
>>> @@ -938,8 +938,16 @@ parseOneConfigFile(struct OptConfData *data,
>>> const char *filename)
>>>   static int
>>>   scandir_filter(const struct dirent *ent)
>>>   {
>>> +#ifndef DT_REG /* systems without d_type in dirent results */
>>> +    struct stat st;
>>> +
>>> +    lstat(ent->d_name, );
>>> +    if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode))
>>> +   return 0;
>>> +#else
>>
>> What about testing for the return code of |lstat()|&&|errno| before
>> looking at the value of |st| ?
> 
> Oh, I suppose there is a small window in which the file could disappear
> after it's read from the directory entry, but before the lstat occurs.
> 
> Attached version checks for that.
> 
> 
> 
> ___
> 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] glsl: fix array assignments of a swizzled vector

2018-10-05 Thread Ilia Mirkin
On Fri, Oct 5, 2018 at 12:55 PM Ian Romanick  wrote:
>
> On 10/04/2018 11:25 PM, Ilia Mirkin wrote:
> > This happens in situations where we might do
> >
> >   vec.wzyx[i] = ...
>
> Ouch.
>
> I haven't looked on the piglit list yet, but is there a piglit test for
> this?

Not yet, but will be tonight. The specific WebGL test is actually
problematic because it's [0] (due to loop unrolling) and not [i] --
the latter should work I believe (but if it doesn't, I'll send a
separate fix for it). I'm going to send tests to cover both cases.

Thanks for taking a look!

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


Re: [Mesa-dev] [PATCH] glsl: fix array assignments of a swizzled vector

2018-10-05 Thread Ian Romanick
On 10/04/2018 11:25 PM, Ilia Mirkin wrote:
> This happens in situations where we might do
> 
>   vec.wzyx[i] = ...

Ouch.

I haven't looked on the piglit list yet, but is there a piglit test for
this?

> The swizzle would get effectively ignored because of the interaction
> between how ir_assignment->set_lhs works and overwriting the write_mask.
> 
> Fixes the following WebGL test:
> 
> https://www.khronos.org/registry/webgl/sdk/tests/conformance2/glsl3/vector-dynamic-indexing-swizzled-lvalue.html
> 
> Signed-off-by: Ilia Mirkin 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/compiler/glsl/lower_vector_derefs.cpp | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compiler/glsl/lower_vector_derefs.cpp 
> b/src/compiler/glsl/lower_vector_derefs.cpp
> index 7583d1fdd3e..af046966491 100644
> --- a/src/compiler/glsl/lower_vector_derefs.cpp
> +++ b/src/compiler/glsl/lower_vector_derefs.cpp
> @@ -59,21 +59,27 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
> if (!deref->array->type->is_vector())
>return ir_rvalue_enter_visitor::visit_enter(ir);
>  
> -   ir_dereference *const new_lhs = (ir_dereference *) deref->array;
> -   ir->set_lhs(new_lhs);
> +   ir_rvalue *const new_lhs = deref->array;
>  
> void *mem_ctx = ralloc_parent(ir);
> ir_constant *old_index_constant =
>deref->array_index->constant_expression_value(mem_ctx);
> if (!old_index_constant) {
> +  ir->set_lhs(new_lhs);
>ir->rhs = new(mem_ctx) ir_expression(ir_triop_vector_insert,
> new_lhs->type,
> new_lhs->clone(mem_ctx, NULL),
> ir->rhs,
> deref->array_index);
>ir->write_mask = (1 << new_lhs->type->vector_elements) - 1;
> +   } else if (new_lhs->ir_type != ir_type_swizzle) {
> +  ir->set_lhs(new_lhs);
> +  ir->write_mask = 1 << old_index_constant->get_uint_component(0);
> } else {
> -  ir->write_mask = 1 << old_index_constant->get_int_component(0);
> +  // If there "new" lhs is a swizzle, use the set_lhs helper to instead
> +  // swizzle the rhs.

LHS and RHS are initialisms, so they should be capitalized.  We also use
/* */ comments even in C++ code.

Aside from those two tiny nits, this patch is

Reviewed-by: Ian Romanick 

> +  unsigned component[1] = { old_index_constant->get_uint_component(0) };
> +  ir->set_lhs(new(mem_ctx) ir_swizzle(new_lhs, component, 1));
> }
>  
> return ir_rvalue_enter_visitor::visit_enter(ir);
> 

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


Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage

2018-10-05 Thread Christian König
If that also fixes the problem, then yeah that makes perfect sense to me 
as well.


Christian.

Am 05.10.2018 um 18:11 schrieb Ilia Mirkin:

This is an improvement, but I think you need to clip the box to

1. Size of the surface
2. Size of the image

I think that there are clipping helpers available to do that (maybe
pipe_box_clip or so? I forget, check the auxiliary dir). Christian -
does that make sense to you?

Cheers,

   -ilia
On Fri, Oct 5, 2018 at 12:01 PM  wrote:

From: Boyuan Zhang 

vlVaGetImage should respect the width, height, and coordinates x and y that
passed in. Therefore, pipe_box should be created with the passed in values
instead of surface width/height.

Signed-off-by: Boyuan Zhang 
---
  src/gallium/state_trackers/va/image.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/image.c 
b/src/gallium/state_trackers/va/image.c
index 3f892c9..c9f6f18 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -400,11 +400,14 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, 
int x, int y,
 }

 for (i = 0; i < vaimage->num_planes; i++) {
-  unsigned width, height;
+  unsigned w = align(width, 2);
+  unsigned h = align(height, 2);
if (!views[i]) continue;
-  vlVaVideoSurfaceSize(surf, i, , );
+  vl_video_buffer_adjust_size(, , i,
+  surf->templat.chroma_format,
+  surf->templat.interlaced);
for (j = 0; j < views[i]->texture->array_size; ++j) {
- struct pipe_box box = {0, 0, j, width, height, 1};
+ struct pipe_box box = {x, y, j, w, h, 1};
   struct pipe_transfer *transfer;
   uint8_t *map;
   map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
--
2.7.4



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


Re: [Mesa-dev] [PATCH mesa] meson: fix vulkan only build with tests

2018-10-05 Thread Dylan Baker
Quoting Eric Engestrom (2018-10-05 05:19:34)
> Fixes the build for
> $ meson -D dri-drivers=[] -D gallium-drivers=[] -D build-tests=true
> 
> Compiling all this unused code isn't an actual problem, until you also
> try to build the tests, at which point you get this:
> 
>   [213/705] Linking target src/mapi/glapi/glapi_static_check_table.
>   FAILED: src/mapi/glapi/glapi_static_check_table
>   ccache c++  -o src/mapi/glapi/glapi_static_check_table 
> 'src/mapi/glapi/src@mapi@glapi@@glapi_static_check_table@exe/tests_check_table.cpp.o'
>  -Wl,--no-undefined -Wl,--as-needed -Wl,--start-group 
> src/mapi/glapi/libglapi_static.a src/gtest/libgtest.a -Wl,--end-group 
> -pthread '-Wl,-rpath,$ORIGIN/:$ORIGIN/../../gtest' 
> -Wl,-rpath-link,/tmp/tmp.qyVZB5kQIB/mesa/build/src/mapi/glapi:/tmp/tmp.qyVZB5kQIB/mesa/build/src/gtest
>   /usr/bin/ld: 
> src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8810): 
> undefined reference to `gl_dispatch_stub_343'
>   /usr/bin/ld: 
> src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8828): 
> undefined reference to `gl_dispatch_stub_343'
>   /usr/bin/ld: 
> src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8840): 
> undefined reference to `gl_dispatch_stub_344'
>   /usr/bin/ld: 
> src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8858): 
> undefined reference to `gl_dispatch_stub_344'
>   /usr/bin/ld: 
> src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8870): 
> undefined reference to `gl_dispatch_stub_345'
>   /usr/bin/ld: 
> src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x): 
> undefined reference to `gl_dispatch_stub_345'
>   collect2: error: ld returned 1 exit status
>   ninja: build stopped: subcommand failed.
> 
> Signed-off-by: Eric Engestrom 
> ---
>  src/meson.build | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

This really looks like the static-glapi tests are broken again. 

> diff --git a/src/meson.build b/src/meson.build
> index 89ffaddf47b7286e4fe0..bde0e2aaca07931d3097 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -51,7 +51,9 @@ idep_git_sha1 = declare_dependency(
>  
>  subdir('gtest')
>  subdir('util')
> -subdir('mapi')
> +if with_gles1 or with_gles2 or with_shared_glapi

We need to mapi for non-shared glapi opengl as well.

> +  subdir('mapi')
> +endif
>  # TODO: opengl
>  subdir('compiler')
>  subdir('egl/wayland/wayland-drm')
> @@ -65,7 +67,9 @@ endif
>  if with_dri_i965 or with_intel_vk
>subdir('intel')
>  endif
> -subdir('mesa')
> +if with_opengl or with_gles1 or with_gles2 or with_shared_glapi
> +  subdir('mesa')
> +endif
>  subdir('loader')
>  if with_platform_haiku
>subdir('hgl')
> -- 
> Cheers,
>   Eric
> 

This makes me really nervous. There are a bunch of things (including libmesa_*,
and all of mapi) that are controlled by build_by_default = false, and are built
only when a top level target pulls them in. I did this specifically to avoid
having long complicated conditionals around whether to traverse into a subdir
like autotools does. I did put guards on the tests because that was easier, and
it's not going to break an end user's system if a test is or isn't built.

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 108095] Window becomes partially transparent for short time

2018-10-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108095

--- Comment #10 from Michel Dänzer  ---
Specifically, with xfwm4, loader_dri3_wait_x() is currently a no-op, because
draw->have_fake_front is false.

-- 
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 108095] Window becomes partially transparent for short time

2018-10-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108095

Michel Dänzer  changed:

   What|Removed |Added

Version|DRI git |git
Product|DRI |Mesa
  Component|DRM/AMDgpu  |GLX
 QA Contact||mesa-dev@lists.freedesktop.
   ||org
   Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org

--- Comment #9 from Michel Dänzer  ---
I think this is because the glXWaitX implementation in Mesa isn't reliable.
xfwm4 relies on it to synchronize between X11 protocol drawing to a pixmap and
sampling from an OpenGL texture bound to the corresponding GLXPixmap via
GLX_EXT_texture_from_pixmap.

-- 
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] st/va: use provided sizes and coords for getimage

2018-10-05 Thread Ilia Mirkin
This is an improvement, but I think you need to clip the box to

1. Size of the surface
2. Size of the image

I think that there are clipping helpers available to do that (maybe
pipe_box_clip or so? I forget, check the auxiliary dir). Christian -
does that make sense to you?

Cheers,

  -ilia
On Fri, Oct 5, 2018 at 12:01 PM  wrote:
>
> From: Boyuan Zhang 
>
> vlVaGetImage should respect the width, height, and coordinates x and y that
> passed in. Therefore, pipe_box should be created with the passed in values
> instead of surface width/height.
>
> Signed-off-by: Boyuan Zhang 
> ---
>  src/gallium/state_trackers/va/image.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/image.c 
> b/src/gallium/state_trackers/va/image.c
> index 3f892c9..c9f6f18 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -400,11 +400,14 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, 
> int x, int y,
> }
>
> for (i = 0; i < vaimage->num_planes; i++) {
> -  unsigned width, height;
> +  unsigned w = align(width, 2);
> +  unsigned h = align(height, 2);
>if (!views[i]) continue;
> -  vlVaVideoSurfaceSize(surf, i, , );
> +  vl_video_buffer_adjust_size(, , i,
> +  surf->templat.chroma_format,
> +  surf->templat.interlaced);
>for (j = 0; j < views[i]->texture->array_size; ++j) {
> - struct pipe_box box = {0, 0, j, width, height, 1};
> + struct pipe_box box = {x, y, j, w, h, 1};
>   struct pipe_transfer *transfer;
>   uint8_t *map;
>   map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage

2018-10-05 Thread boyuan.zhang
From: Boyuan Zhang 

vlVaGetImage should respect the width, height, and coordinates x and y that
passed in. Therefore, pipe_box should be created with the passed in values
instead of surface width/height. 

Signed-off-by: Boyuan Zhang 
---
 src/gallium/state_trackers/va/image.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/image.c 
b/src/gallium/state_trackers/va/image.c
index 3f892c9..c9f6f18 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -400,11 +400,14 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, 
int x, int y,
}
 
for (i = 0; i < vaimage->num_planes; i++) {
-  unsigned width, height;
+  unsigned w = align(width, 2);
+  unsigned h = align(height, 2);
   if (!views[i]) continue;
-  vlVaVideoSurfaceSize(surf, i, , );
+  vl_video_buffer_adjust_size(, , i,
+  surf->templat.chroma_format,
+  surf->templat.interlaced);
   for (j = 0; j < views[i]->texture->array_size; ++j) {
- struct pipe_box box = {0, 0, j, width, height, 1};
+ struct pipe_box box = {x, y, j, w, h, 1};
  struct pipe_transfer *transfer;
  uint8_t *map;
  map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
-- 
2.7.4

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


Re: [Mesa-dev] [PATCH] nir: Add a pass for gathering transform feedback info

2018-10-05 Thread Alejandro Piñeiro
On 05/10/18 17:44, Jason Ekstrand wrote:
> On Fri, Oct 5, 2018 at 10:34 AM Alejandro Piñeiro
> mailto:apinhe...@igalia.com>> wrote:
>
> On 05/10/18 16:13, Jason Ekstrand wrote:
> > This is different from the GL_ARB_spirv pass because it
> generates a much
> > simpler data structure that isn't tied to OpenGL and mtypes.h.
>
> I have just skimmed it (don't have time right now for a full
> check, will
> take a deeper look next Monday), but FWIW, the GL_ARB_spirv pass
> does a
> initial mtypes-free info gathering (get_active_xfb_varyings) and
> then it
> uses that info to fill-up the OpenGL specific mtypes.h. In fact, if we
> just compare the initial GL_ARB_spirv gathering with this new
> pass, your
> pass seems more complete, and with more checks. So I was wondering
> if it
> would be possible to remove some code on the GL_ARB_spirv pass and use
> this new pass instead. Did you check if that would be possible? If not
> I'm willing to check next week.
>
>
> I didn't really look into that too much.  When drafting this one, I
> did base it somewhat on the GL_ARB_spirv pass so I"m not surprised
> it's similar.  At the time, I was just trying to get something working
> and wasn't too worried about code duplication.  If we can use this
> pass for GL_ARB_spirv, that'd be fantastic. 

Ok, then next week I will check for sure if we can do that.

> If we do go that route, however, we may want to do something better
> than assert() for the error handling.

Ok, first I will check if this new pass gather the info GL_ARB_spirv
pass needs, and see how much the GL_ARB_spirv pass would need to be
modified (as the gathering info formatting would change). Then we can
talk about what to do with the asserts.

>  
>
> > ---
> >  src/compiler/Makefile.sources          |   4 +-
> >  src/compiler/nir/meson.build           |   2 +
> >  src/compiler/nir/nir_gather_xfb_info.c | 150
> +
> >  src/compiler/nir/nir_xfb_info.h        |  59 ++
> >  4 files changed, 214 insertions(+), 1 deletion(-)
> >  create mode 100644 src/compiler/nir/nir_gather_xfb_info.c
> >  create mode 100644 src/compiler/nir/nir_xfb_info.h
> >
> > diff --git a/src/compiler/Makefile.sources
> b/src/compiler/Makefile.sources
> > index d3b06564832..46ed5e47b46 100644
> > --- a/src/compiler/Makefile.sources
> > +++ b/src/compiler/Makefile.sources
> > @@ -216,6 +216,7 @@ NIR_FILES = \
> >       nir/nir_format_convert.h \
> >       nir/nir_from_ssa.c \
> >       nir/nir_gather_info.c \
> > +     nir/nir_gather_xfb_info.c \
> >       nir/nir_gs_count_vertices.c \
> >       nir/nir_inline_functions.c \
> >       nir/nir_instr_set.c \
> > @@ -307,7 +308,8 @@ NIR_FILES = \
> >       nir/nir_validate.c \
> >       nir/nir_vla.h \
> >       nir/nir_worklist.c \
> > -     nir/nir_worklist.h
> > +     nir/nir_worklist.h \
> > +     nir/nir_xfb_info.h
> > 
> >  SPIRV_GENERATED_FILES = \
> >       spirv/spirv_info.c \
> > diff --git a/src/compiler/nir/meson.build
> b/src/compiler/nir/meson.build
> > index 090aa7a628f..b416e561eb0 100644
> > --- a/src/compiler/nir/meson.build
> > +++ b/src/compiler/nir/meson.build
> > @@ -100,6 +100,7 @@ files_libnir = files(
> >    'nir_format_convert.h',
> >    'nir_from_ssa.c',
> >    'nir_gather_info.c',
> > +  'nir_gather_xfb_info.c',
> >    'nir_gs_count_vertices.c',
> >    'nir_inline_functions.c',
> >    'nir_instr_set.c',
> > @@ -192,6 +193,7 @@ files_libnir = files(
> >    'nir_vla.h',
> >    'nir_worklist.c',
> >    'nir_worklist.h',
> > +  'nir_xfb_info.h',
> >    '../spirv/GLSL.ext.AMD.h',
> >    '../spirv/GLSL.std.450.h',
> >    '../spirv/gl_spirv.c',
> > diff --git a/src/compiler/nir/nir_gather_xfb_info.c
> b/src/compiler/nir/nir_gather_xfb_info.c
> > new file mode 100644
> > index 000..a53703bb9bf
> > --- /dev/null
> > +++ b/src/compiler/nir/nir_gather_xfb_info.c
> > @@ -0,0 +1,150 @@
> > +/*
> > + * 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
> > 

Re: [Mesa-dev] [PATCH] spirv: mark variables decorated with XfbBuffer as always active

2018-10-05 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Fri, Oct 5, 2018 at 7:39 AM Samuel Pitoiset 
wrote:

> Otherwise, they are removed during NIR linking or in some
> lowering passes.
>
> Cc: Jason Ekstrand 
> Cc: Timothy Arceri 
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/compiler/spirv/vtn_variables.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/compiler/spirv/vtn_variables.c
> b/src/compiler/spirv/vtn_variables.c
> index 358ff4bef7a..636fdb8689a 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1326,6 +1326,7 @@ apply_var_decoration(struct vtn_builder *b,
> case SpvDecorationXfbBuffer:
>var_data->explicit_xfb_buffer = true;
>var_data->xfb_buffer = dec->literals[0];
> +  var_data->always_active_io = true;
>break;
> case SpvDecorationXfbStride:
>var_data->explicit_xfb_stride = true;
> --
> 2.19.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] nir: Add a pass for gathering transform feedback info

2018-10-05 Thread Jason Ekstrand
On Fri, Oct 5, 2018 at 10:34 AM Alejandro Piñeiro 
wrote:

> On 05/10/18 16:13, Jason Ekstrand wrote:
> > This is different from the GL_ARB_spirv pass because it generates a much
> > simpler data structure that isn't tied to OpenGL and mtypes.h.
>
> I have just skimmed it (don't have time right now for a full check, will
> take a deeper look next Monday), but FWIW, the GL_ARB_spirv pass does a
> initial mtypes-free info gathering (get_active_xfb_varyings) and then it
> uses that info to fill-up the OpenGL specific mtypes.h. In fact, if we
> just compare the initial GL_ARB_spirv gathering with this new pass, your
> pass seems more complete, and with more checks. So I was wondering if it
> would be possible to remove some code on the GL_ARB_spirv pass and use
> this new pass instead. Did you check if that would be possible? If not
> I'm willing to check next week.
>

I didn't really look into that too much.  When drafting this one, I did
base it somewhat on the GL_ARB_spirv pass so I"m not surprised it's
similar.  At the time, I was just trying to get something working and
wasn't too worried about code duplication.  If we can use this pass for
GL_ARB_spirv, that'd be fantastic.  If we do go that route, however, we may
want to do something better than assert() for the error handling.


> > ---
> >  src/compiler/Makefile.sources  |   4 +-
> >  src/compiler/nir/meson.build   |   2 +
> >  src/compiler/nir/nir_gather_xfb_info.c | 150 +
> >  src/compiler/nir/nir_xfb_info.h|  59 ++
> >  4 files changed, 214 insertions(+), 1 deletion(-)
> >  create mode 100644 src/compiler/nir/nir_gather_xfb_info.c
> >  create mode 100644 src/compiler/nir/nir_xfb_info.h
> >
> > diff --git a/src/compiler/Makefile.sources
> b/src/compiler/Makefile.sources
> > index d3b06564832..46ed5e47b46 100644
> > --- a/src/compiler/Makefile.sources
> > +++ b/src/compiler/Makefile.sources
> > @@ -216,6 +216,7 @@ NIR_FILES = \
> >   nir/nir_format_convert.h \
> >   nir/nir_from_ssa.c \
> >   nir/nir_gather_info.c \
> > + nir/nir_gather_xfb_info.c \
> >   nir/nir_gs_count_vertices.c \
> >   nir/nir_inline_functions.c \
> >   nir/nir_instr_set.c \
> > @@ -307,7 +308,8 @@ NIR_FILES = \
> >   nir/nir_validate.c \
> >   nir/nir_vla.h \
> >   nir/nir_worklist.c \
> > - nir/nir_worklist.h
> > + nir/nir_worklist.h \
> > + nir/nir_xfb_info.h
> >
> >  SPIRV_GENERATED_FILES = \
> >   spirv/spirv_info.c \
> > diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
> > index 090aa7a628f..b416e561eb0 100644
> > --- a/src/compiler/nir/meson.build
> > +++ b/src/compiler/nir/meson.build
> > @@ -100,6 +100,7 @@ files_libnir = files(
> >'nir_format_convert.h',
> >'nir_from_ssa.c',
> >'nir_gather_info.c',
> > +  'nir_gather_xfb_info.c',
> >'nir_gs_count_vertices.c',
> >'nir_inline_functions.c',
> >'nir_instr_set.c',
> > @@ -192,6 +193,7 @@ files_libnir = files(
> >'nir_vla.h',
> >'nir_worklist.c',
> >'nir_worklist.h',
> > +  'nir_xfb_info.h',
> >'../spirv/GLSL.ext.AMD.h',
> >'../spirv/GLSL.std.450.h',
> >'../spirv/gl_spirv.c',
> > diff --git a/src/compiler/nir/nir_gather_xfb_info.c
> b/src/compiler/nir/nir_gather_xfb_info.c
> > new file mode 100644
> > index 000..a53703bb9bf
> > --- /dev/null
> > +++ b/src/compiler/nir/nir_gather_xfb_info.c
> > @@ -0,0 +1,150 @@
> > +/*
> > + * 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_xfb_info.h"
> > +
> > +#include 
> > +
> > +static void
> > +add_var_xfb_outputs(nir_xfb_info *xfb,
> > +nir_variable *var,
> > +unsigned *location,
> > +unsigned *offset,
> > +

Re: [Mesa-dev] [PATCH] nir: Add a pass for gathering transform feedback info

2018-10-05 Thread Alejandro Piñeiro
On 05/10/18 16:13, Jason Ekstrand wrote:
> This is different from the GL_ARB_spirv pass because it generates a much
> simpler data structure that isn't tied to OpenGL and mtypes.h.

I have just skimmed it (don't have time right now for a full check, will
take a deeper look next Monday), but FWIW, the GL_ARB_spirv pass does a
initial mtypes-free info gathering (get_active_xfb_varyings) and then it
uses that info to fill-up the OpenGL specific mtypes.h. In fact, if we
just compare the initial GL_ARB_spirv gathering with this new pass, your
pass seems more complete, and with more checks. So I was wondering if it
would be possible to remove some code on the GL_ARB_spirv pass and use
this new pass instead. Did you check if that would be possible? If not
I'm willing to check next week.


> ---
>  src/compiler/Makefile.sources  |   4 +-
>  src/compiler/nir/meson.build   |   2 +
>  src/compiler/nir/nir_gather_xfb_info.c | 150 +
>  src/compiler/nir/nir_xfb_info.h|  59 ++
>  4 files changed, 214 insertions(+), 1 deletion(-)
>  create mode 100644 src/compiler/nir/nir_gather_xfb_info.c
>  create mode 100644 src/compiler/nir/nir_xfb_info.h
>
> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
> index d3b06564832..46ed5e47b46 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -216,6 +216,7 @@ NIR_FILES = \
>   nir/nir_format_convert.h \
>   nir/nir_from_ssa.c \
>   nir/nir_gather_info.c \
> + nir/nir_gather_xfb_info.c \
>   nir/nir_gs_count_vertices.c \
>   nir/nir_inline_functions.c \
>   nir/nir_instr_set.c \
> @@ -307,7 +308,8 @@ NIR_FILES = \
>   nir/nir_validate.c \
>   nir/nir_vla.h \
>   nir/nir_worklist.c \
> - nir/nir_worklist.h
> + nir/nir_worklist.h \
> + nir/nir_xfb_info.h
>  
>  SPIRV_GENERATED_FILES = \
>   spirv/spirv_info.c \
> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
> index 090aa7a628f..b416e561eb0 100644
> --- a/src/compiler/nir/meson.build
> +++ b/src/compiler/nir/meson.build
> @@ -100,6 +100,7 @@ files_libnir = files(
>'nir_format_convert.h',
>'nir_from_ssa.c',
>'nir_gather_info.c',
> +  'nir_gather_xfb_info.c',
>'nir_gs_count_vertices.c',
>'nir_inline_functions.c',
>'nir_instr_set.c',
> @@ -192,6 +193,7 @@ files_libnir = files(
>'nir_vla.h',
>'nir_worklist.c',
>'nir_worklist.h',
> +  'nir_xfb_info.h',
>'../spirv/GLSL.ext.AMD.h',
>'../spirv/GLSL.std.450.h',
>'../spirv/gl_spirv.c',
> diff --git a/src/compiler/nir/nir_gather_xfb_info.c 
> b/src/compiler/nir/nir_gather_xfb_info.c
> new file mode 100644
> index 000..a53703bb9bf
> --- /dev/null
> +++ b/src/compiler/nir/nir_gather_xfb_info.c
> @@ -0,0 +1,150 @@
> +/*
> + * 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_xfb_info.h"
> +
> +#include 
> +
> +static void
> +add_var_xfb_outputs(nir_xfb_info *xfb,
> +nir_variable *var,
> +unsigned *location,
> +unsigned *offset,
> +const struct glsl_type *type)
> +{
> +   if (glsl_type_is_array(type) || glsl_type_is_matrix(type)) {
> +  unsigned length = glsl_get_length(type);
> +  const struct glsl_type *child_type = glsl_get_array_element(type);
> +  for (unsigned i = 0; i < length; i++)
> + add_var_xfb_outputs(xfb, var, location, offset, child_type);
> +   } else if (glsl_type_is_struct(type)) {
> +  unsigned length = glsl_get_length(type);
> +  for (unsigned i = 0; i < length; i++) {
> + const struct glsl_type *child_type = glsl_get_struct_field(type, i);
> + add_var_xfb_outputs(xfb, var, location, offset, child_type);
> +  }
> +   } else {
> +  

[Mesa-dev] [Bug 108250] [GLSL] layout-location-struct.shader_test fails to link

2018-10-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108250

--- Comment #1 from vadym  ---
Patch: https://patchwork.freedesktop.org/patch/255084/

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


[Mesa-dev] [PATCH] glsl: Fix input/output structure matching across shader stages

2018-10-05 Thread Vadym Shovkoplias
Section 7.4.1 (Shader Interface Matching) of the OpenGL 4.30 spec says:

"Variables or block members declared as structures are considered
 to match in type if and only if structure members match in name,
 type, qualification, and declaration order."

Fixes:
 * layout-location-struct.shader_test

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108250
Signed-off-by: Vadym Shovkoplias 
---
 src/compiler/glsl/ast_to_hir.cpp|  2 +-
 src/compiler/glsl/link_varyings.cpp | 55 +++--
 src/compiler/glsl_types.cpp | 17 ++---
 src/compiler/glsl_types.h   |  8 +++--
 4 files changed, 56 insertions(+), 26 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 93e7c8ec33..67ad300236 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -7583,7 +7583,7 @@ ast_struct_specifier::hir(exec_list *instructions,
if (!type->is_anonymous() && !state->symbols->add_type(name, type)) {
   const glsl_type *match = state->symbols->get_type(name);
   /* allow struct matching for desktop GL - older UE4 does this */
-  if (match != NULL && state->is_version(130, 0) && 
match->record_compare(type, false))
+  if (match != NULL && state->is_version(130, 0) && 
match->record_compare(type, true, false))
  _mesa_glsl_warning(& loc, state, "struct `%s' previously defined", 
name);
   else
  _mesa_glsl_error(& loc, state, "struct `%s' previously defined", 
name);
diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 52e493cb59..249f475a69 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -214,25 +214,42 @@ cross_validate_types_and_qualifiers(struct gl_context 
*ctx,
}
 
if (type_to_match != output->type) {
-  /* There is a bit of a special case for gl_TexCoord.  This
-   * built-in is unsized by default.  Applications that variable
-   * access it must redeclare it with a size.  There is some
-   * language in the GLSL spec that implies the fragment shader
-   * and vertex shader do not have to agree on this size.  Other
-   * driver behave this way, and one or two applications seem to
-   * rely on it.
-   *
-   * Neither declaration needs to be modified here because the array
-   * sizes are fixed later when update_array_sizes is called.
-   *
-   * From page 48 (page 54 of the PDF) of the GLSL 1.10 spec:
-   *
-   * "Unlike user-defined varying variables, the built-in
-   * varying variables don't have a strict one-to-one
-   * correspondence between the vertex language and the
-   * fragment language."
-   */
-  if (!output->type->is_array() || !is_gl_identifier(output->name)) {
+  if (output->type->is_record()) {
+ /* Structures across shader stages can have different name
+  * and considered to match in type if and only if structure
+  * members match in name, type, qualification, and declaration
+  * order.
+  */
+ if(!output->type->record_compare(type_to_match, false, true)) {
+linker_error(prog,
+  "%s shader output %s' declared as struct `%s', "
+  "doesn't match in type with %s shader input "
+  "declared as struct `%s'\n",
+  _mesa_shader_stage_to_string(producer_stage),
+  output->name,
+  output->type->name,
+  _mesa_shader_stage_to_string(consumer_stage),
+  input->type->name);
+ }
+  } else if (!output->type->is_array() || !is_gl_identifier(output->name)) 
{
+ /* There is a bit of a special case for gl_TexCoord.  This
+  * built-in is unsized by default.  Applications that variable
+  * access it must redeclare it with a size.  There is some
+  * language in the GLSL spec that implies the fragment shader
+  * and vertex shader do not have to agree on this size.  Other
+  * driver behave this way, and one or two applications seem to
+  * rely on it.
+  *
+  * Neither declaration needs to be modified here because the array
+  * sizes are fixed later when update_array_sizes is called.
+  *
+  * From page 48 (page 54 of the PDF) of the GLSL 1.10 spec:
+  *
+  * "Unlike user-defined varying variables, the built-in
+  * varying variables don't have a strict one-to-one
+  * correspondence between the vertex language and the
+  * fragment language."
+  */
  linker_error(prog,
   "%s shader output `%s' declared as type `%s', "
   "but %s shader input declared as type `%s'\n",
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 

[Mesa-dev] [Bug 108250] [GLSL] layout-location-struct.shader_test fails to link

2018-10-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108250

Bug ID: 108250
   Summary: [GLSL] layout-location-struct.shader_test fails to
link
   Product: Mesa
   Version: 18.2
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: glsl-compiler
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: vadym.shovkopl...@globallogic.com
QA Contact: intel-3d-b...@lists.freedesktop.org

# bin/shader_runner
./tests/spec/arb_separate_shader_objects/execution/layout-location-struct.shader_test
 
Failed to link:
error: vertex shader output `s2' declared as type `S2', but fragment shader
input declared as type `S'
error: vertex shader output `s' declared as type `S', but fragment shader input
declared as type `S1'

Test failure on line 50
Probe color at (0,0)
  Expected: 255 0 0
  Observed: 0 0 0
Test failure on line 51
Failed to link:
error: vertex shader output `s2' declared as type `S2', but fragment shader
input declared as type `S'
error: vertex shader output `s' declared as type `S', but fragment shader input
declared as type `S1'



Accordingly to OpenGL 4.30 spec this test shouldn't fail to link:

Section 7.4.1 (Shader Interface Matching):

"Variables or block members declared as structures are considered
to match in type if and only if structure members match in name,
type, qualification, and declaration order."

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


[Mesa-dev] [PATCH] nir: Add a pass for gathering transform feedback info

2018-10-05 Thread Jason Ekstrand
This is different from the GL_ARB_spirv pass because it generates a much
simpler data structure that isn't tied to OpenGL and mtypes.h.
---
 src/compiler/Makefile.sources  |   4 +-
 src/compiler/nir/meson.build   |   2 +
 src/compiler/nir/nir_gather_xfb_info.c | 150 +
 src/compiler/nir/nir_xfb_info.h|  59 ++
 4 files changed, 214 insertions(+), 1 deletion(-)
 create mode 100644 src/compiler/nir/nir_gather_xfb_info.c
 create mode 100644 src/compiler/nir/nir_xfb_info.h

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index d3b06564832..46ed5e47b46 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -216,6 +216,7 @@ NIR_FILES = \
nir/nir_format_convert.h \
nir/nir_from_ssa.c \
nir/nir_gather_info.c \
+   nir/nir_gather_xfb_info.c \
nir/nir_gs_count_vertices.c \
nir/nir_inline_functions.c \
nir/nir_instr_set.c \
@@ -307,7 +308,8 @@ NIR_FILES = \
nir/nir_validate.c \
nir/nir_vla.h \
nir/nir_worklist.c \
-   nir/nir_worklist.h
+   nir/nir_worklist.h \
+   nir/nir_xfb_info.h
 
 SPIRV_GENERATED_FILES = \
spirv/spirv_info.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index 090aa7a628f..b416e561eb0 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -100,6 +100,7 @@ files_libnir = files(
   'nir_format_convert.h',
   'nir_from_ssa.c',
   'nir_gather_info.c',
+  'nir_gather_xfb_info.c',
   'nir_gs_count_vertices.c',
   'nir_inline_functions.c',
   'nir_instr_set.c',
@@ -192,6 +193,7 @@ files_libnir = files(
   'nir_vla.h',
   'nir_worklist.c',
   'nir_worklist.h',
+  'nir_xfb_info.h',
   '../spirv/GLSL.ext.AMD.h',
   '../spirv/GLSL.std.450.h',
   '../spirv/gl_spirv.c',
diff --git a/src/compiler/nir/nir_gather_xfb_info.c 
b/src/compiler/nir/nir_gather_xfb_info.c
new file mode 100644
index 000..a53703bb9bf
--- /dev/null
+++ b/src/compiler/nir/nir_gather_xfb_info.c
@@ -0,0 +1,150 @@
+/*
+ * 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_xfb_info.h"
+
+#include 
+
+static void
+add_var_xfb_outputs(nir_xfb_info *xfb,
+nir_variable *var,
+unsigned *location,
+unsigned *offset,
+const struct glsl_type *type)
+{
+   if (glsl_type_is_array(type) || glsl_type_is_matrix(type)) {
+  unsigned length = glsl_get_length(type);
+  const struct glsl_type *child_type = glsl_get_array_element(type);
+  for (unsigned i = 0; i < length; i++)
+ add_var_xfb_outputs(xfb, var, location, offset, child_type);
+   } else if (glsl_type_is_struct(type)) {
+  unsigned length = glsl_get_length(type);
+  for (unsigned i = 0; i < length; i++) {
+ const struct glsl_type *child_type = glsl_get_struct_field(type, i);
+ add_var_xfb_outputs(xfb, var, location, offset, child_type);
+  }
+   } else {
+  assert(var->data.xfb_buffer < NIR_MAX_XFB_BUFFERS);
+  if (xfb->buffers_written & (1 << var->data.xfb_buffer)) {
+ assert(xfb->strides[var->data.xfb_buffer] == var->data.xfb_stride);
+ assert(xfb->buffer_to_stream[var->data.xfb_buffer] == 
var->data.stream);
+  } else {
+ xfb->buffers_written |= (1 << var->data.xfb_buffer);
+ xfb->strides[var->data.xfb_buffer] = var->data.xfb_stride;
+ xfb->buffer_to_stream[var->data.xfb_buffer] = var->data.stream;
+  }
+
+  assert(var->data.stream < NIR_MAX_XFB_STREAMS);
+  xfb->streams_written |= (1 << var->data.stream);
+
+  unsigned comp_slots = glsl_get_component_slots(type);
+  unsigned attrib_slots = DIV_ROUND_UP(comp_slots, 4);
+  assert(attrib_slots == glsl_count_attribute_slots(type, false));
+
+  /* Ensure that we don't 

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-05 Thread Ian Romanick
On 10/03/2018 11:52 PM, Iago Toral wrote:
> On Wed, 2018-10-03 at 16:24 +0300, Vadim Shovkoplias wrote:
>> Hi Iago,
>>
>> I also think that compiler is the best place for the fix. But from my
>> understanding compiler fix will be limited to distinct shader objects
>> (not the shader stage).
>> In GLSL spec mentioned: "A program will fail to compile or link if any
>> *shader or stage* contains two or more functions with the same name if
>> the name is 
>> associated with a subroutine type".
>> So if I understand the spec correctly such restriction for the shader
>> stage can be fixed only in linker. E.g. consider the use case when
>> fragment shader is linked from 
>> two fragment shader objects. One object contains function foo() which
>> is associated with subroutine type and second shader object has some
>> regular foo() function 
>> with the same name.  I suppose compiler fix won't be able to detect this.
> 
> Yes, that is correct, good point.
> 
>> IMHO the best way to fix this is to implement 2 patches:
>> 1) One patch for linker to implement restriction for the shader stages
>> (already done in this patch)
>> 2) Another one for compiler to check the restriction for distinct
>> shader objects.
>>
>> What do you think ?
> 
> Since we need to keep the link-time check, any compile-time checks we
> add would be redundant with that, so I think we should just go with the
> link-time check only (your original patch)

The spec quotation is pretty clear: "A program will fail to compile ...
if any shader ... contains two or more functions with the same name if
the name is associated with a subroutine type."  To be strictly correct,
both checks are necessary.  Before doing that extra work, someone should
check that glslang has the compile-time check.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/5] glspirv: drop pointless assert (size_t is unsigned)

2018-10-05 Thread Alejandro Piñeiro
Reviewed-by: Alejandro Piñeiro 


On 05/10/18 02:00, Dave Airlie wrote:
> From: Dave Airlie 
>
> Found by coverity
> ---
>  src/mesa/main/glspirv.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
> index fecf7384eb3..972989055e9 100644
> --- a/src/mesa/main/glspirv.c
> +++ b/src/mesa/main/glspirv.c
> @@ -73,8 +73,6 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
> struct gl_spirv_module *module;
> struct gl_shader_spirv_data *spirv_data;
>  
> -   assert(length >= 0);
> -
> module = malloc(sizeof(*module) + length);
> if (!module) {
>_mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary");

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


Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-05 Thread Ian Romanick
On 10/03/2018 01:39 AM, Vadym Shovkoplias wrote:
> From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification
> 
> "A program will fail to compile or link if any shader
>  or stage contains two or more functions with the same
>  name if the name is associated with a subroutine type."
> 
> Fixes:
> * no-overloads.vert
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108109
> Signed-off-by: Vadym Shovkoplias 
> ---
>  src/compiler/glsl/linker.cpp | 40 
>  1 file changed, 40 insertions(+)
> 
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 3fde7e78d3..aca5488a1e 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -4639,6 +4639,45 @@ link_assign_subroutine_types(struct gl_shader_program 
> *prog)
> }
>  }
>  
> +static void
> +verify_subroutine_associated_funcs(struct gl_shader_program *prog)
> +{
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +  const int i = u_bit_scan();
> +  gl_program *p = prog->_LinkedShaders[i]->Program;
> +  glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;
> +
> +  /*
> +   * From OpenGL ES Shading Language 4.00 specification
> +   * (6.1.2 Subroutines):
> +   * "A program will fail to compile or link if any shader
> +   * or stage contains two or more functions with the same
> +   * name if the name is associated with a subroutine type."
> +   */

There is no such thing as the "OpenGL ES Shading Language 4.00
specification", and there are is no subroutine support in GLSL ES.  I'm
assuming you actually meant, "GLSL 4.00".

For future reference, please use the canonical format for spec quotations:

  /* Section 6.1.2 (Subroutines) of the GLSL 4.00 spec says:
   *
   *A program will fail to compile or link if any shader
   *or stage contains two or more functions with the same
   *name if the name is associated with a subroutine type.
   */

Some people, myself included, use grep to find spec quotations in the
code.  Using the canonical format helps those people find things.  This
especially matters when later versions of the spec have bug fixes that
require use to change the behavior of the compiler.

> +  for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {
> + unsigned definitions = 0;
> + char *name = p->sh.SubroutineFunctions[j].name;
> + ir_function *fn = symbols->get_function(name);
> +
> + /* Calculate number of function definitions with the same name */
> + foreach_in_list(ir_function_signature, sig, >signatures) {
> +if (sig->is_defined) {
> +   if (++definitions > 1) {
> +  linker_error(prog, "%s shader contains two or more 
> function "
> +"definitions with name `%s', which is "
> +"associated with a subroutine type.\n",
> +_mesa_shader_stage_to_string(i),
> +fn->name);
> +  return;
> +   }
> +}
> + }
> +  }
> +   }
> +}
> +
> +
>  static void
>  set_always_active_io(exec_list *ir, ir_variable_mode io_mode)
>  {
> @@ -5024,6 +5063,7 @@ link_shaders(struct gl_context *ctx, struct 
> gl_shader_program *prog)
>  
> check_explicit_uniform_locations(ctx, prog);
> link_assign_subroutine_types(prog);
> +   verify_subroutine_associated_funcs(prog);
>  
> if (!prog->data->LinkStatus)
>goto done;
> 

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


[Mesa-dev] [PATCH v2] gallivm: Make it possible to disable some optimization shortcuts in release builds

2018-10-05 Thread Gert Wollny
From: Gert Wollny 

For testing it is of interest that all tests of dEQP pass, e.g. to test
virglrenderer on a host only providing software rendering like in a CI.
Hence make it possible to disable certain optimizations that make tests fail.

While we are there also add some documentation to the flags to make it clear
that this is opt-out.

Setting the environment variable "GALLIVM_PERF=disable_all" can be used to make
the following tests pass in release mode:

  dEQP-GLES2.functional.texture.mipmap.2d.affine.*_linear_*
  dEQP-GLES2.functional.texture.mipmap.cube.generate.*
  dEQP-GLES2.functional.texture.vertex.2d.filtering.*_mipmap_linear_*
  dEQP-GLES2.functional.texture.vertex.2d.wrap.*

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

v2: rename optimization disabling flag to 'safemath' and also move the
nopt flag to the perf flags.

Signed-off-by: Gert Wollny 
---
 src/gallium/auxiliary/gallivm/lp_bld_debug.h  | 16 ---
 src/gallium/auxiliary/gallivm/lp_bld_init.c   | 25 +++
 src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c |  6 +++---
 src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c   |  6 +++---
 4 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_debug.h 
b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
index f96a1afa7a..eeef0d6ba6 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_debug.h
+++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
@@ -39,20 +39,22 @@
 #define GALLIVM_DEBUG_TGSI  (1 << 0)
 #define GALLIVM_DEBUG_IR(1 << 1)
 #define GALLIVM_DEBUG_ASM   (1 << 2)
-#define GALLIVM_DEBUG_NO_OPT(1 << 3)
-#define GALLIVM_DEBUG_PERF  (1 << 4)
-#define GALLIVM_DEBUG_NO_BRILINEAR  (1 << 5)
-#define GALLIVM_DEBUG_NO_RHO_APPROX (1 << 6)
-#define GALLIVM_DEBUG_NO_QUAD_LOD   (1 << 7)
-#define GALLIVM_DEBUG_GC(1 << 8)
-#define GALLIVM_DEBUG_DUMP_BC   (1 << 9)
+#define GALLIVM_DEBUG_PERF  (1 << 3)
+#define GALLIVM_DEBUG_GC(1 << 4)
+#define GALLIVM_DEBUG_DUMP_BC   (1 << 5)
 
+#define GALLIVM_PERF_NO_BRILINEAR  (1 << 0)
+#define GALLIVM_PERF_NO_RHO_APPROX (1 << 1)
+#define GALLIVM_PERF_NO_QUAD_LOD   (1 << 2)
+#define GALLIVM_PERF_NO_OPT(1 << 3)
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 
+extern unsigned gallivm_perf;
+
 #ifdef DEBUG
 extern unsigned gallivm_debug;
 #else
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_init.c 
b/src/gallium/auxiliary/gallivm/lp_bld_init.c
index 1f0a01cde6..3f7c4d3154 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_init.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_init.c
@@ -59,6 +59,17 @@ static const bool use_mcjit = USE_MCJIT;
 static bool use_mcjit = FALSE;
 #endif
 
+unsigned gallivm_perf = 0;
+
+static const struct debug_named_value lp_bld_perf_flags[] = {
+   { "no_brilinear", GALLIVM_PERF_NO_BRILINEAR, "disable brilinear 
optimization" },
+   { "no_rho_approx", GALLIVM_PERF_NO_RHO_APPROX, "disable rho_approx 
optimization" },
+   { "no_quad_lod", GALLIVM_PERF_NO_QUAD_LOD, "disable quad_lod optimization" 
},
+   { "nopt",   GALLIVM_PERF_NO_OPT, "disable optimization passes to speed up 
shader compilation" },
+   { "safemath", GALLIVM_PERF_NO_BRILINEAR | GALLIVM_PERF_NO_RHO_APPROX |
+ GALLIVM_PERF_NO_QUAD_LOD, "disable unsafe optimizations" },
+   DEBUG_NAMED_VALUE_END
+};
 
 #ifdef DEBUG
 unsigned gallivm_debug = 0;
@@ -67,11 +78,7 @@ static const struct debug_named_value lp_bld_debug_flags[] = 
{
{ "tgsi",   GALLIVM_DEBUG_TGSI, NULL },
{ "ir", GALLIVM_DEBUG_IR, NULL },
{ "asm",GALLIVM_DEBUG_ASM, NULL },
-   { "nopt",   GALLIVM_DEBUG_NO_OPT, NULL },
{ "perf",   GALLIVM_DEBUG_PERF, NULL },
-   { "no_brilinear", GALLIVM_DEBUG_NO_BRILINEAR, NULL },
-   { "no_rho_approx", GALLIVM_DEBUG_NO_RHO_APPROX, NULL },
-   { "no_quad_lod", GALLIVM_DEBUG_NO_QUAD_LOD, NULL },
{ "gc", GALLIVM_DEBUG_GC, NULL },
{ "dumpbc", GALLIVM_DEBUG_DUMP_BC, NULL },
DEBUG_NAMED_VALUE_END
@@ -136,7 +143,7 @@ create_pass_manager(struct gallivm_state *gallivm)
   free(td_str);
}
 
-   if ((gallivm_debug & GALLIVM_DEBUG_NO_OPT) == 0) {
+   if ((gallivm_perf & GALLIVM_PERF_NO_OPT) == 0) {
   /*
* TODO: Evaluate passes some more - keeping in mind
* both quality of generated code and compile times.
@@ -240,7 +247,7 @@ init_gallivm_engine(struct gallivm_state *gallivm)
   char *error = NULL;
   int ret;
 
-  if (gallivm_debug & GALLIVM_DEBUG_NO_OPT) {
+  if (gallivm_perf & GALLIVM_PERF_NO_OPT) {
  optlevel = None;
   }
   else {
@@ -420,6 +427,8 @@ lp_build_init(void)
gallivm_debug = debug_get_option_gallivm_debug();
 #endif
 
+   gallivm_perf = debug_get_flags_option("GALLIVM_PERF", lp_bld_perf_flags, 0 
);
+
lp_set_target_options();
 
util_cpu_detect();
@@ -589,10 +598,10 @@ gallivm_compile_module(struct gallivm_state *gallivm)
   LLVMWriteBitcodeToFile(gallivm->module, filename);
  

Re: [Mesa-dev] [PATCH] vulkan/wsi/wayland: don't double free on error in get_presentation_support

2018-10-05 Thread Eric Engestrom
On Friday, 2018-10-05 14:10:13 +0200, Philipp Zabel wrote:
> On Fri, 2018-10-05 at 10:13 +1000, Dave Airlie wrote:
> > From: Dave Airlie 
> > 
> > If we fail the init path then don't call the free path.
> > 
> > Found by coverity
> > ---
> >  src/vulkan/wsi/wsi_common_wayland.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/vulkan/wsi/wsi_common_wayland.c 
> > b/src/vulkan/wsi/wsi_common_wayland.c
> > index 6b34e21bd98..ce20583315a 100644
> > --- a/src/vulkan/wsi/wsi_common_wayland.c
> > +++ b/src/vulkan/wsi/wsi_common_wayland.c
> > @@ -455,8 +455,8 @@ wsi_wl_get_presentation_support(struct wsi_device 
> > *wsi_device,
> >(struct wsi_wayland *)wsi_device->wsi[VK_ICD_WSI_PLATFORM_WAYLAND];
> >  
> > struct wsi_wl_display display;
> > -   int ret = wsi_wl_display_init(wsi, , wl_display, false);
> 
> This removes int ret ...
> 
> > -   wsi_wl_display_finish();
> > +   if (wsi_wl_display_init(wsi, , wl_display, false))
> > +  wsi_wl_display_finish();
> >  
> > return ret == 0;
> 
> ... which is still used here.
> Also it looks like this calls finish in the error case (result != 0).

Agreed.

> 
> Maybe
> 
> int ret = wsi_wl_display_init(wsi, , wl_display, false);
> -   wsi_wl_display_finish();
> +   if (ret == VK_SUCCESS)
> +  wsi_wl_display_finish();
> 
> instead?

That patch would be:
Reviewed-by: Eric Engestrom 

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


[Mesa-dev] [PATCH] spirv: mark variables decorated with XfbBuffer as always active

2018-10-05 Thread Samuel Pitoiset
Otherwise, they are removed during NIR linking or in some
lowering passes.

Cc: Jason Ekstrand 
Cc: Timothy Arceri 
Signed-off-by: Samuel Pitoiset 
---
 src/compiler/spirv/vtn_variables.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/compiler/spirv/vtn_variables.c 
b/src/compiler/spirv/vtn_variables.c
index 358ff4bef7a..636fdb8689a 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1326,6 +1326,7 @@ apply_var_decoration(struct vtn_builder *b,
case SpvDecorationXfbBuffer:
   var_data->explicit_xfb_buffer = true;
   var_data->xfb_buffer = dec->literals[0];
+  var_data->always_active_io = true;
   break;
case SpvDecorationXfbStride:
   var_data->explicit_xfb_stride = true;
-- 
2.19.0

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


[Mesa-dev] [PATCH mesa] meson: fix vulkan only build with tests

2018-10-05 Thread Eric Engestrom
Fixes the build for
$ meson -D dri-drivers=[] -D gallium-drivers=[] -D build-tests=true

Compiling all this unused code isn't an actual problem, until you also
try to build the tests, at which point you get this:

  [213/705] Linking target src/mapi/glapi/glapi_static_check_table.
  FAILED: src/mapi/glapi/glapi_static_check_table
  ccache c++  -o src/mapi/glapi/glapi_static_check_table 
'src/mapi/glapi/src@mapi@glapi@@glapi_static_check_table@exe/tests_check_table.cpp.o'
 -Wl,--no-undefined -Wl,--as-needed -Wl,--start-group 
src/mapi/glapi/libglapi_static.a src/gtest/libgtest.a -Wl,--end-group -pthread 
'-Wl,-rpath,$ORIGIN/:$ORIGIN/../../gtest' 
-Wl,-rpath-link,/tmp/tmp.qyVZB5kQIB/mesa/build/src/mapi/glapi:/tmp/tmp.qyVZB5kQIB/mesa/build/src/gtest
  /usr/bin/ld: 
src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8810): 
undefined reference to `gl_dispatch_stub_343'
  /usr/bin/ld: 
src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8828): 
undefined reference to `gl_dispatch_stub_343'
  /usr/bin/ld: 
src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8840): 
undefined reference to `gl_dispatch_stub_344'
  /usr/bin/ld: 
src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8858): 
undefined reference to `gl_dispatch_stub_344'
  /usr/bin/ld: 
src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x8870): 
undefined reference to `gl_dispatch_stub_345'
  /usr/bin/ld: 
src/mapi/glapi/libglapi_static.a(glapi_getproc.c.o):(.data.rel.ro+0x): 
undefined reference to `gl_dispatch_stub_345'
  collect2: error: ld returned 1 exit status
  ninja: build stopped: subcommand failed.

Signed-off-by: Eric Engestrom 
---
 src/meson.build | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/meson.build b/src/meson.build
index 89ffaddf47b7286e4fe0..bde0e2aaca07931d3097 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -51,7 +51,9 @@ idep_git_sha1 = declare_dependency(
 
 subdir('gtest')
 subdir('util')
-subdir('mapi')
+if with_gles1 or with_gles2 or with_shared_glapi
+  subdir('mapi')
+endif
 # TODO: opengl
 subdir('compiler')
 subdir('egl/wayland/wayland-drm')
@@ -65,7 +67,9 @@ endif
 if with_dri_i965 or with_intel_vk
   subdir('intel')
 endif
-subdir('mesa')
+if with_opengl or with_gles1 or with_gles2 or with_shared_glapi
+  subdir('mesa')
+endif
 subdir('loader')
 if with_platform_haiku
   subdir('hgl')
-- 
Cheers,
  Eric

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


Re: [Mesa-dev] [PATCH] vulkan/wsi/wayland: don't double free on error in get_presentation_support

2018-10-05 Thread Philipp Zabel
On Fri, 2018-10-05 at 10:13 +1000, Dave Airlie wrote:
> From: Dave Airlie 
> 
> If we fail the init path then don't call the free path.
> 
> Found by coverity
> ---
>  src/vulkan/wsi/wsi_common_wayland.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/vulkan/wsi/wsi_common_wayland.c 
> b/src/vulkan/wsi/wsi_common_wayland.c
> index 6b34e21bd98..ce20583315a 100644
> --- a/src/vulkan/wsi/wsi_common_wayland.c
> +++ b/src/vulkan/wsi/wsi_common_wayland.c
> @@ -455,8 +455,8 @@ wsi_wl_get_presentation_support(struct wsi_device 
> *wsi_device,
>(struct wsi_wayland *)wsi_device->wsi[VK_ICD_WSI_PLATFORM_WAYLAND];
>  
> struct wsi_wl_display display;
> -   int ret = wsi_wl_display_init(wsi, , wl_display, false);

This removes int ret ...

> -   wsi_wl_display_finish();
> +   if (wsi_wl_display_init(wsi, , wl_display, false))
> +  wsi_wl_display_finish();
>  
> return ret == 0;

... which is still used here.
Also it looks like this calls finish in the error case (result != 0).

Maybe

int ret = wsi_wl_display_init(wsi, , wl_display, false);
-   wsi_wl_display_finish();
+   if (ret == VK_SUCCESS)
+  wsi_wl_display_finish();

instead?

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


Re: [Mesa-dev] [PATCH] radv: check return from mkdir

2018-10-05 Thread Gustaw Smolarczyk
pt., 5 paź 2018 o 13:10 Grazvydas Ignotas  napisał(a):
>
> On Fri, Oct 5, 2018 at 3:38 AM Dave Airlie  wrote:
> >
> > From: Dave Airlie 
> >
> > There may be some security or sandbox reason this might fail, so
> > check and fail appropriately.
> > ---
> >  src/amd/vulkan/radv_meta.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/amd/vulkan/radv_meta.c b/src/amd/vulkan/radv_meta.c
> > index 1ec8896afa2..6616b1da65a 100644
> > --- a/src/amd/vulkan/radv_meta.c
> > +++ b/src/amd/vulkan/radv_meta.c
> > @@ -248,7 +248,9 @@ radv_builtin_cache_path(char *path)
> >
> > strcpy(path, pwd.pw_dir);
> > strcat(path, "/.cache");
> > -   mkdir(path, 0755);
> > +   ret = mkdir(path, 0755);
> > +   if (ret == -1)
>
> if (ret == -1 && errno != EEXIST) ?

Won't EEXIST be returned even in case the path already exists but is
not a directory? [1]

Regards,
Gustaw Smolarczyk

[1] http://man7.org/linux/man-pages/man2/mkdir.2.html

>
> > +  return false;
> >
> > ret = snprintf(path, PATH_MAX + 1, "%s%s%zd",
> >pwd.pw_dir, suffix2, sizeof(void *) * 8);
> > --
> > 2.17.1
>
> Gražvydas
> ___
> 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] vulkan/wsi/wayland: don't double free on error in get_presentation_support

2018-10-05 Thread Lionel Landwerlin

Reviewed-by: Lionel Landwerlin 

On 05/10/2018 01:13, Dave Airlie wrote:

From: Dave Airlie 

If we fail the init path then don't call the free path.

Found by coverity
---
  src/vulkan/wsi/wsi_common_wayland.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common_wayland.c 
b/src/vulkan/wsi/wsi_common_wayland.c
index 6b34e21bd98..ce20583315a 100644
--- a/src/vulkan/wsi/wsi_common_wayland.c
+++ b/src/vulkan/wsi/wsi_common_wayland.c
@@ -455,8 +455,8 @@ wsi_wl_get_presentation_support(struct wsi_device 
*wsi_device,
(struct wsi_wayland *)wsi_device->wsi[VK_ICD_WSI_PLATFORM_WAYLAND];
  
 struct wsi_wl_display display;

-   int ret = wsi_wl_display_init(wsi, , wl_display, false);
-   wsi_wl_display_finish();
+   if (wsi_wl_display_init(wsi, , wl_display, false))
+  wsi_wl_display_finish();
  
 return ret == 0;

  }



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


Re: [Mesa-dev] [PATCH] radv: check return from mkdir

2018-10-05 Thread Grazvydas Ignotas
On Fri, Oct 5, 2018 at 3:38 AM Dave Airlie  wrote:
>
> From: Dave Airlie 
>
> There may be some security or sandbox reason this might fail, so
> check and fail appropriately.
> ---
>  src/amd/vulkan/radv_meta.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_meta.c b/src/amd/vulkan/radv_meta.c
> index 1ec8896afa2..6616b1da65a 100644
> --- a/src/amd/vulkan/radv_meta.c
> +++ b/src/amd/vulkan/radv_meta.c
> @@ -248,7 +248,9 @@ radv_builtin_cache_path(char *path)
>
> strcpy(path, pwd.pw_dir);
> strcat(path, "/.cache");
> -   mkdir(path, 0755);
> +   ret = mkdir(path, 0755);
> +   if (ret == -1)

if (ret == -1 && errno != EEXIST) ?

> +  return false;
>
> ret = snprintf(path, PATH_MAX + 1, "%s%s%zd",
>pwd.pw_dir, suffix2, sizeof(void *) * 8);
> --
> 2.17.1

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


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

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

Bug 104602 Summary: [apitrace] Graphical artifacts in Civilization VI on RX Vega
https://bugs.freedesktop.org/show_bug.cgi?id=104602

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

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


[Mesa-dev] [ANNOUNCE] mesa 18.2.2

2018-10-05 Thread Juan A. Suarez Romero
Mesa 18.2.2 is now available.

In this release we have:

Different patches for the DirectX9 and DRI state trackers.

A patch to implement vkAcquireNextImage2 in the Intel and AMD vulkan drivers, as
well as a patch for adding support for protected memory properties in
GetPhysicalDeviceProperties2() for the former driver.

RADV also gets a patch to fix some issues with reflections in GTA V, and a path
to fix a GPU hang in SteamVR with Vega.

Finally, there are more fixes for Radeonsi, nvc0, vc4, and vulkan code.


Alex Deucher (1):
  pci_ids: add new polaris pci id

Andres Rodriguez (1):
  radv: only emit ZPASS_DONE for timestamp queries on gfx queues

Axel Davy (3):
  st/nine: Clamp RCP when 0*inf!=0
  st/nine: Avoid redundant SetCursorPos calls
  st/nine: Increase maximum number of temp registers

Dylan Baker (1):
  meson: Don't compile pipe loader with dri support when not using dri

Eric Anholt (1):
  vc4: Fix sin(0.0) and cos(0.0) accuracy to fix SDL rendering rotation.

Eric Engestrom (1):
  vulkan/wsi/display: check if wsi_swapchain_init() succeeded

Jason Ekstrand (1):
  anv,radv: Implement vkAcquireNextImage2

Juan A. Suarez Romero (3):
  docs: add sha256 checksums for 18.2.1
  Update version to 18.2.2
  docs: add release notes for 18.2.2

Leo Liu (1):
  radeon/uvd: use bitstream coded number for symbols of Huffman tables

Marek Olšák (2):
  glsl_to_tgsi: invert gl_SamplePosition.y for the default framebuffer
  radeonsi: NaN should pass kill_if

Maxime (1):
  vulkan: Disable randr lease for libxcb < 1.13

Michal Srb (1):
  st/dri: don't set queryDmaBufFormats/queryDmaBufModifiers if the driver 
does not implement it

Rhys Perry (2):
  nvc0: Update counter reading shaders to new NVC0_CB_AUX_MP_INFO
  nvc0: fix bindless multisampled images on Maxwell+

Samuel Iglesias Gonsálvez (1):
  anv: Add support for protected memory properties on 
anv_GetPhysicalDeviceProperties2()

Samuel Pitoiset (1):
  radv: use the resolve compute path if dest uses multiple layers

Stuart Young (1):
  docs: Update FAQ with respect to s3tc support

Timothy Arceri (1):
  radeonsi: add a workaround for bitfield_extract when count is 0

git tag: mesa-18.2.2

https://mesa.freedesktop.org/archive/mesa-18.2.2.tar.gz
MD5:  bc9b82d55209919a0fb242fb5aac0dad  mesa-18.2.2.tar.gz
SHA1: 3bd974a1f6b783d1a49b989922156ec82a6e789d  mesa-18.2.2.tar.gz
SHA256: c51711168971957037cc7e3e19e8abe1ec6eeab9cf236d419a1e7728a41cac8a  
mesa-18.2.2.tar.gz
SHA512: 
68e4794bf5671925b8ae43ab44a3313fa42ddd0cbfc294481b6477b73a3a29590967361bf4a66f8c2eb070903707e7cbd22981ea7596a7acb92091a60faa47a0
  mesa-18.2.2.tar.gz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.2.2.tar.gz.sig

https://mesa.freedesktop.org/archive/mesa-18.2.2.tar.xz
MD5:  5931dd76a7533c7c5e702a4e5c00d3bb  mesa-18.2.2.tar.xz
SHA1: 4abcb04c7fa261c60775a826a39ae0b28ab3856b  mesa-18.2.2.tar.xz
SHA256: c3ba82b12a89d3d9fed2bdd96b4702dbb7ab675034650a8b1b718320daf073c4  
mesa-18.2.2.tar.xz
SHA512: 
35c27f0673abd35d0581db34b6ad646058523dd826ff751df718e1f9d6a996409a0c5b313fbbf177058d9610a53d646f858fb86537e3ea1000df5edbddcf043a
  mesa-18.2.2.tar.xz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.2.2.tar.xz.sig



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] [Bug 77449] Tracker bug for all bugs related to Steam titles

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

Bug 104602 Summary: [apitrace] Graphical artifacts in Civilization VI on RX Vega
https://bugs.freedesktop.org/show_bug.cgi?id=104602

   What|Removed |Added

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

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


Re: [Mesa-dev] [PATCH] gallivm: Make it possible to disable some optimization shortcuts in release builds

2018-10-05 Thread Gert Wollny
Am Donnerstag, den 04.10.2018, 17:16 + schrieb Roland Scheidegger:
> I've attached the diff (no guarantees, can't test it right now).
> nopt is useful because if there's tons of shaders to compile
> compilation is quite a bit faster (but generally you really don't
> want to do this).
> Not sure about why dumpbc isn't debug only, might not have been a
> deliberate decision. Although using a separate variable has the
> advantage that the compiler can optimise out the unneeded code (we
> didn't really care).
Okay, then I will add the nopt t my patch and keep the variables 
separate. 

thanks, 
Gert 


> 
> 
> From: Gert Wollny 
> Sent: Thursday, October 4, 2018 12:58:41 AM
> To: Roland Scheidegger; mesa-dev@lists.freedesktop.org
> Cc: imir...@alum.mit.edu; Jose Fonseca
> Subject: Re: [PATCH] gallivm: Make it possible to disable some
> optimization shortcuts in release builds
> 
> Am Mittwoch, den 03.10.2018, 20:47 + schrieb Roland Scheidegger:
> > Is it worth it splitting out to another var?
> > We actually have code branches internally where we just define the
> > gallivm_debug var always, and some of the debug flags outside the
> > #ifdef
> > debug (we'll actually need more than just these 3 accessible
> > outside
> > debug builds).
> > If you think this is cleaner though I suppose we can deal with
> > it...
> 
> One part of me says that keeping it separate is indeed cleaner,
> another
> says it  doesn't really matter. Why don't you upstream your version
> with all the flags you need exposed (and document the not so obvious
> ones)?
> 
> Best,
> Gert
> 
> 
> > 
> > Roland
> > 
> > 
> > 
> > On 10/03/2018 09:52 AM, Gert Wollny wrote:
> > > From: Gert Wollny 
> > > 
> > > For testing it is of interetest that all tests of dEQP pass, e.g.
> > > to test
> > > virglrenderer on a host only providing software rendering like in
> > > a
> > > CI.
> > > Hence make it possible to disable certain optimizations that make
> > > tests fail.
> > > 
> > > While we are there also add some documentation to the flags to
> > > make
> > > it clear
> > > that this is opt-out.
> > > 
> > > Setting the environment variable "GALLIVM_PERF=disable_all" can
> > > be
> > > used to make
> > > the follwing tests pass in release mode:
> > > 
> > >dEQP-GLES2.functional.texture.mipmap.2d.affine.*_linear_*
> > >dEQP-GLES2.functional.texture.mipmap.cube.generate.*
> > >dEQP-
> > > GLES2.functional.texture.vertex.2d.filtering.*_mipmap_linear_*
> > >dEQP-GLES2.functional.texture.vertex.2d.wrap.*
> > > 
> > > Related:
> > >https://na01.safelinks.protection.outlook.com/?url=https%3A%2F
> > > %2
> > > Fbugs.freedesktop.org%2Fshow_bug.cgi%3Fid%3D94957data=02%7C0
> > > 1%
> > > 7Csroland%40vmware.com%7Cd8a63cda397e40a6d42808d6290556e7%7Cb3913
> > > 8c
> > > a3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636741500244575630sdata=
> > > UU
> > > 5W053FLBScYWpQtw9yANGRDCcKYQdS4eRyl7k9u9k%3Dreserved=0
> > > 
> > > Signed-off-by: Gert Wollny 
> > > ---
> > >   src/gallium/auxiliary/gallivm/lp_bld_debug.h  | 13 
> > > -
> > >   src/gallium/auxiliary/gallivm/lp_bld_init.c   | 15
> > > ---
> > >   src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c |  6 +++---
> > >   src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c   |  6 +++---
> > >   4 files changed, 26 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_debug.h
> > > b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
> > > index f96a1afa7a..ce09d789cb 100644
> > > --- a/src/gallium/auxiliary/gallivm/lp_bld_debug.h
> > > +++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.h
> > > @@ -41,18 +41,21 @@
> > >   #define GALLIVM_DEBUG_ASM   (1 << 2)
> > >   #define GALLIVM_DEBUG_NO_OPT(1 << 3)
> > >   #define GALLIVM_DEBUG_PERF  (1 << 4)
> > > -#define GALLIVM_DEBUG_NO_BRILINEAR  (1 << 5)
> > > -#define GALLIVM_DEBUG_NO_RHO_APPROX (1 << 6)
> > > -#define GALLIVM_DEBUG_NO_QUAD_LOD   (1 << 7)
> > > -#define GALLIVM_DEBUG_GC(1 << 8)
> > > -#define GALLIVM_DEBUG_DUMP_BC   (1 << 9)
> > > +#define GALLIVM_DEBUG_GC(1 << 5)
> > > +#define GALLIVM_DEBUG_DUMP_BC   (1 << 6)
> > > 
> > > 
> > > +#define GALLIVM_PERF_NO_BRILINEAR  (1 << 0)
> > > +#define GALLIVM_PERF_NO_RHO_APPROX (1 << 1)
> > > +#define GALLIVM_PERF_NO_QUAD_LOD   (1 << 2)
> > > +
> > >   #ifdef __cplusplus
> > >   extern "C" {
> > >   #endif
> > > 
> > > 
> > > +extern unsigned gallivm_perf;
> > > +
> > >   #ifdef DEBUG
> > >   extern unsigned gallivm_debug;
> > >   #else
> > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_init.c
> > > b/src/gallium/auxiliary/gallivm/lp_bld_init.c
> > > index 1f0a01cde6..c8b2a7fcc9 100644
> > > --- a/src/gallium/auxiliary/gallivm/lp_bld_init.c
> > > +++ b/src/gallium/auxiliary/gallivm/lp_bld_init.c
> > > @@ -59,6 +59,16 @@ static const bool use_mcjit = USE_MCJIT;
> > >   static bool use_mcjit = FALSE;
> > >   #endif
> > > 
> > > +unsigned 

Re: [Mesa-dev] [PATCH] glsl: prevent qualifiers modification of predeclared variables

2018-10-05 Thread andrey simiklit
Hello,

+ i...@freedesktop.org
and add missing bugzilla link
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108247

Regards,
Andrii.

On Thu, Oct 4, 2018 at 5:08 PM  wrote:

> From: Andrii Simiklit 
>
> GLSL 3.7 (Identifiers):
> "However, as noted in the specification, there are some cases where
> previously declared variables can be redeclared to change or add some
> property, and predeclared "gl_" names are allowed to be redeclared in a
> shader only for these specific purposes.  More generally, it is an error to
> redeclare a variable, including those starting "gl_"."
>
> This patch should fix piglit tests:
> 'clip-distance-redeclare-without-inout.frag'
> 'clip-distance-redeclare-without-inout.vert'
> and leads to regression in clip-distance-out-values.shader_test
> but probably a fix should be made in the test.
>
> As far as I understood following mailing thread:
> https://lists.freedesktop.org/archives/piglit/2013-October/007935.html
> looks like we have accepted to remove an ability to change qualifiers
> but have not done it yet. Unless I missed something)
>
> Fixes: 8e6cb9fe51a2 "glsl: Refactor AST-to-HIR code handling variable
>  redeclarations"
> Signed-off-by: Andrii Simiklit 
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp
> b/src/compiler/glsl/ast_to_hir.cpp
> index 93e7c8ec33..e26ae6b92a 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -4240,10 +4240,15 @@ get_variable_being_redeclared(ir_variable
> **var_ptr, YYLTYPE loc,
>  */
> if (earlier->type->is_unsized_array() && var->type->is_array()
> && (var->type->fields.array == earlier->type->fields.array)) {
> -  /* FINISHME: This doesn't match the qualifiers on the two
> -   * FINISHME: declarations.  It's not 100% clear whether this is
> -   * FINISHME: required or not.
> -   */
> +
> +  if ((strcmp("gl_ClipDistance", var->name) == 0) &&
> +  earlier->data.mode != var->data.mode) {
> + _mesa_glsl_error(, state,
> +  "redeclaration of '%s %s' with incorrect
> qualifiers '%s'.",
> +  mode_string(earlier),
> +  var->name,
> +  mode_string(var));
> +  }
>
>const int size = var->type->array_size();
>check_builtin_array_max_size(var->name, size, loc, state);
> --
> 2.17.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] gbm: Add GBM_FORMAT_ARGB1555 support

2018-10-05 Thread Michel Dänzer
From: Michel Dänzer 

Signed-off-by: Michel Dänzer 
---
 src/gbm/backends/dri/gbm_dri.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index b3d6ceb15a3..35ec3a1c3a2 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -552,6 +552,10 @@ static const struct gbm_dri_visual gbm_dri_visuals_table[] 
= {
  GBM_FORMAT_GR88, __DRI_IMAGE_FORMAT_GR88,
  { 0x00ff, 0xff00, 0x, 0x },
},
+   {
+ GBM_FORMAT_ARGB1555, __DRI_IMAGE_FORMAT_ARGB1555,
+ { 0x7c00, 0x03e0, 0x001f, 0x8000 },
+   },
{
  GBM_FORMAT_RGB565, __DRI_IMAGE_FORMAT_RGB565,
  { 0xf800, 0x07e0, 0x001f, 0x },
-- 
2.19.0

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


[Mesa-dev] [PATCH 1/2] st/dri: Handle BGRA5551 format

2018-10-05 Thread Michel Dänzer
From: Michel Dänzer 

Signed-off-by: Michel Dänzer 
---
 src/gallium/state_trackers/dri/dri2.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/gallium/state_trackers/dri/dri2.c 
b/src/gallium/state_trackers/dri/dri2.c
index b17c5e16ede..4efc4334b65 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -101,6 +101,10 @@ static int convert_fourcc(int format, int 
*dri_components_p)
 {
int dri_components;
switch(format) {
+   case __DRI_IMAGE_FOURCC_ARGB1555:
+  format = __DRI_IMAGE_FORMAT_ARGB1555;
+  dri_components = __DRI_IMAGE_COMPONENTS_RGBA;
+  break;
case __DRI_IMAGE_FOURCC_RGB565:
   format = __DRI_IMAGE_FORMAT_RGB565;
   dri_components = __DRI_IMAGE_COMPONENTS_RGB;
@@ -187,6 +191,9 @@ static int convert_fourcc(int format, int *dri_components_p)
 static int convert_to_fourcc(int format)
 {
switch(format) {
+   case __DRI_IMAGE_FORMAT_ARGB1555:
+  format = __DRI_IMAGE_FOURCC_ARGB1555;
+  break;
case __DRI_IMAGE_FORMAT_RGB565:
   format = __DRI_IMAGE_FOURCC_RGB565;
   break;
@@ -231,6 +238,9 @@ static enum pipe_format dri2_format_to_pipe_format (int 
format)
enum pipe_format pf;
 
switch (format) {
+   case __DRI_IMAGE_FORMAT_ARGB1555:
+  pf = PIPE_FORMAT_B5G5R5A1_UNORM;
+  break;
case __DRI_IMAGE_FORMAT_RGB565:
   pf = PIPE_FORMAT_B5G6R5_UNORM;
   break;
@@ -523,6 +533,9 @@ dri_image_drawable_get_buffers(struct dri_drawable 
*drawable,
   }
 
   switch (pf) {
+  case PIPE_FORMAT_B5G5R5A1_UNORM:
+ image_format = __DRI_IMAGE_FORMAT_ARGB1555;
+ break;
   case PIPE_FORMAT_B5G6R5_UNORM:
  image_format = __DRI_IMAGE_FORMAT_RGB565;
  break;
-- 
2.19.0

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


[Mesa-dev] [PATCH] docs: update calendar

2018-10-05 Thread Juan A. Suarez Romero
I'll take care of 18.2 releases series on Andres behalf.

CC: Andres Gomez 
CC: Dylan Baker 
CC: Emil Velikov 
---
 docs/release-calendar.html | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/release-calendar.html b/docs/release-calendar.html
index f92f7ba00fb..379d1a089e5 100644
--- a/docs/release-calendar.html
+++ b/docs/release-calendar.html
@@ -42,25 +42,25 @@ if you'd like to nominate a patch in the next stable 
release.
 18.2
 2018-10-03
 18.2.2
-Andres Gomez
+Juan A. Suarez
 
 
 
 2018-10-17
 18.2.3
-Andres Gomez
+Juan A. Suarez
 
 
 
 2018-10-31
 18.2.4
-Andres Gomez
+Juan A. Suarez
 
 
 
 2018-11-14
 18.2.5
-Andres Gomez
+Juan A. Suarez
 Last planned 18.2.x release
 
 
-- 
2.17.1

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


[Mesa-dev] [Bug 35268] initial-exec TLS model breaks dlopen'ed libGL

2018-10-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=35268

Ross Burton  changed:

   What|Removed |Added

 CC||raj.k...@gmail.com

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


[Mesa-dev] [Bug 97444] mesa git crashes in libxshmfence

2018-10-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=97444

Michel Dänzer  changed:

   What|Removed |Added

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

--- Comment #13 from Michel Dänzer  ---
(In reply to Fabian Maurer from comment #12)
> No, doesn't seem to happen anymore.

Glad to hear that, thanks for the follow-up!

-- 
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 v5 2/2] virgl: Pass resource size and transfer offsets

2018-10-05 Thread Gert Wollny
Am Donnerstag, den 04.10.2018, 10:48 -0700 schrieb Gurchetan Singh:
> 
> The idea is to get rid of any adjustments on both the Mesa /
> virglrenderer sides -- so transfer size is just what's needed to
> transfer the box size, and the offset is the just offset into the
> iovec from which the transfer will start.  For v1, we can just
> specify an offset of zero on the virglrenderer side.

I certainly see you point, the problem is (as Tomeu also pointed out)
we would like to enable the CI *now*, and all what is missing is this
patch. If we want to correct the protocol it will likely take another
couple of weeks to get it right and do all the testing, and this while
facing the more challanging changes regarding memory handling, adding
more GL extensions etc. 

> vtest isn't used in production but it will be used in the CI.  Also,
> it's a simpler model of virgl3d and is useful for experimenting with
> new protocol additions (hostmap, vulkan).  We should fix this now --
> it could take a while to disentangle the workarounds should anyone
> look at this again ...
My proposal to unblock this would be open an issue against
virglrenderer (because the problem is on both sides) and doeument there
where the problems are. Whether this gets fixed by correcting this
code, or completely replacing things (like Tomeu proposed) is another
story.

What do you think? 

Gert 

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


Re: [Mesa-dev] [PATCH 3/5] radv: remove unsigned comparison against 0

2018-10-05 Thread Samuel Pitoiset

Patches 2-3 are:

Reviewed-by: Samuel Pitoiset 

On 10/5/18 2:00 AM, Dave Airlie wrote:

From: Dave Airlie 

The value is always >= 0 here.

Found by coverity
---
  src/amd/vulkan/radv_nir_to_llvm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index 32d347ebd0f..bf6b8aee50e 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2282,7 +2282,7 @@ si_llvm_init_export_args(struct radv_shader_context *ctx,
return;
  
  	bool is_16bit = ac_get_type_size(LLVMTypeOf(values[0])) == 2;

-   if (ctx->stage == MESA_SHADER_FRAGMENT && target >= 
V_008DFC_SQ_EXP_MRT) {
+   if (ctx->stage == MESA_SHADER_FRAGMENT) {
unsigned index = target - V_008DFC_SQ_EXP_MRT;
unsigned col_format = (ctx->options->key.fs.col_format >> (4 * 
index)) & 0xf;
bool is_int8 = (ctx->options->key.fs.is_int8 >> index) & 1;


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


Re: [Mesa-dev] [PATCH] radv: don't pass shader key by copy

2018-10-05 Thread Samuel Pitoiset

Reviewed-by: Samuel Pitoiset 

On 10/5/18 1:18 AM, Dave Airlie wrote:

From: Dave Airlie 

Coverity pointed out we were copying 168 bytes here unnecessarily.
---
  src/amd/vulkan/radv_pipeline.c | 13 ++---
  1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 4b626a59af2..c1fe4ece396 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -1989,7 +1989,7 @@ static
  void radv_create_shaders(struct radv_pipeline *pipeline,
   struct radv_device *device,
   struct radv_pipeline_cache *cache,
- struct radv_pipeline_key key,
+ const struct radv_pipeline_key *key,
   const VkPipelineShaderStageCreateInfo **pStages,
   const VkPipelineCreateFlags flags)
  {
@@ -2013,7 +2013,7 @@ void radv_create_shaders(struct radv_pipeline *pipeline,
}
}
  
-	radv_hash_shaders(hash, pStages, pipeline->layout, , get_hash_flags(device));

+   radv_hash_shaders(hash, pStages, pipeline->layout, key, 
get_hash_flags(device));
memcpy(gs_copy_hash, hash, 20);
gs_copy_hash[0] ^= 1;
  
@@ -2094,7 +2094,7 @@ void radv_create_shaders(struct radv_pipeline *pipeline,

nir_print_shader(nir[i], stderr);
}
  
-	radv_fill_shader_keys(keys, , nir);

+   radv_fill_shader_keys(keys, key, nir);
  
  	if (nir[MESA_SHADER_FRAGMENT]) {

if (!pipeline->shaders[MESA_SHADER_FRAGMENT]) {
@@ -3496,9 +3496,8 @@ radv_pipeline_init(struct radv_pipeline *pipeline,
pStages[stage] = >pStages[i];
}
  
-	radv_create_shaders(pipeline, device, cache,

-   radv_generate_graphics_pipeline_key(pipeline, 
pCreateInfo, , has_view_index),
-   pStages, pCreateInfo->flags);
+   struct radv_pipeline_key key = 
radv_generate_graphics_pipeline_key(pipeline, pCreateInfo, , 
has_view_index);
+   radv_create_shaders(pipeline, device, cache, , pStages, 
pCreateInfo->flags);
  
  	pipeline->graphics.spi_baryc_cntl = S_0286E0_FRONT_FACE_ALL_BITS(1);

radv_pipeline_init_multisample_state(pipeline, , pCreateInfo);
@@ -3731,7 +3730,7 @@ static VkResult radv_compute_pipeline_create(
assert(pipeline->layout);
  
  	pStages[MESA_SHADER_COMPUTE] = >stage;

-   radv_create_shaders(pipeline, device, cache, (struct radv_pipeline_key) 
{0}, pStages, pCreateInfo->flags);
+   radv_create_shaders(pipeline, device, cache, &(struct radv_pipeline_key) 
{0}, pStages, pCreateInfo->flags);
  
  	pipeline->user_data_0[MESA_SHADER_COMPUTE] = radv_pipeline_stage_to_user_data_0(pipeline, MESA_SHADER_COMPUTE, device->physical_device->rad_info.chip_class);

pipeline->need_indirect_descriptor_sets |= 
pipeline->shaders[MESA_SHADER_COMPUTE]->info.need_indirect_descriptor_sets;


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


Re: [Mesa-dev] [PATCH] radv: check return from mkdir

2018-10-05 Thread Samuel Pitoiset

Reviewed-by: Samuel Pitoiset 

On 10/5/18 2:37 AM, Dave Airlie wrote:

From: Dave Airlie 

There may be some security or sandbox reason this might fail, so
check and fail appropriately.
---
  src/amd/vulkan/radv_meta.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_meta.c b/src/amd/vulkan/radv_meta.c
index 1ec8896afa2..6616b1da65a 100644
--- a/src/amd/vulkan/radv_meta.c
+++ b/src/amd/vulkan/radv_meta.c
@@ -248,7 +248,9 @@ radv_builtin_cache_path(char *path)
  
  	strcpy(path, pwd.pw_dir);

strcat(path, "/.cache");
-   mkdir(path, 0755);
+   ret = mkdir(path, 0755);
+   if (ret == -1)
+  return false;
  
  	ret = snprintf(path, PATH_MAX + 1, "%s%s%zd",

   pwd.pw_dir, suffix2, sizeof(void *) * 8);


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


[Mesa-dev] [PATCH] glsl: fix array assignments of a swizzled vector

2018-10-05 Thread Ilia Mirkin
This happens in situations where we might do

  vec.wzyx[i] = ...

The swizzle would get effectively ignored because of the interaction
between how ir_assignment->set_lhs works and overwriting the write_mask.

Fixes the following WebGL test:

https://www.khronos.org/registry/webgl/sdk/tests/conformance2/glsl3/vector-dynamic-indexing-swizzled-lvalue.html

Signed-off-by: Ilia Mirkin 
Cc: mesa-sta...@lists.freedesktop.org
---
 src/compiler/glsl/lower_vector_derefs.cpp | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/lower_vector_derefs.cpp 
b/src/compiler/glsl/lower_vector_derefs.cpp
index 7583d1fdd3e..af046966491 100644
--- a/src/compiler/glsl/lower_vector_derefs.cpp
+++ b/src/compiler/glsl/lower_vector_derefs.cpp
@@ -59,21 +59,27 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
if (!deref->array->type->is_vector())
   return ir_rvalue_enter_visitor::visit_enter(ir);
 
-   ir_dereference *const new_lhs = (ir_dereference *) deref->array;
-   ir->set_lhs(new_lhs);
+   ir_rvalue *const new_lhs = deref->array;
 
void *mem_ctx = ralloc_parent(ir);
ir_constant *old_index_constant =
   deref->array_index->constant_expression_value(mem_ctx);
if (!old_index_constant) {
+  ir->set_lhs(new_lhs);
   ir->rhs = new(mem_ctx) ir_expression(ir_triop_vector_insert,
new_lhs->type,
new_lhs->clone(mem_ctx, NULL),
ir->rhs,
deref->array_index);
   ir->write_mask = (1 << new_lhs->type->vector_elements) - 1;
+   } else if (new_lhs->ir_type != ir_type_swizzle) {
+  ir->set_lhs(new_lhs);
+  ir->write_mask = 1 << old_index_constant->get_uint_component(0);
} else {
-  ir->write_mask = 1 << old_index_constant->get_int_component(0);
+  // If there "new" lhs is a swizzle, use the set_lhs helper to instead
+  // swizzle the rhs.
+  unsigned component[1] = { old_index_constant->get_uint_component(0) };
+  ir->set_lhs(new(mem_ctx) ir_swizzle(new_lhs, component, 1));
}
 
return ir_rvalue_enter_visitor::visit_enter(ir);
-- 
2.16.4

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