Re: [Mesa-dev] [PATCH v2] glsl: fix array assignments of a swizzled vector
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
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
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
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
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
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
--- 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
--- 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
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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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