Re: [Mesa-dev] [PATCH 41/59] intel/compiler: split is_partial_write() into two variants
On Wed, 2018-12-12 at 14:15 +0200, Pohjolainen, Topi wrote: > On Wed, Dec 12, 2018 at 09:48:20AM +0100, Iago Toral wrote: > > On Tue, 2018-12-11 at 18:59 +0200, Pohjolainen, Topi wrote: > > > On Fri, Dec 07, 2018 at 03:30:11PM +0200, Pohjolainen, Topi > > > wrote: > > > > On Tue, Dec 04, 2018 at 08:17:05AM +0100, Iago Toral Quiroga > > > > wrote: > > > > > This function is used in two different scenarios that for 32- > > > > > bit > > > > > instructions are the same, but for 16-bit instructions are > > > > > not. > > > > > > > > > > One scenario is that in which we are working at a SIMD8 > > > > > register > > > > > level and we need to know if a register is fully defined or > > > > > written. > > > > > This is useful, for example, in the context of liveness > > > > > analysis > > > > > or > > > > > register allocation, where we work with units of registers. > > > > > > > > > > The other scenario is that in which we want to know if an > > > > > instruction > > > > > is writing a full scalar component or just some subset of it. > > > > > This is > > > > > useful, for example, in the context of some optimization > > > > > passes > > > > > like copy propagation. > > > > > > > > > > For 32-bit instructions (or larger), a SIMD8 dispatch will > > > > > always > > > > > write > > > > > at least a full SIMD8 register (32B) if the write is not > > > > > partial. > > > > > The > > > > > function is_partial_write() checks this to determine if we > > > > > have a > > > > > partial > > > > > write. However, when we deal with 16-bit instructions, that > > > > > logic > > > > > disables > > > > > some optimizations that should be safe. For example, a SIMD8 > > > > > 16- > > > > > bit MOV will > > > > > only update half of a SIMD register, but it is still a > > > > > complete > > > > > write of the > > > > > variable for a SIMD8 dispatch, so we should not prevent copy > > > > > propagation in > > > > > this scenario because we don't write all 32 bytes in the SIMD > > > > > register > > > > > or because the write starts at offset 16B (wehere we pack > > > > > components Y or > > > > > W of 16-bit vectors). > > > > > > > > > > This is a problem for SIMD8 executions (VS, TCS, TES, GS) of > > > > > 16- > > > > > bit > > > > > instructions, which lose a number of optimizations because of > > > > > this, most > > > > > important of which is copy-propagation. > > > > > > > > > > This patch splits is_partial_write() into > > > > > is_partial_reg_write(), > > > > > which > > > > > represents the current is_partial_write(), useful for things > > > > > like > > > > > liveness analysis, and is_partial_var_write(), which > > > > > considers > > > > > the dispatch size to check if we are writing a full variable > > > > > (rather > > > > > than a full register) to decide if the write is partial or > > > > > not, > > > > > which > > > > > is what we really want in many optimization passes. > > > > > > I actually started wondering why would liveness analysis and > > > register > > > coalescing need to treat the 16-bit SIMD8 case differently than > > > optimizations. > > > In virtual register space nothing would read or write the unused > > > second half > > > of the register in case of 16-bit type and SIMD8. > > > > True, we might be able to use the "variable" version in more cases. > > I > > was trying to be conservative when I implemented this because I > > don't > > think the half-float CTS tests provides a good testing ground for > > all > > aspects of the compiler. I can try that and see if it breaks > > anything > > though. > > > > > Real register allocation in turn should be orthogonal to how > > > things > > > are > > > allocated in virtual space. And I guess even there we wouldn't be > > > interested > > > of packing two 16-bit typed SIMD8 variables into one and same > > > hardware > > > register. It is SIMD16 where we get more pressure into register > > > space > > > anyway > > > and there the 16-bit typed variables occupy full registers. In > > > other > > > words, > > > if things fit in SIMD16, would we bother packing things more > > > tightly > > > in > > > SIMD8? Or even if SIMD8 was the only option, would we be > > > interested > > > packing > > > channels for two variables in one hw reg even then? > > > > > > Jason, we discussed this a little in the spring time. > > > > > > As a recap my approach shortly. Instead of ignoring the second > > > half > > > of > > > registers case by case I addressed it more generally: > > > > > > - changed all the open coded checks to use helpers, > > > - added a padding bit into fs_reg telling about the unused space, > > > - change nir -> fs step to set that bit for 16-bit typed regs > > > - and finally changed the helpers to consider the padding bit. > > > > So if I understand how this works, you mostly make the vgrf > > infrastructure think that half-float registers actually use twice > > the > > space they require by including the padding into the > > component_size() > > help
Re: [Mesa-dev] [PATCH] radv/xfb: fix counter buffer bounds checks.
Reviewed-by: Samuel Pitoiset On 12/13/18 4:32 AM, Dave Airlie wrote: From: Dave Airlie If we gave this function 0 counter buffers, we'd still try and access pCounterBuffers[0] as this check was incorrect. Fixes crash with ext_transform_feedback-pipeline-basic-primgen on zink on radv. Fixes: 677b496b6 (radv: fix begin/end transform feedback with 0 counter buffers.) Signed-off-by: Dave Airlie --- src/amd/vulkan/radv_cmd_buffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c index 23909a0f7dd..e08d37ee1f2 100644 --- a/src/amd/vulkan/radv_cmd_buffer.c +++ b/src/amd/vulkan/radv_cmd_buffer.c @@ -4830,7 +4830,7 @@ void radv_CmdBeginTransformFeedbackEXT( assert(firstCounterBuffer + counterBufferCount <= MAX_SO_BUFFERS); for_each_bit(i, so->enabled_mask) { int32_t counter_buffer_idx = i - firstCounterBuffer; - if (counter_buffer_idx >= 0 && counter_buffer_idx > counterBufferCount) + if (counter_buffer_idx >= 0 && counter_buffer_idx >= counterBufferCount) counter_buffer_idx = -1; /* SI binds streamout buffers as shader resources. @@ -4892,7 +4892,7 @@ void radv_CmdEndTransformFeedbackEXT( assert(firstCounterBuffer + counterBufferCount <= MAX_SO_BUFFERS); for_each_bit(i, so->enabled_mask) { int32_t counter_buffer_idx = i - firstCounterBuffer; - if (counter_buffer_idx >= 0 && counter_buffer_idx > counterBufferCount) + if (counter_buffer_idx >= 0 && counter_buffer_idx >= counterBufferCount) counter_buffer_idx = -1; if (counter_buffer_idx >= 0 && pCounterBuffers && pCounterBuffers[counter_buffer_idx]) { ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] ac: split 16-bit ssbo loads that may not be dword aligned
This refactoring is really not easy to read as is. Can you please split this in different parts? Maybe refactor first, then fix 16-bit ssbo loads? If we want this to be backported, we will just need to squash the different parts and send a separate patch to mesa-stable. On 12/6/18 12:13 AM, Rhys Perry wrote: This ends up refactoring visit_load_buffer() a little. Fixes: 7e7ee826982 ('ac: add support for 16bit buffer loads') Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108114 Signed-off-by: Rhys Perry --- Unfortunately I was not able to test this patch on a Polaris due to hardware issues. It fixed the deqp-vk tests without regressions on Vega though. src/amd/common/ac_llvm_build.c | 8 ++-- src/amd/common/ac_nir_to_llvm.c | 80 - src/compiler/nir/nir.h | 2 +- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c index abc18da13d..154cc696a2 100644 --- a/src/amd/common/ac_llvm_build.c +++ b/src/amd/common/ac_llvm_build.c @@ -2943,9 +2943,11 @@ LLVMValueRef ac_trim_vector(struct ac_llvm_context *ctx, LLVMValueRef value, if (count == num_components) return value; - LLVMValueRef masks[] = { - ctx->i32_0, ctx->i32_1, - LLVMConstInt(ctx->i32, 2, false), LLVMConstInt(ctx->i32, 3, false)}; + LLVMValueRef masks[MAX2(count, 2)]; + masks[0] = ctx->i32_0; + masks[1] = ctx->i32_1; + for (unsigned i = 2; i < count; i++) + masks[i] = LLVMConstInt(ctx->i32, i, false); if (count == 1) return LLVMBuildExtractElement(ctx->builder, value, masks[0], diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c index a109f5a815..4a4c09cf5f 100644 --- a/src/amd/common/ac_nir_to_llvm.c +++ b/src/amd/common/ac_nir_to_llvm.c @@ -1623,37 +1623,45 @@ static LLVMValueRef visit_atomic_ssbo(struct ac_nir_context *ctx, static LLVMValueRef visit_load_buffer(struct ac_nir_context *ctx, const nir_intrinsic_instr *instr) { - LLVMValueRef results[2]; - int load_bytes; int elem_size_bytes = instr->dest.ssa.bit_size / 8; int num_components = instr->num_components; - int num_bytes = num_components * elem_size_bytes; enum gl_access_qualifier access = nir_intrinsic_access(instr); LLVMValueRef glc = ctx->ac.i1false; if (access & (ACCESS_VOLATILE | ACCESS_COHERENT)) glc = ctx->ac.i1true; - for (int i = 0; i < num_bytes; i += load_bytes) { - load_bytes = MIN2(num_bytes - i, 16); - const char *load_name; - LLVMTypeRef data_type; - LLVMValueRef offset = get_src(ctx, instr->src[1]); - LLVMValueRef immoffset = LLVMConstInt(ctx->ac.i32, i, false); - LLVMValueRef rsrc = ctx->abi->load_ssbo(ctx->abi, - get_src(ctx, instr->src[0]), false); - LLVMValueRef vindex = ctx->ac.i32_0; + LLVMValueRef offset = get_src(ctx, instr->src[1]); + LLVMValueRef rsrc = ctx->abi->load_ssbo(ctx->abi, + get_src(ctx, instr->src[0]), false); + LLVMValueRef vindex = ctx->ac.i32_0; + + LLVMTypeRef def_type = get_def_type(ctx, &instr->dest.ssa); + LLVMTypeRef def_elem_type = num_components > 1 ? LLVMGetElementType(def_type) : def_type; - int idx = i ? 1 : 0; + LLVMValueRef results[4]; + for (int i = 0; i < num_components;) { + int num_elems = num_components - i; + if (elem_size_bytes < 4 && nir_intrinsic_align(instr) % 4 != 0) + num_elems = 1; + if (num_elems * elem_size_bytes > 16) + num_elems = 16 / elem_size_bytes; + int load_bytes = num_elems * elem_size_bytes; + + LLVMValueRef immoffset = LLVMConstInt(ctx->ac.i32, i * elem_size_bytes, false); + + LLVMValueRef ret; if (load_bytes == 2) { - results[idx] = ac_build_tbuffer_load_short(&ctx->ac, - rsrc, - vindex, - offset, - ctx->ac.i32_0, - immoffset, - glc); + ret = ac_build_tbuffer_load_short(&ctx->ac, + rsrc, + vindex, + offset, +
Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float
On 12/6/18 3:18 PM, Rhys Perry wrote: ./deqp-vk --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag should crash with something like: deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst* llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion `castIsValid(op, S, Ty) && "Invalid cast!"' failed. because it's trying to zext/sext a half float to a i32. and ./deqp-vk --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert should crash with something like: deqp-vk: lib/IR/Instructions.cpp:348: void llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef, llvm::ArrayRef >, const llvm::Twine&): Assertion `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"' failed. because it's calling the export intrinsic with incorrect argument types. For both tests, it seems to only assert with LLVM 8 for some reason. I guess you use a debug llvm build? Can you figure out what change introduces this crash? On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset wrote: On 12/6/18 2:15 PM, Rhys Perry wrote: So that the signature is correct and consistent, the inputs to a export intrinsic should always be 32-bit floats. This and the previous commit fixes a large amount crashes from dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_* tests They don't crash for me? Please explain how to reproduce. Fixes: b722b29f10d ('radv: add support for 16bit input/output') Signed-off-by: Rhys Perry --- src/amd/vulkan/radv_nir_to_llvm.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c index 0c91118e5a..90bcc8dbfe 100644 --- a/src/amd/vulkan/radv_nir_to_llvm.c +++ b/src/amd/vulkan/radv_nir_to_llvm.c @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context *ctx, } else memcpy(&args->out[0], values, sizeof(values[0]) * 4); - for (unsigned i = 0; i < 4; ++i) { - if (!(args->enabled_channels & (1 << i))) - continue; - + for (unsigned i = 0; i < 4; ++i) args->out[i] = ac_to_float(&ctx->ac, args->out[i]); - } } static void ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] nir: add if opt opt_if_loop_last_continue()
This introduces crashes for dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tesse dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert Test case 'dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag'.. deqp-vk: nir/nir_control_flow.c:553: stitch_blocks: Assertion `exec_list_is_empty(&after->instr_list)' failed. Aborted (core dumped) Did you run CTS? On 11/28/18 4:25 AM, Timothy Arceri wrote: From: Danylo Piliaiev Removing the last continue can allow more loops to unroll. Also inserting code into the if branch can allow the various if opts to progress further. The insertion of some loops into the if branch also reduces VGPR use in some shaders. vkpipeline-db results (VEGA): Totals from affected shaders: SGPRS: 6552 -> 6576 (0.37 %) VGPRS: 6544 -> 6532 (-0.18 %) Spilled SGPRs: 0 -> 0 (0.00 %) Spilled VGPRs: 0 -> 0 (0.00 %) Private memory VGPRs: 0 -> 0 (0.00 %) Scratch size: 0 -> 0 (0.00 %) dwords per thread Code Size: 481952 -> 478032 (-0.81 %) bytes LDS: 13 -> 13 (0.00 %) blocks Max Waves: 241 -> 242 (0.41 %) Wait states: 0 -> 0 (0.00 %) Shader-db results radeonsi (VEGA): Totals from affected shaders: SGPRS: 168 -> 168 (0.00 %) VGPRS: 144 -> 140 (-2.78 %) Spilled SGPRs: 157 -> 157 (0.00 %) Spilled VGPRs: 0 -> 0 (0.00 %) Private memory VGPRs: 0 -> 0 (0.00 %) Scratch size: 0 -> 0 (0.00 %) dwords per thread Code Size: 8524 -> 8488 (-0.42 %) bytes LDS: 0 -> 0 (0.00 %) blocks Max Waves: 7 -> 7 (0.00 %) Wait states: 0 -> 0 (0.00 %) v2: (Timothy Arceri): - allow for continues in either branch - move any trailing loops inside the if as well as blocks. - leave nir_opt_trivial_continues() to actually remove the continue. Signed-off-by: Timothy Arceri Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211 --- src/compiler/nir/nir_opt_if.c | 95 +++ 1 file changed, 95 insertions(+) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index dd488b1787..4a9dffb782 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -263,6 +263,100 @@ rewrite_phi_predecessor_blocks(nir_if *nif, } } +static bool +nir_block_ends_in_continue(nir_block *block) +{ + if (exec_list_is_empty(&block->instr_list)) + return false; + + nir_instr *instr = nir_block_last_instr(block); + return instr->type == nir_instr_type_jump && + nir_instr_as_jump(instr)->type == nir_jump_continue; +} + +/** + * This optimization turns: + * + * loop { + *... + *if (cond) { + * do_work_1(); + * continue; + *} else { + *} + *do_work_2(); + * } + * + * into: + * + * loop { + *... + *if (cond) { + * do_work_1(); + * continue; + *} else { + * do_work_2(); + *} + * } + * + * The continue should then be removed by nir_opt_trivial_continues() and the + * loop can potentially be unrolled. + * + * Note: do_work_2() is only ever blocks and nested loops. We could also nest + * other if-statments in the branch which would allow further continues to + * be removed. However in practice this can result in increased register + * pressure. + */ +static bool +opt_if_loop_last_continue(nir_loop *loop) +{ + /* Get the last if-stament in the loop */ + nir_block *last_block = nir_loop_last_block(loop); + nir_cf_node *if_node = nir_cf_node_prev(&last_block->cf_node); + while (if_node) { + if (if_node->type == nir_cf_node_if) + break; + + if_node = nir_cf_node_prev(if_node); + } + + if (!if_node || if_node->type != nir_cf_node_if) + return false; + + nir_if *nif = nir_cf_node_as_if(if_node); + nir_block *then_block = nir_if_last_then_block(nif); + nir_block *else_block = nir_if_last_else_block(nif); + + bool then_ends_in_continue = nir_block_ends_in_continue(then_block); + bool else_ends_in_continue = nir_block_ends_in_continue(else_block); + + /* If both branches end in a continue do nothing, this should be handled +* by nir_opt_dead_cf(). +*/ + if (then_ends_in_continue && else_ends_in_continue) + return false; + + if (!then_ends_in_continue && !else_ends_in_continue) + return false; + + /* Move the last block of the loop inside the last if-statement */ + nir_cf_list tmp; + nir_cf_extract(&tmp, nir_after_cf_node(if_node), +nir_after_block(last_block)); + if (then_ends_in_continue) { + nir_cf_reinsert(&tmp, nir_after_cf_list(&nif->else_list)); + } else { + nir_cf_reinsert(&tmp, nir_after_cf_list(&nif->then_list)); + } + + /* In order to avoid running nir_lower
Re: [Mesa-dev] [PATCH v2 09/29] nir: Make boolean conversions sized just like the others
On Thu, 2018-12-06 at 13:45 -0600, Jason Ekstrand wrote: (...) > diff --git a/src/compiler/nir/nir_builder.h > b/src/compiler/nir/nir_builder.h > index 30fa1d7ec8b..e0cdcd4ba23 100644 > --- a/src/compiler/nir/nir_builder.h > +++ b/src/compiler/nir/nir_builder.h > @@ -963,6 +963,18 @@ nir_load_param(nir_builder *build, uint32_t > param_idx) > > #include "nir_builder_opcodes.h" > > +static inline nir_ssa_def * > +nir_f2b(nir_builder *build, nir_ssa_def *f) > +{ > + return nir_f2b32(build, f); > +} > + > +static inline nir_ssa_def * > +nir_i2b(nir_builder *build, nir_ssa_def *i) > +{ > + return nir_i2b32(build, i); > +} > + any particular reason why wanted these instead of just calling the 32- bit opcode directly from the caller? I need to do b2f conversions in a couple of places where the destination can be 16, 32 or 64 and I think it is more convenient to have helpers that take the destination bit-size as parameter and then emit the appropriate opcode instead. What do you think? Iago ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Let's talk about -DDEBUG
On Wednesday, 2018-12-12 19:45:06 -0500, Marek Olšák wrote: > On Wed, Dec 12, 2018 at 7:35 PM Rob Clark wrote: > > > On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker wrote: > > > > > > Quoting Rob Clark (2018-12-12 15:52:47) > > > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker > > wrote: > > > > > > > > > > In the autotools discussion I've come to realize that we also need > > to talk about > > > > > the -DDEBUG guard. It seems that there are two different uses, and > > thus two > > > > > different asks about it: > > > > > > > > > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > > > > > - NIR and Intel (at least) use -DDEBUG to hide really expensive > > checks that are > > > > > useful, but necessarily tank performance. > > > > > > > > > > The first group would like -DDEBUG in debugoptimized builds, the > > second > > > > > obviously doesn't. > > > > > > > > > > Is the right solution to move the first group being !NDEBUG, or > > would it be > > > > > better to split DEBUG into two different defines such as > > DEBUG_MESSAGES and > > > > > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), > > with the > > > > > first for both debug and debugoptimized and the second only in debug > > builds? > > > > > > > > I guess my use cases for !=release builds are: > > > > > > > > + I want all the expensive checking because I'm not in it to win the > > > > deqp/piglit fps race > > > > + I want debug syms for profiling and/or valgrind, but otherwise > > > > want something close to a release build but with debug syms > > > > > > > > > > > > That said, I can get behind replacing DEBUG with !NDEBUG or > > > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer > > > > > > > > > > > > BR, > > > > -R > > > > > > I guess I should have covered that: > > > > > > autotools had effectively two build types "debug" and "not debug", > > "debug" set > > > "-DDEBUG -g -O2", "not debug" set -DNDEBUG > > > > > > Meson has 4 build types, and a separate toggle for NDEBUG: > > > debug: -O0 -DDEBUG (we add -DDEBUG) > > > debugoptimzed: -O2 -g > > > release: -O2 > > > plain: (nothing) > > > > I generally view meson as an upgrade in this respect, I guess mostly > > because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine > > by me).. maybe making meson debug config use -O2 would bridge the gap? > > If so I have no problem with dropping -O0 and making debug config > > also use -O2 > > > > Sounds good. > > Also I think we should make -Db_ndebug=true the default for release builds. > !NDEBUG enables a lot more debugging code than just assertions. That's already the case :) The default value (line 29 of the root meson.build) is: b_ndebug=if-release which means it will default to `true` if `buildtype=release`, and `false` otherwise, while still being overridden if you manually set `b_ndebug`. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 41/59] intel/compiler: split is_partial_write() into two variants
On Thu, Dec 13, 2018 at 09:10:24AM +0100, Iago Toral wrote: > On Wed, 2018-12-12 at 14:15 +0200, Pohjolainen, Topi wrote: > > On Wed, Dec 12, 2018 at 09:48:20AM +0100, Iago Toral wrote: > > > On Tue, 2018-12-11 at 18:59 +0200, Pohjolainen, Topi wrote: > > > > On Fri, Dec 07, 2018 at 03:30:11PM +0200, Pohjolainen, Topi > > > > wrote: > > > > > On Tue, Dec 04, 2018 at 08:17:05AM +0100, Iago Toral Quiroga > > > > > wrote: > > > > > > This function is used in two different scenarios that for 32- > > > > > > bit > > > > > > instructions are the same, but for 16-bit instructions are > > > > > > not. > > > > > > > > > > > > One scenario is that in which we are working at a SIMD8 > > > > > > register > > > > > > level and we need to know if a register is fully defined or > > > > > > written. > > > > > > This is useful, for example, in the context of liveness > > > > > > analysis > > > > > > or > > > > > > register allocation, where we work with units of registers. > > > > > > > > > > > > The other scenario is that in which we want to know if an > > > > > > instruction > > > > > > is writing a full scalar component or just some subset of it. > > > > > > This is > > > > > > useful, for example, in the context of some optimization > > > > > > passes > > > > > > like copy propagation. > > > > > > > > > > > > For 32-bit instructions (or larger), a SIMD8 dispatch will > > > > > > always > > > > > > write > > > > > > at least a full SIMD8 register (32B) if the write is not > > > > > > partial. > > > > > > The > > > > > > function is_partial_write() checks this to determine if we > > > > > > have a > > > > > > partial > > > > > > write. However, when we deal with 16-bit instructions, that > > > > > > logic > > > > > > disables > > > > > > some optimizations that should be safe. For example, a SIMD8 > > > > > > 16- > > > > > > bit MOV will > > > > > > only update half of a SIMD register, but it is still a > > > > > > complete > > > > > > write of the > > > > > > variable for a SIMD8 dispatch, so we should not prevent copy > > > > > > propagation in > > > > > > this scenario because we don't write all 32 bytes in the SIMD > > > > > > register > > > > > > or because the write starts at offset 16B (wehere we pack > > > > > > components Y or > > > > > > W of 16-bit vectors). > > > > > > > > > > > > This is a problem for SIMD8 executions (VS, TCS, TES, GS) of > > > > > > 16- > > > > > > bit > > > > > > instructions, which lose a number of optimizations because of > > > > > > this, most > > > > > > important of which is copy-propagation. > > > > > > > > > > > > This patch splits is_partial_write() into > > > > > > is_partial_reg_write(), > > > > > > which > > > > > > represents the current is_partial_write(), useful for things > > > > > > like > > > > > > liveness analysis, and is_partial_var_write(), which > > > > > > considers > > > > > > the dispatch size to check if we are writing a full variable > > > > > > (rather > > > > > > than a full register) to decide if the write is partial or > > > > > > not, > > > > > > which > > > > > > is what we really want in many optimization passes. > > > > > > > > I actually started wondering why would liveness analysis and > > > > register > > > > coalescing need to treat the 16-bit SIMD8 case differently than > > > > optimizations. > > > > In virtual register space nothing would read or write the unused > > > > second half > > > > of the register in case of 16-bit type and SIMD8. > > > > > > True, we might be able to use the "variable" version in more cases. > > > I > > > was trying to be conservative when I implemented this because I > > > don't > > > think the half-float CTS tests provides a good testing ground for > > > all > > > aspects of the compiler. I can try that and see if it breaks > > > anything > > > though. > > > > > > > Real register allocation in turn should be orthogonal to how > > > > things > > > > are > > > > allocated in virtual space. And I guess even there we wouldn't be > > > > interested > > > > of packing two 16-bit typed SIMD8 variables into one and same > > > > hardware > > > > register. It is SIMD16 where we get more pressure into register > > > > space > > > > anyway > > > > and there the 16-bit typed variables occupy full registers. In > > > > other > > > > words, > > > > if things fit in SIMD16, would we bother packing things more > > > > tightly > > > > in > > > > SIMD8? Or even if SIMD8 was the only option, would we be > > > > interested > > > > packing > > > > channels for two variables in one hw reg even then? > > > > > > > > Jason, we discussed this a little in the spring time. > > > > > > > > As a recap my approach shortly. Instead of ignoring the second > > > > half > > > > of > > > > registers case by case I addressed it more generally: > > > > > > > > - changed all the open coded checks to use helpers, > > > > - added a padding bit into fs_reg telling about the unused space, > > > > - change nir -> fs step to set that bit for 16-bit typ
Re: [Mesa-dev] Let's talk about -DDEBUG
On Wednesday, 2018-12-12 15:24:25 -0800, Dylan Baker wrote: > In the autotools discussion I've come to realize that we also need to talk > about > the -DDEBUG guard. It seems that there are two different uses, and thus two > different asks about it: > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > - NIR and Intel (at least) use -DDEBUG to hide really expensive checks that > are > useful, but necessarily tank performance. > > The first group would like -DDEBUG in debugoptimized builds, the second > obviously doesn't. > > Is the right solution to move the first group being !NDEBUG, or would it be > better to split DEBUG into two different defines such as DEBUG_MESSAGES and > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), with the > first for both debug and debugoptimized and the second only in debug builds? Replacing DEBUG with !NDEBUG is obviously trivially simpler, but I think the right thing would be to split it into !NDEBUG and EXPENSIVE_VALIDATION (the color suits me just fine :P), as both solutions satisfy the first group but only the latter solution satisfies the 2nd group. I think a first pass might be to simply s/DEBUG/EXPENSIVE_VALIDATION/ so that it expresses the intent more clearly, with a prior patch to convert Nine and other obvious !NDEBUG candidates, then, later on, some of the EXPENSIVE_VALIDATION can be promoted to !NDEBUG on a case-by-case basis. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] virgl: work around bad assumptions in virglrenderer
This fixes rendering in Supertuxkart, Serious Sam 3, and Shadow Warrior via virgl for me. So for the series: Tested-By: Gert Wollny Am Dienstag, den 11.12.2018, 15:26 +0100 schrieb Erik Faye-Lund: > Virglrenderer does the wrong thing when given an instance divisor; > it tries to use the element-index rather than the binding-index as > the argument to glVertexBindingDivisor(). This worked fine as long > as there was a 1:1 relationship between elements and bindings, > which was the case util 19a91841c34 "st/mesa: Use Array._DrawVAO in > st_atom_array.c.". > > So let's detect instance divisors, and restore a 1:1 relationship in > that case. This will make old versions of virglrenderer behave > correctly. For newer versions, we can consider making a better > interface, where the instance divisor isn't specified per element, > but rather per binding. But let's save that for another day. > > Signed-off-by: Erik Faye-Lund > --- > src/gallium/drivers/virgl/virgl_context.c | 29 > ++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/virgl/virgl_context.c > b/src/gallium/drivers/virgl/virgl_context.c > index 121d9139f28..ce72f73a0f6 100644 > --- a/src/gallium/drivers/virgl/virgl_context.c > +++ b/src/gallium/drivers/virgl/virgl_context.c > @@ -50,6 +50,8 @@ > > struct virgl_vertex_elements_state { > uint32_t handle; > + uint8_t binding_map[PIPE_MAX_ATTRIBS]; > + uint8_t num_bindings; > }; > > static uint32_t next_handle; > @@ -390,10 +392,24 @@ static void > *virgl_create_vertex_elements_state(struct pipe_context *ctx, > unsigned > num_elements, > const struct > pipe_vertex_element *elements) > { > + struct pipe_vertex_element new_elements[PIPE_MAX_ATTRIBS]; > struct virgl_context *vctx = virgl_context(ctx); > struct virgl_vertex_elements_state *state = >CALLOC_STRUCT(virgl_vertex_elements_state); > > + for (int i = 0; i < num_elements; ++i) { > + if (elements[i].instance_divisor) { > + for (int j = 0; j < num_elements; ++j) { > +new_elements[j] = elements[j]; > +new_elements[j].vertex_buffer_index = j; > +state->binding_map[j] = elements[j].vertex_buffer_index; > + } > + elements = new_elements; > + state->num_bindings = num_elements; > + break; > + } > + } > + > state->handle = virgl_object_assign_handle(); > virgl_encoder_create_vertex_elements(vctx, state->handle, > num_elements, elements); > @@ -419,6 +435,7 @@ static void > virgl_bind_vertex_elements_state(struct pipe_context *ctx, > vctx->vertex_elements = state; > virgl_encode_bind_object(vctx, state ? state->handle : 0, > VIRGL_OBJECT_VERTEX_ELEMENTS); > + vctx->vertex_array_dirty = TRUE; > } > > static void virgl_set_vertex_buffers(struct pipe_context *ctx, > @@ -438,7 +455,17 @@ static void virgl_set_vertex_buffers(struct > pipe_context *ctx, > static void virgl_hw_set_vertex_buffers(struct virgl_context *vctx) > { > if (vctx->vertex_array_dirty) { > - virgl_encoder_set_vertex_buffers(vctx, vctx- > >num_vertex_buffers, vctx->vertex_buffer); > + struct virgl_vertex_elements_state *ve = vctx- > >vertex_elements; > + > + if (ve->num_bindings) { > + struct pipe_vertex_buffer vertex_buffers[PIPE_MAX_ATTRIBS]; > + for (int i = 0; i < ve->num_bindings; ++i) > +vertex_buffers[i] = vctx->vertex_buffer[ve- > >binding_map[i]]; > + > + virgl_encoder_set_vertex_buffers(vctx, ve->num_bindings, > vertex_buffers); > + } else > + virgl_encoder_set_vertex_buffers(vctx, vctx- > >num_vertex_buffers, vctx->vertex_buffer); > + >virgl_attach_res_vertex_buffers(vctx); > } > } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 41/59] intel/compiler: split is_partial_write() into two variants
El 13/12/18 a las 11:49, Pohjolainen, Topi escribió: > On Thu, Dec 13, 2018 at 09:10:24AM +0100, Iago Toral wrote: >> On Wed, 2018-12-12 at 14:15 +0200, Pohjolainen, Topi wrote: >>> On Wed, Dec 12, 2018 at 09:48:20AM +0100, Iago Toral wrote: On Tue, 2018-12-11 at 18:59 +0200, Pohjolainen, Topi wrote: > On Fri, Dec 07, 2018 at 03:30:11PM +0200, Pohjolainen, Topi > wrote: >> On Tue, Dec 04, 2018 at 08:17:05AM +0100, Iago Toral Quiroga >> wrote: >>> This function is used in two different scenarios that for 32- >>> bit >>> instructions are the same, but for 16-bit instructions are >>> not. >>> >>> One scenario is that in which we are working at a SIMD8 >>> register >>> level and we need to know if a register is fully defined or >>> written. >>> This is useful, for example, in the context of liveness >>> analysis >>> or >>> register allocation, where we work with units of registers. >>> >>> The other scenario is that in which we want to know if an >>> instruction >>> is writing a full scalar component or just some subset of it. >>> This is >>> useful, for example, in the context of some optimization >>> passes >>> like copy propagation. >>> >>> For 32-bit instructions (or larger), a SIMD8 dispatch will >>> always >>> write >>> at least a full SIMD8 register (32B) if the write is not >>> partial. >>> The >>> function is_partial_write() checks this to determine if we >>> have a >>> partial >>> write. However, when we deal with 16-bit instructions, that >>> logic >>> disables >>> some optimizations that should be safe. For example, a SIMD8 >>> 16- >>> bit MOV will >>> only update half of a SIMD register, but it is still a >>> complete >>> write of the >>> variable for a SIMD8 dispatch, so we should not prevent copy >>> propagation in >>> this scenario because we don't write all 32 bytes in the SIMD >>> register >>> or because the write starts at offset 16B (wehere we pack >>> components Y or >>> W of 16-bit vectors). >>> >>> This is a problem for SIMD8 executions (VS, TCS, TES, GS) of >>> 16- >>> bit >>> instructions, which lose a number of optimizations because of >>> this, most >>> important of which is copy-propagation. >>> >>> This patch splits is_partial_write() into >>> is_partial_reg_write(), >>> which >>> represents the current is_partial_write(), useful for things >>> like >>> liveness analysis, and is_partial_var_write(), which >>> considers >>> the dispatch size to check if we are writing a full variable >>> (rather >>> than a full register) to decide if the write is partial or >>> not, >>> which >>> is what we really want in many optimization passes. > > I actually started wondering why would liveness analysis and > register > coalescing need to treat the 16-bit SIMD8 case differently than > optimizations. > In virtual register space nothing would read or write the unused > second half > of the register in case of 16-bit type and SIMD8. True, we might be able to use the "variable" version in more cases. I was trying to be conservative when I implemented this because I don't think the half-float CTS tests provides a good testing ground for all aspects of the compiler. I can try that and see if it breaks anything though. > Real register allocation in turn should be orthogonal to how > things > are > allocated in virtual space. And I guess even there we wouldn't be > interested > of packing two 16-bit typed SIMD8 variables into one and same > hardware > register. It is SIMD16 where we get more pressure into register > space > anyway > and there the 16-bit typed variables occupy full registers. In > other > words, > if things fit in SIMD16, would we bother packing things more > tightly > in > SIMD8? Or even if SIMD8 was the only option, would we be > interested > packing > channels for two variables in one hw reg even then? > > Jason, we discussed this a little in the spring time. > > As a recap my approach shortly. Instead of ignoring the second > half > of > registers case by case I addressed it more generally: > > - changed all the open coded checks to use helpers, > - added a padding bit into fs_reg telling about the unused space, > - change nir -> fs step to set that bit for 16-bit typed regs > - and finally changed the helpers to consider the padding bit. So if I understand how this works, you mostly make the vgrf infrastructure think that half-float registers actually use twice the space they require by including the padding into the component_size() helper, righ
[Mesa-dev] [Bug 108900] Non-recoverable GPU hangs with GfxBench v5 Aztec Ruins Vulkan test
https://bugs.freedesktop.org/show_bug.cgi?id=108900 --- Comment #4 from Eero Tamminen --- FYI: https://gfxbench.com/ site works again. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] spirv/nir: adjust location assignment for the case of arrays of blocks
This is needed due how the types get rearranged after the struct splitting. So for example, this array of blocks: layout(location = 0) out block { vec4 v; vec3 v2; } x[2]; Would be splitted on two nir variables with the following types: * vec4 v[2] * vec3 v2[2] So we need to take into account the length of the array to avoid locations overlaps one with the other. --- Hi Jason, again, sending in advance patches, just in case you are working on the same. I was able to fix the location overlapping without all those crazy ideas about lowering array of blocks into individual blocks, by just adjusting the locations as this patch shows. FWIW, the resulting locations are equivalent to those that we get with GLSL IR, that results on a similar splitting. With this change I got the following working: * SPIR-V simple arrays of blocks input/outputs * The arrays of blocks inputs/outputs + interpolator qualifiers test I mentioned to you last week [1] when run its SPIR-V equivalent. * SPIR-V xfb tests using arrays of blocks, where the xfb offset are assigned to all block members. * SPIR-V xfb tests using arrays of blocks, where the xfb offset is assigned to just one member, so just that member is captured, although as many times as the array length (yes! afaiu by spec that needs to work) So now, the only pending thing is a cleanup and send the series to review. Specifically, I think that this series can be put on top of current master instead of the arb_gl_spirv. Will try that and send a final series this week or early next week. BR [1] https://github.com/Igalia/piglit/blob/master/tests/spec/glsl-1.50/execution/interface-block-interpolation-array.shader_test src/compiler/spirv/vtn_variables.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index a8f2fdfa534..87386cee42f 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1672,6 +1672,14 @@ add_missing_member_locations(struct vtn_variable *var, glsl_get_length(glsl_without_array(var->type->type)); int location = var->base_location; + /* To know if it is a interface block we can't ask directly for +* var->type->block because on the case of arrays of blocks, block is set +* on the array_element. +*/ + bool is_array_block = var->var->interface_type != NULL && + glsl_type_is_array(var->type->type); + int adjustment = is_array_block ? glsl_get_length(var->type->type) : 1; + for (unsigned i = 0; i < length; i++) { /* From the Vulkan spec: * @@ -1702,8 +1710,12 @@ add_missing_member_locations(struct vtn_variable *var, const struct glsl_type *member_type = glsl_get_struct_field(glsl_without_array(var->type->type), i); + /* For arrays of interface blocks we can't just add the attribute slots + * of a member type due how the splitting would rearrange the types, so + * we need to adjust for the array length in that case. + */ location += - glsl_count_attribute_slots(member_type, is_vertex_input); + glsl_count_attribute_slots(member_type, is_vertex_input) * adjustment; } } -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108578] RADV reports wrong hardcoded Vulkan API Version
https://bugs.freedesktop.org/show_bug.cgi?id=108578 --- Comment #7 from Shmerl --- For those following this, reported API version should be now at 1.1.90: https://gitlab.freedesktop.org/mesa/mesa/commit/2ac6d55f38c1665a16d8d02675df2e3a858a7fec -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 09/29] nir: Make boolean conversions sized just like the others
On December 13, 2018 04:07:56 Iago Toral wrote: On Thu, 2018-12-06 at 13:45 -0600, Jason Ekstrand wrote: (...) diff --git a/src/compiler/nir/nir_builder.h b/src/compiler/nir/nir_builder.h index 30fa1d7ec8b..e0cdcd4ba23 100644 --- a/src/compiler/nir/nir_builder.h +++ b/src/compiler/nir/nir_builder.h @@ -963,6 +963,18 @@ nir_load_param(nir_builder *build, uint32_t param_idx) #include "nir_builder_opcodes.h" +static inline nir_ssa_def * +nir_f2b(nir_builder *build, nir_ssa_def *f) +{ + return nir_f2b32(build, f); +} + +static inline nir_ssa_def * +nir_i2b(nir_builder *build, nir_ssa_def *i) +{ + return nir_i2b32(build, i); +} + any particular reason why wanted these instead of just calling the 32- bit opcode directly from the caller? In my 1-bit booleans series, it lets me switch most of NIR without having to manually edit every single place we do a x2b conversion. I need to do b2f conversions in a couple of places where the destination can be 16, 32 or 64 and I think it is more convenient to have helpers that take the destination bit-size as parameter and then emit the appropriate opcode instead. What do you think? For b2f, I totally agree. In fact, I've considered adding a small bit of codegen to nir_builder to add such a pole of helpers. One of my patch series (I don't remember which at this point) adds i2i and u2u helpers that take a bit size like you suggest. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir: remove unused variable
To avoid the following warning: ./src/compiler/nir/nir_loop_analyze.c:807:16: warning: unused variable ‘ns’ [-Wunused-variable] nir_shader *ns = impl->function->shader; --- Perhaps this is solved on any of the loop analysis patches pending to be reviewed, but just in case, sending it. src/compiler/nir/nir_loop_analyze.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index 3de45401975..259f02a854e 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -803,7 +803,6 @@ get_loop_info(loop_info_state *state, nir_function_impl *impl) /* Run through each of the terminators and try to compute a trip-count */ find_trip_count(state); - nir_shader *ns = impl->function->shader; nir_foreach_block_in_cf_node(block, &state->loop->cf_node) { if (force_unroll_heuristics(state, block)) { state->loop->info->force_unroll = true; -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float
Seems my LLVM configuration was messed up and I might have used my distro's LLVM too. On Thu, 13 Dec 2018 at 08:38, Samuel Pitoiset wrote: > > > > On 12/6/18 3:18 PM, Rhys Perry wrote: > > ./deqp-vk > > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag > > should crash with something like: > > deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst* > > llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, > > llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion > > `castIsValid(op, S, Ty) && "Invalid cast!"' failed. > > because it's trying to zext/sext a half float to a i32. > > > > and ./deqp-vk > > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert > > should crash with something like: > > deqp-vk: lib/IR/Instructions.cpp:348: void > > llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, > > llvm::ArrayRef, > > llvm::ArrayRef >, const > > llvm::Twine&): Assertion `(i >= FTy->getNumParams() || > > FTy->getParamType(i) == Args[i]->getType()) && "Calling a function > > with a bad signature!"' failed. > > because it's calling the export intrinsic with incorrect argument types. > > > > For both tests, it seems to only assert with LLVM 8 for some reason. > > I guess you use a debug llvm build? Can you figure out what change > introduces this crash? > > > On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset > > wrote: > >> > >> > >> > >> On 12/6/18 2:15 PM, Rhys Perry wrote: > >>> So that the signature is correct and consistent, the inputs to a export > >>> intrinsic should always be 32-bit floats. > >>> > >>> This and the previous commit fixes a large amount crashes from > >>> dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_* > >>> tests > >>> > >> > >> They don't crash for me? Please explain how to reproduce. > >> > >>> Fixes: b722b29f10d ('radv: add support for 16bit input/output') > >>> Signed-off-by: Rhys Perry > >>> --- > >>>src/amd/vulkan/radv_nir_to_llvm.c | 6 +- > >>>1 file changed, 1 insertion(+), 5 deletions(-) > >>> > >>> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c > >>> b/src/amd/vulkan/radv_nir_to_llvm.c > >>> index 0c91118e5a..90bcc8dbfe 100644 > >>> --- a/src/amd/vulkan/radv_nir_to_llvm.c > >>> +++ b/src/amd/vulkan/radv_nir_to_llvm.c > >>> @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct > >>> radv_shader_context *ctx, > >>>} else > >>>memcpy(&args->out[0], values, sizeof(values[0]) * 4); > >>> > >>> - for (unsigned i = 0; i < 4; ++i) { > >>> - if (!(args->enabled_channels & (1 << i))) > >>> - continue; > >>> - > >>> + for (unsigned i = 0; i < 4; ++i) > >>>args->out[i] = ac_to_float(&ctx->ac, args->out[i]); > >>> - } > >>>} > >>> > >>>static void > >>> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: remove unused variable
Reviewed-by: Lionel Landwerlin On 13/12/2018 14:25, Alejandro Piñeiro wrote: To avoid the following warning: ./src/compiler/nir/nir_loop_analyze.c:807:16: warning: unused variable ‘ns’ [-Wunused-variable] nir_shader *ns = impl->function->shader; --- Perhaps this is solved on any of the loop analysis patches pending to be reviewed, but just in case, sending it. src/compiler/nir/nir_loop_analyze.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index 3de45401975..259f02a854e 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -803,7 +803,6 @@ get_loop_info(loop_info_state *state, nir_function_impl *impl) /* Run through each of the terminators and try to compute a trip-count */ find_trip_count(state); - nir_shader *ns = impl->function->shader; nir_foreach_block_in_cf_node(block, &state->loop->cf_node) { if (force_unroll_heuristics(state, block)) { state->loop->info->force_unroll = true; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float
(accidently sent an incomplete email) Seems my LLVM configuration was messed up and I might have used my distro's LLVM too. LLVM 8 and 7 with a release build passes. A debug build of 8 (and my messed up builds of 7 and 8 which I thought were release ones) results in an assert. On Thu, 13 Dec 2018 at 08:38, Samuel Pitoiset wrote: > > > > On 12/6/18 3:18 PM, Rhys Perry wrote: > > ./deqp-vk > > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag > > should crash with something like: > > deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst* > > llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, > > llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion > > `castIsValid(op, S, Ty) && "Invalid cast!"' failed. > > because it's trying to zext/sext a half float to a i32. > > > > and ./deqp-vk > > --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert > > should crash with something like: > > deqp-vk: lib/IR/Instructions.cpp:348: void > > llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, > > llvm::ArrayRef, > > llvm::ArrayRef >, const > > llvm::Twine&): Assertion `(i >= FTy->getNumParams() || > > FTy->getParamType(i) == Args[i]->getType()) && "Calling a function > > with a bad signature!"' failed. > > because it's calling the export intrinsic with incorrect argument types. > > > > For both tests, it seems to only assert with LLVM 8 for some reason. > > I guess you use a debug llvm build? Can you figure out what change > introduces this crash? > > > On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset > > wrote: > >> > >> > >> > >> On 12/6/18 2:15 PM, Rhys Perry wrote: > >>> So that the signature is correct and consistent, the inputs to a export > >>> intrinsic should always be 32-bit floats. > >>> > >>> This and the previous commit fixes a large amount crashes from > >>> dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_* > >>> tests > >>> > >> > >> They don't crash for me? Please explain how to reproduce. > >> > >>> Fixes: b722b29f10d ('radv: add support for 16bit input/output') > >>> Signed-off-by: Rhys Perry > >>> --- > >>>src/amd/vulkan/radv_nir_to_llvm.c | 6 +- > >>>1 file changed, 1 insertion(+), 5 deletions(-) > >>> > >>> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c > >>> b/src/amd/vulkan/radv_nir_to_llvm.c > >>> index 0c91118e5a..90bcc8dbfe 100644 > >>> --- a/src/amd/vulkan/radv_nir_to_llvm.c > >>> +++ b/src/amd/vulkan/radv_nir_to_llvm.c > >>> @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct > >>> radv_shader_context *ctx, > >>>} else > >>>memcpy(&args->out[0], values, sizeof(values[0]) * 4); > >>> > >>> - for (unsigned i = 0; i < 4; ++i) { > >>> - if (!(args->enabled_channels & (1 << i))) > >>> - continue; > >>> - > >>> + for (unsigned i = 0; i < 4; ++i) > >>>args->out[i] = ac_to_float(&ctx->ac, args->out[i]); > >>> - } > >>>} > >>> > >>>static void > >>> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [ANNOUNCE] mesa 18.2.7
Mesa 18.2.7 is now available. In this release we have: Several patches fixing leaks in glsl, winsys and r600. Improvements in the scripts that helps in preparing releases. Added PCI IDs for Amber Lake and Whiskey Lake. Fixes for radv, anv, i965 and vc4 drivers. A couple of fixes in NIR backend. Finally, several fixes in meson build system. Alex Smith (1): radv: Flush before vkCmdWriteTimestamp() if needed Bas Nieuwenhuizen (4): radv: Align large buffers to the fragment size. radv: Clamp gfx9 image view extents to the allocated image extents. radv/android: Mark android WSI image as shareable. radv/android: Use buffer metadata to determine scanout compat. Dave Airlie (2): r600: make suballocator 256-bytes align radv: use 3d shader for gfx9 copies if dst is 3d Emil Velikov (2): egl/wayland: bail out when drmGetMagic fails egl/wayland: plug memory leak in drm_handle_device() Eric Anholt (3): v3d: Fix a leak of the transfer helper on screen destroy. vc4: Fix a leak of the transfer helper on screen destroy. v3d: Fix a leak of the disassembled instruction string during debug dumps. Eric Engestrom (3): anv: correctly use vulkan 1.0 by default wsi/display: fix mem leak when freeing swapchains vulkan/wsi: fix s/,/;/ typo Gurchetan Singh (3): virgl: quadruple command buffer size virgl: avoid large inline transfers virgl: don't mark buffers as unclean after a write Juan A. Suarez Romero (5): docs: add sha256 checksums for 18.2.6 cherry-ignore: freedreno: Fix autotools build. cherry-ignore: mesa: Revert INTEL_fragment_shader_ordering support Update version to 18.2.7 docs: add release notes for 18.2.7 Karol Herbst (1): nv50,nvc0: Fix gallium nine regression regarding sampler bindings Lionel Landwerlin (2): anv: flush pipeline before query result copies anv/query: flush render target before copying results Michal Srb (2): gallium: Constify drisw_loader_funcs struct drisw: Use separate drisw_loader_funcs for shm Nicolai Hähnle (2): egl/wayland: rather obvious build fix meson: link LLVM 'native' component when LLVM is available Samuel Pitoiset (1): radv: rework the TC-compat HTILE hardware bug with COND_EXEC Thomas Hellstrom (2): st/xa: Fix a memory leak winsys/svga: Fix a memory leak Tobias Klausmann (1): amd/vulkan: meson build - use radv_deps for libvulkan_radeon Vinson Lee (1): st/xvmc: Add X11 include path. git tag: mesa-18.2.7 https://mesa.freedesktop.org/archive/mesa-18.2.7.tar.gz MD5: 6da03050077d073bfee765ae330bc2d5 mesa-18.2.7.tar.gz SHA1: 916a26ff5b31bf49fa70ac433b6cf0287e0d6249 mesa-18.2.7.tar.gz SHA256: 092351cfbcd430ec595fbd3a3d8d253fd62c29074e1740d7198b00289ab400f8 mesa-18.2.7.tar.gz SHA512: 0df3465cadbcf9352d5b0e816f8b421ef4d4ac0c01f9ac2adfdeb351fe38f814c47201d59ad655d96caf12eb78d41b7432e43a06b70eed6c14f8dc3b0df8f6d4 mesa-18.2.7.tar.gz PGP: https://mesa.freedesktop.org/archive/mesa-18.2.7.tar.gz.sig https://mesa.freedesktop.org/archive/mesa-18.2.7.tar.xz MD5: 37905033b6d0aa9740c9cba6399b6de5 mesa-18.2.7.tar.xz SHA1: c5a204daab96a65a9e7958a0795877ec4e7ec83f mesa-18.2.7.tar.xz SHA256: 9c7b02560d89d77ca279cd21f36ea9a49e9ffc5611f6fe35099357d744d07ae6 mesa-18.2.7.tar.xz SHA512: 81f7b7108352cb3d8cf4b600ce2b0db6eb8420df550bf9f7087d83c2ab0f8dc32abe188f8ce5f52066e432fb64d9d17fc0edfbc9d0cb0650d0041f14064d9d77 mesa-18.2.7.tar.xz PGP: https://mesa.freedesktop.org/archive/mesa-18.2.7.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
Re: [Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float
On 12/13/18 3:45 PM, Rhys Perry wrote: (accidently sent an incomplete email) Seems my LLVM configuration was messed up and I might have used my distro's LLVM too. LLVM 8 and 7 with a release build passes. A debug build of 8 (and my messed up builds of 7 and 8 which I thought were release ones) results in an assert. Just tried to reproduce with a LLVM 8 debug build, I don't get an assertion. Maybe I messed up my build too? On Thu, 13 Dec 2018 at 08:38, Samuel Pitoiset wrote: On 12/6/18 3:18 PM, Rhys Perry wrote: ./deqp-vk --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag should crash with something like: deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst* llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion `castIsValid(op, S, Ty) && "Invalid cast!"' failed. because it's trying to zext/sext a half float to a i32. and ./deqp-vk --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert should crash with something like: deqp-vk: lib/IR/Instructions.cpp:348: void llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef, llvm::ArrayRef >, const llvm::Twine&): Assertion `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"' failed. because it's calling the export intrinsic with incorrect argument types. For both tests, it seems to only assert with LLVM 8 for some reason. I guess you use a debug llvm build? Can you figure out what change introduces this crash? On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset wrote: On 12/6/18 2:15 PM, Rhys Perry wrote: So that the signature is correct and consistent, the inputs to a export intrinsic should always be 32-bit floats. This and the previous commit fixes a large amount crashes from dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_* tests They don't crash for me? Please explain how to reproduce. Fixes: b722b29f10d ('radv: add support for 16bit input/output') Signed-off-by: Rhys Perry --- src/amd/vulkan/radv_nir_to_llvm.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c index 0c91118e5a..90bcc8dbfe 100644 --- a/src/amd/vulkan/radv_nir_to_llvm.c +++ b/src/amd/vulkan/radv_nir_to_llvm.c @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context *ctx, } else memcpy(&args->out[0], values, sizeof(values[0]) * 4); - for (unsigned i = 0; i < 4; ++i) { - if (!(args->enabled_channels & (1 << i))) - continue; - + for (unsigned i = 0; i < 4; ++i) args->out[i] = ac_to_float(&ctx->ac, args->out[i]); - } } static void ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 1/6] mesa: wire up InvalidateFramebuffer
On 12/12/2018 08:48 AM, Rob Clark wrote: > And before someone actually starts implementing DiscardFramebuffer() > lets rework the interface to something that is actually usable. > > Signed-off-by: Rob Clark > Reviewed-by: Ian Romanick > --- > src/mesa/main/dd.h | 5 ++- > src/mesa/main/fbobject.c | 77 +--- > 2 files changed, 75 insertions(+), 7 deletions(-) > > diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h > index f14c3e04e91..1214eeaa474 100644 > --- a/src/mesa/main/dd.h > +++ b/src/mesa/main/dd.h > @@ -784,9 +784,8 @@ struct dd_function_table { > GLint srcX0, GLint srcY0, GLint srcX1, GLint > srcY1, > GLint dstX0, GLint dstY0, GLint dstX1, GLint > dstY1, > GLbitfield mask, GLenum filter); > - void (*DiscardFramebuffer)(struct gl_context *ctx, > - GLenum target, GLsizei numAttachments, > - const GLenum *attachments); > + void (*DiscardFramebuffer)(struct gl_context *ctx, struct gl_framebuffer > *fb, > + struct gl_renderbuffer_attachment *att); > > /** > * \name Functions for GL_ARB_sample_locations > diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c > index 23e49396199..442435655fa 100644 > --- a/src/mesa/main/fbobject.c > +++ b/src/mesa/main/fbobject.c > @@ -4637,6 +4637,65 @@ invalid_enum: > return; > } > > +static struct gl_renderbuffer_attachment * > +get_fb_attachment(struct gl_context *ctx, struct gl_framebuffer *fb, > + const GLenum attachment) > +{ > + int idx; > + > + switch (attachment) { > + case GL_COLOR: > + case GL_COLOR_ATTACHMENT0: > + case GL_COLOR_ATTACHMENT1: > + case GL_COLOR_ATTACHMENT2: > + case GL_COLOR_ATTACHMENT3: > + case GL_COLOR_ATTACHMENT4: > + case GL_COLOR_ATTACHMENT5: > + case GL_COLOR_ATTACHMENT6: > + case GL_COLOR_ATTACHMENT7: > + case GL_COLOR_ATTACHMENT8: > + case GL_COLOR_ATTACHMENT9: > + case GL_COLOR_ATTACHMENT10: > + case GL_COLOR_ATTACHMENT11: > + case GL_COLOR_ATTACHMENT12: > + case GL_COLOR_ATTACHMENT13: > + case GL_COLOR_ATTACHMENT14: > + case GL_COLOR_ATTACHMENT15: > + if (attachment == GL_COLOR) { > + idx = 0; > + } else { > + idx = attachment - GL_COLOR_ATTACHMENT0; > + } > + return &fb->Attachment[BUFFER_COLOR0 + idx]; > + case GL_DEPTH: > + case GL_DEPTH_ATTACHMENT: > + case GL_DEPTH_STENCIL_ATTACHMENT: > + return &fb->Attachment[BUFFER_DEPTH]; > + case GL_STENCIL: > + case GL_STENCIL_ATTACHMENT: > + return &fb->Attachment[BUFFER_STENCIL]; > + default: > + return NULL; > + } > +} That function seems pretty similar to get_attachment(). Can you use (or modify) that one instead? > + > +static void > +discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb, > +GLsizei numAttachments, const GLenum *attachments) Please put a comment on this new function. > +{ > + if (!ctx->Driver.DiscardFramebuffer) > + return; > + > + for (int i = 0; i < numAttachments; i++) { > + struct gl_renderbuffer_attachment *att = > +get_fb_attachment(ctx, fb, attachments[i]); > + > + if (!att) > + continue; > + > + ctx->Driver.DiscardFramebuffer(ctx, fb, att); Or, if (att) { ctx->Driver.DiscardFramebuffer(ctx, fb, att); } > + } > +} > > void GLAPIENTRY > _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei > numAttachments, > @@ -4695,14 +4754,23 @@ _mesa_InvalidateNamedFramebufferSubData(GLuint > framebuffer, > invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments, > x, y, width, height, > "glInvalidateNamedFramebufferSubData"); > -} > > + discard_sub_framebuffer(ctx, fb, numAttachments, attachments, > + x, y, width, height); > +} > > void GLAPIENTRY > _mesa_InvalidateFramebuffer_no_error(GLenum target, GLsizei numAttachments, >const GLenum *attachments) > { > - /* no-op */ > + struct gl_framebuffer *fb; > + GET_CURRENT_CONTEXT(ctx); > + > + fb = get_framebuffer_target(ctx, target); > + if (!fb) > + return; > + > + discard_framebuffer(ctx, fb, numAttachments, attachments); > } > > > @@ -4738,6 +4806,8 @@ _mesa_InvalidateFramebuffer(GLenum target, GLsizei > numAttachments, > ctx->Const.MaxViewportWidth, > ctx->Const.MaxViewportHeight, > "glInvalidateFramebuffer"); > + > + discard_framebuffer(ctx, fb, numAttachments, attachments); > } > > > @@ -4824,8 +4894,7 @@ _mesa_DiscardFramebufferEXT(GLenum target, GLsizei > numAttachments, > } > } > > - if (ctx->Driv
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset wrote: > > Personally, I will continue to use the list, at least for a simplicity > point of view. I'm not sure if using a new tool will improve quality and > code review process. > > Though, if the majority reports that is really nice to use, I will > probably change my mind. Not a strong reject. I agree. We've been using the MR interface for xf86-video-amdgpu and I find it awkward compared to the mailing list. Maybe it just takes getting used to. I also feel less inclined to do drive by patch review if I have to explicitly delve into the browser to look at the outstanding MRs. Over email, sometimes I see a patch set in my in box that piques my interest and I find some time to review it when I might not have otherwise if the bar were higher. Out of curiosity what do others like about the MR interface? How are you using it? What advantages does it give you over the mailing list? I feel like maybe I'm not using it right or missing features that make it more useful and less awkward. I feel like the interface makes it harder than it needs to be to see the actual changes in MR to be reviewed. All of the links click through to the source view rather than the patch view. Alex > > On 12/6/18 12:32 AM, Jordan Justen wrote: > > This documents a process for using GitLab Merge Requests as an second > > way to submit code changes for Mesa. Only one of the two methods is > > allowed for each patch series. > > > > We will *not* require all patches to be emailed. Some code changes may > > be reviewed and merged without any discussion on the mesa-dev email > > list. > > > > v2: > > * No longer require email. Allow submitter to choose email or a > > GitLab merge request. > > * Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason, > > Matt, Michel and Rob. > > > > Signed-off-by: Jordan Justen > > --- > > docs/submittingpatches.html | 76 ++--- > > 1 file changed, 71 insertions(+), 5 deletions(-) > > > > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html > > index 92d954a2d09..21175988d0b 100644 > > --- a/docs/submittingpatches.html > > +++ b/docs/submittingpatches.html > > @@ -21,7 +21,7 @@ > > Basic guidelines > > Patch formatting > > Testing Patches > > -Mailing Patches > > +Submitting Patches > > Reviewing Patches > > Nominating a commit for a stable branch > > Criteria for accepting patches to the stable > > branch > > @@ -42,8 +42,10 @@ components. > > git bisect.) > > Patches should be properly formatted. > > Patches should be sufficiently tested before > > submitting. > > -Patches should be submitted to mesa-dev > > -for review using git send-email. > > +Patches should be submitted > > +to mesa-dev or with > > +a merge request > > +for review. > > > > > > > > @@ -180,10 +182,19 @@ run. > > > > > > > > -Mailing Patches > > +Submitting Patches > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > +Patches may be submitted to the Mesa project by > > +email or with a > > +GitLab merge request. To prevent > > +duplicate code review, only use one method to submit your changes. > > + > > + > > +Mailing Patches > > + > > + > > +Patches may be sent to the mesa-dev mailing list for review: > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev";> > > mesa-dev@lists.freedesktop.org. > > When submitting a patch make sure to use > > @@ -217,8 +228,63 @@ disabled before sending your patches. (Note that you > > may need to contact > > your email administrator for this.) > > > > > > +GitLab Merge Requests > > + > > + > > + https://gitlab.freedesktop.org/mesa/mesa";>GitLab Merge > > + Requests (MR) can also be used to submit patches for Mesa. > > + > > + > > + > > + If the MR may have interest for most of the Mesa community, you can > > + send an email to the mesa-dev email list including a link to the MR. > > + Don't send the patch to mesa-dev, just the MR link. > > + > > + > > + Add labels to your MR to help reviewers find it. For example: > > + > > +Mesa changes affecting all drivers: mesa > > +Hardware vendor specific code: amd, intel, nvidia, ... > > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > > + radv, vc4, ... > > +Other tag examples: gallium, util > > + > > + > > + > > + If you revise your patches based on code review and push an update > > + to your branch, you should maintain a clean history > > + in your patches. There should not be "fixup" patches in the history. > > + The series should be buildable and functional after every commit > > + whenever you push the branch. > > + > > + > > + It is your responsibility to keep the MR alive and making progress, > > + as there are no guarantees that a Mesa dev will independently take > > + interest in it. > > + > > + > > + Some other notes: > > + > > +Make changes and update your branch based on feedback > >
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher wrote: > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: > > > > Personally, I will continue to use the list, at least for a simplicity > > point of view. I'm not sure if using a new tool will improve quality and > > code review process. > > > > Though, if the majority reports that is really nice to use, I will > > probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. FWIW I also do a lot of drive-by reviews. Perhaps those aren't valuable -- dunno. Either way, if it's not in email, I won't end up seeing it. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 00/18] Meson fixes and Travis integration
Hi guys, While hacking on the integration I've spotted a few issues with our meson build - patches 1-6. The rest of the series is Dylan's patches followed by a build matrix analogous to the autotools/make one. As-is this causes one issue on macOS due to overly strict guards in the meson code. I've respinned the series more than 10 times and everything else seems solid. Personally I prefer if we get this (or anything functionally analogous) and address macOS as follow-up. Let me know what you think. Emil Cc: Rhys Kidd Cc: Dylan Baker Dylan Baker (3): travis: meson: use native files to override llvm-config travis: Don't try to read libdrm out of configure.ac travis: meson: enable unit tests Emil Velikov (15): meson: don't require glx/egl/gbm with gallium drivers pipe-loader: meson: reference correct library glx: meson: build src/glx only with -Dglx=dri glx: meson: drop includes from a link-only library glx: meson: wire up the dispatch-index-check test glx/test: meson: assorted include fixes configure: add CXX11_CXXFLAGS to LLVM_CXXFLAGS travis: flip to distro xenial, sudo true travis: meson: print the configured state travis: printout llvm-config --version travis: meson: use FOO_DRIVERS directly travis: meson: add unwind handling travis: meson: explicitly control the DRI loaders travis: meson: add explicit handling to gallium ST travis: meson: port gallium build combinations over .travis.yml | 353 +--- configure.ac| 1 + meson.build | 6 +- src/gallium/targets/pipe-loader/meson.build | 2 +- src/glx/meson.build | 31 +- src/glx/tests/meson.build | 9 +- src/meson.build | 2 +- 7 files changed, 267 insertions(+), 137 deletions(-) -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/18] glx: meson: wire up the dispatch-index-check test
From: Emil Velikov Accidentally dropped with earlier commit.! Fixes: 4ccb9816737 ("meson: Use consistent style for tests") Signed-off-by: Emil Velikov --- src/glx/tests/meson.build | 5 + 1 file changed, 5 insertions(+) diff --git a/src/glx/tests/meson.build b/src/glx/tests/meson.build index fa3ca9db8cd..2420c90c437 100644 --- a/src/glx/tests/meson.build +++ b/src/glx/tests/meson.build @@ -33,6 +33,11 @@ if with_shared_glapi files_glx_test += files('query_renderer_implementation_unittest.cpp') endif + test( +'dispatch-index-check', +files('dispatch-index-check'), +suite : ['glx'], + ) test( 'glx-test', executable( -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/18] glx: meson: build src/glx only with -Dglx=dri
From: Emil Velikov The library is the dri capable one, push the check src/meson.build, instead of the current partial handling in src/glx/meson.build. Fixes: a47c525f328 ("meson: build glx") Signed-off-by: Emil Velikov --- src/glx/meson.build | 32 +++- src/meson.build | 2 +- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/glx/meson.build b/src/glx/meson.build index 3fd74439b11..898ed1f5826 100644 --- a/src/glx/meson.build +++ b/src/glx/meson.build @@ -152,23 +152,21 @@ libglx = static_library( build_by_default : false, ) -if with_glx == 'dri' - libgl = shared_library( -gl_lib_name, -[], -include_directories : [inc_common, inc_glapi, inc_loader, inc_gl_internal], -link_with : [libglapi_static, libglapi], -link_whole : libglx, -link_args : [ld_args_bsymbolic, ld_args_gc_sections, extra_ld_args_libgl], -dependencies : [ - dep_libdrm, dep_dl, dep_m, dep_thread, dep_x11, dep_xcb_glx, dep_xcb, - dep_x11_xcb, dep_xcb_dri2, dep_xext, dep_xfixes, dep_xdamage, dep_xxf86vm, - extra_deps_libgl, -], -version : gl_lib_version, -install : true, - ) -endif +libgl = shared_library( + gl_lib_name, + [], + include_directories : [inc_common, inc_glapi, inc_loader, inc_gl_internal], + link_with : [libglapi_static, libglapi], + link_whole : libglx, + link_args : [ld_args_bsymbolic, ld_args_gc_sections, extra_ld_args_libgl], + dependencies : [ +dep_libdrm, dep_dl, dep_m, dep_thread, dep_x11, dep_xcb_glx, dep_xcb, +dep_x11_xcb, dep_xcb_dri2, dep_xext, dep_xfixes, dep_xdamage, dep_xxf86vm, +extra_deps_libgl, + ], + version : gl_lib_version, + install : true, +) if with_tests subdir('tests') diff --git a/src/meson.build b/src/meson.build index 915441fb2ce..ae094fccf6c 100644 --- a/src/meson.build +++ b/src/meson.build @@ -74,7 +74,7 @@ subdir('loader') if with_platform_haiku subdir('hgl') endif -if with_glx != 'disabled' +if with_glx == 'dri' subdir('glx') endif if with_gbm -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/18] meson: don't require glx/egl/gbm with gallium drivers
From: Emil Velikov The gallium drivers do not require a DRI loader. Drop the artificial and unnecessary restriction. Fixes: af9d276134d ("meson: build libmesa_gallium") Signed-off-by: Emil Velikov --- There are a few use cases: - building a gallium state-tracker, say opencl in st/clover, without having to build the dri one (st/dri) Use case: shipping only a opencl/omx/va driver. - building dri loader without an actual DRI driver. Use case: working/updating libGL/libGLX_mesa and egl/gbm equivalents. This patch should address the former, although the latter seems figgly to fix from a quick look. Input and patches will be appreciated. --- meson.build | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index db2d27d2459..2aa2794f971 100644 --- a/meson.build +++ b/meson.build @@ -387,9 +387,9 @@ endif if with_any_vk and (with_platform_x11 and not with_dri3) error('Vulkan drivers require dri3 for X11 support') endif -if with_dri or with_gallium - if with_glx == 'disabled' and not with_egl and not with_platform_haiku -error('building dri or gallium drivers require at least one window system') +if with_dri + if with_glx == 'disabled' and not with_egl and not with_gbm +error('building dri drivers require at least one windowing system') endif endif -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/18] glx: meson: drop includes from a link-only library
From: Emil Velikov When producing the final libGL.so/libGLX_mesa.so we only link the local static helper lib (libglx). Thus there's no reason for the includes. Fixes: a47c525f328 ("meson: build glx") Signed-off-by: Emil Velikov --- src/glx/meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/src/glx/meson.build b/src/glx/meson.build index 898ed1f5826..4eb2e0a21d2 100644 --- a/src/glx/meson.build +++ b/src/glx/meson.build @@ -155,7 +155,6 @@ libglx = static_library( libgl = shared_library( gl_lib_name, [], - include_directories : [inc_common, inc_glapi, inc_loader, inc_gl_internal], link_with : [libglapi_static, libglapi], link_whole : libglx, link_args : [ld_args_bsymbolic, ld_args_gc_sections, extra_ld_args_libgl], -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/18] glx/test: meson: assorted include fixes
From: Emil Velikov Swap '..' with the symbolic inc_glx and add glproto as dependency. That will pull the correct include, effectively fixing the tests on macOS. Fixes: a47c525f328 ("meson: build glx") Signed-off-by: Emil Velikov --- src/glx/tests/meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/glx/tests/meson.build b/src/glx/tests/meson.build index 2420c90c437..68e1ca59c22 100644 --- a/src/glx/tests/meson.build +++ b/src/glx/tests/meson.build @@ -46,9 +46,9 @@ if with_shared_glapi link_with : [libglx, libglapi], include_directories : [ inc_src, inc_include, inc_mesa, inc_mapi, inc_gl_internal, -include_directories('..'), +inc_glx, ], - dependencies : [dep_libdrm, dep_thread, idep_gtest] + dependencies : [dep_libdrm, dep_glproto, dep_thread, idep_gtest] ), suite : ['glx'], ) -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/18] travis: meson: print the configured state
From: Emil Velikov Signed-off-by: Emil Velikov --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index deec40cb135..6fefe06617a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -670,5 +670,6 @@ script: export CFLAGS="$CFLAGS -isystem`pwd`" meson _build $MESON_OPTIONS + meson configure _build ninja -C _build fi -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/18] pipe-loader: meson: reference correct library
From: Emil Velikov The library is called libgalliumvl_stub - note singular. Fixes: 42ea0631f10 ("meson: build clover") Signed-off-by: Emil Velikov --- src/gallium/targets/pipe-loader/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/targets/pipe-loader/meson.build b/src/gallium/targets/pipe-loader/meson.build index 5a44102a69d..e9454d5666a 100644 --- a/src/gallium/targets/pipe-loader/meson.build +++ b/src/gallium/targets/pipe-loader/meson.build @@ -31,7 +31,7 @@ if (with_gallium_va or with_gallium_vdpau or with_gallium_omx != 'disabled' or with_gallium_xvmc or with_dri) pipe_loader_link_with += libgalliumvl else - pipe_loader_link_with += libgalliumvl_stubs + pipe_loader_link_with += libgalliumvl_stub endif if (with_gallium_va or with_gallium_vdpau or with_gallium_omx != 'disabled' or with_gallium_xvmc) -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/18] configure: add CXX11_CXXFLAGS to LLVM_CXXFLAGS
From: Emil Velikov Seemingly with LLVM7 and GCC 5.0, the former won't properly advertise -std=c++11 and the latter will choke. dd this temporary workaround, otherwise we'll get errors like: In file included from /usr/include/c++/5/type_traits:35:0, from /usr/lib/llvm-7/include/llvm/Support/type_traits.h:18, from /usr/lib/llvm-7/include/llvm/ADT/Optional.h:22, from /usr/lib/llvm-7/include/llvm/ADT/STLExtras.h:20, from /usr/lib/llvm-7/include/llvm/ADT/StringRef.h:13, from /usr/lib/llvm-7/include/llvm/Target/TargetMachine.h:17, from ../../../src/amd/common/ac_llvm_helper.cpp:36: /usr/include/c++/5/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options. Signed-off-by: Emil Velikov --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 5d3da4b7c48..e5e95f12732 100644 --- a/configure.ac +++ b/configure.ac @@ -2903,6 +2903,7 @@ if test "x$enable_llvm" = xyes; then LLVM_LDFLAGS=`$LLVM_CONFIG --ldflags` LLVM_CFLAGS=$LLVM_CPPFLAGS # CPPFLAGS seem to be sufficient LLVM_CXXFLAGS=`strip_unwanted_llvm_flags "$LLVM_CONFIG --cxxflags"` +LLVM_CXXFLAGS="$CXX11_CXXFLAGS $LLVM_CXXFLAGS" dnl Set LLVM_LIBS - This is done after the driver configuration so dnl that drivers can add additional components to LLVM_COMPONENTS. -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/18] travis: meson: enable unit tests
From: Dylan Baker v2: [Emil] pass the argument directly to meson Reviewed-by: Emil Velikov (v1) Signed-off-by: Emil Velikov --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3c34942cadb..8cb7f8b95c1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -649,7 +649,9 @@ script: export CFLAGS="$CFLAGS -isystem`pwd`" meson _build $MESON_OPTIONS \ - --native-file=native.file + --native-file=native.file \ + -Dbuild-tests=true meson configure _build ninja -C _build + ninja -C _build test fi -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/18] travis: meson: use native files to override llvm-config
From: Dylan Baker This is the supported way to do this, and should be more robust and reliable. v2: [Emil] - enable backslash escapes - don't hardcode the path - pass the argument directly to meson Reviewed-by: Emil Velikov (v1) Signed-off-by: Emil Velikov --- .travis.yml | 22 +- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index c69f322d7b3..c398ebc748a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -650,28 +650,16 @@ script: MESON_OPTIONS="-Ddri-drivers=${DRI_DRIVERS:-[]} -Dgallium-drivers=${GALLIUM_DRIVERS:-[]} -Dvulkan-drivers=${VULKAN_DRIVERS:-[]}" fi - # Travis CI has moved to LLVM 5.0, and meson is detecting - # automatically the available version in /usr/local/bin based on - # the PATH env variable order preference. + # We need to control the version of llvm-config we're using, so we'll + # generate a native file to do so. This requires meson >=0.49 # - # As for 0.44.x, Meson cannot receive the path to the - # llvm-config binary as a configuration parameter. See - # https://github.com/mesonbuild/meson/issues/2887 and - # https://github.com/dcbaker/meson/commit/7c8b6ee3fa42f43c9ac7dcacc61a77eca3f1bcef - # - # We want to use the custom (APT) installed version. Therefore, - # let's make Meson find our wanted version sooner than the one - # at /usr/local/bin - # - # Once this is corrected, we would still need a patch similar - # to: - # https://lists.freedesktop.org/archives/mesa-dev/2017-December/180217.html - test -f /usr/bin/$LLVM_CONFIG && ln -s /usr/bin/$LLVM_CONFIG $HOME/prefix/bin/llvm-config + echo -e "[binaries]\nllvm-config = '`which $LLVM_CONFIG`'" > native.file $LLVM_CONFIG --version export CFLAGS="$CFLAGS -isystem`pwd`" - meson _build $MESON_OPTIONS + meson _build $MESON_OPTIONS \ + --native-file=native.file meson configure _build ninja -C _build fi -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 15/18] travis: meson: add unwind handling
From: Emil Velikov Signed-off-by: Emil Velikov --- .travis.yml | 4 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 4966f7eb1bf..d16c896b8a1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,7 @@ matrix: - env: - LABEL="meson Vulkan" - BUILD=meson +- UNWIND="false" - VULKAN_DRIVERS="intel,amd" - LLVM_VERSION=7 - LLVM_CONFIG="llvm-config-${LLVM_VERSION}" @@ -55,6 +56,7 @@ matrix: - env: - LABEL="meson loaders/classic DRI" - BUILD=meson +- UNWIND="false" - DRI_DRIVERS="i915,i965,r100,r200,swrast,nouveau" addons: apt: @@ -457,6 +459,7 @@ matrix: - env: - LABEL="macOS meson" - BUILD=meson +- UNWIND="false" os: osx before_install: @@ -643,6 +646,7 @@ script: meson _build $MESON_OPTIONS \ --native-file=native.file \ -Dbuild-tests=true \ + -Dlibunwind=${UNWIND} \ -Ddri-drivers=${DRI_DRIVERS:-[]} \ -Dgallium-drivers=${GALLIUM_DRIVERS:-[]} \ -Dvulkan-drivers=${VULKAN_DRIVERS:-[]} -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/18] travis: printout llvm-config --version
From: Emil Velikov Provides quick and easy feedback. Signed-off-by: Emil Velikov --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 6fefe06617a..c69f322d7b3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -668,6 +668,8 @@ script: # https://lists.freedesktop.org/archives/mesa-dev/2017-December/180217.html test -f /usr/bin/$LLVM_CONFIG && ln -s /usr/bin/$LLVM_CONFIG $HOME/prefix/bin/llvm-config + $LLVM_CONFIG --version + export CFLAGS="$CFLAGS -isystem`pwd`" meson _build $MESON_OPTIONS meson configure _build -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/18] travis: Don't try to read libdrm out of configure.ac
From: Dylan Baker Since we're going to delete it shortly Reviewed-by: Emil Velikov --- .travis.yml | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index c398ebc748a..3c34942cadb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ env: - GLPROTO_VERSION=glproto-1.4.17 - DRI2PROTO_VERSION=dri2proto-2.8 - LIBPCIACCESS_VERSION=libpciaccess-0.13.4 -- LIBDRM_VERSION=libdrm-2.4.74 +- LIBDRM_VERSION=libdrm-2.4.95 - XCBPROTO_VERSION=xcb-proto-1.13 - RANDRPROTO_VERSION=randrproto-1.3.0 - LIBXRANDR_VERSION=libXrandr-1.3.0 @@ -505,16 +505,6 @@ install: pip2 install --user mako; fi - # Since libdrm gets updated in configure.ac regularly, try to pick up the - # latest version from there. - - for line in `grep "^LIBDRM.*_REQUIRED=" configure.ac`; do - old_ver=`echo $LIBDRM_VERSION | sed 's/libdrm-//'`; - new_ver=`echo $line | sed 's/.*REQUIRED=//'`; - if `echo "$old_ver,$new_ver" | tr ',' '\n' | sort -Vc 2> /dev/null`; then -export LIBDRM_VERSION="libdrm-$new_ver"; - fi; -done - # Install dependencies where we require specific versions (or where # disallowed by Travis CI's package whitelisting). -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/18] travis: flip to distro xenial, sudo true
From: Emil Velikov The latter is the default these days and Travis will be removing sudo soonish. Flipping to xenial, allows us to remove a bunch of hacks we have. Plus it prevents us from adding new ones, to workaround what seems like a gcc/binutils bug. For example (from the upcoming meson build): FAILED: ccache c++ -o src/gallium/targets/pipe-loader/pipe_r600.so ... ... src/util/libmesa_util.a ... /usr/lib/x86_64-linux-gnu/libz.so ... src/util/libmesa_util.a(disk_cache.c.o): In function `deflate_and_write_to_disk': _build/../src/util/disk_cache.c:746: undefined reference to `deflateInit_' _build/../src/util/disk_cache.c:765: undefined reference to `deflate' ... As we can see, even though libz.so is explicitly passed after the object that requires it - the linker still fails to see the symbols. Avoid all those situations - flip the switch. Signed-off-by: Emil Velikov --- .travis.yml | 99 +++-- 1 file changed, 27 insertions(+), 72 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8eceb4e86f5..deec40cb135 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: c -sudo: false -dist: trusty +dist: xenial cache: apt: true @@ -43,12 +42,9 @@ matrix: addons: apt: sources: -- sourceline: 'deb http://apt.llvm.org/trusty/ llvm-toolchain-trusty-7 main' +- sourceline: 'deb http://apt.llvm.org/xenial/ llvm-toolchain-xenial-7 main' key_url: https://apt.llvm.org/llvm-snapshot.gpg.key -# llvm-7 requires libstdc++4.9 which is not in main repo -- ubuntu-toolchain-r-test packages: -# From sources above - llvm-7-dev # Common - xz-utils @@ -57,6 +53,7 @@ matrix: - libelf-dev - python3.5 - python3-pip +- python3-setuptools - env: - LABEL="meson loaders/classic DRI" - BUILD=meson @@ -68,12 +65,14 @@ matrix: packages: - xz-utils - x11proto-xf86vidmode-dev +- libxxf86vm-dev - libexpat1-dev - libx11-xcb-dev - libxdamage-dev - libxfixes-dev - python3.5 - python3-pip +- python3-setuptools - env: - LABEL="make loaders/classic DRI" - BUILD=make @@ -90,11 +89,13 @@ matrix: packages: - xz-utils - x11proto-xf86vidmode-dev +- libxxf86vm-dev - libexpat1-dev - libx11-xcb-dev - libxdamage-dev - libxfixes-dev - python3-pip +- python3-setuptools - env: # NOTE: Building SWR is 2x (yes two) times slower than all the other # gallium drivers combined. @@ -113,12 +114,7 @@ matrix: - LIBUNWIND_FLAGS="--enable-libunwind" addons: apt: - sources: -- llvm-toolchain-trusty-6.0 -# llvm-6 requires libstdc++4.9 which is not in main repo -- ubuntu-toolchain-r-test packages: -# From sources above - llvm-6.0-dev # Common - xz-utils @@ -127,6 +123,7 @@ matrix: - libelf-dev - libunwind8-dev - python3-pip +- python3-setuptools - env: - LABEL="make Gallium Drivers RadeonSI" - BUILD=make @@ -143,10 +140,8 @@ matrix: addons: apt: sources: -- sourceline: 'deb http://apt.llvm.org/trusty/ llvm-toolchain-trusty-7 main' +- sourceline: 'deb http://apt.llvm.org/xenial/ llvm-toolchain-xenial-7 main' key_url: https://apt.llvm.org/llvm-snapshot.gpg.key -# llvm-7 requires libstdc++4.9 which is not in main repo -- ubuntu-toolchain-r-test packages: # From sources above - llvm-7-dev @@ -157,6 +152,7 @@ matrix: - libelf-dev - libunwind8-dev - python3-pip +- python3-setuptools - env: - LABEL="make Gallium Drivers Other" - BUILD=make @@ -164,8 +160,6 @@ matrix: - MAKE_CHECK_COMMAND="true" - LLVM_VERSION=3.9 - LLVM_CONFIG="llvm-config-${LLVM_VERSION}" -# New binutils linker is required for llvm-3.9 -- OVERRIDE_PATH=/usr/lib/binutils-2.26/bin - DRI_LOADERS="--disable-glx --disable-gbm --disable-egl" - DRI_DRIVERS="" - GALLIUM_ST="--enable-dri --disable-opencl --disable-xa --disable-nine --disable-xvmc --disable-vdpau --disable-va --disable-omx-bellagio --disable-gallium-osmesa" @@ -174,13 +168,9 @@ matrix: - LIBUNWIND_FLAGS="--enable-libunwind" addons: apt: - sources: -- llvm-toolchain-trusty-3.9 packages: -- binutils-2.26
[Mesa-dev] [PATCH 16/18] travis: meson: explicitly control the DRI loaders
From: Emil Velikov Signed-off-by: Emil Velikov --- .travis.yml | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index d16c896b8a1..4933d8d78a2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,6 +35,7 @@ matrix: - LABEL="meson Vulkan" - BUILD=meson - UNWIND="false" +- DRI_LOADERS="-Dglx=disabled -Dgbm=false -Degl=false -Dplatforms=x11,wayland,drm -Dosmesa=none" - VULKAN_DRIVERS="intel,amd" - LLVM_VERSION=7 - LLVM_CONFIG="llvm-config-${LLVM_VERSION}" @@ -57,6 +58,7 @@ matrix: - LABEL="meson loaders/classic DRI" - BUILD=meson - UNWIND="false" +- DRI_LOADERS="-Dglx=dri -Dgbm=true -Degl=true -Dplatforms=x11,wayland,drm,surfaceless -Dosmesa=classic" - DRI_DRIVERS="i915,i965,r100,r200,swrast,nouveau" addons: apt: @@ -460,6 +462,7 @@ matrix: - LABEL="macOS meson" - BUILD=meson - UNWIND="false" +- DRI_LOADERS="-Dglx=dri -Dgbm=false -Degl=false -Dplatforms=x11 -Dosmesa=none" os: osx before_install: @@ -630,11 +633,6 @@ script: - | if test "x$BUILD" = xmeson; then - - if test "x$TRAVIS_OS_NAME" == xosx; then -MESON_OPTIONS="-Degl=false" - fi - # We need to control the version of llvm-config we're using, so we'll # generate a native file to do so. This requires meson >=0.49 # @@ -643,10 +641,11 @@ script: $LLVM_CONFIG --version export CFLAGS="$CFLAGS -isystem`pwd`" - meson _build $MESON_OPTIONS \ + meson _build \ --native-file=native.file \ -Dbuild-tests=true \ -Dlibunwind=${UNWIND} \ + ${DRI_LOADERS} \ -Ddri-drivers=${DRI_DRIVERS:-[]} \ -Dgallium-drivers=${GALLIUM_DRIVERS:-[]} \ -Dvulkan-drivers=${VULKAN_DRIVERS:-[]} -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 17/18] travis: meson: add explicit handling to gallium ST
From: Emil Velikov Signed-off-by: Emil Velikov --- .travis.yml | 4 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 4933d8d78a2..125d6ce3c68 100644 --- a/.travis.yml +++ b/.travis.yml @@ -36,6 +36,7 @@ matrix: - BUILD=meson - UNWIND="false" - DRI_LOADERS="-Dglx=disabled -Dgbm=false -Degl=false -Dplatforms=x11,wayland,drm -Dosmesa=none" +- GALLIUM_ST="-Ddri3=true -Dgallium-vdpau=false -Dgallium-xvmc=false -Dgallium-omx=disabled -Dgallium-va=false -Dgallium-xa=false -Dgallium-nine=false -Dgallium-opencl=disabled" - VULKAN_DRIVERS="intel,amd" - LLVM_VERSION=7 - LLVM_CONFIG="llvm-config-${LLVM_VERSION}" @@ -60,6 +61,7 @@ matrix: - UNWIND="false" - DRI_LOADERS="-Dglx=dri -Dgbm=true -Degl=true -Dplatforms=x11,wayland,drm,surfaceless -Dosmesa=classic" - DRI_DRIVERS="i915,i965,r100,r200,swrast,nouveau" +- GALLIUM_ST="-Ddri3=true -Dgallium-vdpau=false -Dgallium-xvmc=false -Dgallium-omx=disabled -Dgallium-va=false -Dgallium-xa=false -Dgallium-nine=false -Dgallium-opencl=disabled" addons: apt: packages: @@ -463,6 +465,7 @@ matrix: - BUILD=meson - UNWIND="false" - DRI_LOADERS="-Dglx=dri -Dgbm=false -Degl=false -Dplatforms=x11 -Dosmesa=none" +- GALLIUM_ST="-Ddri3=true -Dgallium-vdpau=false -Dgallium-xvmc=false -Dgallium-omx=disabled -Dgallium-va=false -Dgallium-xa=false -Dgallium-nine=false -Dgallium-opencl=disabled" os: osx before_install: @@ -647,6 +650,7 @@ script: -Dlibunwind=${UNWIND} \ ${DRI_LOADERS} \ -Ddri-drivers=${DRI_DRIVERS:-[]} \ + ${GALLIUM_ST} \ -Dgallium-drivers=${GALLIUM_DRIVERS:-[]} \ -Dvulkan-drivers=${VULKAN_DRIVERS:-[]} meson configure _build -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 14/18] travis: meson: use FOO_DRIVERS directly
From: Emil Velikov It makes for a shorter MESON_OPTIONS and cleaner handling. Signed-off-by: Emil Velikov --- .travis.yml | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8cb7f8b95c1..4966f7eb1bf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,8 +34,6 @@ matrix: - env: - LABEL="meson Vulkan" - BUILD=meson -- DRI_DRIVERS="" -- GALLIUM_DRIVERS="" - VULKAN_DRIVERS="intel,amd" - LLVM_VERSION=7 - LLVM_CONFIG="llvm-config-${LLVM_VERSION}" @@ -58,8 +56,6 @@ matrix: - LABEL="meson loaders/classic DRI" - BUILD=meson - DRI_DRIVERS="i915,i965,r100,r200,swrast,nouveau" -- GALLIUM_DRIVERS="" -- VULKAN_DRIVERS="" addons: apt: packages: @@ -636,10 +632,6 @@ script: MESON_OPTIONS="-Degl=false" fi - if test "x$TRAVIS_OS_NAME" == xlinux; then -MESON_OPTIONS="-Ddri-drivers=${DRI_DRIVERS:-[]} -Dgallium-drivers=${GALLIUM_DRIVERS:-[]} -Dvulkan-drivers=${VULKAN_DRIVERS:-[]}" - fi - # We need to control the version of llvm-config we're using, so we'll # generate a native file to do so. This requires meson >=0.49 # @@ -650,7 +642,10 @@ script: export CFLAGS="$CFLAGS -isystem`pwd`" meson _build $MESON_OPTIONS \ --native-file=native.file \ - -Dbuild-tests=true + -Dbuild-tests=true \ + -Ddri-drivers=${DRI_DRIVERS:-[]} \ + -Dgallium-drivers=${GALLIUM_DRIVERS:-[]} \ + -Dvulkan-drivers=${VULKAN_DRIVERS:-[]} meson configure _build ninja -C _build ninja -C _build test -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 18/18] travis: meson: port gallium build combinations over
From: Emil Velikov This commit adds a number of build combos: - Gallium Drivers {SWR, RadeonSI, Others) Each one has different LLVM requirements. Building SWR alone is twice as slow as all other drivers combined. - Gallium ST Clover LLVM {5,6,7} Because C++ API changes all the time. Analogous to above building Clover takes as much time as building all other ST combined. - Gallium ST Others Nouveau is used, instead of i915g since meson has explicit target tracking. Meaning that a configure error is throws if we use i915g with say va, vdpau or others. Note: LLVM prior to 5.0 is intentionally dropped. If needed we can add that later. Signed-off-by: Emil Velikov --- .travis.yml | 187 1 file changed, 187 insertions(+) diff --git a/.travis.yml b/.travis.yml index 125d6ce3c68..b70df99d67e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -98,6 +98,193 @@ matrix: - libxfixes-dev - python3-pip - python3-setuptools +- env: +# NOTE: Building SWR is 2x (yes two) times slower than all the other +# gallium drivers combined. +# Start this early so that it doesn't hunder the run time. +- LABEL="meson Gallium Drivers SWR" +- BUILD=meson +- UNWIND="true" +- DRI_LOADERS="-Dglx=disabled -Degl=false -Dgbm=false" +- GALLIUM_ST="-Ddri3=false -Dgallium-vdpau=false -Dgallium-xvmc=false -Dgallium-omx=disabled -Dgallium-va=false -Dgallium-xa=false -Dgallium-nine=false -Dgallium-opencl=disabled" +- GALLIUM_DRIVERS="swr" +- LLVM_VERSION=6.0 +- LLVM_CONFIG="llvm-config-${LLVM_VERSION}" + addons: +apt: + packages: +- llvm-6.0-dev +# Common +- xz-utils +- libexpat1-dev +- libx11-xcb-dev +- libelf-dev +- libunwind8-dev +- python3.5 +- python3-pip +- python3-setuptools +- env: +- LABEL="meson Gallium Drivers RadeonSI" +- BUILD=meson +- UNWIND="true" +- DRI_LOADERS="-Dglx=disabled -Degl=false -Dgbm=false" +- GALLIUM_ST="-Ddri3=false -Dgallium-vdpau=false -Dgallium-xvmc=false -Dgallium-omx=disabled -Dgallium-va=false -Dgallium-xa=false -Dgallium-nine=false -Dgallium-opencl=disabled" +- GALLIUM_DRIVERS="radeonsi" +- LLVM_VERSION=7 +- LLVM_CONFIG="llvm-config-${LLVM_VERSION}" + addons: +apt: + sources: +- sourceline: 'deb http://apt.llvm.org/xenial/ llvm-toolchain-xenial-7 main' + key_url: https://apt.llvm.org/llvm-snapshot.gpg.key + packages: +# From sources above +- llvm-7-dev +# Common +- xz-utils +- libexpat1-dev +- libx11-xcb-dev +- libelf-dev +- libunwind8-dev +- python3.5 +- python3-pip +- python3-setuptools +- env: +- LABEL="meson Gallium Drivers Other" +- BUILD=meson +- UNWIND="true" +- DRI_LOADERS="-Dglx=disabled -Degl=false -Dgbm=false" +- GALLIUM_ST="-Ddri3=false -Dgallium-vdpau=false -Dgallium-xvmc=false -Dgallium-omx=disabled -Dgallium-va=false -Dgallium-xa=false -Dgallium-nine=false -Dgallium-opencl=disabled" +- GALLIUM_DRIVERS="i915,nouveau,pl111,r300,r600,freedreno,svga,swrast,v3d,vc4,virgl,etnaviv,imx" +- LLVM_VERSION=5.0 +- LLVM_CONFIG="llvm-config-${LLVM_VERSION}" + addons: +apt: + packages: +# LLVM packaging is broken and misses these dependencies +- libedit-dev +- llvm-5.0-dev +# Common +- xz-utils +- libexpat1-dev +- libx11-xcb-dev +- libelf-dev +- libunwind8-dev +- python3.5 +- python3-pip +- python3-setuptools +- env: +- LABEL="meson Gallium ST Clover LLVM-5.0" +- BUILD=meson +- UNWIND="true" +- DRI_LOADERS="-Dglx=disabled -Degl=false -Dgbm=false" +- GALLIUM_ST="-Ddri3=false -Dgallium-vdpau=false -Dgallium-xvmc=false -Dgallium-omx=disabled -Dgallium-va=false -Dgallium-xa=false -Dgallium-nine=false -Dgallium-opencl=icd" +- GALLIUM_DRIVERS="r600" +- LLVM_VERSION=5.0 +- LLVM_CONFIG="llvm-config-${LLVM_VERSION}" + addons: +apt: + packages: +- libclc-dev +# LLVM packaging is broken and misses these dependencies +- libedit-dev +- llvm-5.0-dev +- clang-5.0 +- libclang-5.0-dev +# Common +- xz-utils +- libexpat1-dev +- libx11-xcb-dev +- libelf-dev +- libunwind8-dev +- python3-pip +- python3-setuptools +- env: +- LABEL="meson Gallium ST Clover
Re: [Mesa-dev] Let's talk about -DDEBUG
Hi, On 13.12.2018 12.19, Eric Engestrom wrote: On Wednesday, 2018-12-12 19:45:06 -0500, Marek Olšák wrote: On Wed, Dec 12, 2018 at 7:35 PM Rob Clark wrote: On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker wrote: Quoting Rob Clark (2018-12-12 15:52:47) ... I guess I should have covered that: autotools had effectively two build types "debug" and "not debug", "debug" set "-DDEBUG -g -O2", "not debug" set -DNDEBUG Meson has 4 build types, and a separate toggle for NDEBUG: debug: -O0 -DDEBUG (we add -DDEBUG) debugoptimzed: -O2 -g release: -O2 plain: (nothing) I generally view meson as an upgrade in this respect, I guess mostly because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine by me).. maybe making meson debug config use -O2 would bridge the gap? If so I have no problem with dropping -O0 and making debug config also use -O2 Target without optimizations is good to track down issues triggered by compiler optimizations, and to have faster builds. Sounds good. Also I think we should make -Db_ndebug=true the default for release builds. !NDEBUG enables a lot more debugging code than just assertions. That's already the case :) The default value (line 29 of the root meson.build) is: b_ndebug=if-release which means it will default to `true` if `buildtype=release`, and `false` otherwise, while still being overridden if you manually set `b_ndebug`. There needs to be something that's release with debug symbols, but without _any_ other differences. If build type assert handling differs from release, debug symbols are less of a use for debugging issues with release build. Build type names could also be more descriptive, although something like this seems overkill: - release - release_with_symbols - release_with_symbols_and_asserts - full_debug Meson can help by outputting exact description of each build types, e.g. when one doesn't specify one, or unrecognized one is provided. - Eero ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 2018-12-13 4:52 p.m., Alex Deucher wrote: > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: >> >> Personally, I will continue to use the list, at least for a simplicity >> point of view. I'm not sure if using a new tool will improve quality and >> code review process. >> >> Though, if the majority reports that is really nice to use, I will >> probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. Hopefully we can get notifications of new MRs to the mailing lists at some point, but in the meantime, this should be solvable by tuning your notification settings in GitLab. That might even allow you to fine-tune what you receive notifications for, compared to the all-or-nothing with e-mail based review. > Out of curiosity what do others like about the MR interface? How are > you using it? What advantages does it give you over the mailing list? One things that's nice about the MR interface is the integration with the CI pipeline. But for me, it's not so much about the MR interface per se, but about the integration between different parts. With e-mail based review, it's sometimes a bit awkward to link between patches for testing / review and the final changes in Git, e.g. in bug reports. All of these things can be linked together by MRs. This should be even better once we use GitLab issues instead of Bugzilla. > I feel like the interface makes it harder than it needs to be to see > the actual changes in MR to be reviewed. All of the links click > through to the source view rather than the patch view. There are certainly still rough edges around review of MRs with multiple commits. There have been some improvements based on feedback provided by freedesktop.org folks to GitLab developers, but there's still need for more. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] WIP: meson: allow building DRI loaders without a DRI driver
From: Emil Velikov Reasonably often people will want to build the loader w/o any drivers. Be that debugging an issue in the former, distribution bundle or other. Currently there is an artificial restriction preventing people to build a config like the following: meson build/ -Dglx=dri -Ddri-drivers= -Dvulkan-drivers= -Dgallium-drivers= Cc: Dylan Baker Signed-off-by: Emil Velikov --- This is a WIP, since it's obviously incomplete/wrong. Input or fix would be appreciated. Thanks Emil --- meson.build | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/meson.build b/meson.build index 2aa2794f971..e1d6561c1a3 100644 --- a/meson.build +++ b/meson.build @@ -285,14 +285,6 @@ if with_glx == 'dri' endif endif -if not (with_dri or with_gallium or with_glx == 'xlib' or with_glx == 'gallium-xlib') - with_gles1 = false - with_gles2 = false - with_opengl = false - with_any_opengl = false - with_shared_glapi = false -endif - _gbm = get_option('gbm') if _gbm == 'auto' with_gbm = system_has_kms_drm and with_dri @@ -352,9 +344,7 @@ if with_glx != 'disabled' error('xlib conflicts with any dri driver') endif elif with_glx == 'dri' -if not with_dri - error('dri based GLX requires at least one DRI driver') -elif not with_shared_glapi +if not with_shared_glapi error('dri based GLX requires shared-glapi') endif endif -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Let's talk about -DDEBUG
On Thu, Dec 13, 2018 at 5:06 AM Eric Engestrom wrote: > On Wednesday, 2018-12-12 15:24:25 -0800, Dylan Baker wrote: > > In the autotools discussion I've come to realize that we also need to > talk about > > the -DDEBUG guard. It seems that there are two different uses, and thus > two > > different asks about it: > > > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > > - NIR and Intel (at least) use -DDEBUG to hide really expensive checks > that are > > useful, but necessarily tank performance. > > > > The first group would like -DDEBUG in debugoptimized builds, the second > > obviously doesn't. > > > > Is the right solution to move the first group being !NDEBUG, or would it > be > > better to split DEBUG into two different defines such as DEBUG_MESSAGES > and > > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), with > the > > first for both debug and debugoptimized and the second only in debug > builds? > > Replacing DEBUG with !NDEBUG is obviously trivially simpler, but I think > the right thing would be to split it into !NDEBUG and EXPENSIVE_VALIDATION > (the color suits me just fine :P), as both solutions satisfy the first > group but only the latter solution satisfies the 2nd group. > > I think a first pass might be to simply s/DEBUG/EXPENSIVE_VALIDATION/ so > that it expresses the intent more clearly, with a prior patch to convert > Nine and other obvious !NDEBUG candidates, then, later on, some of the > EXPENSIVE_VALIDATION can be promoted to !NDEBUG on a case-by-case basis. > I think this whole discussion is way over-thinking this. With autools, we had two options: --enable-debug or not which, as I understand it, corresponds to release and debug. Great. Now meson adds a new one. Let's just pick something that makes sense and call it a day; it really doesn't matter. Anyone who wants more control can just set their own CFLAGS. Regardless of what we do, we're not really loosing anything by doing this as people who build Nine today with --enable-debug are getting an unoptimized build the same as they would with -Dbuild-type=debug. Users/devs can also always set -Db_ndebug manually to get the behavior that they want. I don't know that I have all that strong of a preference as I'm not likely to use it anyway. On the one hand, the name implies that it's a debug build only optimized. This is different than CMake's RelWithDebugInfo which is clearly a release build with debug symbols. Based on that naming, i'd say we should leave asserts on. I think the root of the issue is that different developers have tied way too much stuff to -DDEBUG. The Nine people can add a -Dnine-logging=true flag that can turn on logging even in release builds. In the NIR-based drivers, we already have environment variables to shut off NIR validation to make things go faster even in debug builds. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: > On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher > wrote: > > > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > > wrote: > > > > > > Personally, I will continue to use the list, at least for a simplicity > > > point of view. I'm not sure if using a new tool will improve quality > and > > > code review process. > > > > > > Though, if the majority reports that is really nice to use, I will > > > probably change my mind. Not a strong reject. > > > > I agree. We've been using the MR interface for xf86-video-amdgpu and > > I find it awkward compared to the mailing list. Maybe it just takes > > getting used to. I also feel less inclined to do drive by patch > > review if I have to explicitly delve into the browser to look at the > > outstanding MRs. Over email, sometimes I see a patch set in my in box > > that piques my interest and I find some time to review it when I might > > not have otherwise if the bar were higher. > > FWIW I also do a lot of drive-by reviews. Perhaps those aren't > valuable -- dunno. Either way, if it's not in email, I won't end up > seeing it. > I find this commentary on drive-by reviews interesting. Personally, I don't find going to the web interface to be a problem at all but that may be because my e-mail is also in my web browser and it's as easy as clicking a link. Also, since I'm an active contributor who's likely to be making MRs and commenting on various MRs on a regular basis, I'm already on the MR page looking at it. If you randomly scrolled through the MR list in the same way you randomly scroll through the mailing list, would you be equally inclined to give drive-by reviews? That's an honest hypothetical question. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Let's talk about -DDEBUG
On 13/12/2018 17:26, Jason Ekstrand wrote: On Thu, Dec 13, 2018 at 5:06 AM Eric Engestrom mailto:eric.engest...@intel.com>> wrote: On Wednesday, 2018-12-12 15:24:25 -0800, Dylan Baker wrote: > In the autotools discussion I've come to realize that we also need to talk about > the -DDEBUG guard. It seems that there are two different uses, and thus two > different asks about it: > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > - NIR and Intel (at least) use -DDEBUG to hide really expensive checks that are > useful, but necessarily tank performance. > > The first group would like -DDEBUG in debugoptimized builds, the second > obviously doesn't. > > Is the right solution to move the first group being !NDEBUG, or would it be > better to split DEBUG into two different defines such as DEBUG_MESSAGES and > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), with the > first for both debug and debugoptimized and the second only in debug builds? Replacing DEBUG with !NDEBUG is obviously trivially simpler, but I think the right thing would be to split it into !NDEBUG and EXPENSIVE_VALIDATION (the color suits me just fine :P), as both solutions satisfy the first group but only the latter solution satisfies the 2nd group. I think a first pass might be to simply s/DEBUG/EXPENSIVE_VALIDATION/ so that it expresses the intent more clearly, with a prior patch to convert Nine and other obvious !NDEBUG candidates, then, later on, some of the EXPENSIVE_VALIDATION can be promoted to !NDEBUG on a case-by-case basis. I think this whole discussion is way over-thinking this. With autools, we had two options: --enable-debug or not which, as I understand it, corresponds to release and debug. Great. Now meson adds a new one. Let's just pick something that makes sense and call it a day; it really doesn't matter. Anyone who wants more control can just set their own CFLAGS. Regardless of what we do, we're not really loosing anything by doing this as people who build Nine today with --enable-debug are getting an unoptimized build the same as they would with -Dbuild-type=debug. Users/devs can also always set -Db_ndebug manually to get the behavior that they want. I don't know that I have all that strong of a preference as I'm not likely to use it anyway. On the one hand, the name implies that it's a debug build only optimized. This is different than CMake's RelWithDebugInfo which is clearly a release build with debug symbols. Based on that naming, i'd say we should leave asserts on. I think the root of the issue is that different developers have tied way too much stuff to -DDEBUG. The Nine people can add a -Dnine-logging=true flag that can turn on logging even in release builds. In the NIR-based drivers, we already have environment variables to shut off NIR validation to make things go faster even in debug builds. --Jason Hi, I agree with Jason that there seems to be a multitude of needs and that it may be hard to handle for all these needs in a simple way. Devs who want to stress specific parts of their code can indeed use CFLAGS, and thus there isn't need to have a meson build mode for every specific need. However I believe using a debug build option should be all that is needed for a user to help report bugs. If the user is investigating a crash, he wants to enable asserts and debug info. He may want to enable nine logging, etc. Dev flags may change between releases, while the user would always have the same debug option to enable all it may need. I think the autotools way was simple for the user, and the new meson way should be as simple. 'debugoptimized' is counter-intuitive for an user, who may expect all the mentioned debug info. To me debugoptimized should be similar to debug, but with -O2. Other dev specific debug options can be added with CFLAGS. Axel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Hi, On Thursday, 13 December 2018 17:40:49 CET Jason Ekstrand wrote: > On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: > > > On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher > > wrote: > > > > > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > > > wrote: > > > > > > > > Personally, I will continue to use the list, at least for a simplicity > > > > point of view. I'm not sure if using a new tool will improve quality > > and > > > > code review process. > > > > > > > > Though, if the majority reports that is really nice to use, I will > > > > probably change my mind. Not a strong reject. > > > > > > I agree. We've been using the MR interface for xf86-video-amdgpu and > > > I find it awkward compared to the mailing list. Maybe it just takes > > > getting used to. I also feel less inclined to do drive by patch > > > review if I have to explicitly delve into the browser to look at the > > > outstanding MRs. Over email, sometimes I see a patch set in my in box > > > that piques my interest and I find some time to review it when I might > > > not have otherwise if the bar were higher. > > > > FWIW I also do a lot of drive-by reviews. Perhaps those aren't > > valuable -- dunno. Either way, if it's not in email, I won't end up > > seeing it. > > > > I find this commentary on drive-by reviews interesting. Personally, I > don't find going to the web interface to be a problem at all but that may > be because my e-mail is also in my web browser and it's as easy as clicking > a link. Also, since I'm an active contributor who's likely to be making > MRs and commenting on various MRs on a regular basis, I'm already on the MR > page looking at it. If you randomly scrolled through the MR list in the > same way you randomly scroll through the mailing list, would you be equally > inclined to give drive-by reviews? That's an honest hypothetical question. Initially it seemed to me that I am about the only one sticking with mailing lists. And I personally feel like a too small contributor to really try to influence your decisions too much. But these recent hand full of mails all tell me that I am not that alone. I personally did contribute to several projects during the past years. All that only in part time, thus it had to be *very* efficient for myself. And that is something that I achieved by a consistent 'interface' to all those projects. Just my widely used and highly convenient mail client. So, all that worked in a sufficiently efficient way because I could combine this kind of 'work' even with my private mail that I could handle in between with that single 'interface'. So going to any web site there is already a detour and having multiple of them for each such project gives an even longer detour. Okay today it's mostly mesa that is left as well as a communication middle end used in vizsim applications. But going away too far away from a mailing list will be mostly a loss of efficiency for me. As I said, my two cents, that should not keep you all from doing changes that finally increase your all efficiency ... best Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 11:41 AM Jason Ekstrand wrote: > > On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: >> >> On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher wrote: >> > >> > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset >> > wrote: >> > > >> > > Personally, I will continue to use the list, at least for a simplicity >> > > point of view. I'm not sure if using a new tool will improve quality and >> > > code review process. >> > > >> > > Though, if the majority reports that is really nice to use, I will >> > > probably change my mind. Not a strong reject. >> > >> > I agree. We've been using the MR interface for xf86-video-amdgpu and >> > I find it awkward compared to the mailing list. Maybe it just takes >> > getting used to. I also feel less inclined to do drive by patch >> > review if I have to explicitly delve into the browser to look at the >> > outstanding MRs. Over email, sometimes I see a patch set in my in box >> > that piques my interest and I find some time to review it when I might >> > not have otherwise if the bar were higher. >> >> FWIW I also do a lot of drive-by reviews. Perhaps those aren't >> valuable -- dunno. Either way, if it's not in email, I won't end up >> seeing it. > > > I find this commentary on drive-by reviews interesting. Personally, I don't > find going to the web interface to be a problem at all but that may be > because my e-mail is also in my web browser and it's as easy as clicking a > link. Also, since I'm an active contributor who's likely to be making MRs > and commenting on various MRs on a regular basis, I'm already on the MR page > looking at it. If you randomly scrolled through the MR list in the same way > you randomly scroll through the mailing list, would you be equally inclined > to give drive-by reviews? That's an honest hypothetical question. > I'm still very much email driven and most of the projects I work most in are still email based. I think the problem is that you don't see the patches, just the MR list so you have to kind of guess based on the MR names. When I'm reading through email, a function name or something in a patch title may catch my eye and I'll then take a look at that patch and probably the whole series. E.g., "oh, they are changing foo(), I just did some work there a few weeks ago, let me take a look..", whereas the MR title may be something like "Restructure to support new extension BAR." which I may not really be too familiar with. Alex ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 11:41 AM Jason Ekstrand wrote: > > On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: >> >> On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher wrote: >> > >> > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset >> > wrote: >> > > >> > > Personally, I will continue to use the list, at least for a simplicity >> > > point of view. I'm not sure if using a new tool will improve quality and >> > > code review process. >> > > >> > > Though, if the majority reports that is really nice to use, I will >> > > probably change my mind. Not a strong reject. >> > >> > I agree. We've been using the MR interface for xf86-video-amdgpu and >> > I find it awkward compared to the mailing list. Maybe it just takes >> > getting used to. I also feel less inclined to do drive by patch >> > review if I have to explicitly delve into the browser to look at the >> > outstanding MRs. Over email, sometimes I see a patch set in my in box >> > that piques my interest and I find some time to review it when I might >> > not have otherwise if the bar were higher. >> >> FWIW I also do a lot of drive-by reviews. Perhaps those aren't >> valuable -- dunno. Either way, if it's not in email, I won't end up >> seeing it. > > > I find this commentary on drive-by reviews interesting. Personally, I don't > find going to the web interface to be a problem at all but that may be > because my e-mail is also in my web browser and it's as easy as clicking a > link. Also, since I'm an active contributor who's likely to be making MRs > and commenting on various MRs on a regular basis, I'm already on the MR page > looking at it. If you randomly scrolled through the MR list in the same way > you randomly scroll through the mailing list, would you be equally inclined > to give drive-by reviews? That's an honest hypothetical question. Less so, but greater than 0. My e-mail client is also web-based (gmail). I auto-archive all mailing list email, and attach various labels like "mesa-dev" and "dri-devel" (yeah, I'm creative). I will frequently run through those emails looking at subjects of new ones. I will also click into patches, and look at them for 0.5-3 seconds each (I've gotten good at this). If they're short, or I happen to notice something, or they're interesting, or they're in "my domain", I'll just reply. Otherwise I do nothing. With a web UI... I wouldn't end up on it in the first place. I always have an email client open. I don't have gitlab open. Even if I did, I'd have to keep flipping between various projects I might be interested in. I also like to stay apprised of various goings on, like the various display work going on in other drivers. Having it all stream through email is convenient. If it's hidden behind some web UI ... I'll never look at it. Here's a practical example, even if it's not a 100% match for this situation -- a company I formerly worked at had a vibrant mailing list of ex-employees. Lots of traffic, job postings, etc. Then a well-intentioned HR person at that company who managed the community created an online job board and additional environment, with additional logins, etc -- where posts, mailing lists, etc would go. The job board posts are gone now, the new mailing lists never got traffic since it no longer went to people's primary emails, and the community is now dead. They rolled it all back, but it never recovered. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] ac: split 16-bit ssbo loads that may not be dword aligned
Fixes: 7e7ee826982 ('ac: add support for 16bit buffer loads') Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108114 Signed-off-by: Rhys Perry --- src/amd/common/ac_nir_to_llvm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c index c05b45e084..4a4c09cf5f 100644 --- a/src/amd/common/ac_nir_to_llvm.c +++ b/src/amd/common/ac_nir_to_llvm.c @@ -1642,6 +1642,8 @@ static LLVMValueRef visit_load_buffer(struct ac_nir_context *ctx, LLVMValueRef results[4]; for (int i = 0; i < num_components;) { int num_elems = num_components - i; + if (elem_size_bytes < 4 && nir_intrinsic_align(instr) % 4 != 0) + num_elems = 1; if (num_elems * elem_size_bytes > 16) num_elems = 16 / elem_size_bytes; int load_bytes = num_elems * elem_size_bytes; -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] ac: refactor visit_load_buffer
This is so that we can split different types of loads more easily. Signed-off-by: Rhys Perry --- src/amd/common/ac_llvm_build.c | 8 ++-- src/amd/common/ac_nir_to_llvm.c | 80 - src/compiler/nir/nir.h | 2 +- 3 files changed, 44 insertions(+), 46 deletions(-) diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c index abc18da13d..154cc696a2 100644 --- a/src/amd/common/ac_llvm_build.c +++ b/src/amd/common/ac_llvm_build.c @@ -2943,9 +2943,11 @@ LLVMValueRef ac_trim_vector(struct ac_llvm_context *ctx, LLVMValueRef value, if (count == num_components) return value; - LLVMValueRef masks[] = { - ctx->i32_0, ctx->i32_1, - LLVMConstInt(ctx->i32, 2, false), LLVMConstInt(ctx->i32, 3, false)}; + LLVMValueRef masks[MAX2(count, 2)]; + masks[0] = ctx->i32_0; + masks[1] = ctx->i32_1; + for (unsigned i = 2; i < count; i++) + masks[i] = LLVMConstInt(ctx->i32, i, false); if (count == 1) return LLVMBuildExtractElement(ctx->builder, value, masks[0], diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c index a109f5a815..c05b45e084 100644 --- a/src/amd/common/ac_nir_to_llvm.c +++ b/src/amd/common/ac_nir_to_llvm.c @@ -1623,37 +1623,43 @@ static LLVMValueRef visit_atomic_ssbo(struct ac_nir_context *ctx, static LLVMValueRef visit_load_buffer(struct ac_nir_context *ctx, const nir_intrinsic_instr *instr) { - LLVMValueRef results[2]; - int load_bytes; int elem_size_bytes = instr->dest.ssa.bit_size / 8; int num_components = instr->num_components; - int num_bytes = num_components * elem_size_bytes; enum gl_access_qualifier access = nir_intrinsic_access(instr); LLVMValueRef glc = ctx->ac.i1false; if (access & (ACCESS_VOLATILE | ACCESS_COHERENT)) glc = ctx->ac.i1true; - for (int i = 0; i < num_bytes; i += load_bytes) { - load_bytes = MIN2(num_bytes - i, 16); - const char *load_name; - LLVMTypeRef data_type; - LLVMValueRef offset = get_src(ctx, instr->src[1]); - LLVMValueRef immoffset = LLVMConstInt(ctx->ac.i32, i, false); - LLVMValueRef rsrc = ctx->abi->load_ssbo(ctx->abi, - get_src(ctx, instr->src[0]), false); - LLVMValueRef vindex = ctx->ac.i32_0; - - int idx = i ? 1 : 0; + LLVMValueRef offset = get_src(ctx, instr->src[1]); + LLVMValueRef rsrc = ctx->abi->load_ssbo(ctx->abi, + get_src(ctx, instr->src[0]), false); + LLVMValueRef vindex = ctx->ac.i32_0; + + LLVMTypeRef def_type = get_def_type(ctx, &instr->dest.ssa); + LLVMTypeRef def_elem_type = num_components > 1 ? LLVMGetElementType(def_type) : def_type; + + LLVMValueRef results[4]; + for (int i = 0; i < num_components;) { + int num_elems = num_components - i; + if (num_elems * elem_size_bytes > 16) + num_elems = 16 / elem_size_bytes; + int load_bytes = num_elems * elem_size_bytes; + + LLVMValueRef immoffset = LLVMConstInt(ctx->ac.i32, i * elem_size_bytes, false); + + LLVMValueRef ret; if (load_bytes == 2) { - results[idx] = ac_build_tbuffer_load_short(&ctx->ac, - rsrc, - vindex, - offset, - ctx->ac.i32_0, - immoffset, - glc); + ret = ac_build_tbuffer_load_short(&ctx->ac, + rsrc, + vindex, + offset, + ctx->ac.i32_0, + immoffset, + glc); } else { + const char *load_name; + LLVMTypeRef data_type; switch (load_bytes) { case 16: case 12: @@ -1679,33 +1685,23 @@ static LLVMValueRef visit_load_buffer(struct ac_nir_context *ctx, glc, ctx->ac.i1false, }; - results[idx] = ac_build_intrinsic(&ctx->ac, load_nam
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 13/12/2018 17:57, Mathias Fröhlich wrote: Hi, Initially it seemed to me that I am about the only one sticking with mailing lists. And I personally feel like a too small contributor to really try to influence your decisions too much. But these recent hand full of mails all tell me that I am not that alone. I personally did contribute to several projects during the past years. All that only in part time, thus it had to be *very* efficient for myself. And that is something that I achieved by a consistent 'interface' to all those projects. Just my widely used and highly convenient mail client. So, all that worked in a sufficiently efficient way because I could combine this kind of 'work' even with my private mail that I could handle in between with that single 'interface'. So going to any web site there is already a detour and having multiple of them for each such project gives an even longer detour. Okay today it's mostly mesa that is left as well as a communication middle end used in vizsim applications. But going away too far away from a mailing list will be mostly a loss of efficiency for me. As I said, my two cents, that should not keep you all from doing changes that finally increase your all efficiency ... best Mathias Hi, I have to add my voice here as well. Even though I do not feel able to give review for most of the mesa code base, I appreciate to have all patches in the mailing list in my mail client. From time to time, I give feedback for some set of patches, for example when I see patches related to dri3 or that could impact Nine. It also enables me to get an overview of all the recent works and new features Nine could use. I feel like if most patches go through MR without getting a mail feedback, I would not be able to do those as efficiently. I would appreciate if I could *flag* some files or directories, and when a MR impacts those (for example dri3 files, gallium interface, gallium Nine, etc), I could get an automated mail with a summary of the MR, in order to encourage me to look at it. Yours, Axel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 4:52 PM Alex Deucher wrote: > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: > > > > Personally, I will continue to use the list, at least for a simplicity > > point of view. I'm not sure if using a new tool will improve quality and > > code review process. > > > > Though, if the majority reports that is really nice to use, I will > > probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. > > Out of curiosity what do others like about the MR interface? How are > you using it? What advantages does it give you over the mailing list? > I feel like maybe I'm not using it right or missing features that > make it more useful and less awkward. I feel like the interface makes > it harder than it needs to be to see the actual changes in MR to be > reviewed. All of the links click through to the source view rather > than the patch view. FWIW, the killer feature for me (if it actually works out in practice) is that I can keep track of patches that still need attention. With me doing this in my free time mostly still and the mesa list being pretty high volume, as well the tendency of giving regular reviews by r-b, it is very hard for me to track what MRs I still need to review, what has been pushed and what has been abandoned. If a series is not nearly trivial and I review it in one evening I lose track easily. Furthermore, I think patchwork has pretty much shown that deriving this from email is not going to work. Doing this in a more integrated environment where we can actually get people to indicate the status is IMO the only way to work towards getting there. As far as the actual reviewing itself, sure I agree that it needs improvement. > > Alex > > > > > On 12/6/18 12:32 AM, Jordan Justen wrote: > > > This documents a process for using GitLab Merge Requests as an second > > > way to submit code changes for Mesa. Only one of the two methods is > > > allowed for each patch series. > > > > > > We will *not* require all patches to be emailed. Some code changes may > > > be reviewed and merged without any discussion on the mesa-dev email > > > list. > > > > > > v2: > > > * No longer require email. Allow submitter to choose email or a > > > GitLab merge request. > > > * Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason, > > > Matt, Michel and Rob. > > > > > > Signed-off-by: Jordan Justen > > > --- > > > docs/submittingpatches.html | 76 ++--- > > > 1 file changed, 71 insertions(+), 5 deletions(-) > > > > > > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html > > > index 92d954a2d09..21175988d0b 100644 > > > --- a/docs/submittingpatches.html > > > +++ b/docs/submittingpatches.html > > > @@ -21,7 +21,7 @@ > > > Basic guidelines > > > Patch formatting > > > Testing Patches > > > -Mailing Patches > > > +Submitting Patches > > > Reviewing Patches > > > Nominating a commit for a stable branch > > > Criteria for accepting patches to the stable > > > branch > > > @@ -42,8 +42,10 @@ components. > > > git bisect.) > > > Patches should be properly formatted. > > > Patches should be sufficiently tested before > > > submitting. > > > -Patches should be submitted to mesa-dev > > > -for review using git send-email. > > > +Patches should be submitted > > > +to mesa-dev or with > > > +a merge request > > > +for review. > > > > > > > > > > > > @@ -180,10 +182,19 @@ run. > > > > > > > > > > > > -Mailing Patches > > > +Submitting Patches > > > > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > > +Patches may be submitted to the Mesa project by > > > +email or with a > > > +GitLab merge request. To prevent > > > +duplicate code review, only use one method to submit your changes. > > > + > > > + > > > +Mailing Patches > > > + > > > + > > > +Patches may be sent to the mesa-dev mailing list for review: > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev";> > > > mesa-dev@lists.freedesktop.org. > > > When submitting a patch make sure to use > > > @@ -217,8 +228,63 @@ disabled before sending your patches. (Note that you > > > may need to contact > > > your email administrator for this.) > > > > > > > > > +GitLab Merge Requests > > > + > > > + > > > + https://gitlab.freedesktop.org/mesa/mesa";>GitLab Merge > > > + Requests (MR) can also be used to submit patches for Mesa. > > > + > > > + > > > + > > > + If the MR may have interest for most of the Mesa community, you can > > > + send
Re: [Mesa-dev] [PATCH 1/3] glsl: XFB TSC per-vertex output varyings match as not declared as arrays
Ping. El 22/11/18 a las 0:28, Chema Casanova escribió: > > > On 21/11/18 20:04, Ilia Mirkin wrote: >> On Wed, Nov 21, 2018 at 1:45 PM Jose Maria Casanova Crespo >> wrote: >>> >>> Recent change on OpenGL CTS ("Use non-arrayed varying name for TCS blocks") >>> on KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage >>> tests changed how to name per-vertex Tessellation Control Shader output >>> varyings in transform feedback using interface block as "BLOCK_INOUT.value" >>> rather than "BLOCK_INOUT[0].value" >>> >>> So Tessellation control shader per-vertex output variables and blocks that >>> are required to be declared as arrays, with each element representing output >>> values for a single vertex of a multi-vertex primitive are expected to be >>> named as they were not declared as arrays. >>> >>> This patch adds a new is_xfb_per_vertex_output flag at ir_variable level so >>> we mark when an ir_variable is an per-vertex TCS output varying. So we >>> treat it in terms on XFB its naming as a non array variable. >>> >>> As we don't support NV_gpu_shader5, so PATCHES mode is not accepted as >>> primitiveMode parameter of BeginTransformFeedback the test expects a >>> failure as we can use the XFB results. >>> >>> This patch uncovers that we were passing the GLES version of the tests >>> because candidates naming didn't match, not because on GLES the Tessellation >>> Control stage varyings shouldn't be XFB candidates in any case. This >>> is addressed in the following patch. >>> >>> Fixes: >>> KHR-GL4*.tessellation_shader.single.xfb_captures_data_from_correct_stage >>> >>> Cc: mesa-sta...@lists.freedesktop.org >>> --- >>> src/compiler/glsl/ir.cpp| 1 + >>> src/compiler/glsl/ir.h | 6 ++ >>> src/compiler/glsl/link_uniforms.cpp | 6 -- >>> src/compiler/glsl/link_varyings.cpp | 8 +++- >>> 4 files changed, 18 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp >>> index 1d1a56ae9a5..582111d71f5 100644 >>> --- a/src/compiler/glsl/ir.cpp >>> +++ b/src/compiler/glsl/ir.cpp >>> @@ -1750,6 +1750,7 @@ ir_variable::ir_variable(const struct glsl_type >>> *type, const char *name, >>> this->data.fb_fetch_output = false; >>> this->data.bindless = false; >>> this->data.bound = false; >>> + this->data.is_xfb_per_vertex_output = false; >>> >>> if (type != NULL) { >>>if (type->is_interface()) >>> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h >>> index f478b29a6b5..e09f053b77c 100644 >>> --- a/src/compiler/glsl/ir.h >>> +++ b/src/compiler/glsl/ir.h >>> @@ -766,6 +766,12 @@ public: >>> */ >>>unsigned is_xfb_only:1; >>> >>> + /** >>> + * Is this varying a TSC per-vertex output candidate for transform >> >> TCS? > > > Yes. I've fixed it locally at the commit summary too. > > >>> + * feedback? >>> + */ >>> + unsigned is_xfb_per_vertex_output:1; >>> + >>>/** >>> * Was a transfor feedback buffer set in the shader? >> >> ugh, not your problem, but "transform" :( >> >>> */ >>> diff --git a/src/compiler/glsl/link_uniforms.cpp >>> b/src/compiler/glsl/link_uniforms.cpp >>> index 63e688b19a7..547da68e216 100644 >>> --- a/src/compiler/glsl/link_uniforms.cpp >>> +++ b/src/compiler/glsl/link_uniforms.cpp >>> @@ -72,8 +72,10 @@ program_resource_visitor::process(ir_variable *var, bool >>> use_std430_as_default) >>> get_internal_ifc_packing(use_std430_as_default) : >>>var->type->get_internal_ifc_packing(use_std430_as_default); >>> >>> - const glsl_type *t = >>> - var->data.from_named_ifc_block ? var->get_interface_type() : >>> var->type; >>> + const glsl_type *t = var->data.from_named_ifc_block ? >>> + (var->data.is_xfb_per_vertex_output ? >>> + var->get_interface_type()->without_array() : >>> + var->get_interface_type()) : var->type; >>> const glsl_type *t_without_array = t->without_array(); >>> >>> /* false is always passed for the row_major parameter to the other >>> diff --git a/src/compiler/glsl/link_varyings.cpp >>> b/src/compiler/glsl/link_varyings.cpp >>> index 52e493cb599..1964dcc0a22 100644 >>> --- a/src/compiler/glsl/link_varyings.cpp >>> +++ b/src/compiler/glsl/link_varyings.cpp >>> @@ -2150,7 +2150,10 @@ private: >>>tfeedback_candidate *candidate >>> = rzalloc(this->mem_ctx, tfeedback_candidate); >>>candidate->toplevel_var = this->toplevel_var; >>> - candidate->type = type; >>> + if (this->toplevel_var->data.is_xfb_per_vertex_output) >>> + candidate->type = type->without_array(); >>> + else >>> + candidate->type = type; >>>candidate->offset = this->varying_floats; >>>_mesa_hash_table_insert(this->tfeedback_candidates, >>>ralloc_strdup(this->mem_ctx, name), >>> @@ -2499,6 +2502,9 @@ assign_varying_locations(struct gl_context *ctx, >>> >>> if (num_t
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 11:07 AM Axel Davy wrote: > On 13/12/2018 17:57, Mathias Fröhlich wrote: > > Hi, > > Initially it seemed to me that I am about the only one sticking with > mailing lists. > > And I personally feel like a too small contributor to really try to > influence your > > decisions too much. But these recent hand full of mails all tell me that > I am not > > that alone. I personally did contribute to several projects during the > past years. > > All that only in part time, thus it had to be *very* efficient for > myself. And that is > > something that I achieved by a consistent 'interface' to all those > projects. Just > > my widely used and highly convenient mail client. So, all that worked in > a sufficiently > > efficient way because I could combine this kind of 'work' even with my > private mail > > that I could handle in between with that single 'interface'. So going to > any web site > > there is already a detour and having multiple of them for each such > project gives an > > even longer detour. Okay today it's mostly mesa that is left as well as > a communication > > middle end used in vizsim applications. But going away too far away from > a mailing list > > will be mostly a loss of efficiency for me. > > As I said, my two cents, that should not keep you all from doing changes > that finally > > increase your all efficiency ... > > > > best > > > > Mathias > > > > > > > > Hi, > > > I have to add my voice here as well. > > Even though I do not feel able to give review for most of the mesa code > base, > I appreciate to have all patches in the mailing list in my mail client. > > From time to time, I give feedback for some set of patches, for example > when I see patches related to dri3 or that could impact Nine. > > It also enables me to get an overview of all the recent works and new > features Nine could use. > > I feel like if most patches go through MR without getting a mail > feedback, I would not be able to do those as efficiently. > I find it interesting that multiple people have had this same sentiment. For me, the exact opposite is true. I'm someone who is responsible for tracking and reviewing dozens of series at a time often with many patches each. My current review backlog is probably 200 patches or more. Having them in a linear stream in my mail isn't at all what I want because once I blow past the first page (50 e-mails) stuff easily gets lost. The ability to look at them at the series level, filter by tags, etc. is a huge advantage. I also really like the fact that GitLab pulls all the comments for the series together which makes cross-patch discussions easier to track. > I would appreciate if I could *flag* some files or directories, and when > a MR impacts those (for example dri3 files, gallium interface, gallium > Nine, etc), > I could get an automated mail with a summary of the MR, in order to > encourage me to look at it. > You can at least sort-of. If you go here: https://gitlab.freedesktop.org/mesa/mesa/labels You can subscribe to an individual label and it will e-mail you every time a MR is posted and tagged with that label. Now, it will probably take a bit before we get label discipline sufficient that you can rely on it but the mechanism is there. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Let's talk about -DDEBUG
Quoting Rob Clark (2018-12-12 16:35:24) > On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker wrote: > > > > Quoting Rob Clark (2018-12-12 15:52:47) > > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker wrote: > > > > > > > > In the autotools discussion I've come to realize that we also need to > > > > talk about > > > > the -DDEBUG guard. It seems that there are two different uses, and thus > > > > two > > > > different asks about it: > > > > > > > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > > > > - NIR and Intel (at least) use -DDEBUG to hide really expensive checks > > > > that are > > > > useful, but necessarily tank performance. > > > > > > > > The first group would like -DDEBUG in debugoptimized builds, the second > > > > obviously doesn't. > > > > > > > > Is the right solution to move the first group being !NDEBUG, or would > > > > it be > > > > better to split DEBUG into two different defines such as DEBUG_MESSAGES > > > > and > > > > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), with > > > > the > > > > first for both debug and debugoptimized and the second only in debug > > > > builds? > > > > > > I guess my use cases for !=release builds are: > > > > > > + I want all the expensive checking because I'm not in it to win the > > > deqp/piglit fps race > > > + I want debug syms for profiling and/or valgrind, but otherwise > > > want something close to a release build but with debug syms > > > > > > > > > That said, I can get behind replacing DEBUG with !NDEBUG or > > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer > > > > > > > > > BR, > > > -R > > > > I guess I should have covered that: > > > > autotools had effectively two build types "debug" and "not debug", "debug" > > set > > "-DDEBUG -g -O2", "not debug" set -DNDEBUG > > > > Meson has 4 build types, and a separate toggle for NDEBUG: > > debug: -O0 -DDEBUG (we add -DDEBUG) > > debugoptimzed: -O2 -g > > release: -O2 > > plain: (nothing) > > I generally view meson as an upgrade in this respect, I guess mostly > because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine > by me).. maybe making meson debug config use -O2 would bridge the gap? > If so I have no problem with dropping -O0 and making debug config > also use -O2 > > BR. > -R Unfortunately buildtypes are part of meson itself, you can't override them. It does this to ensure that each buildtype is equivalent when changing compiler, ie, MSVC doesn't have -O0 and -O2, but it does have equivalent commands options. Basically our choices (as I see them are): 1. Status Quo 2. Split DEBUG into two flags, one that is only in debug builds, one that is in both debug and debugoptimized builds 3. Stop defining DEBUG by default in any build type and tell people to add -Dc_args=-DDEBUG -Dcpp_args=-DDEBUG if they want DEBUG. Dylan signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Let's talk about -DDEBUG
On Thu, Dec 13, 2018 at 11:34 AM Dylan Baker wrote: > Quoting Rob Clark (2018-12-12 16:35:24) > > On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker wrote: > > > > > > Quoting Rob Clark (2018-12-12 15:52:47) > > > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker > wrote: > > > > > > > > > > In the autotools discussion I've come to realize that we also need > to talk about > > > > > the -DDEBUG guard. It seems that there are two different uses, and > thus two > > > > > different asks about it: > > > > > > > > > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > > > > > - NIR and Intel (at least) use -DDEBUG to hide really expensive > checks that are > > > > > useful, but necessarily tank performance. > > > > > > > > > > The first group would like -DDEBUG in debugoptimized builds, the > second > > > > > obviously doesn't. > > > > > > > > > > Is the right solution to move the first group being !NDEBUG, or > would it be > > > > > better to split DEBUG into two different defines such as > DEBUG_MESSAGES and > > > > > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), > with the > > > > > first for both debug and debugoptimized and the second only in > debug builds? > > > > > > > > I guess my use cases for !=release builds are: > > > > > > > > + I want all the expensive checking because I'm not in it to win the > > > > deqp/piglit fps race > > > > + I want debug syms for profiling and/or valgrind, but otherwise > > > > want something close to a release build but with debug syms > > > > > > > > > > > > That said, I can get behind replacing DEBUG with !NDEBUG or > > > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer > > > > > > > > > > > > BR, > > > > -R > > > > > > I guess I should have covered that: > > > > > > autotools had effectively two build types "debug" and "not debug", > "debug" set > > > "-DDEBUG -g -O2", "not debug" set -DNDEBUG > > > > > > Meson has 4 build types, and a separate toggle for NDEBUG: > > > debug: -O0 -DDEBUG (we add -DDEBUG) > > > debugoptimzed: -O2 -g > > > release: -O2 > > > plain: (nothing) > > > > I generally view meson as an upgrade in this respect, I guess mostly > > because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine > > by me).. maybe making meson debug config use -O2 would bridge the gap? > > If so I have no problem with dropping -O0 and making debug config > > also use -O2 > > > > BR. > > -R > > Unfortunately buildtypes are part of meson itself, you can't override > them. It > does this to ensure that each buildtype is equivalent when changing > compiler, > ie, MSVC doesn't have -O0 and -O2, but it does have equivalent commands > options. > > Basically our choices (as I see them are): > > 1. Status Quo > 2. Split DEBUG into two flags, one that is only in debug builds, one that > is in >both debug and debugoptimized builds > Whatever we do, let's please not split DEBUG and NDEBUG. We've had no end of trouble with sorting out the fallout of them being different. > 3. Stop defining DEBUG by default in any build type and tell people to add >-Dc_args=-DDEBUG -Dcpp_args=-DDEBUG if they want DEBUG. > Can we not please? Yes, I have a script that wraps things but -Dbuild-type=debug should do something reasonable and reasonable includes -DDEBUG. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Quoting Alex Deucher (2018-12-13 07:52:04) > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: > > > > Personally, I will continue to use the list, at least for a simplicity > > point of view. I'm not sure if using a new tool will improve quality and > > code review process. > > > > Though, if the majority reports that is really nice to use, I will > > probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. > > Out of curiosity what do others like about the MR interface? How are > you using it? What advantages does it give you over the mailing list? > I feel like maybe I'm not using it right or missing features that > make it more useful and less awkward. I feel like the interface makes > it harder than it needs to be to see the actual changes in MR to be > reviewed. All of the links click through to the source view rather > than the patch view. > > Alex My perspective is based on the two other projects I work on, meson and alot (which, incidentally, I started working on alot because doing patch review on mailing lists is such a pain) - Using MRs drives a *lot* of drive by, one time, contributors. They run into some bug, write a patch, send an MR, and it's that easy. - It's easy to see all of the issues I've opened, and MRs I've opened (and the closed vs still open ones) quickly and easily. This makes tracking my outstanding work much easier (and is something I struggle with greatly) - All review happens in the same view. As I read the code I immediately see what other reviewers have said, which saves getting the same review comment multiple times and prevents conflicting review - by tagging issues (ala, Fixes #1234) in the commit message then pushing the commit the issue is automatically closed. This is super nice both to keep the bug tracker tidy and to figure out why an issue was closed. - not dealing with git am - V2s are automatically updated, and you can see the changes easily between various versions, as well as when each version was sent - Doesn't require huge setup to become efficient. I have now set up a pretty complicated email setup with notmuch+alot+afew to get tagging, open things in vim, added countless macros to vim, and sunk countless hours into alot to be able to do patch review efficiently. With github and gitlab, I open firefox, log in, and start doing work. - tags make it easy to see that something does or doesn't affect an area of the mesa that I know/care about My perspective coming into mesa was the doing mailing list review was very daunting and scary. There are a lot of rules that are expected that common clients (gmail) don't respect (don't send HTML mail), use inline comments, snip long quotes when they're not relevant. MRs take a lot of that away. We don't have to train people to use git-send-email *and* help them get it all set up. Hopefully that helps, 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 109004] blob.c:114: multiple definition of `blob_init'
https://bugs.freedesktop.org/show_bug.cgi?id=109004 Emil Velikov changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #2 from Emil Velikov --- commit 9578dde1c8751fd5372bce0fce0a448259160650 Author: Kristian H. Kristensen Date: Mon Dec 10 14:22:47 2018 -0800 freedreno: Fix the Makefile.am fix -- 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] Let's talk about -DDEBUG
Dylan Baker writes: > [ Unknown signature status ] > In the autotools discussion I've come to realize that we also need to talk > about > the -DDEBUG guard. It seems that there are two different uses, and thus two > different asks about it: > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > - NIR and Intel (at least) use -DDEBUG to hide really expensive checks that > are > useful, but necessarily tank performance. > > The first group would like -DDEBUG in debugoptimized builds, the second > obviously doesn't. > > Is the right solution to move the first group being !NDEBUG, or would it be > better to split DEBUG into two different defines such as DEBUG_MESSAGES and > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), with the > first for both debug and debugoptimized and the second only in debug builds? I would like to see NIR validation in debugoptimized builds (which is the build I use on a regular basis: "please catch all bugs you can at runtime with asserts, but don't waste CPU time by compiling with -O0"); signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Jason Ekstrand writes: > On Thu, Dec 13, 2018 at 11:07 AM Axel Davy wrote: > >> On 13/12/2018 17:57, Mathias Fröhlich wrote: >> > Hi, >> > Initially it seemed to me that I am about the only one sticking with >> mailing lists. >> > And I personally feel like a too small contributor to really try to >> influence your >> > decisions too much. But these recent hand full of mails all tell me that >> I am not >> > that alone. I personally did contribute to several projects during the >> past years. >> > All that only in part time, thus it had to be *very* efficient for >> myself. And that is >> > something that I achieved by a consistent 'interface' to all those >> projects. Just >> > my widely used and highly convenient mail client. So, all that worked in >> a sufficiently >> > efficient way because I could combine this kind of 'work' even with my >> private mail >> > that I could handle in between with that single 'interface'. So going to >> any web site >> > there is already a detour and having multiple of them for each such >> project gives an >> > even longer detour. Okay today it's mostly mesa that is left as well as >> a communication >> > middle end used in vizsim applications. But going away too far away from >> a mailing list >> > will be mostly a loss of efficiency for me. >> > As I said, my two cents, that should not keep you all from doing changes >> that finally >> > increase your all efficiency ... >> > >> > best >> > >> > Mathias >> > >> > >> > >> >> Hi, >> >> >> I have to add my voice here as well. >> >> Even though I do not feel able to give review for most of the mesa code >> base, >> I appreciate to have all patches in the mailing list in my mail client. >> >> From time to time, I give feedback for some set of patches, for example >> when I see patches related to dri3 or that could impact Nine. >> >> It also enables me to get an overview of all the recent works and new >> features Nine could use. >> >> I feel like if most patches go through MR without getting a mail >> feedback, I would not be able to do those as efficiently. >> > > I find it interesting that multiple people have had this same sentiment. > For me, the exact opposite is true. I'm someone who is responsible for > tracking and reviewing dozens of series at a time often with many patches > each. My current review backlog is probably 200 patches or more. Having > them in a linear stream in my mail isn't at all what I want because once I > blow past the first page (50 e-mails) stuff easily gets lost. The ability > to look at them at the series level, filter by tags, etc. is a huge > advantage. I also really like the fact that GitLab pulls all the comments > for the series together which makes cross-patch discussions easier to track. Complete agreement here. I'm both a regular contributor to Mesa (I've been in Jason's position before 100s-of-patches review backlog), and an occasional contributor to other projects (xserver is currently "occasional" status, unfortunately). I've found MRs to be better at tracking outsanding review for me than email, and I even use the email client written by cworth to try to solve this problem for us. > You can subscribe to an individual label and it will e-mail you every time > a MR is posted and tagged with that label. Now, it will probably take a > bit before we get label discipline sufficient that you can rely on it but > the mechanism is there. I'm curious if we can do something with this to replace discipline with software: https://about.gitlab.com/2016/08/19/applying-gitlab-labels-automatically/ signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108935] [LLVM Regression, bisected] Witcher 3 Corrupted Terrain
https://bugs.freedesktop.org/show_bug.cgi?id=108935 --- Comment #2 from Philip Rebohle --- This regression also affects Hitman 2, https://reviews.llvm.org/D55602 fixes it. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 109058] Machine freezes during early stages of gfxbench startup
https://bugs.freedesktop.org/show_bug.cgi?id=109058 Bug ID: 109058 Summary: Machine freezes during early stages of gfxbench startup Product: Mesa Version: 18.2 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Vulkan/radeon Assignee: mesa-dev@lists.freedesktop.org Reporter: cst...@google.com QA Contact: mesa-dev@lists.freedesktop.org Created attachment 142805 --> https://bugs.freedesktop.org/attachment.cgi?id=142805&action=edit vkinfo I'm trying out the radv driver on Ubuntu 18.10 on the Intel NUC 8i7HVK with AMD gpu: https://www.intel.com/content/www/us/en/products/boards-kits/nuc/kits/nuc8i7hvk.html vkinfo reports as attached. Is this hardware known to work? Is there a recommended test app to validate that the driver works on my system? I'm trying to run gfxbench 5.0, the vulkan_5_normal scenario. The app attempts to do some shader warmup early in initialization, and after a few seconds, the system freezes. Thanks. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 109023] error: inlining failed in call to always_inline ‘__m512 _mm512_and_ps(__m512, __m512)’: target specific option mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=109023 --- Comment #1 from Vinson Lee --- Build command to reproduce. meson builddir -Dgallium-drivers=swr -Dswr-arches=avx,avx2,knl,skx ninja -C builddir -- 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] last call for autotools
Another issue with meson is this: I have to set PKG_CONFIG_PATH when I type meson for 32-bit builds. If I do meson configure --clearcache and then type "ninja", it will reconfigure, but will use 64-bit libraries instead because PKG_CONFIG_PATH is not set when ninja is run, which will fail to link. Luckily, at least CC="gcc -m32" and CXX="g++ -m32" weren't wiped out by --clearcache. Additionally, it would be nice if the default build type was release instead of plain. Thanks, Marek On Mon, Dec 10, 2018 at 6:11 PM Dylan Baker wrote: > Meson 0.49.0 has been out for a couple of days now, and I'd like to make > the > final call for autotools. My patch is so massive that it's a huge pain to > send > to the list, the latest versions is here: > https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools > > Dylan > ___ > 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] [Bug 108578] RADV reports wrong hardcoded Vulkan API Version
https://bugs.freedesktop.org/show_bug.cgi?id=108578 --- Comment #8 from bmil...@gmail.com --- @Samuel Pitoiset Still reports 1.1.70, this function wasn't updated. line 292 of rad_extensions.py: uint32_t radv_physical_device_api_version(struct radv_physical_device *dev) { if (!ANDROID && dev->rad_info.has_syncobj_wait_for_submit) return VK_MAKE_VERSION(1, 1, 70); return VK_MAKE_VERSION(1, 0, 68); } """) -- 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] last call for autotools
On Wed, Dec 12, 2018 at 2:52 PM Rob Clark wrote: > On Wed, Dec 12, 2018 at 3:45 PM Marek Olšák wrote: > > > > On Wed, Dec 12, 2018 at 3:37 PM Rob Clark wrote: > >> > >> On Wed, Dec 12, 2018 at 3:13 PM Bas Nieuwenhuizen > >> wrote: > >> > > >> > On Wed, Dec 12, 2018 at 8:59 PM Marek Olšák wrote: > >> > > > >> > > There are 2 issues with meson: > >> > > * -DDEBUG is not present in debugoptimized builds. > >> > > >> > Do people expect -DDEBUG for debugoptimized? I would think that debug > >> > optimized would be an optimized build with debug symbols, but not > >> > expensive checks & asserts, which would match the current > >> > debugoptimized build? > >> > >> please, no -DDEBUG for debugoptimized.. I use that when I want debug > >> syms but not (for example) nir_validate and other expensive checks. > > > > > > If nir_validate is so bad, perhaps it shouldn't be run at all. If you > work on NIR and it's not important for you to run nir_validate, perhaps it > shouldn't be run at all. It doesn't have anything to do with build systems. > > > > I do actually want it enabled when I piglit/deqp.. for which I use > debug builds. But I don't want it if I'm profiling or valgrinding, > where I use debugoptimized.. > IMO, you shouldn't do any profiling with any build that has the world "debug" in the name. What it sounds like you want is a meson equivalent of CMake's RelWithDebugInfo which meson doesn't currently have. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] last call for autotools
On Wed, Dec 12, 2018 at 2:42 PM Marek Olšák wrote: > Most assertions and checks are enabled, because NDEBUG is not defined, but > DEBUG is not defined either, which is a Mesa-specific definition. > > The default debug build for autotools is debugoptimized (-g -O2 -DDEBUG). > meson changed that behavior by removing -DDEBUG. Autotools doesn't have > what meson calls "debug", which indicates that most people don't want to > use "debug" much. People who wanted to -O0 had to set CFLAGS. Now people > who want the autotools debug build have to add -DDEBUG into CFLAGS. > For whatever it's worth, I never do a debug build with either build system without throwing in -O0 because otherwise the symbols are nearly useless. > On top of that, I discovered a 3rd issue with meson: > * meson --reconfigure doesn't re-run llvm-config. > > Marek > > On Wed, Dec 12, 2018 at 3:13 PM Bas Nieuwenhuizen > wrote: > >> On Wed, Dec 12, 2018 at 8:59 PM Marek Olšák wrote: >> > >> > There are 2 issues with meson: >> > * -DDEBUG is not present in debugoptimized builds. >> >> Do people expect -DDEBUG for debugoptimized? I would think that debug >> optimized would be an optimized build with debug symbols, but not >> expensive checks & asserts, which would match the current >> debugoptimized build? >> >> > * meson ignores CFLAGS with --reconfigure, for example: >> CFLAGS="-DDEBUG" meson --reconfigure ... doesn't update CFLAGS. >> > >> > Marek >> > >> > On Mon, Dec 10, 2018 at 6:11 PM Dylan Baker >> wrote: >> >> >> >> Meson 0.49.0 has been out for a couple of days now, and I'd like to >> make the >> >> final call for autotools. My patch is so massive that it's a huge pain >> to send >> >> to the list, the latest versions is here: >> >> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools >> >> >> >> Dylan >> >> ___ >> >> mesa-dev mailing list >> >> mesa-dev@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > >> > ___ >> > mesa-dev mailing list >> > mesa-dev@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] meson: make it possible to build etnaviv's cmdline compiler
Signed-off-by: Christian Gmeiner --- meson.build | 2 +- meson_options.txt | 2 +- src/gallium/drivers/etnaviv/meson.build | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index fe647f682c..f516780115 100644 --- a/meson.build +++ b/meson.build @@ -57,7 +57,7 @@ with_osmesa = get_option('osmesa') with_swr_arches = get_option('swr-arches') with_tools = get_option('tools') if with_tools.contains('all') - with_tools = ['freedreno', 'glsl', 'intel', 'nir', 'nouveau', 'xvmc'] + with_tools = ['etnaviv', 'freedreno', 'glsl', 'intel', 'nir', 'nouveau', 'xvmc'] endif dri_drivers_path = get_option('dri-drivers-path') diff --git a/meson_options.txt b/meson_options.txt index a1d5ab0e18..005356b14c 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -301,7 +301,7 @@ option( 'tools', type : 'array', value : [], - choices : ['freedreno', 'glsl', 'intel', 'intel-ui', 'nir', 'nouveau', 'xvmc', 'all'], + choices : ['etnaviv', 'freedreno', 'glsl', 'intel', 'intel-ui', 'nir', 'nouveau', 'xvmc', 'all'], description : 'List of tools to build. (Note: `intel-ui` selects `intel`)', ) option( diff --git a/src/gallium/drivers/etnaviv/meson.build b/src/gallium/drivers/etnaviv/meson.build index 1733024ac9..63553dec51 100644 --- a/src/gallium/drivers/etnaviv/meson.build +++ b/src/gallium/drivers/etnaviv/meson.build @@ -101,7 +101,8 @@ etnaviv_compiler = executable( include_directories : [inc_include, inc_src, inc_gallium, inc_gallium_aux], link_with : [libmesa_util, libgallium, libetnaviv], dependencies : [dep_libdrm_etnaviv], - build_by_default : false, + build_by_default : with_tools.contains('etnaviv'), + install : with_tools.contains('etnaviv'), ) driver_etnaviv = declare_dependency( -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] last call for autotools
On Thu, Dec 13, 2018 at 2:41 PM Jason Ekstrand wrote: > > On Wed, Dec 12, 2018 at 2:52 PM Rob Clark wrote: >> >> On Wed, Dec 12, 2018 at 3:45 PM Marek Olšák wrote: >> > >> > On Wed, Dec 12, 2018 at 3:37 PM Rob Clark wrote: >> >> >> >> On Wed, Dec 12, 2018 at 3:13 PM Bas Nieuwenhuizen >> >> wrote: >> >> > >> >> > On Wed, Dec 12, 2018 at 8:59 PM Marek Olšák wrote: >> >> > > >> >> > > There are 2 issues with meson: >> >> > > * -DDEBUG is not present in debugoptimized builds. >> >> > >> >> > Do people expect -DDEBUG for debugoptimized? I would think that debug >> >> > optimized would be an optimized build with debug symbols, but not >> >> > expensive checks & asserts, which would match the current >> >> > debugoptimized build? >> >> >> >> please, no -DDEBUG for debugoptimized.. I use that when I want debug >> >> syms but not (for example) nir_validate and other expensive checks. >> > >> > >> > If nir_validate is so bad, perhaps it shouldn't be run at all. If you work >> > on NIR and it's not important for you to run nir_validate, perhaps it >> > shouldn't be run at all. It doesn't have anything to do with build systems. >> > >> >> I do actually want it enabled when I piglit/deqp.. for which I use >> debug builds. But I don't want it if I'm profiling or valgrinding, >> where I use debugoptimized.. > > > IMO, you shouldn't do any profiling with any build that has the world "debug" > in the name. What it sounds like you want is a meson equivalent of CMake's > RelWithDebugInfo which meson doesn't currently have. this is kinda what I expect debugoptimzied to be. And in fact we only set -DDEBUG for 'debug' and not 'debugoptimized' if I'm reading meson.build correctly.. BR, -R ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
IMHO allowing MRs is a good thing, so Acked-by: Gert Wollny I've added a little remark below. Best, Gert Am Mittwoch, den 05.12.2018, 15:32 -0800 schrieb Jordan Justen: > This documents a process for using GitLab Merge Requests as an second > way to submit code changes for Mesa. Only one of the two methods is > allowed for each patch series. > > We will *not* require all patches to be emailed. Some code changes > may > be reviewed and merged without any discussion on the mesa-dev email > list. > > v2: > * No longer require email. Allow submitter to choose email or a >GitLab merge request. > * Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason, >Matt, Michel and Rob. > > Signed-off-by: Jordan Justen > --- > docs/submittingpatches.html | 76 ++- > -- > 1 file changed, 71 insertions(+), 5 deletions(-) > > diff --git a/docs/submittingpatches.html > b/docs/submittingpatches.html > index 92d954a2d09..21175988d0b 100644 > --- a/docs/submittingpatches.html > +++ b/docs/submittingpatches.html > @@ -21,7 +21,7 @@ > Basic guidelines > Patch formatting > Testing Patches > -Mailing Patches > +Submitting Patches > Reviewing Patches > Nominating a commit for a stable > branch > Criteria for accepting patches to the stable > branch > @@ -42,8 +42,10 @@ components. > git bisect.) > Patches should be properly formatted. > Patches should be sufficiently tested > before submitting. > -Patches should be submitted to mesa-dev > -for review using git send- > email. > +Patches should be submitted > +to mesa-dev or with > +a merge request > +for review. > > > > @@ -180,10 +182,19 @@ run. > > > > -Mailing Patches > +Submitting Patches > > > -Patches should be sent to the mesa-dev mailing list for review: > +Patches may be submitted to the Mesa project by > +email or with a > +GitLab merge request. To prevent > +duplicate code review, only use one method to submit your changes. > + > + > +Mailing Patches > + > + > +Patches may be sent to the mesa-dev mailing list for review: > https://lists.freedesktop.org/mailman/listinfo/mesa-dev";>; > mesa-dev@lists.freedesktop.org. > When submitting a patch make sure to use > @@ -217,8 +228,63 @@ disabled before sending your patches. (Note that > you may need to contact > your email administrator for this.) > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa";>GitLab; > Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + If the MR may have interest for most of the Mesa community, you > can > + send an email to the mesa-dev email list including a link to the > MR. > + Don't send the patch to mesa-dev, just the MR link. > + > + > + Add labels to your MR to help reviewers find it. For example: > + > +Mesa changes affecting all drivers: mesa > +Hardware vendor specific code: amd, intel, nvidia, ... > +Driver specific code: anvil, freedreno, i965, iris, > radeonsi, > + radv, vc4, ... > +Other tag examples: gallium, util > + > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean > history > + in your patches. There should not be "fixup" patches in the > history. > + The series should be buildable and functional after every commit > + whenever you push the branch. > + > + > + It is your responsibility to keep the MR alive and making > progress, > + as there are no guarantees that a Mesa dev will independently take > + interest in it. > + > + > + Some other notes: > + > +Make changes and update your branch based on feedback > +Old, stale MR may be closed, but you can reopen it if you > + still want to pursue the changes > +You should periodically check to see if your MR needs to be > + rebased Just a remark: With virglrenderer I actually get a notification email when a MR can no longer be applied cleanly, I don't think I had to configure this explicitely. > +Make sure your MR is closed if your patches get pushed > outside > + of GitLab > + > + > + > Reviewing Patches > > + > + To participate in code review, you should monitor the > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev";> > ; > + mesa-dev email list and the GitLab > + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_reque > sts">Merge > + Requests page. > + > + > > When you've reviewed a patch on the mailing list, please be > unambiguous > about your review. That is, state either ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 109039] [CLOVER][CLANG-SVN] build failure CodeGenOptions.h: No such file or directory
https://bugs.freedesktop.org/show_bug.cgi?id=109039 --- Comment #2 from Jan Vesely --- (In reply to Yurii Kolesnykov from comment #1) > Created attachment 142791 [details] [review] > fix Have you tested the patch with LLVM-7? It looks like it would fail the build. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 109039] [CLOVER][CLANG-SVN] build failure CodeGenOptions.h: No such file or directory
https://bugs.freedesktop.org/show_bug.cgi?id=109039 Jan Vesely changed: What|Removed |Added Blocks||99553 Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=99553 [Bug 99553] Tracker bug for runnning OpenCL applications on Clover -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 99553] Tracker bug for runnning OpenCL applications on Clover
https://bugs.freedesktop.org/show_bug.cgi?id=99553 Jan Vesely changed: What|Removed |Added Depends on||109039 Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=109039 [Bug 109039] [CLOVER][CLANG-SVN] build failure CodeGenOptions.h: No such file or directory -- 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] [PATCH 1/1] clover: Fix build after clang r348827
CodeGenOptions were moved to Basic. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109039 Signed-off-by: Jan Vesely --- src/gallium/state_trackers/clover/llvm/compat.hpp | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp b/src/gallium/state_trackers/clover/llvm/compat.hpp index 975012cbda..b91cb95a29 100644 --- a/src/gallium/state_trackers/clover/llvm/compat.hpp +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp @@ -58,9 +58,14 @@ #include #include -#include #include +#if HAVE_LLVM >= 0x0800 +#include +#else +#include +#endif + namespace clover { namespace llvm { namespace compat { -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v6 5/8] mesa/st: rework support for sRGB framebuffer attachements
On Thu, 2018-11-15 at 13:45 +0100, Gert Wollny wrote: > @@ -457,14 +458,15 @@ st_framebuffer_create(struct st_context *st, > * format such that util_format_srgb(visual->color_format) can be > supported > * by the pipe driver. We still need to advertise the capability > here. > * > -* For GLES, however, sRGB framebuffer write is controlled only > by the > -* capability of the framebuffer. There is > GL_EXT_sRGB_write_control to > -* give applications the control back, but sRGB write is still > enabled by > -* default. To avoid unexpected results, we should not advertise > the > -* capability. This could change when we add support for > -* EGL_KHR_gl_colorspace. > +* For GLES, however, sRGB framebuffer write is initially only > controlled > +* by the capability of the framebuffer, with > GL_EXT_sRGB_write_control > +* control is given back to the applications, but > GL_FRAMEBUFFER_SRGB is > +* still enabled by default since this is the behaviour when > +* EXT_sRGB_write_control is not available. Since > GL_EXT_sRGB_write_control > +* brings GLES on par with desktop GLs EXT_framebuffer_sRGB, in > mesa this > +* is also expressed by using the same extension flag > */ > - if (_mesa_is_desktop_gl(st->ctx)) { > + if (st->ctx->Extensions.EXT_framebuffer_sRGB) { This allows this behavior on OpenGL ES 1.x and 2.0 also, which I don't think we want... ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v6 0/8] Add and enable extension EXT_sRGB_write_control
On Thu, 2018-11-15 at 13:45 +0100, Gert Wollny wrote: > From: Gert Wollny > > Dear all, > > after the RFC and Ilias comments I reworked the series another > time. > Changes with respect to the RFC are > - renaming the new CAP > - reordering of the patches that no double checking of > EXT_sRGB and EXT_framebuffer_sRGB is needed. > > thanks for reviewing, > Gert > > Gert Wollny (8): > Gallium: Add new CAPS to indicate whether a driver can switch SRGB > write > virgl: Set sRGB write control CAP based on host capabilities > mesa:main: Add flag for EXT_sRGB to gl_extensions > i965: Set flag for EXT_sRGB > mesa/st: rework support for sRGB framebuffer attachements > mesa/main: Use flag for EXT_sRGB instead of EXT_framebuffer_sRGB > where > possible > mesa/main/version: Lower the requirements for GLES 3.0 > mesa/main: Expose EXT_sRGB_write_control > Hmm. So you only add a driver cap for EXT_sRGB, you don't actually expose the extension, right? I can't see it added to extensions_table.h... Is there a reason for that? I guess it's more work than you'd want, considering it requires stuff like disallowing glGenerateMipmap() with sRGB textures... ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 109058] Machine freezes during early stages of gfxbench startup
https://bugs.freedesktop.org/show_bug.cgi?id=109058 --- Comment #1 from Bas Nieuwenhuizen --- I don't think there are any outstanding bugreports for that GPU specifically though it does not receive much testing. Looking at https://bugs.freedesktop.org/show_bug.cgi?id=108900 it looks like there is a general issue with gfxbench5. Sascha Willems Vulkan demos are a safe bet at https://github.com/SaschaWillems/Vulkan If you don't want to run a game or CTS. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] nvc0/ir: replace cvt instructions with add to improve shader performance
gives me an performance boost of 0.2% in pixmark_piano on my gk106, gm204 and gp107. changes in shader-db: total instructions in shared programs : 7614782 -> 7614782 (0.00%) total cvt instructions in shared programs : 139343 -> 95856 (-31.21%) total gprs used in shared programs: 798045 -> 798045 (0.00%) total shared used in shared programs : 639636 -> 639636 (0.00%) total local used in shared programs : 24648 -> 24648 (0.00%) total bytes used in shared programs : 81330696 -> 81330696 (0.00%) local sharedgpr inst cvts bytes helped 0 0 0 0 14037 0 hurt 0 0 0 0 0 0 Signed-off-by: Karol Herbst --- .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 55 +++ .../nouveau/codegen/nv50_ir_lowering_nvc0.h | 1 + 2 files changed, 56 insertions(+) diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp index 295497be2f9..267fef0898d 100644 --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp @@ -719,6 +719,58 @@ NVC0LegalizePostRA::propagateJoin(BasicBlock *bb) bb->remove(bb->getEntry()); } +// replaces instructions which would end up as f2f or i2i with faster +// alternatives: +// - abs(a) -> add(0, abs a) +// - neg(a) -> add(0, neg a) +// - neg(abs a) -> add(0, neg abs a) +// - sat(a) -> sat add(0, a) +void +NVC0LegalizePostRA::replaceCvt(Instruction *cvt) +{ + if (typeSizeof(cvt->sType) == 8) + return; + if (cvt->sType != cvt->dType) + return; + // we could make it work, but in this case we have optimizations disabled + // and we don't really care either way. + if (cvt->src(0).getFile() == FILE_IMMEDIATE) + return; + + switch (cvt->op) { + case OP_ABS: + if (cvt->src(0).mod) + break; + if (!isFloatType(cvt->sType)) + break; + cvt->op = OP_ADD; + cvt->moveSources(0, 1); + cvt->src(1).mod = NV50_IR_MOD_ABS; + cvt->setSrc(0, rZero); + cvt->src(0).mod = 0; + break; + case OP_NEG: + if (cvt->src(0).mod && (cvt->src(0).mod.neg() || !isFloatType(cvt->sType))) + break; + cvt->op = OP_ADD; + cvt->moveSources(0, 1); + cvt->src(1).mod = cvt->src(0).mod ? NV50_IR_MOD_NEG_ABS : NV50_IR_MOD_NEG; + cvt->setSrc(0, rZero); + cvt->src(0).mod = 0; + break; + case OP_SAT: + if (cvt->src(0).mod) + break; + cvt->op = OP_ADD; + cvt->moveSources(0, 1); + cvt->setSrc(0, rZero); + cvt->saturate = true; + break; + default: + break; + } +} + bool NVC0LegalizePostRA::visit(BasicBlock *bb) { @@ -758,6 +810,9 @@ NVC0LegalizePostRA::visit(BasicBlock *bb) next = hi; } + if (i->isCvt()) +replaceCvt(i); + if (i->op != OP_MOV && i->op != OP_PFETCH) replaceZero(i); } diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h index e0f50ab0904..4679c56471b 100644 --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h @@ -81,6 +81,7 @@ private: virtual bool visit(Function *); virtual bool visit(BasicBlock *); + void replaceCvt(Instruction *); void replaceZero(Instruction *); bool tryReplaceContWithBra(BasicBlock *); void propagateJoin(BasicBlock *); -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] nvc0: count cvt instructions and print it
Signed-off-by: Karol Herbst --- src/gallium/drivers/nouveau/codegen/nv50_ir.h | 1 + .../drivers/nouveau/codegen/nv50_ir_driver.h| 1 + .../drivers/nouveau/codegen/nv50_ir_inlines.h | 17 + .../drivers/nouveau/codegen/nv50_ir_target.cpp | 2 ++ src/gallium/drivers/nouveau/nvc0/nvc0_program.c | 4 ++-- 5 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h b/src/gallium/drivers/nouveau/codegen/nv50_ir.h index 8085bb2f542..b00a005bdef 100644 --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h @@ -867,6 +867,7 @@ public: inline bool isPseudo() const { return op < OP_MOV; } bool isDead() const; bool isNop() const; + inline bool isCvt() const; bool isCommutationLegal(const Instruction *) const; // must be adjacent ! bool isActionEqual(const Instruction *) const; bool isResultEqual(const Instruction *) const; diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h index 7c835ceab8d..5bee7bfd108 100644 --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h @@ -93,6 +93,7 @@ struct nv50_ir_prog_info uint32_t *code; uint32_t codeSize; uint32_t instructions; + uint32_t cvt_instructions; uint8_t sourceRep; /* PIPE_SHADER_IR_* */ const void *source; void *relocData; diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h b/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h index 4cb53ab42ed..06882058dc9 100644 --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h @@ -419,4 +419,21 @@ LValue *Function::getLValue(int id) return reinterpret_cast(allLValues.get(id)); } +bool Instruction::isCvt() const +{ + switch (op) { + case OP_ABS: + case OP_CEIL: + case OP_FLOOR: + case OP_NEG: + case OP_TRUNC: + case OP_SAT: + return true; + case OP_CVT: + return def(0).getFile() != FILE_PREDICATE && src(0).getFile() != FILE_PREDICATE; + default: + return false; + } +} + #endif // __NV50_IR_INLINES_H__ diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp index 9193a01f189..85adb5b1852 100644 --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp @@ -393,6 +393,8 @@ Program::emitBinary(struct nv50_ir_prog_info *info) for (Instruction *i = fn->bbArray[b]->getEntry(); i; i = i->next) { emit->emitInstruction(i); info->bin.instructions++; +if (i->isCvt()) + info->bin.cvt_instructions++; if ((typeSizeof(i->sType) == 8 || typeSizeof(i->dType) == 8) && (isFloatType(i->sType) || isFloatType(i->dType))) info->io.fp64 = true; diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c index 57d98753f45..139e4fec9c0 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c @@ -699,9 +699,9 @@ nvc0_program_translate(struct nvc0_program *prog, uint16_t chipset, &prog->pipe.stream_output); pipe_debug_message(debug, SHADER_INFO, - "type: %d, local: %d, shared: %d, gpr: %d, inst: %d, bytes: %d", + "type: %d, local: %d, shared: %d, gpr: %d, inst: %d, cvts: %d, bytes: %d", prog->type, info->bin.tlsSpace, info->bin.smemSize, - prog->num_gprs, info->bin.instructions, + prog->num_gprs, info->bin.instructions, info->bin.cvt_instructions, info->bin.codeSize); #ifdef DEBUG -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir: fix opt_if_loop_last_continue()
The pass did not correctly handle loops ending in: if ssa_7 { block block_8: /* preds: block_7 */ continue /* succs: block_1 */ } else { block block_9: /* preds: block_7 */ break /* succs: block_11 */ } The break will get eliminated by another opt but if this pass gets called first (as it does on RADV) we ended up inserting instructions after the break. Fixes: 5921a19d4b0c ("nir: add if opt opt_if_loop_last_continue()") --- src/compiler/nir/nir_opt_if.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index 691448a96e..c21ac9219f 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -318,9 +318,13 @@ opt_if_loop_last_continue(nir_loop *loop) nir_cf_extract(&tmp, nir_after_cf_node(if_node), nir_after_block(last_block)); if (then_ends_in_continue) { - nir_cf_reinsert(&tmp, nir_after_cf_list(&nif->else_list)); + nir_cursor last_blk_cursor = nir_after_cf_list(&nif->else_list); + nir_cf_reinsert(&tmp, + nir_after_block_before_jump(last_blk_cursor.block)); } else { - nir_cf_reinsert(&tmp, nir_after_cf_list(&nif->then_list)); + nir_cursor last_blk_cursor = nir_after_cf_list(&nif->then_list); + nir_cf_reinsert(&tmp, + nir_after_block_before_jump(last_blk_cursor.block)); } /* In order to avoid running nir_lower_regs_to_ssa_impl() every time an if -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nvc0/ir: replace cvt instructions with add to improve shader performance
On Thu, Dec 13, 2018 at 6:14 PM Karol Herbst wrote: > > gives me an performance boost of 0.2% in pixmark_piano on my gk106, gm204 and > gp107. > > changes in shader-db: > > total instructions in shared programs : 7614782 -> 7614782 (0.00%) > total cvt instructions in shared programs : 139343 -> 95856 (-31.21%) > total gprs used in shared programs: 798045 -> 798045 (0.00%) > total shared used in shared programs : 639636 -> 639636 (0.00%) > total local used in shared programs : 24648 -> 24648 (0.00%) > total bytes used in shared programs : 81330696 -> 81330696 (0.00%) > > local sharedgpr inst cvts bytes > helped 0 0 0 0 14037 0 > hurt 0 0 0 0 0 0 > > Signed-off-by: Karol Herbst > --- > .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 55 +++ > .../nouveau/codegen/nv50_ir_lowering_nvc0.h | 1 + > 2 files changed, 56 insertions(+) > > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > index 295497be2f9..267fef0898d 100644 > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > @@ -719,6 +719,58 @@ NVC0LegalizePostRA::propagateJoin(BasicBlock *bb) > bb->remove(bb->getEntry()); > } > > +// replaces instructions which would end up as f2f or i2i with faster > +// alternatives: > +// - abs(a) -> add(0, abs a) > +// - neg(a) -> add(0, neg a) > +// - neg(abs a) -> add(0, neg abs a) > +// - sat(a) -> sat add(0, a) > +void > +NVC0LegalizePostRA::replaceCvt(Instruction *cvt) > +{ > + if (typeSizeof(cvt->sType) == 8) != 4 Otherwise you're just asking for trouble. > + return; > + if (cvt->sType != cvt->dType) > + return; > + // we could make it work, but in this case we have optimizations disabled > + // and we don't really care either way. > + if (cvt->src(0).getFile() == FILE_IMMEDIATE) > + return; > + > + switch (cvt->op) { > + case OP_ABS: > + if (cvt->src(0).mod) > + break; > + if (!isFloatType(cvt->sType)) Why? What's wrong with IABS -> IADD? Similar question for OP_NEG. > + break; > + cvt->op = OP_ADD; > + cvt->moveSources(0, 1); These two lines, along with moveSources are always repeated. Would be nice to centralize this somehow. > + cvt->src(1).mod = NV50_IR_MOD_ABS; > + cvt->setSrc(0, rZero); > + cvt->src(0).mod = 0; > + break; > + case OP_NEG: > + if (cvt->src(0).mod && (cvt->src(0).mod.neg() || > !isFloatType(cvt->sType))) > + break; > + cvt->op = OP_ADD; > + cvt->moveSources(0, 1); > + cvt->src(1).mod = cvt->src(0).mod ? NV50_IR_MOD_NEG_ABS : > NV50_IR_MOD_NEG; > + cvt->setSrc(0, rZero); > + cvt->src(0).mod = 0; > + break; > + case OP_SAT: > + if (cvt->src(0).mod) > + break; > + cvt->op = OP_ADD; > + cvt->moveSources(0, 1); > + cvt->setSrc(0, rZero); > + cvt->saturate = true; > + break; > + default: > + break; > + } > +} > + > bool > NVC0LegalizePostRA::visit(BasicBlock *bb) > { > @@ -758,6 +810,9 @@ NVC0LegalizePostRA::visit(BasicBlock *bb) > next = hi; > } > > + if (i->isCvt()) > +replaceCvt(i); > + > if (i->op != OP_MOV && i->op != OP_PFETCH) > replaceZero(i); >} > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > index e0f50ab0904..4679c56471b 100644 > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > @@ -81,6 +81,7 @@ private: > virtual bool visit(Function *); > virtual bool visit(BasicBlock *); > > + void replaceCvt(Instruction *); > void replaceZero(Instruction *); > bool tryReplaceContWithBra(BasicBlock *); > void propagateJoin(BasicBlock *); > -- > 2.19.2 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] nir: add if opt opt_if_loop_last_continue()
On 13/12/18 8:10 pm, Samuel Pitoiset wrote: This introduces crashes for dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tesse dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert Test case 'dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag'.. deqp-vk: nir/nir_control_flow.c:553: stitch_blocks: Assertion `exec_list_is_empty(&after->instr_list)' failed. Aborted (core dumped) Did you run CTS? Yes I'd run it through Intels CI countless times. I've sent a fix, seems the ordering of optimisations on RADV vs intels drivers causes us to trip up on this one were the other drivers did not. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: remove unused variable
I don't think I've seen the warning so its likely I use this (or removing it) in a future patch. But feel free to push this for now. On 14/12/18 1:25 am, Alejandro Piñeiro wrote: To avoid the following warning: ./src/compiler/nir/nir_loop_analyze.c:807:16: warning: unused variable ‘ns’ [-Wunused-variable] nir_shader *ns = impl->function->shader; --- Perhaps this is solved on any of the loop analysis patches pending to be reviewed, but just in case, sending it. src/compiler/nir/nir_loop_analyze.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index 3de45401975..259f02a854e 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -803,7 +803,6 @@ get_loop_info(loop_info_state *state, nir_function_impl *impl) /* Run through each of the terminators and try to compute a trip-count */ find_trip_count(state); - nir_shader *ns = impl->function->shader; nir_foreach_block_in_cf_node(block, &state->loop->cf_node) { if (force_unroll_heuristics(state, block)) { state->loop->info->force_unroll = true; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] spirv/nir: adjust location assignment for the case of arrays of blocks
On 13/12/18 11:11 pm, Alejandro Piñeiro wrote: This is needed due how the types get rearranged after the struct splitting. So for example, this array of blocks: layout(location = 0) out block { vec4 v; vec3 v2; } x[2]; Would be splitted on two nir variables with the following types: * vec4 v[2] * vec3 v2[2] So we need to take into account the length of the array to avoid locations overlaps one with the other. --- Hi Jason, again, sending in advance patches, just in case you are working on the same. I was able to fix the location overlapping without all those crazy ideas about lowering array of blocks into individual blocks, by just adjusting the locations as this patch shows. FWIW, the resulting locations are equivalent to those that we get with GLSL IR, that results on a similar splitting. With this change I got the following working: * SPIR-V simple arrays of blocks input/outputs * The arrays of blocks inputs/outputs + interpolator qualifiers test I mentioned to you last week [1] when run its SPIR-V equivalent. * SPIR-V xfb tests using arrays of blocks, where the xfb offset are assigned to all block members. * SPIR-V xfb tests using arrays of blocks, where the xfb offset is assigned to just one member, so just that member is captured, although as many times as the array length (yes! afaiu by spec that needs to work) So now, the only pending thing is a cleanup and send the series to review. Specifically, I think that this series can be put on top of current master instead of the arb_gl_spirv. Will try that and send a final series this week or early next week. BR [1] https://github.com/Igalia/piglit/blob/master/tests/spec/glsl-1.50/execution/interface-block-interpolation-array.shader_test src/compiler/spirv/vtn_variables.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index a8f2fdfa534..87386cee42f 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1672,6 +1672,14 @@ add_missing_member_locations(struct vtn_variable *var, glsl_get_length(glsl_without_array(var->type->type)); int location = var->base_location; + /* To know if it is a interface block we can't ask directly for +* var->type->block because on the case of arrays of blocks, block is set +* on the array_element. +*/ + bool is_array_block = var->var->interface_type != NULL && + glsl_type_is_array(var->type->type); + int adjustment = is_array_block ? glsl_get_length(var->type->type) : 1; + I think you probably want to use glsl_get_aoa_size() here instead of glsl_get_length() ? for (unsigned i = 0; i < length; i++) { /* From the Vulkan spec: * @@ -1702,8 +1710,12 @@ add_missing_member_locations(struct vtn_variable *var, const struct glsl_type *member_type = glsl_get_struct_field(glsl_without_array(var->type->type), i); + /* For arrays of interface blocks we can't just add the attribute slots + * of a member type due how the splitting would rearrange the types, so + * we need to adjust for the array length in that case. + */ location += - glsl_count_attribute_slots(member_type, is_vertex_input); + glsl_count_attribute_slots(member_type, is_vertex_input) * adjustment; } } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108946] High memory usage in Black Mesa
https://bugs.freedesktop.org/show_bug.cgi?id=108946 --- Comment #8 from i...@yahoo.com --- (In reply to Ian Romanick from comment #6) > (I'm quite sure I submitted a similar comment earlier today, but it seems to > have just vanished.) > > I ran the API trace in Valgrind massif memory profiler. As far as I can > tell, the application never calls glDeleteShader. We don't release any of > the memory because the app hasn't told us that we can. In that trace there > is is about 2.6GB (on a 64-bit build of apitrace) of intermediate IR that > could be released by calling glDeleteShader on all the shaders that have > been linked. Do you really need to keep all Intermediate Representations of all shaders? The standard does not deal with internal stuff and IR is just that. I thought that in core context you do not need to recompile them and that there is a method to patch the prologue of the binary in compatibility mode (at least for radeon si). -- 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 2/2] nvc0/ir: replace cvt instructions with add to improve shader performance
On Fri, Dec 14, 2018 at 12:27 AM Ilia Mirkin wrote: > > On Thu, Dec 13, 2018 at 6:14 PM Karol Herbst wrote: > > > > gives me an performance boost of 0.2% in pixmark_piano on my gk106, gm204 > > and > > gp107. > > > > changes in shader-db: > > > > total instructions in shared programs : 7614782 -> 7614782 (0.00%) > > total cvt instructions in shared programs : 139343 -> 95856 (-31.21%) > > total gprs used in shared programs: 798045 -> 798045 (0.00%) > > total shared used in shared programs : 639636 -> 639636 (0.00%) > > total local used in shared programs : 24648 -> 24648 (0.00%) > > total bytes used in shared programs : 81330696 -> 81330696 (0.00%) > > > > local sharedgpr inst cvts bytes > > helped 0 0 0 0 14037 0 > > hurt 0 0 0 0 0 0 > > > > Signed-off-by: Karol Herbst > > --- > > .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 55 +++ > > .../nouveau/codegen/nv50_ir_lowering_nvc0.h | 1 + > > 2 files changed, 56 insertions(+) > > > > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > > index 295497be2f9..267fef0898d 100644 > > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp > > @@ -719,6 +719,58 @@ NVC0LegalizePostRA::propagateJoin(BasicBlock *bb) > > bb->remove(bb->getEntry()); > > } > > > > +// replaces instructions which would end up as f2f or i2i with faster > > +// alternatives: > > +// - abs(a) -> add(0, abs a) > > +// - neg(a) -> add(0, neg a) > > +// - neg(abs a) -> add(0, neg abs a) > > +// - sat(a) -> sat add(0, a) > > +void > > +NVC0LegalizePostRA::replaceCvt(Instruction *cvt) > > +{ > > + if (typeSizeof(cvt->sType) == 8) > > != 4 > > Otherwise you're just asking for trouble. > > > + return; > > + if (cvt->sType != cvt->dType) > > + return; > > + // we could make it work, but in this case we have optimizations > > disabled > > + // and we don't really care either way. > > + if (cvt->src(0).getFile() == FILE_IMMEDIATE) > > + return; > > + > > + switch (cvt->op) { > > + case OP_ABS: > > + if (cvt->src(0).mod) > > + break; > > + if (!isFloatType(cvt->sType)) > > Why? What's wrong with IABS -> IADD? Similar question for OP_NEG. > iadd can't have abs modifiers > > + break; > > + cvt->op = OP_ADD; > > + cvt->moveSources(0, 1); > > These two lines, along with moveSources are always repeated. Would be > nice to centralize this somehow. yeah.. just didn't figure out a super nice way to do that except checking twice for valid opcodes :/ > > > + cvt->src(1).mod = NV50_IR_MOD_ABS; > > + cvt->setSrc(0, rZero); > > + cvt->src(0).mod = 0; > > + break; > > + case OP_NEG: > > + if (cvt->src(0).mod && (cvt->src(0).mod.neg() || > > !isFloatType(cvt->sType))) > > + break; > > + cvt->op = OP_ADD; > > + cvt->moveSources(0, 1); > > + cvt->src(1).mod = cvt->src(0).mod ? NV50_IR_MOD_NEG_ABS : > > NV50_IR_MOD_NEG; > > + cvt->setSrc(0, rZero); > > + cvt->src(0).mod = 0; > > + break; > > + case OP_SAT: > > + if (cvt->src(0).mod) > > + break; > > + cvt->op = OP_ADD; > > + cvt->moveSources(0, 1); > > + cvt->setSrc(0, rZero); > > + cvt->saturate = true; > > + break; > > + default: > > + break; > > + } > > +} > > + > > bool > > NVC0LegalizePostRA::visit(BasicBlock *bb) > > { > > @@ -758,6 +810,9 @@ NVC0LegalizePostRA::visit(BasicBlock *bb) > > next = hi; > > } > > > > + if (i->isCvt()) > > +replaceCvt(i); > > + > > if (i->op != OP_MOV && i->op != OP_PFETCH) > > replaceZero(i); > >} > > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > > index e0f50ab0904..4679c56471b 100644 > > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h > > @@ -81,6 +81,7 @@ private: > > virtual bool visit(Function *); > > virtual bool visit(BasicBlock *); > > > > + void replaceCvt(Instruction *); > > void replaceZero(Instruction *); > > bool tryReplaceContWithBra(BasicBlock *); > > void propagateJoin(BasicBlock *); > > -- > > 2.19.2 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] last call for autotools
Quoting Marek Olšák (2018-12-13 11:28:10) > Another issue with meson is this: > > I have to set PKG_CONFIG_PATH when I type meson for 32-bit builds. If I do > meson configure --clearcache and then type "ninja", it will reconfigure, but > will use 64-bit libraries instead because PKG_CONFIG_PATH is not set when > ninja > is run, which will fail to link. > > Luckily, at least CC="gcc -m32" and CXX="g++ -m32" weren't wiped out by > --clearcache. > > Additionally, it would be nice if the default build type was release instead > of > plain. > > Thanks, > Marek > > On Mon, Dec 10, 2018 at 6:11 PM Dylan Baker wrote: > > Meson 0.49.0 has been out for a couple of days now, and I'd like to make > the > final call for autotools. My patch is so massive that it's a huge pain to > send > to the list, the latest versions is here: > https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools > > Dylan > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > I've had much better luck using a cross file for doing x86 builds on x86_64, I believe I've posted my cross file before but I can do so again. Dylan signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/3] i965/gen9: Add workarounds for object preemption.
On Wed, Oct 31, 2018 at 04:27:31PM -0700, Kenneth Graunke wrote: > On Wednesday, October 31, 2018 11:15:28 AM PDT Rafael Antognolli wrote: > > On Tue, Oct 30, 2018 at 04:32:54PM -0700, Kenneth Graunke wrote: > > > On Monday, October 29, 2018 10:19:54 AM PDT Rafael Antognolli wrote: > > > Do we need any stalling when whacking CS_CHICKEN1...? > > > > Hmmm... there's this: > > > > "A fixed function pipe flush is required before modifying this field" > > > > in the programming notes. I'm not sure what that is, but I assume it's > > some type of PIPE_CONTROL? > > Yeah. I'm not honestly sure what kind - "fixed function pipe flush" > isn't a thing. Nobody ever uses wording that corresponds to actual > mechanics of the hardware. :( > > Maybe this would work: > > brw_emit_end_of_pipe_sync(brw, PIPE_CONTROL_RENDER_TARGET_FLUSH); Hey Ken, Ressurrecting this old thread... I just noticed that I had this in patch 2/3 (inside brw_enable_obj_preemption()): brw_emit_pipe_control_flush(brw, PIPE_CONTROL_FLUSH_ENABLE); That's bit 7 of the PIPE_CONTROL, and from the docs: "Hardware on parsing PIPECONTROL command with Pipe Control Flush Enable set will wait for all the outstanding post sync operations corresponding to previously executed PIPECONTROL commands are complete before making forward progress." Do you think that's maybe what they meant? And in that case, I guess maybe I would need a PIPE_CONTROL with post sync operation right before this one, right? > > > We may also need to disable autostripping by whacking some chicken > > > registers if it's enabled (Gen9 WA #0799). Which would be lame, > > > > Looking again at #0799, it seems it's only applicable up to C0 on SKL, > > and B0 on BXT. So maybe we should be fine here? Or just disable it on > > BXT? > > You're right, I misread that. (I saw the Gen8 "FROM" tag and didn't > notice that it uses "UNTIL" on Gen9...) Nothing to do here. > > > > because that's likely a useful optimization. I guess we could disable > > > preemption for TRILIST and LINELIST as well to be safe. > > > > Is this because of the autostripping mentioned above, or some other > > workaround? Or just your impression? > > > > I can update it to include that, but just want to be sure that it's > > still applicable, once we figure the thing about #0799. > > Autostrip converts TRILIST/LINELIST into TRISTRIP/LINESTRIP, so > the idea would be to avoid preemption for anything that hits the > autostrip feature. But, no need, as you noted above. > > --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/18] Meson fixes and Travis integration
On Thu, 13 Dec 2018 at 11:08, Emil Velikov wrote: > Hi guys, > > While hacking on the integration I've spotted a few issues with our > meson build - patches 1-6. > > The rest of the series is Dylan's patches followed by a build matrix > analogous to the autotools/make one. > > As-is this causes one issue on macOS due to overly strict guards in the > meson code. > > I've respinned the series more than 10 times and everything else seems > solid. Personally I prefer if we get this (or anything functionally > analogous) and address macOS as follow-up. > > Let me know what you think. > Emil > > Cc: Rhys Kidd > Cc: Dylan Baker > > For the series: Acked-by: Rhys Kidd > Dylan Baker (3): > travis: meson: use native files to override llvm-config > travis: Don't try to read libdrm out of configure.ac > travis: meson: enable unit tests > > Emil Velikov (15): > meson: don't require glx/egl/gbm with gallium drivers > pipe-loader: meson: reference correct library > glx: meson: build src/glx only with -Dglx=dri > glx: meson: drop includes from a link-only library > glx: meson: wire up the dispatch-index-check test > glx/test: meson: assorted include fixes > configure: add CXX11_CXXFLAGS to LLVM_CXXFLAGS > travis: flip to distro xenial, sudo true > travis: meson: print the configured state > travis: printout llvm-config --version > travis: meson: use FOO_DRIVERS directly > travis: meson: add unwind handling > travis: meson: explicitly control the DRI loaders > travis: meson: add explicit handling to gallium ST > travis: meson: port gallium build combinations over > > .travis.yml | 353 +--- > configure.ac| 1 + > meson.build | 6 +- > src/gallium/targets/pipe-loader/meson.build | 2 +- > src/glx/meson.build | 31 +- > src/glx/tests/meson.build | 9 +- > src/meson.build | 2 +- > 7 files changed, 267 insertions(+), 137 deletions(-) > > -- > 2.19.2 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108946] High memory usage in Black Mesa
https://bugs.freedesktop.org/show_bug.cgi?id=108946 --- Comment #9 from Ian Romanick --- (In reply to iive from comment #8) > (In reply to Ian Romanick from comment #6) > > (I'm quite sure I submitted a similar comment earlier today, but it seems to > > have just vanished.) > > > > I ran the API trace in Valgrind massif memory profiler. As far as I can > > tell, the application never calls glDeleteShader. We don't release any of > > the memory because the app hasn't told us that we can. In that trace there > > is is about 2.6GB (on a 64-bit build of apitrace) of intermediate IR that > > could be released by calling glDeleteShader on all the shaders that have > > been linked. > > Do you really need to keep all Intermediate Representations of all shaders? > The standard does not deal with internal stuff and IR is just that. > > I thought that in core context you do not need to recompile them and that > there is a method to patch the prologue of the binary in compatibility mode > (at least for radeon si). The IR is needed in case the same shader is used to link with another program. It is common for the same vertex shader to be used with many different fragment shaders. Until the application calls glDeleteShader, it is impossible for the driver to know that the application is done with it. Separate data is stored with the linked programs for state-based recompiles. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] tgsi/scan: correctly walk instructions in tgsi_scan_tess_ctrl()
The previous code used a do while loop and continues after walking a nested loop/if-statement. This means we end up evaluating the last instruction from the nested block against the while condition and potentially exit early if it matches the exit condition of the outer block. Fixes: 386d165d8d09 ("tgsi/scan: add a new pass that analyzes tess factor writes") --- No changes in shader-db. Didn't run against piglit. src/gallium/auxiliary/tgsi/tgsi_scan.c | 72 +++--- 1 file changed, 43 insertions(+), 29 deletions(-) diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c b/src/gallium/auxiliary/tgsi/tgsi_scan.c index 6e2e23822c..d56844bdc2 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c @@ -1028,11 +1028,12 @@ get_block_tessfactor_writemask(const struct tgsi_shader_info *info, struct tgsi_full_instruction *inst; unsigned writemask = 0; - do { - tgsi_parse_token(parse); - assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION); - inst = &parse->FullToken.FullInstruction; - check_no_subroutines(inst); + tgsi_parse_token(parse); + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION); + inst = &parse->FullToken.FullInstruction; + check_no_subroutines(inst); + + while (inst->Instruction.Opcode != end_opcode) { /* Recursively process nested blocks. */ switch (inst->Instruction.Opcode) { @@ -1040,20 +1041,26 @@ get_block_tessfactor_writemask(const struct tgsi_shader_info *info, case TGSI_OPCODE_UIF: writemask |= get_block_tessfactor_writemask(info, parse, TGSI_OPCODE_ENDIF); - continue; + break; case TGSI_OPCODE_BGNLOOP: writemask |= get_block_tessfactor_writemask(info, parse, TGSI_OPCODE_ENDLOOP); - continue; + break; case TGSI_OPCODE_BARRIER: unreachable("nested BARRIER is illegal"); - continue; + break; + + default: + writemask |= get_inst_tessfactor_writemask(info, inst); } - writemask |= get_inst_tessfactor_writemask(info, inst); - } while (inst->Instruction.Opcode != end_opcode); + tgsi_parse_token(parse); + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION); + inst = &parse->FullToken.FullInstruction; + check_no_subroutines(inst); + } return writemask; } @@ -1067,18 +1074,20 @@ get_if_block_tessfactor_writemask(const struct tgsi_shader_info *info, struct tgsi_full_instruction *inst; unsigned then_tessfactor_writemask = 0; unsigned else_tessfactor_writemask = 0; + unsigned writemask; bool is_then = true; - do { - tgsi_parse_token(parse); - assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION); - inst = &parse->FullToken.FullInstruction; - check_no_subroutines(inst); + tgsi_parse_token(parse); + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION); + inst = &parse->FullToken.FullInstruction; + check_no_subroutines(inst); + + while (inst->Instruction.Opcode != TGSI_OPCODE_ENDIF) { switch (inst->Instruction.Opcode) { case TGSI_OPCODE_ELSE: is_then = false; - continue; + break; /* Recursively process nested blocks. */ case TGSI_OPCODE_IF: @@ -1087,28 +1096,33 @@ get_if_block_tessfactor_writemask(const struct tgsi_shader_info *info, is_then ? &then_tessfactor_writemask : &else_tessfactor_writemask, cond_block_tf_writemask); - continue; + break; case TGSI_OPCODE_BGNLOOP: *cond_block_tf_writemask |= get_block_tessfactor_writemask(info, parse, TGSI_OPCODE_ENDLOOP); - continue; + break; case TGSI_OPCODE_BARRIER: unreachable("nested BARRIER is illegal"); - continue; - } - - /* Process an instruction in the current block. */ - unsigned writemask = get_inst_tessfactor_writemask(info, inst); + break; + default: + /* Process an instruction in the current block. */ + writemask = get_inst_tessfactor_writemask(info, inst); - if (writemask) { - if (is_then) -then_tessfactor_writemask |= writemask; - else -else_tessfactor_writemask |= writemask; + if (writemask) { +if (is_then) + then_tessfactor_writemask |= writemask; +else + else_tessfactor_writemask |= writemask; + } } - } while (inst->Instruction.Opcode != TGSI_OPCODE_ENDIF); + + tgsi_parse_token(parse); + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION); + inst = &parse->FullToken.FullInstruction; + check_no_subroutines(inst); + } if (then_tessf
[Mesa-dev] [PATCH 1/2] tgsi/scan: fix loop exit point in tgsi_scan_tess_ctrl()
This just happened not to crash/assert because all loops have at least 1 if-statement and due to a second bug we end up matching the same ENDIF to exit both the iteration over the if-statment and the loop. The second bug is fixed in the following patch. Fixes: 386d165d8d09 ("tgsi/scan: add a new pass that analyzes tess factor writes") --- No changes in shader-db. Didn't run against piglit. src/gallium/auxiliary/tgsi/tgsi_scan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c b/src/gallium/auxiliary/tgsi/tgsi_scan.c index 4ca84902dd..6e2e23822c 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c @@ -1171,7 +1171,7 @@ tgsi_scan_tess_ctrl(const struct tgsi_token *tokens, case TGSI_OPCODE_BGNLOOP: cond_block_tf_writemask |= -get_block_tessfactor_writemask(info, &parse, TGSI_OPCODE_ENDIF); +get_block_tessfactor_writemask(info, &parse, TGSI_OPCODE_ENDLOOP); continue; case TGSI_OPCODE_BARRIER: -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] last call for autotools
I have to say that the user experience for autotools is WAY better than for meson. As a concrete example, I had a meson build. Then I updated meson (0.48.1 to 0.48.2). Now ninja -C foo doesn't work. meson --reconfigure (which presumably is what ninja would end up running) doesn't work. http://paste.debian.net/hidden/cf777f3e/ So now what? I don't remember how that config was done, except that it was done the way I decided I needed it at the time. I have no way to recover it. With autotools, in such cases (which are immensely rare), you just run "head config.log" and it tells you what you did last time. And by updating the build component, now I have to rebuild EVERYTHING? meson is not at a point where it Just Works. It ... sometimes works. The fact that everyone has scripts which wrap meson is a symptom of that. I don't feel good about dumping the system that everyone (and I don't just mean people on this list -- I mean the wider open source community as well) knows how to use and has worked reliably for years (decades, really) to be replaced by a system that everyone is having problems with (it's not just me -- others are running into trouble too -- just look at this thread). It's just not ready yet. -ilia On Mon, Dec 10, 2018 at 6:11 PM Dylan Baker wrote: > > Meson 0.49.0 has been out for a couple of days now, and I'd like to make the > final call for autotools. My patch is so massive that it's a huge pain to send > to the list, the latest versions is here: > https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools > > Dylan > ___ > 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] last call for autotools
On Thu, Dec 13, 2018 at 10:19 PM Ilia Mirkin wrote: > So now what? I don't remember how that config was done, except that it > was done the way I decided I needed it at the time. I have no way to > recover it. With autotools, in such cases (which are immensely rare), > you just run "head config.log" and it tells you what you did last > time. And by updating the build component, now I have to rebuild > EVERYTHING? Since it uses ccache, that should take about 20 seconds... ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] last call for autotools
So your issue is that meson upgrade broke the build and you are unable to get config.log. Let me tell you something. Ninja upgrade also breaks the build. I think that's the way it's going to be now. I think people will use the new build system anyway as conf and compile time is all that matters. Marek On Fri, Dec 14, 2018, 1:19 AM Ilia Mirkin I have to say that the user experience for autotools is WAY better > than for meson. As a concrete example, I had a meson build. Then I > updated meson (0.48.1 to 0.48.2). Now ninja -C foo doesn't work. meson > --reconfigure (which presumably is what ninja would end up running) > doesn't work. http://paste.debian.net/hidden/cf777f3e/ > > So now what? I don't remember how that config was done, except that it > was done the way I decided I needed it at the time. I have no way to > recover it. With autotools, in such cases (which are immensely rare), > you just run "head config.log" and it tells you what you did last > time. And by updating the build component, now I have to rebuild > EVERYTHING? > > meson is not at a point where it Just Works. It ... sometimes works. > The fact that everyone has scripts which wrap meson is a symptom of > that. I don't feel good about dumping the system that everyone (and I > don't just mean people on this list -- I mean the wider open source > community as well) knows how to use and has worked reliably for years > (decades, really) to be replaced by a system that everyone is having > problems with (it's not just me -- others are running into trouble too > -- just look at this thread). It's just not ready yet. > > -ilia > > On Mon, Dec 10, 2018 at 6:11 PM Dylan Baker wrote: > > > > Meson 0.49.0 has been out for a couple of days now, and I'd like to make > the > > final call for autotools. My patch is so massive that it's a huge pain > to send > > to the list, the latest versions is here: > > https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools > > > > Dylan > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] last call for autotools
On Fri, Dec 14, 2018 at 2:13 AM Matt Turner wrote: > > On Thu, Dec 13, 2018 at 10:19 PM Ilia Mirkin wrote: > > So now what? I don't remember how that config was done, except that it > > was done the way I decided I needed it at the time. I have no way to > > recover it. With autotools, in such cases (which are immensely rare), > > you just run "head config.log" and it tells you what you did last > > time. And by updating the build component, now I have to rebuild > > EVERYTHING? > > Since it uses ccache, that should take about 20 seconds... I don't have ccache. No clue how to set it up, and don't have the disk space for it anyways -- constantly teetering on the edge of fullness. It takes about 10 minutes to build. And if the hope is that meson becomes used for other projects, then this is multiplied by all the other meson-using projects. Your answer is an apology for tool shortcomings. As someone who frequently builds a particular project, I'm sure you have everything all set up perfectly (enough) for your situation. But you're not the average user. I have things set up "well enough" as well. I edit a handful of files, and expect a ~full rebuild when I rebase, which I do relatively rarely. The average user is the one who knows just a smidgen more than "./configure; make install". That's what I mean by the user experience being better in autotools. But like I said on IRC ... if I'm the lone cranky old man who doesn't like the new thing, then I won't (and probably couldn't even if I wanted to) stand in the way of this. However my impression is that there's a larger contingent dissatisfied with meson. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev