Re: [Mesa-dev] [PATCH 3/3] intel/compiler: add an optimization pass for booleans
On Thu, 2018-07-05 at 15:47 -0700, Caio Marcelo de Oliveira Filho wrote: > (I had to stop reading to go home last Tuesday, so here are the > remaining comments.) > > > On Tue, May 15, 2018 at 01:05:21PM +0200, Iago Toral Quiroga wrote: > > NIR assumes that all booleans are 32-bit but Intel hardware > > produces > > booleans of the same size as the operands to the CMP instruction, > > so we > > can actually have 8-bit and 16-bit booleans. To work around this > > mismatch between NIR and the hardware, we emit boolean conversions > > to > > 32-bit right after emitting the CMP instruction during the NIR->FS > > pass, which makes interfacing with NIR a lot easier, but can leave > > unnecessary boolean conversions in the shader code. > > Question: have you explored handling this at the NIR->FS conversion? > I.e. instead of generate the cmp + mov and then look for the cmp + > mov > to fix up; when generating a cmp, perform those checks (at nir level) > and then pick the right bitsize. It is not that easy. The problem is that NIR will continue to come at us with 32-bit boolean instructions after the CMP+MOV, so instead of prpagating forward for every conversion, now, for every bool we find in IR we'd need to go back in the FS program to check if it is a real 32- bit boolean or not to decide what to emit. I don't see any benefit, plus we would be coupling all this with the translation implementation, which I think is less nice than having it being a completely separate thing. Anyway, there is a major issue with the current patch that I have found this week thanks to some new CTS tests: when we propagate the bitsize of a logical instruction to its destination, that affects all its consumers even outside the current block, so we need to handle propagation across blocks, which adds a few more problems, so I still need to figure out how to deal with that properly and whether that is something we want to do (there is a reason why no other opt/lowering passes do cross-block changes...). Iago > > > +/** > > + * Propagates the bit-size of the destination of a boolean > > instruction to > > + * all its consumers. If propagate_from_source is True, then the > > producer > > + * is a conversion MOV from a low bit-size boolean to 32-bit, and > > in that > > + * case the propagation happens from the source of the instruction > > instead > > + * of its destination. > > + */ > > +static bool > > +propagate_bool_bit_size(fs_inst *inst, bool propagate_from_source) > > +{ > > + assert(!propagate_from_source || inst->opcode == > > BRW_OPCODE_MOV); > > + > > + bool progress = false; > > + > > + const unsigned bit_size = 8 * (propagate_from_source ? > > + type_sz(inst->src[0].type) : type_sz(inst->dst.type)); > > + > > + /* Look for any follow-up instructions that sources from the > > boolean > > +* result of the producer instruction and rewrite them to use > > the correct > > +* bit-size. > > +*/ > > + foreach_inst_in_block_starting_from(fs_inst, fixup_inst, inst) > > { > > + if (!inst_supports_boolean(fixup_inst)) > > + continue; > > Should we care about other instruction clobbering the contents of > inst->dst, or at this point of the optimization we can count on it > not > being? > > > > + /* If it is a plain boolean conversion to 32-bit, then > > look for any > > + * follow-up instructions that source from the 32-bit > > boolean and > > + * rewrite them to source from the output of the CMP > > (which is the > > + * source of the conversion instruction) directly if > > possible. > > + */ > > + progress = propagate_bool_bit_size(conv_inst, true) || > > progress; > > + } > > +#if 0 > > + else if (inst_supports_boolean(inst) && inst->sources > 1) > > { > > If you end up enabling this section, I suggest move the > inst_supports_boolean() check to the beginning of the for-loop, as an > early return. Makes the condition for the cases we are handling > cleaner. > > > > > + /* For all logical instructions that can take more than > > one operand > > + * we need to ensure that all of them have matching bit- > > sizes. If they > > + * don't, it means that the original shader code is > > operating boolean > > + * expressions with different native bit-sizes and we > > need to choose > > + * a canonical boolean form for all the operands, which > > requires to > > + * inject conversions to temporaries. We choose the bit- > > size of the > > + * destination as the canonical form (which must be a 32- > > bit boolean > > + * since we only propagate smaller bit-sizes to the > > destination if we > > + * managed to convert all the operands to the same bit- > > size) because > > + * that way we don't need to worry about propagating the > > destination > > + * bit-size down the line. > > + */ > > To make this comment less heavy, I'd
Re: [Mesa-dev] [PATCH rfc 0/3] Be able to disable EGL/GLX_EXT_buffer_age
On Fri, Jul 6, 2018 at 6:20 AM Eric Anholt wrote: > > Qiang Yu writes: > > > Hi Emil, > > > > On Thu, Jul 5, 2018 at 9:54 PM Emil Velikov > > wrote: > >> > >> On 5 July 2018 at 14:31, Emil Velikov wrote: > >> > Hi Qiang Yu > >> > > >> > On 5 July 2018 at 03:31, Qiang Yu wrote: > >> >> For GPU like ARM mali Utgard EGL/GLX_EXT_buffer_age will make > >> >> performace worse. But mesa has no way to disable it. > >> >> > >> >> This patch series make driver be able to disable it and add a > >> >> gallium pipe cap for gallium driver usage. Due to currently > >> >> only out of tree lima driver need it, and not sure if this is > >> >> the right way to disable it, so I send this RFC before lima be > >> >> able to upstream. > >> >> > >> > Pretty sure we already have a way to handle that. Look for > >> > glx_disable_ext_buffer_age - it was introduced for the VMWare driver. > >> > Although we should: > >> > a) tweak the name - kind of split if we need to > >> > b) add glx/dri2 and egl support > >> > > >> Looking at the implementation - it uses a driver query, meaning that > >> one could enable it at a later stage. > >> No need to worry if the user has old drirc/etc as with current solution. > > > > Yes, use drirc to disable buffer age means it can be enabled by changing > > the config (driver has to support it). But GPU like mali just don't want to > > support it at all (need extra code do bad things). > > You must have the ability to load back into the tile buffer, so I think > the driver should continue to support buffer age. You don't know the > complexity of recreating their whole frame, that decision should be up > to them. Yeah, mali can load back into the tile buffer. But buffer age ext has no way to tell how much partial draw cost for different GPU, expose it just tell application partial draw is cheaper. Regards, Qiang > That said, it would be really nice for EGL to decide not to > preserve buffers (and emit the resource invalidate call through gallium) > when the client hasn't asked for a buffer age recently, and then just > report the undefined buffer age value the first time they ask for it. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Restrict the nuber of color regions to those actually written
On Wed, Jun 27, 2018 at 07:00:56PM -0700, Jason Ekstrand wrote: > The back-end compiler emits the number of color writes specified by > wm_prog_key::nr_color_regions regardless of what nir_store_outputs we > have. Once we've gone through and figured out which render targets > actually exist and are written by the shader, we should restrict the key > to avoid extra RT write messages. > --- > src/intel/vulkan/anv_pipeline.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c > index 523202cf3d9..6a27d305dbc 100644 > --- a/src/intel/vulkan/anv_pipeline.c > +++ b/src/intel/vulkan/anv_pipeline.c > @@ -965,6 +965,11 @@ anv_pipeline_compile_fs(struct anv_pipeline *pipeline, > var->data.location = rt_to_bindings[rt] + FRAG_RESULT_DATA0; >} > > + /* Now that we've determined the actual number of render targets, > adjust > + * the key accordingly. > + */ > + key.nr_color_regions = num_rts; > + Question: earlier in the code we call populate_wm_prog_key(pipeline, info, &key); which does key->nr_color_regions = pipeline->subpass->color_count; key->replicate_alpha = key->nr_color_regions > 1 && info->pMultisampleState && info->pMultisampleState->alphaToCoverageEnable; so key->replicate_alpha is calculated based on the old value. Should this be (re)calculated using the new value? Thanks, Caio ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965/fs: Properly handle sign(-abs(x))
Reviewed-by: Caio Marcelo de Oliveira Filho Tested the patch with the shaders mentioned. Do you think would be worth to have a pass at NIR level to perform these transformations? Thanks, Caio On Wed, Jun 27, 2018 at 07:37:57AM -0700, Ian Romanick wrote: > From: Ian Romanick > > Fixes new piglit tests: > > - glsl-1.10/execution/fs-sign-neg-abs.shader_test > - glsl-1.10/execution/fs-sign-sat-neg-abs.shader_test > - glsl-1.10/execution/vs-sign-neg-abs.shader_test > - glsl-1.10/execution/vs-sign-sat-neg-abs.shader_test > > Signed-off-by: Ian Romanick > Cc: mesa-sta...@lists.freedesktop.org > --- > src/intel/compiler/brw_fs_nir.cpp | 15 --- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > b/src/intel/compiler/brw_fs_nir.cpp > index 0abb4798e70..2ad7a2d0dfc 100644 > --- a/src/intel/compiler/brw_fs_nir.cpp > +++ b/src/intel/compiler/brw_fs_nir.cpp > @@ -807,11 +807,20 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, > nir_alu_instr *instr) > > case nir_op_fsign: { >if (op[0].abs) { > - /* Straightforward since the source can be assumed to be > - * non-negative. > + /* Straightforward since the source can be assumed to be either > + * strictly >= 0 or strictly <= 0 depending on the setting of the > + * negate flag. >*/ > set_condmod(BRW_CONDITIONAL_NZ, bld.MOV(result, op[0])); > - set_predicate(BRW_PREDICATE_NORMAL, bld.MOV(result, > brw_imm_f(1.0f))); > + > + inst = (op[0].negate) > +? bld.MOV(result, brw_imm_f(-1.0f)) > +: bld.MOV(result, brw_imm_f(1.0f)); > + > + set_predicate(BRW_PREDICATE_NORMAL, inst); > + > + if (instr->dest.saturate) > +inst->saturate = true; > >} else if (type_sz(op[0].type) < 8) { > /* AND(val, 0x8000) gives the sign bit. > -- > 2.14.4 > > ___ > 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] nir: delete not needed for reinserted nir_cf_list
It wasn't causing problems since there's nothing to delete, but better be consistent with the rest of existing codebase. --- src/compiler/nir/nir_opt_if.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index ec5bf1c9027..a52de120ad6 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -279,7 +279,6 @@ opt_if_simplification(nir_builder *b, nir_if *nif) nir_cf_extract(&tmp, nir_before_cf_list(&nif->else_list), nir_after_cf_list(&nif->else_list)); nir_cf_reinsert(&tmp, nir_before_cf_list(&nif->then_list)); - nir_cf_delete(&tmp); return true; } @@ -345,7 +344,6 @@ opt_if_loop_terminator(nir_if *nif) nir_cf_extract(&tmp, nir_before_block(first_continue_from_blk), nir_after_block(continue_from_blk)); nir_cf_reinsert(&tmp, nir_after_cf_node(&nif->cf_node)); - nir_cf_delete(&tmp); return true; } -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 106644] [llvmpipe] Mesa 18.1.2 fails lp_test_format, lp_test_arit, lp_test_blend, lp_test_printf, lp_test_conv tests
https://bugs.freedesktop.org/show_bug.cgi?id=106644 --- Comment #27 from Roland Scheidegger --- (In reply to Ben Crocker from comment #25) > According to the Wikipedia article, the 970 is a Power ISA v2.03 (2002-2007) > CPU, > while VSX was not introduced until Power ISA v2.06 (February 2009) and > POWER7. > > So it seems to me that llc should recognize which version of the ISA each > CPU implements, and that it should disallow combinations like > -mcpu=970 -mattr=+altivec,+vsx I don't think you can blame llvm for that. This looks perfectly acceptable to me - you're specifying the cpu model (and that governs a lot more than just cpu extensions), but then specify some extensions on top of it. It's not llvm's fault that the cpu really can't do the extensions. (fwiw on x86 this is perfectly acceptable too, and it's even more weird there, for instance if you explicitly specify -avx but +f16c avx will get reenabled nevertheless, since the latter option actually requires avx (or more accurately, vex encoding) - you are responsible for specifying attributes which make sense.) Env vars to specify this are ok, but we should never manually enable attributes which the cpu can't support (and certainly not by default...). Surely it's possible to recognize these cpu features? Also, maybe llvm would recognize the availability of these features on its own nowadays correctly (as it does for x86)? Although I remember there were problems due to LE/BE differentation on ppc. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] nir/worklist: Rework the foreach macro
Reviewed-by: Caio Marcelo de Oliveira Filho On Tue, Jul 03, 2018 at 11:13:07PM -0700, Jason Ekstrand wrote: > This makes the arguments match the (thing, container) pattern used in > other nir_foreach macros and also renames it to make that a bit more > clear. > --- > src/compiler/nir/nir_opt_dce.c | 3 +-- > src/compiler/nir/nir_worklist.h | 4 ++-- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/src/compiler/nir/nir_opt_dce.c b/src/compiler/nir/nir_opt_dce.c > index c9b338862e6..70532be33d7 100644 > --- a/src/compiler/nir/nir_opt_dce.c > +++ b/src/compiler/nir/nir_opt_dce.c > @@ -129,8 +129,7 @@ nir_opt_dce_impl(nir_function_impl *impl) >init_block(block, worklist); > } > > - nir_instr *instr = NULL; > - nir_instr_worklist_foreach(worklist, instr) > + nir_foreach_instr_in_worklist(instr, worklist) >nir_foreach_src(instr, mark_live_cb, worklist); > > nir_instr_worklist_destroy(worklist); > diff --git a/src/compiler/nir/nir_worklist.h b/src/compiler/nir/nir_worklist.h > index 3fb391fceff..05aa757eb79 100644 > --- a/src/compiler/nir/nir_worklist.h > +++ b/src/compiler/nir/nir_worklist.h > @@ -154,8 +154,8 @@ nir_instr_worklist_pop_head(nir_instr_worklist *wl) > return *vec_instr; > } > > -#define nir_instr_worklist_foreach(wl, instr)\ > - while ((instr = nir_instr_worklist_pop_head(wl))) > +#define nir_foreach_instr_in_worklist(instr, wl) \ > + for (nir_instr *instr; (instr = nir_instr_worklist_pop_head(wl));) > > #ifdef __cplusplus > } /* extern "C" */ > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1.5/3] nir: Add a nir_instr_move helper
> > Should we also consider the block-based cursor options here? Moving > > the first instruction of a block before that block > > (nir_cursor_before_block), etc. > > > > That should be fine as-is as nir_instr_insert should do the right thing. > The reason the cursor.instr == instr case is a problem is because you can't > insert an instruction relative to itself. Got it. Reviewed-by: Caio Marcelo de Oliveira Filho ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] intel/compiler: add an optimization pass for booleans
(I had to stop reading to go home last Tuesday, so here are the remaining comments.) On Tue, May 15, 2018 at 01:05:21PM +0200, Iago Toral Quiroga wrote: > NIR assumes that all booleans are 32-bit but Intel hardware produces > booleans of the same size as the operands to the CMP instruction, so we > can actually have 8-bit and 16-bit booleans. To work around this > mismatch between NIR and the hardware, we emit boolean conversions to > 32-bit right after emitting the CMP instruction during the NIR->FS > pass, which makes interfacing with NIR a lot easier, but can leave > unnecessary boolean conversions in the shader code. Question: have you explored handling this at the NIR->FS conversion? I.e. instead of generate the cmp + mov and then look for the cmp + mov to fix up; when generating a cmp, perform those checks (at nir level) and then pick the right bitsize. > +/** > + * Propagates the bit-size of the destination of a boolean instruction to > + * all its consumers. If propagate_from_source is True, then the producer > + * is a conversion MOV from a low bit-size boolean to 32-bit, and in that > + * case the propagation happens from the source of the instruction instead > + * of its destination. > + */ > +static bool > +propagate_bool_bit_size(fs_inst *inst, bool propagate_from_source) > +{ > + assert(!propagate_from_source || inst->opcode == BRW_OPCODE_MOV); > + > + bool progress = false; > + > + const unsigned bit_size = 8 * (propagate_from_source ? > + type_sz(inst->src[0].type) : type_sz(inst->dst.type)); > + > + /* Look for any follow-up instructions that sources from the boolean > +* result of the producer instruction and rewrite them to use the correct > +* bit-size. > +*/ > + foreach_inst_in_block_starting_from(fs_inst, fixup_inst, inst) { > + if (!inst_supports_boolean(fixup_inst)) > + continue; Should we care about other instruction clobbering the contents of inst->dst, or at this point of the optimization we can count on it not being? > + /* If it is a plain boolean conversion to 32-bit, then look for any > + * follow-up instructions that source from the 32-bit boolean and > + * rewrite them to source from the output of the CMP (which is the > + * source of the conversion instruction) directly if possible. > + */ > + progress = propagate_bool_bit_size(conv_inst, true) || progress; > + } > +#if 0 > + else if (inst_supports_boolean(inst) && inst->sources > 1) { If you end up enabling this section, I suggest move the inst_supports_boolean() check to the beginning of the for-loop, as an early return. Makes the condition for the cases we are handling cleaner. > + /* For all logical instructions that can take more than one operand > + * we need to ensure that all of them have matching bit-sizes. If > they > + * don't, it means that the original shader code is operating > boolean > + * expressions with different native bit-sizes and we need to choose > + * a canonical boolean form for all the operands, which requires to > + * inject conversions to temporaries. We choose the bit-size of the > + * destination as the canonical form (which must be a 32-bit boolean > + * since we only propagate smaller bit-sizes to the destination if > we > + * managed to convert all the operands to the same bit-size) because > + * that way we don't need to worry about propagating the destination > + * bit-size down the line. > + */ To make this comment less heavy, I'd move the assumption about the destination being 32-bit right above the assert, which is kind of an explanation of the assertion. > @@ -6240,6 +6471,7 @@ fs_visitor::optimize() > > OPT(opt_drop_redundant_mov_to_flags); > OPT(remove_extra_rounding_modes); > + OPT(opt_bool_bit_size); It could be useful to have a short comment here about the importance of calling opt_bool_bit_size at this point. Thanks, Caio ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 106644] [llvmpipe] Mesa 18.1.2 fails lp_test_format, lp_test_arit, lp_test_blend, lp_test_printf, lp_test_conv tests
https://bugs.freedesktop.org/show_bug.cgi?id=106644 --- Comment #26 from Ben Crocker --- I'm not seeing any assembly code in the output you posted, and I'm wondering whether the attempt to generate VSX code might be leading to the "Relocation type not implemented" errors and failure to generate any assembly code. But yes, you were exactly right in zeroing in on the combination of -mcpu=970 -mattrs=+altivec,+vsx, which SHOULD be caught and disallowed. -- 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 106644] [llvmpipe] Mesa 18.1.2 fails lp_test_format, lp_test_arit, lp_test_blend, lp_test_printf, lp_test_conv tests
https://bugs.freedesktop.org/show_bug.cgi?id=106644 --- Comment #25 from Ben Crocker --- According to the Wikipedia article, the 970 is a Power ISA v2.03 (2002-2007) CPU, while VSX was not introduced until Power ISA v2.06 (February 2009) and POWER7. So it seems to me that llc should recognize which version of the ISA each CPU implements, and that it should disallow combinations like -mcpu=970 -mattr=+altivec,+vsx I filed the following bug against LLVM/llc: https://bugs.llvm.org/show_bug.cgi?id=38075 (llc for Power allows illegal combinations of -mcpu and -mattr) In the meantime, you can control VSX usage from Mesa in a couple of ways: % export GALLIVM_MATTRS=, e.g. % export GALLIVM_MATTRS="+altivec,-vsx" to keep AltiVec but turn off VSX code generation, or % export GALLIVM_VSX=0 to disable VSX code generation. -- 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] [RFC 10/10] glsl: use only copy_propagation_elements
Caio Marcelo de Oliveira Filho writes: > Now that the elements version handles both cases, remove the > non-elements version. I made some progress on reviewing the series today. I think this is very worthwhile to pursue, given that it gets rid of one of our GLSL IR passes that felt like nasty duplication at the time that I wrote the second copy. Hopefully it improves compiler performance for all. Depending on the previous patches, this patch is: Reviewed-by: Eric Anholt 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] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx)
Adam Jackson writes: > Fixes 14 piglits, mostly in egl_khr_create_context. > > Fixes: https://github.com/anholt/libepoxy/issues/177 > Signed-off-by: Adam Jackson > --- > src/mesa/drivers/dri/swrast/swrast.c | 34 +++- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/src/mesa/drivers/dri/swrast/swrast.c > b/src/mesa/drivers/dri/swrast/swrast.c > index ae5874f5927..7f08107c24f 100644 > --- a/src/mesa/drivers/dri/swrast/swrast.c > +++ b/src/mesa/drivers/dri/swrast/swrast.c > @@ -675,6 +675,9 @@ swrast_check_and_update_window_size( struct gl_context > *ctx, struct gl_framebuff > { > GLsizei width, height; > > +if (!fb) > +return; > + > get_window_size(fb, &width, &height); > if (fb->Width != width || fb->Height != height) { > _mesa_resize_framebuffer(ctx, fb, width, height); > @@ -857,30 +860,29 @@ dri_make_current(__DRIcontext * cPriv, >__DRIdrawable * driReadPriv) > { > struct gl_context *mesaCtx; > -struct gl_framebuffer *mesaDraw; > -struct gl_framebuffer *mesaRead; > +struct gl_framebuffer *mesaDraw = NULL; > +struct gl_framebuffer *mesaRead = NULL; > TRACE; > > if (cPriv) { > - struct dri_context *ctx = dri_context(cPriv); > struct dri_drawable *draw; > struct dri_drawable *read; > > - if (!driDrawPriv || !driReadPriv) > - return GL_FALSE; > +mesaCtx = &dri_context(cPriv)->Base; > > - draw = dri_drawable(driDrawPriv); > - read = dri_drawable(driReadPriv); > - mesaCtx = &ctx->Base; > - mesaDraw = &draw->Base; > - mesaRead = &read->Base; > + if (driDrawPriv && driReadPriv) { > + draw = dri_drawable(driDrawPriv); > + read = dri_drawable(driReadPriv); > + mesaDraw = &draw->Base; > + mesaRead = &read->Base; > > - /* check for same context and buffer */ > - if (mesaCtx == _mesa_get_current_context() > - && mesaCtx->DrawBuffer == mesaDraw > - && mesaCtx->ReadBuffer == mesaRead) { > - return GL_TRUE; > - } > + /* check for same context and buffer */ > + if (mesaCtx == _mesa_get_current_context() > + && mesaCtx->DrawBuffer == mesaDraw > + && mesaCtx->ReadBuffer == mesaRead) { > + return GL_TRUE; > + } > +} Didn't you mean for this block to be outside of the driDrawPriv && driReadPriv condition? That way you can get the short-circuit in the no-drawable case, too. Other than that, Reviewed-by: Eric Anholt 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 rfc 0/3] Be able to disable EGL/GLX_EXT_buffer_age
Qiang Yu writes: > Hi Emil, > > On Thu, Jul 5, 2018 at 9:54 PM Emil Velikov wrote: >> >> On 5 July 2018 at 14:31, Emil Velikov wrote: >> > Hi Qiang Yu >> > >> > On 5 July 2018 at 03:31, Qiang Yu wrote: >> >> For GPU like ARM mali Utgard EGL/GLX_EXT_buffer_age will make >> >> performace worse. But mesa has no way to disable it. >> >> >> >> This patch series make driver be able to disable it and add a >> >> gallium pipe cap for gallium driver usage. Due to currently >> >> only out of tree lima driver need it, and not sure if this is >> >> the right way to disable it, so I send this RFC before lima be >> >> able to upstream. >> >> >> > Pretty sure we already have a way to handle that. Look for >> > glx_disable_ext_buffer_age - it was introduced for the VMWare driver. >> > Although we should: >> > a) tweak the name - kind of split if we need to >> > b) add glx/dri2 and egl support >> > >> Looking at the implementation - it uses a driver query, meaning that >> one could enable it at a later stage. >> No need to worry if the user has old drirc/etc as with current solution. > > Yes, use drirc to disable buffer age means it can be enabled by changing > the config (driver has to support it). But GPU like mali just don't want to > support it at all (need extra code do bad things). You must have the ability to load back into the tile buffer, so I think the driver should continue to support buffer age. You don't know the complexity of recreating their whole frame, that decision should be up to them. That said, it would be really nice for EGL to decide not to preserve buffers (and emit the resource invalidate call through gallium) when the client hasn't asked for a buffer age recently, and then just report the undefined buffer age value the first time they ask for it. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/dri: fix a crash in server_wait_Sync
From: Marek Olšák Ported from i965 including the comment. This fixes: dEQP-EGL.functional.reusable_sync.valid.wait_server Cc: 18.1 --- src/gallium/state_trackers/dri/dri_helpers.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/gallium/state_trackers/dri/dri_helpers.c b/src/gallium/state_trackers/dri/dri_helpers.c index f1501bfb815..5d42873a208 100644 --- a/src/gallium/state_trackers/dri/dri_helpers.c +++ b/src/gallium/state_trackers/dri/dri_helpers.c @@ -207,20 +207,26 @@ dri2_client_wait_sync(__DRIcontext *_ctx, void *_fence, unsigned flags, return false; } } static void dri2_server_wait_sync(__DRIcontext *_ctx, void *_fence, unsigned flags) { struct pipe_context *ctx = dri_context(_ctx)->st->pipe; struct dri2_fence *fence = (struct dri2_fence*)_fence; + /* We might be called here with a NULL fence as a result of WaitSyncKHR +* on a EGL_KHR_reusable_sync fence. Nothing to do here in such case. +*/ + if (!fence) + return; + if (ctx->fence_server_sync) ctx->fence_server_sync(ctx, fence->pipe_fence); } const __DRI2fenceExtension dri2FenceExtension = { .base = { __DRI2_FENCE, 2 }, .create_fence = dri2_create_fence, .get_fence_from_cl_event = dri2_get_fence_from_cl_event, .destroy_fence = dri2_destroy_fence, -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/10] glsl: separate copy propagation state
Caio Marcelo de Oliveira Filho writes: > Separate higher level logic of visiting instructions and chosing when > to store and use new copy data from the datastructure holding the copy > propagation information. This will also make easier later patches that > change the structure. > --- > .../glsl/opt_copy_propagation_elements.cpp| 269 ++ > 1 file changed, 143 insertions(+), 126 deletions(-) > > diff --git a/src/compiler/glsl/opt_copy_propagation_elements.cpp > b/src/compiler/glsl/opt_copy_propagation_elements.cpp > index 8975e727522..5d3664a503b 100644 > --- a/src/compiler/glsl/opt_copy_propagation_elements.cpp > +++ b/src/compiler/glsl/opt_copy_propagation_elements.cpp > @@ -89,6 +89,121 @@ public: > acp_ref rhs_node; > }; > > +class copy_propagation_state { Since this copy_propagation_state covers just the acp and not the kills list (whcih is still part of the copy propagation state in the visitor class), could we call it "acp"? > @@ -191,26 +283,21 @@ > ir_copy_propagation_elements_visitor::visit_enter(ir_function_signature *ir) > exec_list *orig_kills = this->kills; > bool orig_killed_all = this->killed_all; > > - hash_table *orig_lhs_ht = lhs_ht; > - hash_table *orig_rhs_ht = rhs_ht; > - > this->kills = new(mem_ctx) exec_list; > this->killed_all = false; > > - create_acp(); > + copy_propagation_state *orig_state = state; > + this->state = new(mem_ctx) copy_propagation_state(mem_ctx, lin_ctx); > > visit_list_elements(this, &ir->body); > > - ralloc_free(this->kills); > - > - destroy_acp(); > + delete this->state; > + this->state = orig_state; Don't you want destroy_acp()'s body in ~copy_propagation_state()? Other than that, it looks good. 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 08/10] glsl: don't let an 'if' then-branch kill copy propagation (elements) for else-branch
Caio Marcelo de Oliveira Filho writes: > When handling 'if' in copy propagation elements, if a certain variable > was killed when processing the first branch of the 'if', then the > second would get any propagation from previous nodes. > > x = y; > if (...) { > z = x; // This would turn into z = y. > x = 22; // x gets killed. > } else { > w = x; // This would NOT turn into w = y. > } Reviewed-by: Eric Anholt 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 2/3] nir: Add a discard optimization pass
On Thu, Jul 5, 2018 at 2:18 PM, Jason Ekstrand wrote: > On Thu, Jul 5, 2018 at 11:03 AM, Francisco Jerez > wrote: > >> Jason Ekstrand writes: >> >> > On Wed, Jul 4, 2018 at 1:20 PM, Francisco Jerez >> > wrote: >> > >> >> Jason Ekstrand writes: >> >> >> >> > Many fragment shaders do a discard using relatively little >> information >> >> > but still put the discard fairly far down in the shader for no good >> >> > reason. If the discard is moved higher up, we can possibly avoid >> doing >> >> > some or almost all of the work in the shader. When this lets us skip >> >> > texturing operations, it's an especially high win. >> >> > >> >> > One of the biggest offenders here is DXVK. The D3D APIs have >> different >> >> > rules for discards than OpenGL and Vulkan. One effective way (which >> is >> >> > what DXVK uses) to implement DX behavior on top of GL or Vulkan is to >> >> > wait until the very end of the shader to discard. This ends up in >> the >> >> > pessimal case where we always do all of the work before discarding. >> >> > This pass helps some DXVK shaders significantly. >> >> > >> >> >> >> One thing to keep in mind is that this sort of transformation is >> trading >> >> off run-time of fragment shader invocations that don't call discard (or >> >> do so non-uniformly, which means that the code the discard jump is >> >> protecting will be executed anyway, so doing this can actually increase >> >> the critical path of the program) in favour of invocations that call >> >> discard uniformly (so executing discard early will effectively >> terminate >> >> the program early). >> > >> > >> > It's not really a uniform vs. non-uniform thing. Even if a shader only >> > discards some of the fragments, it sill reduces the number of live >> channels >> > which reduces the cost of later non-uniform control-flow. >> > >> >> Which only helps if the shader's control flow is sufficiently >> non-uniform that the additional cost from performing those computations >> early pays off -- Or not at all if the discarded fragments need to be >> executed (non-compliantly) anyway in order to provide >> derivatives_safe_after_discard. However, if the discard condition is >> uniform (across a warp), the thread can be terminated early by the >> back-end most certainly, which gives you the maximum pay-off. Uniform >> discard conditions are therefore the best-case scenario for this >> optimization pass. >> > > Yes, that is correct. Fortunately, things that discard tend to discard > fairly large chunks of the polygon at one time so this case is fairly > common. > > >> > >> >> Optimizing for the latter case is an essentially >> >> heuristic assumption that needs to be verified experimentally. Have >> you >> >> tested the effect of this pass on non-DX workloads extensively? >> >> >> > >> > Yes, it is a trade-off. No, I have not done particularly extensive >> > testing. We do, however, know of non-DXVK workloads that would benefit >> > from this. I believe Manhattan is one such example though I have not >> yet >> > benchmarked it. >> > >> >> You should grab some numbers then to make sure there are no >> regressions... > > > I'm working on that. Unfortunately the perf system is giving me trouble > so I don't have the numbers yet. > > >> But keep in mind that the i965 scheduler is already >> performing a similar optimization (locally, but with cycle-count >> information). This will only help over the existing optimization if the >> shaders that represent a bottleneck in Manhattan have sufficient control >> flow for the basic block boundaries to represent a problem to the >> (local) scheduler. >> > > I'm not sure about the manhattan shader but the Skyrim shader does have > control flow which the discard has to get moved above. > I have results from the perf system now and somehow this pass makes manhattan noticeably worse. I'll look into that. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH rfc 2/3] gallium: add PIPE_CAP_BUFFER_AGE
Reviewed-by: Marek Olšák Marek On Wed, Jul 4, 2018 at 10:31 PM, Qiang Yu wrote: > For gallium drivers to expose EGL/GLX_EXT_buffer_age. > > Signed-off-by: Qiang Yu > --- > src/gallium/docs/source/screen.rst | 1 + > src/gallium/drivers/etnaviv/etnaviv_screen.c| 1 + > src/gallium/drivers/freedreno/freedreno_screen.c| 1 + > src/gallium/drivers/i915/i915_screen.c | 1 + > src/gallium/drivers/llvmpipe/lp_screen.c| 1 + > src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 + > src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 + > src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 + > src/gallium/drivers/r300/r300_screen.c | 1 + > src/gallium/drivers/r600/r600_pipe.c| 1 + > src/gallium/drivers/radeonsi/si_get.c | 1 + > src/gallium/drivers/softpipe/sp_screen.c| 1 + > src/gallium/drivers/svga/svga_screen.c | 1 + > src/gallium/drivers/swr/swr_screen.cpp | 1 + > src/gallium/drivers/vc4/vc4_screen.c| 1 + > src/gallium/drivers/vc5/vc5_screen.c| 1 + > src/gallium/drivers/virgl/virgl_screen.c| 2 ++ > src/gallium/include/pipe/p_defines.h| 1 + > src/gallium/state_trackers/dri/dri_query_renderer.c | 4 +++- > 19 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/docs/source/screen.rst > b/src/gallium/docs/source/screen.rst > index 3837360fb4..427944bf70 100644 > --- a/src/gallium/docs/source/screen.rst > +++ b/src/gallium/docs/source/screen.rst > @@ -420,6 +420,7 @@ The integer capabilities: >by the driver, and the driver can throw assertion failures. > * ``PIPE_CAP_PACKED_UNIFORMS``: True if the driver supports packed uniforms >as opposed to padding to vec4s. > +* ``PIPE_CAP_BUFFER_AGE``: True if the driver wants to expose > EGL/GLX_EXT_buffer_age. > > > .. _pipe_capf: > diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c > b/src/gallium/drivers/etnaviv/etnaviv_screen.c > index b0f8b4bebe..1b4276a36e 100644 > --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c > +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c > @@ -144,6 +144,7 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum > pipe_cap param) > case PIPE_CAP_TGSI_TEXCOORD: > case PIPE_CAP_VERTEX_COLOR_UNCLAMPED: > case PIPE_CAP_MIXED_COLOR_DEPTH_BITS: > + case PIPE_CAP_BUFFER_AGE: >return 1; > case PIPE_CAP_NATIVE_FENCE_FD: >return screen->drm_version >= ETNA_DRM_VERSION_FENCE_FD; > diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c > b/src/gallium/drivers/freedreno/freedreno_screen.c > index f338d756df..a7c6f4453e 100644 > --- a/src/gallium/drivers/freedreno/freedreno_screen.c > +++ b/src/gallium/drivers/freedreno/freedreno_screen.c > @@ -186,6 +186,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum > pipe_cap param) > case PIPE_CAP_MIXED_COLOR_DEPTH_BITS: > case PIPE_CAP_TEXTURE_BARRIER: > case PIPE_CAP_INVALIDATE_BUFFER: > + case PIPE_CAP_BUFFER_AGE: > return 1; > > case PIPE_CAP_VERTEXID_NOBASE: > diff --git a/src/gallium/drivers/i915/i915_screen.c > b/src/gallium/drivers/i915/i915_screen.c > index 59d2ec6628..168912946c 100644 > --- a/src/gallium/drivers/i915/i915_screen.c > +++ b/src/gallium/drivers/i915/i915_screen.c > @@ -205,6 +205,7 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap > cap) > case PIPE_CAP_VERTEX_COLOR_CLAMPED: > case PIPE_CAP_USER_VERTEX_BUFFERS: > case PIPE_CAP_MIXED_COLOR_DEPTH_BITS: > + case PIPE_CAP_BUFFER_AGE: >return 1; > > /* Unsupported features (boolean caps). */ > diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c > b/src/gallium/drivers/llvmpipe/lp_screen.c > index 3f5d0327bf..495e3f96b6 100644 > --- a/src/gallium/drivers/llvmpipe/lp_screen.c > +++ b/src/gallium/drivers/llvmpipe/lp_screen.c > @@ -110,6 +110,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum > pipe_cap param) > case PIPE_CAP_NPOT_TEXTURES: > case PIPE_CAP_MIXED_FRAMEBUFFER_SIZES: > case PIPE_CAP_MIXED_COLOR_DEPTH_BITS: > + case PIPE_CAP_BUFFER_AGE: >return 1; > case PIPE_CAP_SM3: >return 1; > diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c > b/src/gallium/drivers/nouveau/nv30/nv30_screen.c > index 1d1fbaad60..47030210b3 100644 > --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c > +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c > @@ -94,6 +94,7 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum > pipe_cap param) > case PIPE_CAP_VERTEX_ELEMENT_SRC_OFFSET_4BYTE_ALIGNED_ONLY: > case PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER: > case PIPE_CAP_ALLOW_MAPPED_BUFFERS_DURING_EXECUTION: > + case PIPE_CAP_BUFFER_AGE: >return 1; > /* nv35 capabilities */ > case PIPE_CAP_DEPTH_BOUNDS_TEST: > diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
Re: [Mesa-dev] [PATCH 01/10] glsl: don't let an 'if' then-branch kill const propagation for else-branch
Caio Marcelo de Oliveira Filho writes: > When handling 'if' in constant propagation, if a certain variable was > killed when processing the first branch of the 'if', then the second > would get any propagation from previous nodes. This is similar to the > change done for copy propagation code. > > x = 1; > if (...) { > z = x; // This would turn into z = 1. > x = 22; // x gets killed. > } else { > w = x; // This would NOT turn into w = 1. > } > > With the change, we let constant propagation happen independently in > the two branches and only then apply the killed values for the > subsequent code. > > The new code use a single hash table for keeping the kills of both > branches (the branches only write to it), and it gets deleted after we > use -- instead of waiting for mem_ctx to collect it. > > NIR deals well with constant propagation, so it already covered for > the missing ones that this patch fixes. > --- > .../glsl/opt_constant_propagation.cpp | 43 +++ > 1 file changed, 25 insertions(+), 18 deletions(-) > > diff --git a/src/compiler/glsl/opt_constant_propagation.cpp > b/src/compiler/glsl/opt_constant_propagation.cpp > index 05dc71efb72..25db3622aba 100644 > --- a/src/compiler/glsl/opt_constant_propagation.cpp > +++ b/src/compiler/glsl/opt_constant_propagation.cpp > @@ -122,7 +122,7 @@ public: > void constant_folding(ir_rvalue **rvalue); > void constant_propagation(ir_rvalue **rvalue); > void kill(ir_variable *ir, unsigned write_mask); > - void handle_if_block(exec_list *instructions); > + void handle_if_block(exec_list *instructions, hash_table *kills, bool > *killed_all); > void handle_rvalue(ir_rvalue **rvalue); > > /** List of acp_entry: The available constants to propagate */ > @@ -356,15 +356,14 @@ ir_constant_propagation_visitor::visit_enter(ir_call > *ir) > } > > void > -ir_constant_propagation_visitor::handle_if_block(exec_list *instructions) > +ir_constant_propagation_visitor::handle_if_block(exec_list *instructions, > hash_table *kills, bool *killed_all) > { > exec_list *orig_acp = this->acp; > hash_table *orig_kills = this->kills; > bool orig_killed_all = this->killed_all; > > this->acp = new(mem_ctx) exec_list; > - this->kills = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer, > - _mesa_key_pointer_equal); > + this->kills = kills; > this->killed_all = false; > > /* Populate the initial acp with a constant of the original */ > @@ -374,20 +373,10 @@ > ir_constant_propagation_visitor::handle_if_block(exec_list *instructions) > > visit_list_elements(this, instructions); > > - if (this->killed_all) { > - orig_acp->make_empty(); > - } > - > - hash_table *new_kills = this->kills; > + *killed_all = this->killed_all; > this->kills = orig_kills; > this->acp = orig_acp; > - this->killed_all = this->killed_all || orig_killed_all; > - > - hash_entry *htk; > - hash_table_foreach(new_kills, htk) { > - kill_entry *k = (kill_entry *) htk->data; > - kill(k->var, k->write_mask); > - } > + this->killed_all = orig_killed_all; > } > > ir_visitor_status > @@ -396,8 +385,26 @@ ir_constant_propagation_visitor::visit_enter(ir_if *ir) > ir->condition->accept(this); > handle_rvalue(&ir->condition); > > - handle_if_block(&ir->then_instructions); > - handle_if_block(&ir->else_instructions); > + hash_table *new_kills = _mesa_hash_table_create(mem_ctx, > _mesa_hash_pointer, > + _mesa_key_pointer_equal); > + bool then_killed_all = false; > + bool else_killed_all = false; > + > + handle_if_block(&ir->then_instructions, new_kills, &then_killed_all); > + handle_if_block(&ir->else_instructions, new_kills, &else_killed_all); Passing in the new_kills the second time and using it as the starting kills looked strange, but since nobody else will kill from that list until our block below, it seems like this patch does what it meant to. Reviewed-by: Eric Anholt Possible follow-up change for someone looking at compiler perf: kill_entry doesn't seem to need to be a list since 4654439fdd766f79a78fe0d812fd916f5815e7e6, and we could probably just store the write_mask in the entry->data field and not have struct kill_entry at all! 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 2/3] nir: Add a discard optimization pass
On Thu, Jul 5, 2018 at 11:03 AM, Francisco Jerez wrote: > Jason Ekstrand writes: > > > On Wed, Jul 4, 2018 at 1:20 PM, Francisco Jerez > > wrote: > > > >> Jason Ekstrand writes: > >> > >> > Many fragment shaders do a discard using relatively little information > >> > but still put the discard fairly far down in the shader for no good > >> > reason. If the discard is moved higher up, we can possibly avoid > doing > >> > some or almost all of the work in the shader. When this lets us skip > >> > texturing operations, it's an especially high win. > >> > > >> > One of the biggest offenders here is DXVK. The D3D APIs have > different > >> > rules for discards than OpenGL and Vulkan. One effective way (which > is > >> > what DXVK uses) to implement DX behavior on top of GL or Vulkan is to > >> > wait until the very end of the shader to discard. This ends up in the > >> > pessimal case where we always do all of the work before discarding. > >> > This pass helps some DXVK shaders significantly. > >> > > >> > >> One thing to keep in mind is that this sort of transformation is trading > >> off run-time of fragment shader invocations that don't call discard (or > >> do so non-uniformly, which means that the code the discard jump is > >> protecting will be executed anyway, so doing this can actually increase > >> the critical path of the program) in favour of invocations that call > >> discard uniformly (so executing discard early will effectively terminate > >> the program early). > > > > > > It's not really a uniform vs. non-uniform thing. Even if a shader only > > discards some of the fragments, it sill reduces the number of live > channels > > which reduces the cost of later non-uniform control-flow. > > > > Which only helps if the shader's control flow is sufficiently > non-uniform that the additional cost from performing those computations > early pays off -- Or not at all if the discarded fragments need to be > executed (non-compliantly) anyway in order to provide > derivatives_safe_after_discard. However, if the discard condition is > uniform (across a warp), the thread can be terminated early by the > back-end most certainly, which gives you the maximum pay-off. Uniform > discard conditions are therefore the best-case scenario for this > optimization pass. > Yes, that is correct. Fortunately, things that discard tend to discard fairly large chunks of the polygon at one time so this case is fairly common. > > > >> Optimizing for the latter case is an essentially > >> heuristic assumption that needs to be verified experimentally. Have you > >> tested the effect of this pass on non-DX workloads extensively? > >> > > > > Yes, it is a trade-off. No, I have not done particularly extensive > > testing. We do, however, know of non-DXVK workloads that would benefit > > from this. I believe Manhattan is one such example though I have not yet > > benchmarked it. > > > > You should grab some numbers then to make sure there are no > regressions... I'm working on that. Unfortunately the perf system is giving me trouble so I don't have the numbers yet. > But keep in mind that the i965 scheduler is already > performing a similar optimization (locally, but with cycle-count > information). This will only help over the existing optimization if the > shaders that represent a bottleneck in Manhattan have sufficient control > flow for the basic block boundaries to represent a problem to the > (local) scheduler. > I'm not sure about the manhattan shader but the Skyrim shader does have control flow which the discard has to get moved above. > > > >> > v2 (Jason Ekstrand): > >> > - Fix a couple of typos (Grazvydas, Ian) > >> > - Use the new nir_instr_move helper > >> > - Find all movable discards before moving anything so we don't > >> >accidentally re-order anything and break dependencies > >> > --- > >> > src/compiler/Makefile.sources | 1 + > >> > src/compiler/nir/meson.build | 1 + > >> > src/compiler/nir/nir.h | 10 + > >> > src/compiler/nir/nir_opt_discard.c | 396 > + > >> > 4 files changed, 408 insertions(+) > >> > create mode 100644 src/compiler/nir/nir_opt_discard.c > >> > > >> > diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile. > >> sources > >> > index 9e3fbdc2612..8600ce81281 100644 > >> > --- a/src/compiler/Makefile.sources > >> > +++ b/src/compiler/Makefile.sources > >> > @@ -271,6 +271,7 @@ NIR_FILES = \ > >> > nir/nir_opt_cse.c \ > >> > nir/nir_opt_dce.c \ > >> > nir/nir_opt_dead_cf.c \ > >> > + nir/nir_opt_discard.c \ > >> > nir/nir_opt_gcm.c \ > >> > nir/nir_opt_global_to_local.c \ > >> > nir/nir_opt_if.c \ > >> > diff --git a/src/compiler/nir/meson.build > b/src/compiler/nir/meson.build > >> > index 28aa8de7014..e339258bb94 100644 > >> > --- a/src/compiler/nir/meson.build > >> > +++ b/src/compiler/nir/meson.build > >> > @@ -156,6 +156,7 @@ files_libnir = files( > >
Re: [Mesa-dev] [PATCH 1.5/3] nir: Add a nir_instr_move helper
On Thu, Jul 5, 2018 at 12:55 PM, Caio Marcelo de Oliveira Filho < caio.olive...@intel.com> wrote: > On Wed, Jul 04, 2018 at 09:55:26AM -0700, Jason Ekstrand wrote: > > Removes an instruction from one place and inserts it at another while > > working around a weird cursor corner-case. > > Is the weird corner case the move to the same place case? > Yes > > --- a/src/compiler/nir/nir.c > > +++ b/src/compiler/nir/nir.c > > @@ -845,6 +845,21 @@ nir_instr_insert(nir_cursor cursor, nir_instr > *instr) > >nir_handle_add_jump(instr->block); > > } > > > > +void > > +nir_instr_move(nir_cursor cursor, nir_instr *instr) > > +{ > > + /* If the cursor happens to refer to this instruction (either before > or > > +* after), don't do anything. > > +*/ > > + if ((cursor.option == nir_cursor_before_instr || > > +cursor.option == nir_cursor_after_instr) && > > + cursor.instr == instr) > > + return; > > Should we also consider the block-based cursor options here? Moving > the first instruction of a block before that block > (nir_cursor_before_block), etc. > That should be fine as-is as nir_instr_insert should do the right thing. The reason the cursor.instr == instr case is a problem is because you can't insert an instruction relative to itself. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] winsys/amdgpu: remove RADEON_SURF_FMASK leftover
From: Marek Olšák RADEON_SURF_FMASK is never set. --- src/gallium/winsys/amdgpu/drm/amdgpu_surface.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_surface.c b/src/gallium/winsys/amdgpu/drm/amdgpu_surface.c index d5fa37bb6d9..eaf10349355 100644 --- a/src/gallium/winsys/amdgpu/drm/amdgpu_surface.c +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_surface.c @@ -89,26 +89,23 @@ static int amdgpu_surface_init(struct radeon_winsys *rws, config.info.color_samples = num_color_samples; config.info.levels = tex->last_level + 1; config.info.num_channels = util_format_get_nr_components(tex->format); config.is_3d = !!(tex->target == PIPE_TEXTURE_3D); config.is_cube = !!(tex->target == PIPE_TEXTURE_CUBE); /* Use different surface counters for color and FMASK, so that MSAA MRTs * always use consecutive surface indices when FMASK is allocated between * them. */ - if (flags & RADEON_SURF_FMASK) - config.info.surf_index = &ws->surf_index_fmask; - else if (!(flags & RADEON_SURF_Z_OR_SBUFFER)) - config.info.surf_index = &ws->surf_index_color; - else - config.info.surf_index = NULL; - + config.info.surf_index = &ws->surf_index_color; config.info.fmask_surf_index = &ws->surf_index_fmask; + if (flags & RADEON_SURF_Z_OR_SBUFFER) + config.info.surf_index = NULL; + return ac_compute_surface(ws->addrlib, &ws->info, &config, mode, surf); } void amdgpu_surface_init_functions(struct amdgpu_winsys *ws) { ws->base.surface_init = amdgpu_surface_init; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] ac: run LLVM optimization passes only on the final function after inlining
From: Marek Olšák --- src/amd/common/ac_llvm_helper.cpp | 6 ++ src/amd/common/ac_llvm_util.c | 7 +++ src/amd/common/ac_llvm_util.h | 1 + 3 files changed, 14 insertions(+) diff --git a/src/amd/common/ac_llvm_helper.cpp b/src/amd/common/ac_llvm_helper.cpp index 4348ebd36ee..e0943135fad 100644 --- a/src/amd/common/ac_llvm_helper.cpp +++ b/src/amd/common/ac_llvm_helper.cpp @@ -29,20 +29,21 @@ #pragma push_macro("DEBUG") #undef DEBUG #include "ac_binary.h" #include "ac_llvm_util.h" #include #include #include #include +#include #include #if HAVE_LLVM < 0x0700 #include "llvm/Support/raw_ostream.h" #endif void ac_add_attr_dereferenceable(LLVMValueRef val, uint64_t bytes) { llvm::Argument *A = llvm::unwrap(val); A->addAttr(llvm::Attribute::getWithDereferenceableBytes(A->getContext(), bytes)); @@ -158,10 +159,15 @@ bool ac_compile_module_to_binary(struct ac_compiler_passes *p, LLVMModuleRef mod p->passmgr.run(*llvm::unwrap(module)); llvm::StringRef data = p->ostream.str(); bool success = ac_elf_read(data.data(), data.size(), binary); p->code_string = ""; /* release the ELF shader binary */ if (!success) fprintf(stderr, "amd: cannot read an ELF shader binary\n"); return success; } + +void ac_llvm_add_barrier_noop_pass(LLVMPassManagerRef passmgr) +{ + llvm::unwrap(passmgr)->add(llvm::createBarrierNoopPass()); +} diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c index 5e04452d5a8..9e7a106afa5 100644 --- a/src/amd/common/ac_llvm_util.c +++ b/src/amd/common/ac_llvm_util.c @@ -172,20 +172,27 @@ static LLVMPassManagerRef ac_create_passmgr(LLVMTargetLibraryInfoRef target_libr if (!passmgr) return NULL; if (target_library_info) LLVMAddTargetLibraryInfo(target_library_info, passmgr); if (check_ir) LLVMAddVerifierPass(passmgr); LLVMAddAlwaysInlinerPass(passmgr); + /* Normally, the pass manager runs all passes on one function before +* moving onto another. Adding a barrier no-op pass forces the pass +* manager to run the inliner on all functions first, which makes sure +* that the following passes are only run on the remaining non-inline +* function. +*/ + ac_llvm_add_barrier_noop_pass(passmgr); /* This pass should eliminate all the load and store instructions. */ LLVMAddPromoteMemoryToRegisterPass(passmgr); LLVMAddScalarReplAggregatesPass(passmgr); LLVMAddLICMPass(passmgr); LLVMAddAggressiveDCEPass(passmgr); LLVMAddCFGSimplificationPass(passmgr); /* This is recommended by the instruction combining pass. */ LLVMAddEarlyCSEMemSSAPass(passmgr); LLVMAddInstructionCombiningPass(passmgr); return passmgr; diff --git a/src/amd/common/ac_llvm_util.h b/src/amd/common/ac_llvm_util.h index 373fd8d28db..e5b93037d26 100644 --- a/src/amd/common/ac_llvm_util.h +++ b/src/amd/common/ac_llvm_util.h @@ -126,16 +126,17 @@ void ac_init_llvm_once(void); bool ac_init_llvm_compiler(struct ac_llvm_compiler *compiler, bool okay_to_leak_target_library_info, enum radeon_family family, enum ac_target_machine_options tm_options); void ac_destroy_llvm_compiler(struct ac_llvm_compiler *compiler); struct ac_compiler_passes *ac_create_llvm_passes(LLVMTargetMachineRef tm); void ac_destroy_llvm_passes(struct ac_compiler_passes *p); bool ac_compile_module_to_binary(struct ac_compiler_passes *p, LLVMModuleRef module, struct ac_shader_binary *binary); +void ac_llvm_add_barrier_noop_pass(LLVMPassManagerRef passmgr); #ifdef __cplusplus } #endif #endif /* AC_LLVM_UTIL_H */ -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/10] util/set: helper to remove entry by key
Caio Marcelo de Oliveira Filho writes: > --- This one should be trivial to unit test, with a present and non-present key. 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 05/10] util/set: add a clone function
Caio Marcelo de Oliveira Filho writes: > --- Could you add a little unit test with it? Maybe make a table with a couple entries and a deleted entry, and verify that the clone has both entries and not the deleted one? I tend to try to create the API in both hash and set at the same time. If I get around to putting your changes up in the source project, I will. 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 1.5/3] nir: Add a nir_instr_move helper
On Wed, Jul 04, 2018 at 09:55:26AM -0700, Jason Ekstrand wrote: > Removes an instruction from one place and inserts it at another while > working around a weird cursor corner-case. Is the weird corner case the move to the same place case? > --- a/src/compiler/nir/nir.c > +++ b/src/compiler/nir/nir.c > @@ -845,6 +845,21 @@ nir_instr_insert(nir_cursor cursor, nir_instr *instr) >nir_handle_add_jump(instr->block); > } > > +void > +nir_instr_move(nir_cursor cursor, nir_instr *instr) > +{ > + /* If the cursor happens to refer to this instruction (either before or > +* after), don't do anything. > +*/ > + if ((cursor.option == nir_cursor_before_instr || > +cursor.option == nir_cursor_after_instr) && > + cursor.instr == instr) > + return; Should we also consider the block-based cursor options here? Moving the first instruction of a block before that block (nir_cursor_before_block), etc. Thanks, Caio ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: Apply fragment color clamping to gl_FragData[] as well.
Rob Clark writes: > On Thu, Jul 5, 2018 at 12:49 PM, Eric Anholt wrote: >> From the ARB_color_buffer_float spec: >> >>35. Should the clamping of fragment shader output gl_FragData[n] >>be controlled by the fragment color clamp. >> >>RESOLVED: Since the destination of the FragData is a color >>buffer, the fragment color clamp control should apply. >> >> Fixes arb_color_buffer_float-mrt mixed on v3d. > > Reviewed-by: Rob Clark > > *possibly* over thinking this, the comment in the gl_frag_result enum > implies that FRAG_RESULT_DATAn come at the end of the enum (ie. if > there was somehow ever a new value added it should come before > FRAG_RESULT_DATA0).. maybe that comment should be less subtle to avoid > accidentally breaking the logic below? I think that's overthinking it. We do these kinds of comparisons with VARYING_SLOT_VAR too and I don't recall anyone messing it up. 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] st/nir: Disable varying packing when doing transform feedback.
Timothy Arceri writes: > On 03/07/18 05:51, Eric Anholt wrote: > >> Eric Anholt writes: >> >>> [ Unknown signature status ] >>> Timothy Arceri writes: >>> nir_compact_varyings() is meant to skip over varyings used by xfb: /* We can't repack xfb varyings. */ if (var->data.always_active_io) continue; Any idea why that isn't working in this case? >>> Looks like GLSL IR has that flag wrong. points.7 has v_var6,7,8,9 >>> transform feedback output, but the IR says: >> [...] >> >> Any feedback on this? This is my remaining TF issue for V3D. > Hmm. I think this still gets messed up because if packing the non-xfb > varyings results in freeing up a location then st_nir_assign_var_locations() > will end up assigning a new location for the xfb varyings. > > I think your patch to disable for xfb is the easiest way to work around > this for now. I still hope we might one day have a full NIR GLSL linker > to replace GLSL IR but my hopes of ever seeing one are slowly fading. > > Anyway for this patch: > > Reviewed-by: Timothy Arceri Thanks! I do wish for better linking as well, but state_tracker's program handling is so convoluted that I can only ever hold a fraction of it in my head at a time. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx)
Fixes 14 piglits, mostly in egl_khr_create_context. Fixes: https://github.com/anholt/libepoxy/issues/177 Signed-off-by: Adam Jackson --- src/mesa/drivers/dri/swrast/swrast.c | 34 +++- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/mesa/drivers/dri/swrast/swrast.c b/src/mesa/drivers/dri/swrast/swrast.c index ae5874f5927..7f08107c24f 100644 --- a/src/mesa/drivers/dri/swrast/swrast.c +++ b/src/mesa/drivers/dri/swrast/swrast.c @@ -675,6 +675,9 @@ swrast_check_and_update_window_size( struct gl_context *ctx, struct gl_framebuff { GLsizei width, height; +if (!fb) +return; + get_window_size(fb, &width, &height); if (fb->Width != width || fb->Height != height) { _mesa_resize_framebuffer(ctx, fb, width, height); @@ -857,30 +860,29 @@ dri_make_current(__DRIcontext * cPriv, __DRIdrawable * driReadPriv) { struct gl_context *mesaCtx; -struct gl_framebuffer *mesaDraw; -struct gl_framebuffer *mesaRead; +struct gl_framebuffer *mesaDraw = NULL; +struct gl_framebuffer *mesaRead = NULL; TRACE; if (cPriv) { - struct dri_context *ctx = dri_context(cPriv); struct dri_drawable *draw; struct dri_drawable *read; - if (!driDrawPriv || !driReadPriv) - return GL_FALSE; +mesaCtx = &dri_context(cPriv)->Base; - draw = dri_drawable(driDrawPriv); - read = dri_drawable(driReadPriv); - mesaCtx = &ctx->Base; - mesaDraw = &draw->Base; - mesaRead = &read->Base; + if (driDrawPriv && driReadPriv) { + draw = dri_drawable(driDrawPriv); + read = dri_drawable(driReadPriv); + mesaDraw = &draw->Base; + mesaRead = &read->Base; - /* check for same context and buffer */ - if (mesaCtx == _mesa_get_current_context() - && mesaCtx->DrawBuffer == mesaDraw - && mesaCtx->ReadBuffer == mesaRead) { - return GL_TRUE; - } + /* check for same context and buffer */ + if (mesaCtx == _mesa_get_current_context() + && mesaCtx->DrawBuffer == mesaDraw + && mesaCtx->ReadBuffer == mesaRead) { + return GL_TRUE; + } +} _glapi_check_multithread(); -- 2.17.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/26] python: Use the print function
On Thu, 2018-07-05 at 08:40 -0700, Dylan Baker wrote: > This is a really big patch that should be mostly mechanical, It's mostly me running `2to3 --fix=print` on all those Python scripts, and adding the `from __future__ import print_function` so that it's compatible with Python 2. In a few rare instances I had to manually fix some things (e.g 2to3 had used `end=' '` and I had to replace it by `end=''`) so that the generated code would be identical to the one generated on master. -- Mathieu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: don't check ccs_e support if isl_format is ISL_FORMAT_UNSUPPORTED
'ISL_FORMAT_UNSUPPORTED' shouldn't be passed down for evaluation as it is strictly prohibited in isl code (e.g. format_info_exists). Signed-off-by: Dongwon Kim --- src/mesa/drivers/dri/i965/intel_screen.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index cb357419a7..a65042da72 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -346,8 +346,16 @@ modifier_is_supported(const struct gen_device_info *devinfo, */ format = _mesa_format_fallback_rgbx_to_rgba(format); format = _mesa_get_srgb_format_linear(format); - if (!isl_format_supports_ccs_e(devinfo, - brw_isl_format_for_mesa_format(format))) + + enum isl_format isl_format; + isl_format = brw_isl_format_for_mesa_format(format); + + /* whether there is supported ISL format for given mesa format */ + if (isl_format == ISL_FORMAT_UNSUPPORTED) + return false; + + /* check if isl_fomat supports ccs_e */ + if (!isl_format_supports_ccs_e(devinfo, isl_format)) return false; } -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: Only enable depth writes if the function isn't EQUAL.
Reviewed-by: Marek Olšák Marek On Thu, Jul 5, 2018 at 6:00 AM, Kenneth Graunke wrote: > If the depth function is EQUAL, then we'll only write the depth value > when it already matches what's in the buffer, which is pointless. > Skipping these writes can save bandwidth. > > The state tracker can easily take care of this, so all drivers benefit. > --- > src/mesa/state_tracker/st_atom_depth.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Not well tested, sorry. > > diff --git a/src/mesa/state_tracker/st_atom_depth.c > b/src/mesa/state_tracker/st_atom_depth.c > index 6ddb8f525cf..9e12361f881 100644 > --- a/src/mesa/state_tracker/st_atom_depth.c > +++ b/src/mesa/state_tracker/st_atom_depth.c > @@ -107,8 +107,9 @@ st_update_depth_stencil_alpha(struct st_context *st) > if (ctx->DrawBuffer->Visual.depthBits > 0) { >if (ctx->Depth.Test) { > dsa->depth.enabled = 1; > - dsa->depth.writemask = ctx->Depth.Mask; > dsa->depth.func = st_compare_func_to_pipe(ctx->Depth.Func); > + if (dsa->depth.func != PIPE_FUNC_EQUAL) > +dsa->depth.writemask = ctx->Depth.Mask; >} >if (ctx->Depth.BoundsTest) { > dsa->depth.bounds_test = 1; > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/3] nir: Add a discard optimization pass
Jason Ekstrand writes: > On Wed, Jul 4, 2018 at 1:20 PM, Francisco Jerez > wrote: > >> Jason Ekstrand writes: >> >> > Many fragment shaders do a discard using relatively little information >> > but still put the discard fairly far down in the shader for no good >> > reason. If the discard is moved higher up, we can possibly avoid doing >> > some or almost all of the work in the shader. When this lets us skip >> > texturing operations, it's an especially high win. >> > >> > One of the biggest offenders here is DXVK. The D3D APIs have different >> > rules for discards than OpenGL and Vulkan. One effective way (which is >> > what DXVK uses) to implement DX behavior on top of GL or Vulkan is to >> > wait until the very end of the shader to discard. This ends up in the >> > pessimal case where we always do all of the work before discarding. >> > This pass helps some DXVK shaders significantly. >> > >> >> One thing to keep in mind is that this sort of transformation is trading >> off run-time of fragment shader invocations that don't call discard (or >> do so non-uniformly, which means that the code the discard jump is >> protecting will be executed anyway, so doing this can actually increase >> the critical path of the program) in favour of invocations that call >> discard uniformly (so executing discard early will effectively terminate >> the program early). > > > It's not really a uniform vs. non-uniform thing. Even if a shader only > discards some of the fragments, it sill reduces the number of live channels > which reduces the cost of later non-uniform control-flow. > Which only helps if the shader's control flow is sufficiently non-uniform that the additional cost from performing those computations early pays off -- Or not at all if the discarded fragments need to be executed (non-compliantly) anyway in order to provide derivatives_safe_after_discard. However, if the discard condition is uniform (across a warp), the thread can be terminated early by the back-end most certainly, which gives you the maximum pay-off. Uniform discard conditions are therefore the best-case scenario for this optimization pass. > >> Optimizing for the latter case is an essentially >> heuristic assumption that needs to be verified experimentally. Have you >> tested the effect of this pass on non-DX workloads extensively? >> > > Yes, it is a trade-off. No, I have not done particularly extensive > testing. We do, however, know of non-DXVK workloads that would benefit > from this. I believe Manhattan is one such example though I have not yet > benchmarked it. > You should grab some numbers then to make sure there are no regressions... But keep in mind that the i965 scheduler is already performing a similar optimization (locally, but with cycle-count information). This will only help over the existing optimization if the shaders that represent a bottleneck in Manhattan have sufficient control flow for the basic block boundaries to represent a problem to the (local) scheduler. > >> > v2 (Jason Ekstrand): >> > - Fix a couple of typos (Grazvydas, Ian) >> > - Use the new nir_instr_move helper >> > - Find all movable discards before moving anything so we don't >> >accidentally re-order anything and break dependencies >> > --- >> > src/compiler/Makefile.sources | 1 + >> > src/compiler/nir/meson.build | 1 + >> > src/compiler/nir/nir.h | 10 + >> > src/compiler/nir/nir_opt_discard.c | 396 + >> > 4 files changed, 408 insertions(+) >> > create mode 100644 src/compiler/nir/nir_opt_discard.c >> > >> > diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile. >> sources >> > index 9e3fbdc2612..8600ce81281 100644 >> > --- a/src/compiler/Makefile.sources >> > +++ b/src/compiler/Makefile.sources >> > @@ -271,6 +271,7 @@ NIR_FILES = \ >> > nir/nir_opt_cse.c \ >> > nir/nir_opt_dce.c \ >> > nir/nir_opt_dead_cf.c \ >> > + nir/nir_opt_discard.c \ >> > nir/nir_opt_gcm.c \ >> > nir/nir_opt_global_to_local.c \ >> > nir/nir_opt_if.c \ >> > diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build >> > index 28aa8de7014..e339258bb94 100644 >> > --- a/src/compiler/nir/meson.build >> > +++ b/src/compiler/nir/meson.build >> > @@ -156,6 +156,7 @@ files_libnir = files( >> >'nir_opt_cse.c', >> >'nir_opt_dce.c', >> >'nir_opt_dead_cf.c', >> > + 'nir_opt_discard.c', >> >'nir_opt_gcm.c', >> >'nir_opt_global_to_local.c', >> >'nir_opt_if.c', >> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h >> > index c40a88c8ccc..dac019c17e8 100644 >> > --- a/src/compiler/nir/nir.h >> > +++ b/src/compiler/nir/nir.h >> > @@ -2022,6 +2022,13 @@ typedef struct nir_shader_compiler_options { >> > */ >> > bool vs_inputs_dual_locations; >> > >> > + /** >> > +* Whether or not derivatives are still a safe operation after a >> discard >> > +* has occurred. Optimization passes may be ab
Re: [Mesa-dev] [PATCH 16/26] python: Explicitly use lists
CC'ing Ian who wrote this code, just to make sure that changing the order is going to be OK. Quoting Dylan Baker (2018-07-05 09:31:30) > Quoting Mathieu Bridon (2018-07-05 06:17:47) > > On Python 2, the builtin functions filter() and zip() would return > > lists. > > > > On Python 3, they return iterators. > > > > Since we want to use those objects in contexts where we need lists, we > > need to explicitly turn them into lists. > > > > This makes the code compatible with both Python 2 and Python 3. > > > > Signed-off-by: Mathieu Bridon > > --- > > src/compiler/nir/nir_opt_algebraic.py | 2 +- > > src/mesa/main/get_hash_generator.py | 4 ++-- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/src/compiler/nir/nir_opt_algebraic.py > > b/src/compiler/nir/nir_opt_algebraic.py > > index 5e07d662b0..7b2ba56990 100644 > > --- a/src/compiler/nir/nir_opt_algebraic.py > > +++ b/src/compiler/nir/nir_opt_algebraic.py > > @@ -633,7 +633,7 @@ optimizations = [ > > > > invert = OrderedDict([('feq', 'fne'), ('fne', 'feq'), ('fge', 'flt'), > > ('flt', 'fge')]) > > > > -for left, right in list(itertools.combinations(invert.keys(), 2)) + > > zip(invert.keys(), invert.keys()): > > +for left, right in list(itertools.combinations(invert.keys(), 2)) + > > list(zip(invert.keys(), invert.keys())): > > Isn't this just a really expenisve re-implementation of: > itertools.combinations_with_replacement(invert.keys(), 2) > > There's really no reason to make this concrete either, so lets not. > > This will change the order of the output slightly, but I think that's okay. > > > optimizations.append((('inot', ('ior(is_used_once)', (left, a, b), > > (right, c, d))), > > ('iand', (invert[left], a, b), (invert[right], c, > > d > > optimizations.append((('inot', ('iand(is_used_once)', (left, a, b), > > (right, c, d))), > > diff --git a/src/mesa/main/get_hash_generator.py > > b/src/mesa/main/get_hash_generator.py > > index facdccd8a5..37dae45e0b 100644 > > --- a/src/mesa/main/get_hash_generator.py > > +++ b/src/mesa/main/get_hash_generator.py > > @@ -117,8 +117,8 @@ def print_tables(tables): > > def merge_tables(tables): > > merged_tables = [] > > for api, indices in sorted(tables.items()): > > - matching_table = filter(lambda mt:mt["indices"] == indices, > > - merged_tables) > > + matching_table = list(filter(lambda mt:mt["indices"] == indices, > > + merged_tables)) > > how about: > matching_table = [m for m in merged_tables if m["indicies" == indicies"] > since that works in both, is more common python, and avoids the extra function > call and the lambda. > > >if matching_table: > > matching_table[0]["apis"].append(api) > >else: > > -- > > 2.17.1 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev 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] vma/tests: Fix compilation if limits.h defines PAGE_SIZE
On 05/07/2018 16:13, Eric Engestrom wrote: On Thursday, 2018-07-05 15:52:01 +0100, Jon Turney wrote: per POSIX, limits.h may define PAGE_SIZE when the value is not indeterminate "may define" -> don't you need #ifdef around #undef? Annoyingly, I though exactly this, and then forgot :( Patch superseded, in any case. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] vma/tests: Fix compilation if limits.h defines PAGE_SIZE (v2)
per POSIX, limits.h may define PAGE_SIZE when the value is not indeterminate v2: just change the variable name, since there's no intended correlation here between this value and the machine's actual page size. Cc: Scott D Phillips Signed-off-by: Jon Turney --- src/util/tests/vma/vma_random_test.cpp | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/util/tests/vma/vma_random_test.cpp b/src/util/tests/vma/vma_random_test.cpp index de887fead3..1f194fcdf9 100644 --- a/src/util/tests/vma/vma_random_test.cpp +++ b/src/util/tests/vma/vma_random_test.cpp @@ -40,7 +40,7 @@ namespace { -static const uint64_t PAGE_SIZE = 4096; +static const uint64_t MEM_PAGE_SIZE = 4096; struct allocation { uint64_t start_page; @@ -62,12 +62,12 @@ constexpr uint64_t allocation_end_page(const allocation& a) { struct random_test { static const uint64_t MEM_START_PAGE = 1; static const uint64_t MEM_SIZE = 0xf000; - static const uint64_t MEM_PAGES = MEM_SIZE / PAGE_SIZE; + static const uint64_t MEM_PAGES = MEM_SIZE / MEM_PAGE_SIZE; random_test(uint_fast32_t seed) : heap_holes{allocation{MEM_START_PAGE, MEM_PAGES}}, rand{seed} { - util_vma_heap_init(&heap, MEM_START_PAGE * PAGE_SIZE, MEM_SIZE); + util_vma_heap_init(&heap, MEM_START_PAGE * MEM_PAGE_SIZE, MEM_SIZE); } void test(unsigned long count) @@ -89,12 +89,12 @@ struct random_test { if (align_order > 51) align_order = std::min(dist(rand), 51); uint64_t align_pages = 1ULL << align_order; - uint64_t align = align_pages * PAGE_SIZE; + uint64_t align = align_pages * MEM_PAGE_SIZE; if (size_order > 51) size_order = std::min(dist(rand), 51); uint64_t size_pages = 1ULL << size_order; - uint64_t size = size_pages * PAGE_SIZE; + uint64_t size = size_pages * MEM_PAGE_SIZE; uint64_t addr = util_vma_heap_alloc(&heap, size, align); @@ -110,7 +110,7 @@ struct random_test { return false; } else { assert(addr % align == 0); - uint64_t addr_page = addr / PAGE_SIZE; + uint64_t addr_page = addr / MEM_PAGE_SIZE; allocation a{addr_page, size_pages}; auto i = heap_holes.find(a); assert(i != end(heap_holes)); @@ -146,8 +146,8 @@ struct random_test { allocation a = allocations.back(); allocations.pop_back(); - util_vma_heap_free(&heap, a.start_page * PAGE_SIZE, - a.num_pages * PAGE_SIZE); + util_vma_heap_free(&heap, a.start_page * MEM_PAGE_SIZE, + a.num_pages * MEM_PAGE_SIZE); assert(heap_holes.find(a) == end(heap_holes)); auto next = heap_holes.upper_bound(a); -- 2.17.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radv: fix emitting the view index on GFX9
Reviewed-by: Bas Nieuwenhuizen On Thu, Jul 5, 2018 at 6:56 PM, Samuel Pitoiset wrote: > For merged shaders, VS as HS for example. > > Signed-off-by: Samuel Pitoiset > Cc: > --- > src/amd/vulkan/radv_cmd_buffer.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/amd/vulkan/radv_cmd_buffer.c > b/src/amd/vulkan/radv_cmd_buffer.c > index b7519dce49..1ea023a811 100644 > --- a/src/amd/vulkan/radv_cmd_buffer.c > +++ b/src/amd/vulkan/radv_cmd_buffer.c > @@ -3079,8 +3079,9 @@ static void radv_emit_view_index(struct radv_cmd_buffer > *cmd_buffer, unsigned in > { > struct radv_pipeline *pipeline = cmd_buffer->state.pipeline; > for (unsigned stage = 0; stage < MESA_SHADER_STAGES; ++stage) { > - if (!pipeline->shaders[stage]) > + if (!radv_get_shader(pipeline, stage)) > continue; > + > struct radv_userdata_info *loc = > radv_lookup_user_sgpr(pipeline, stage, AC_UD_VIEW_INDEX); > if (loc->sgpr_idx == -1) > continue; > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 106644] [llvmpipe] Mesa 18.1.2 fails lp_test_format, lp_test_arit, lp_test_blend, lp_test_printf, lp_test_conv tests
https://bugs.freedesktop.org/show_bug.cgi?id=106644 --- Comment #24 from erhar...@mailbox.org --- Hope this output is helpful to you! I am certainly no expert in this area, but could the problem be the 970 trying to execute vsx instructions? llc -mattr option(s): +altivec,+vsx llc -mcpu option: 970 -- 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 106644] [llvmpipe] Mesa 18.1.2 fails lp_test_format, lp_test_arit, lp_test_blend, lp_test_printf, lp_test_conv tests
https://bugs.freedesktop.org/show_bug.cgi?id=106644 --- Comment #23 from erhar...@mailbox.org --- Created attachment 140477 --> https://bugs.freedesktop.org/attachment.cgi?id=140477&action=edit output from lp_test_* (ppc) -- 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 17/26] python: Better check for string types
Quoting Eric Engestrom (2018-07-05 09:55:10) > On Thursday, 2018-07-05 09:33:24 -0700, Dylan Baker wrote: > > I've done enough python 2 -> 3 porting to feel very nervous about this, my > > experience tells me that mixing bytes and unicode always leads to subtle and > > hard to track down bugs. I'd much rather enforce that we're always getting > > unicode or bytes, but not mixing them. > > Agreed, but the mix was already there, this patch doesn't change it. The problem is that python 2 tries to be helpful and coerces bytes and unicode for you. Python 3 does the right thing and dies in a fire when you try to do operations one both types. > I think it should be cleaned up at some point, but for now this patch is > ok with me, although I'd like to replace the exception with a version > check like you suggested in 18/26; with that: > Reviewed-by: Eric Engestrom > > > > > Quoting Mathieu Bridon (2018-07-05 06:17:48) > > > Python 2 byte strings were called "str", and its unicode strings were > > > called "unicode". > > > > > > In Python 3, they are called "bytes" and "str". > > > > > > This commit makes the script compatible with Python 2 and Python 3, > > > checking for the right types on both. > > > > > > Signed-off-by: Mathieu Bridon > > > --- > > > src/compiler/nir/nir_algebraic.py | 9 - > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/compiler/nir/nir_algebraic.py > > > b/src/compiler/nir/nir_algebraic.py > > > index fda72d3c69..e17e2d26b9 100644 > > > --- a/src/compiler/nir/nir_algebraic.py > > > +++ b/src/compiler/nir/nir_algebraic.py > > > @@ -35,6 +35,13 @@ import traceback > > > > > > from nir_opcodes import opcodes > > > > > > +try: > > > +string_types = (str, unicode) > > > + > > > +except NameError: > > > +# This is Python 3 > > > +string_types = (bytes, str) > > > + > > > _type_re = re.compile(r"(?Pint|uint|bool|float)?(?P\d+)?") > > > > > > def type_bits(type_str): > > > @@ -70,7 +77,7 @@ class Value(object): > > > return Expression(val, name_base, varset) > > >elif isinstance(val, Expression): > > > return val > > > - elif isinstance(val, (str, unicode)): > > > + elif isinstance(val, string_types): > > > return Variable(val, name_base, varset) > > >elif isinstance(val, (bool, int, long, float)): > > > return Constant(val, name_base) > > > -- > > > 2.17.1 > > > > > > ___ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600: Add spill output to group only if register or target index changes
The current spill code checks in each instruction of an instruction group whether spliing is needed and if so, it adds spilling for each component as a seperate instruction and it allocates a new temporary for each component and since it takes the write mask from the TGSI representation, all components might be written each time and as a result already written components might be overwritten with garbage like: ... y: MOVR9.y, [0x4214 37].x t: MOVR8.x, [0x4204 33].y ... MEM_SCRATCH WRITE_IND_ACK 0 R9.xy__, @R4.x ES:3 MEM_SCRATCH WRITE_IND_ACK 0 R8.xy__, @R4.x ES:3 ... To resolve this isse accululate spills to the same memory location so that only one memory write instruction is emmited for an instruction group that writes up to all four components. Signed-off-by: Gert Wollny --- src/gallium/drivers/r600/r600_shader.c | 93 +- 1 file changed, 57 insertions(+), 36 deletions(-) diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c index 5479861c58..354091322a 100644 --- a/src/gallium/drivers/r600/r600_shader.c +++ b/src/gallium/drivers/r600/r600_shader.c @@ -4342,8 +4342,32 @@ static void tgsi_dst(struct r600_shader_ctx *ctx, if (spilled) { struct r600_bytecode_output cf; - int reg = r600_get_temp(ctx); + int reg = 0; int r; + bool add_pending_output = true; + + memset(&cf, 0, sizeof(struct r600_bytecode_output)); + get_spilled_array_base_and_size(ctx, tgsi_dst->Register.Index, + &cf.array_base, &cf.array_size); + + /* If no componet has spilled, reserve a register and add the spill code + * ctx->bc->n_pending_outputs is cleared after each instruction group */ + if (ctx->bc->n_pending_outputs == 0) { + reg = r600_get_temp(ctx); + } else { + /* If we already spilling and the output address is the same like +* before then just reuse the same slot */ + struct r600_bytecode_output *tmpl = &ctx->bc->pending_outputs[ctx->bc->n_pending_outputs-1]; + if ((cf.array_base + idx == tmpl->array_base) || + (cf.array_base == tmpl->array_base && + tmpl->index_gpr == ctx->bc->ar_reg && + tgsi_dst->Register.Indirect)) { + reg = ctx->bc->pending_outputs[0].gpr; + add_pending_output = false; + } else { + reg = r600_get_temp(ctx); + } + } r600_dst->sel = reg; r600_dst->chan = swizzle; @@ -4352,42 +4376,39 @@ static void tgsi_dst(struct r600_shader_ctx *ctx, r600_dst->clamp = 1; } - // needs to be added after op using tgsi_dst - memset(&cf, 0, sizeof(struct r600_bytecode_output)); - cf.op = CF_OP_MEM_SCRATCH; - cf.elem_size = 3; - cf.gpr = reg; - cf.type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_WRITE; - cf.mark = 1; - cf.comp_mask = inst->Dst[0].Register.WriteMask; - cf.swizzle_x = 0; - cf.swizzle_y = 1; - cf.swizzle_z = 2; - cf.swizzle_w = 3; - cf.burst_count = 1; - - get_spilled_array_base_and_size(ctx, tgsi_dst->Register.Index, - &cf.array_base, &cf.array_size); - - if (tgsi_dst->Register.Indirect) { - if (ctx->bc->chip_class < R700) - cf.type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_WRITE_IND; - else - cf.type = 3; // V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_WRITE_IND_ACK; - cf.index_gpr = ctx->bc->ar_reg; - } - else { - cf.array_base += idx; - cf.array_size = 0; + /* Add new outputs as pending */ + if (add_pending_output) { + cf.op = CF_OP_MEM_SCRATCH; + cf.elem_size = 3; + cf.gpr = reg; + cf.type = V
Re: [Mesa-dev] [PATCH 26/26] meson: Build with Python 3
On Thursday, 2018-07-05 15:17:57 +0200, Mathieu Bridon wrote: > Now that all the build scripts are compatible with both Python 2 and 3, > we can flip the switch and tell Meson to use the latter. > > Since Meson already depends on Python 3 anyway, this means we don't need > two different Python stacks to build Mesa. > > Signed-off-by: Mathieu Bridon Looking forward to the day we land this patch :P Reviewed-by: Eric Engestrom > --- > meson.build | 6 ++--- > src/amd/common/meson.build| 2 +- > src/amd/vulkan/meson.build| 10 +++ > src/broadcom/cle/meson.build | 4 +-- > src/compiler/glsl/meson.build | 4 +-- > src/compiler/meson.build | 2 +- > src/compiler/nir/meson.build | 14 +- > src/compiler/spirv/meson.build| 4 +-- > src/egl/meson.build | 4 +-- > src/gallium/auxiliary/meson.build | 6 ++--- > src/gallium/drivers/freedreno/meson.build | 2 +- > src/gallium/drivers/r600/meson.build | 2 +- > src/gallium/drivers/radeonsi/meson.build | 2 +- > .../swr/rasterizer/codegen/meson.build| 8 +++--- > .../swr/rasterizer/core/backends/meson.build | 4 +-- > .../drivers/swr/rasterizer/jitter/meson.build | 6 ++--- > src/intel/compiler/meson.build| 2 +- > src/intel/genxml/meson.build | 6 ++--- > src/intel/isl/meson.build | 2 +- > src/intel/vulkan/meson.build | 10 +++ > src/mapi/es1api/meson.build | 2 +- > src/mapi/es2api/meson.build | 2 +- > src/mapi/glapi/gen/meson.build| 26 +-- > src/mapi/shared-glapi/meson.build | 2 +- > src/mesa/drivers/dri/i965/meson.build | 2 +- > src/mesa/main/meson.build | 6 ++--- > src/mesa/meson.build | 6 ++--- > src/meson.build | 2 +- > src/util/meson.build | 2 +- > src/util/xmlpool/meson.build | 2 +- > src/vulkan/util/meson.build | 2 +- > 31 files changed, 77 insertions(+), 77 deletions(-) > > diff --git a/meson.build b/meson.build > index b2722c71e5..1b41373793 100644 > --- a/meson.build > +++ b/meson.build > @@ -693,10 +693,10 @@ if with_platform_haiku >pre_args += '-DHAVE_HAIKU_PLATFORM' > endif > > -prog_python2 = find_program('python2') > +prog_python = find_program('python3') > -has_mako = run_command(prog_python2, '-c', 'import mako') > +has_mako = run_command(prog_python, '-c', 'import mako') > if has_mako.returncode() != 0 > - error('Python (2.x) mako module required to build mesa.') > + error('Python (3.x) mako module required to build mesa.') > endif > > if cc.get_id() == 'gcc' and cc.version().version_compare('< 4.4.6') > diff --git a/src/amd/common/meson.build b/src/amd/common/meson.build > index 0967b1adb7..6827a02094 100644 > --- a/src/amd/common/meson.build > +++ b/src/amd/common/meson.build > @@ -22,7 +22,7 @@ sid_tables_h = custom_target( >'sid_tables_h', >input : ['sid_tables.py', 'sid.h', 'gfx9d.h'], >output : 'sid_tables.h', > - command : [prog_python2, '@INPUT@'], > + command : [prog_python, '@INPUT@'], >capture : true, > ) > > diff --git a/src/amd/vulkan/meson.build b/src/amd/vulkan/meson.build > index 22857926fa..c5c9b308c6 100644 > --- a/src/amd/vulkan/meson.build > +++ b/src/amd/vulkan/meson.build > @@ -23,7 +23,7 @@ radv_entrypoints = custom_target( >input : ['radv_entrypoints_gen.py', vk_api_xml], >output : ['radv_entrypoints.h', 'radv_entrypoints.c'], >command : [ > -prog_python2, '@INPUT0@', '--xml', '@INPUT1@', '--outdir', > +prog_python, '@INPUT0@', '--xml', '@INPUT1@', '--outdir', > meson.current_build_dir() >], >depend_files : files('radv_extensions.py'), > @@ -34,7 +34,7 @@ radv_extensions_c = custom_target( >input : ['radv_extensions.py', vk_api_xml], >output : ['radv_extensions.c', 'radv_extensions.h'], >command : [ > -prog_python2, '@INPUT0@', '--xml', '@INPUT1@', '--out-c', '@OUTPUT0@', > +prog_python, '@INPUT0@', '--xml', '@INPUT1@', '--out-c', '@OUTPUT0@', > '--out-h', '@OUTPUT1@' >], > ) > @@ -43,7 +43,7 @@ vk_format_table_c = custom_target( >'vk_format_table.c', >input : ['vk_format_table.py', 'vk_format_layout.csv'], >output : 'vk_format_table.c', > - command : [prog_python2, '@INPUT@'], > + command : [prog_python, '@INPUT@'], >depend_files : files('vk_format_parse.py'), >capture : true, > ) > @@ -151,7 +151,7 @@ radeon_icd = custom_target( >input : 'radv_icd.py', >output : 'radeon_icd.@0@.json'.format(host_machine.cpu()), >command : [ > -prog_python2, '@INPUT@', > +prog_python, '@INPUT@', > '--lib-path', join_paths(get_opti
[Mesa-dev] [Bug 106958] Mass Effect Andromeda renders correctly on RX480 POLARIS but BAD ON RX VEGA 64 on wine 3.10 stagingf with DXVK
https://bugs.freedesktop.org/show_bug.cgi?id=106958 --- Comment #9 from Samuel Pitoiset --- Can you try this patch https://patchwork.freedesktop.org/series/46024/ please? I'm not really not sure if that will fix your issue (I still don't have a renderdoc capture). -- 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 22/26] python: Use open(), not file()
On Thursday, 2018-07-05 15:17:53 +0200, Mathieu Bridon wrote: > The latter is a constructor for file objects, but when actually opening > a file, using the former is more idiomatic. > > In addition, file() is not a builtin any more in Python 3, so this makes > the script compatible with both Python 2 and Python 3. > > Signed-off-by: Mathieu Bridon Reviewed-by: Eric Engestrom > --- > src/util/xmlpool/gen_xmlpool.py | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/util/xmlpool/gen_xmlpool.py b/src/util/xmlpool/gen_xmlpool.py > index 886c1854f3..b0db183854 100644 > --- a/src/util/xmlpool/gen_xmlpool.py > +++ b/src/util/xmlpool/gen_xmlpool.py > @@ -168,7 +168,7 @@ > print("/***\ > > # Process the options template and generate options.h with all > # translations. > -template = file (template_header_path, "r") > +template = open (template_header_path, "r") > descMatches = [] > for line in template: > if len(descMatches) > 0: > @@ -199,6 +199,8 @@ for line in template: > else: > print(line, end='') > > +template.close() > + > if len(descMatches) > 0: > sys.stderr.write ("Warning: unterminated description at end of file.\n") > expandMatches (descMatches, translations) > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 21/26] python: Use key-functions when sorting containers
On Thursday, 2018-07-05 09:38:19 -0700, Dylan Baker wrote: > Quoting Mathieu Bridon (2018-07-05 06:17:52) > > In Python 2, the traditional way to sort containers was to use a > > comparison function (which returned either -1, 0 or 1 when passed two > > objects) and pass that as the "cmp" argument to the container's sort() > > method. > > > > Python 2.4 introduced key-functions, which instead only operate on a > > given item, and return a sorting key for this item. > > > > In general, this runs faster, because the cmp-function has to get run > > multiple times for each item of the container. > > > > Python 3 removed the cmp-function, enforcing usage of key-functions > > instead. > > > > This change makes the script compatible with Python 2 and Python 3. > > > > Signed-off-by: Mathieu Bridon > > --- > > src/mapi/mapi_abi.py | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/src/mapi/mapi_abi.py b/src/mapi/mapi_abi.py > > index 67fdb10650..19fdc4572a 100644 > > --- a/src/mapi/mapi_abi.py > > +++ b/src/mapi/mapi_abi.py > > @@ -32,6 +32,7 @@ import os > > GLAPI = os.path.join(".", os.path.dirname(sys.argv[0]), "glapi/gen") > > sys.path.append(GLAPI) > > > > +from operator import attrgetter > > import re > > from optparse import OptionParser > > import gl_XML > > @@ -305,7 +306,7 @@ class ABIPrinter(object): > > > > # sort entries by their names > > self.entries_sorted_by_names = self.entries[:] > > -self.entries_sorted_by_names.sort(lambda x, y: cmp(x.name, y.name)) > > +self.entries_sorted_by_names.sort(key=attrgetter('name')) > > how about: > self.entries_sorted_by_names = list(sorted(self.entries, > key=attrgetter('name'))) > > To avoid the copy and in-place sort. Good idea; with that: Reviewed-by: Eric Engestrom > > > > > self.indent = ' ' * 3 > > self.noop_warn = 'noop_warn' > > @@ -455,7 +456,7 @@ class ABIPrinter(object): > > """Return the string pool for use by stubs.""" > > # sort entries by their names > > sorted_entries = self.entries[:] > > -sorted_entries.sort(lambda x, y: cmp(x.name, y.name)) > > +sorted_entries.sort(key=attrgetter('name')) > > > > pool = [] > > offsets = {} > > -- > > 2.17.1 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2] Check if the window is non-NULL before setting swap interval.
On 5 July 2018 at 17:17, Eric Engestrom wrote: > On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote: >> On 5 July 2018 at 10:53, Eric Engestrom wrote: >> > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote: >> >> This fixes crash due to NULL window when swap interval is set >> >> for pbuffer surface. >> >> >> >> Test: CtsDisplayTestCases pass >> >> >> >> Signed-off-by: samiuddi >> >> --- >> >> >> >> Kindly ignore this patch >> >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html >> >> >> >> src/egl/drivers/dri2/platform_android.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/src/egl/drivers/dri2/platform_android.c >> >> b/src/egl/drivers/dri2/platform_android.c >> >> index ca8708a..b5b960a 100644 >> >> --- a/src/egl/drivers/dri2/platform_android.c >> >> +++ b/src/egl/drivers/dri2/platform_android.c >> >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy, >> >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); >> >> struct ANativeWindow *window = dri2_surf->window; >> >> >> >> - if (window->setSwapInterval(window, interval)) >> >> + if (window && window->setSwapInterval(window, interval)) >> >>return EGL_FALSE; >> > >> > Shouldn't we return false if we couldn't set the swap interval? >> > >> > I think this should be: >> >if (!window || window->setSwapInterval(window, interval)) >> > return EGL_FALSE; >> > >> Changing the patch as above will lead to eglSwapInterval consistently >> failing for pbuffer surfaces. > > I'm not that familiar with pbuffers, but does SwapInterval really make > sense for them? I thought you couldn't swap a pbuffer. > > If so, failing an invalid op seems like the right thing to do. > Otherwise, I guess sure, no-op returning success is fine. > Looking at eglSwapInterval manpage [1] (with my annotation): "eglSwapInterval — specifies the minimum number of video frame periods per buffer swap for the _window_ associated with the current context." While the spec does not mention window/pbuffer/pixmap at all - it extensively refers to eglSwapBuffers. Wrt eglSwapBuffers manpage [2] and spec are consistent: "If surface is a pixel buffer or a pixmap, eglSwapBuffers has no effect, and no error is generated." Perhaps it's wrong to set eglSwapInterval for !window surfaces, but its sibling (eglSwapBuffers) does not error out. Hence I doubt many users make a distinction between window and pbuffer surfaces for eglSwap*. Worth bringing to the WG meeting - it' planned for 1st August. -Emil [1] https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglSwapInterval.xhtml [2] https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglSwapBuffers.xhtml ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 19/26] python: Don't abuse hex()
On Thursday, 2018-07-05 15:17:50 +0200, Mathieu Bridon wrote: > The hex() builtin returns a string containing the hexa-decimal > representation of an integer. > > When the argument is not an integer, then the function calls that > object's __hex__() method, if one is defined. That method is supposed to > return a string. > > While that's not explicitly documented, that string is supposed to be a > valid hexa-decimal representation for a number. Python 2 doesn't enforce > this though, which is why we got away with returning things like > 'NIR_TRUE' which are not numbers. > > In Python 3, the hex() builtin instead calls an object's __index__() > method, which itself must return an integer. That integer is then > automatically converted to a string with its hexa-decimal representation > by the rest of the hex() function. > > As a result, we really can't make this compatible with Python 3 as it > is. > > The solution is to stop using the hex() builtin, and instead use a hex() > object method, which can return whatever we want, in Python 2 and 3. > > Signed-off-by: Mathieu Bridon Reviewed-by: Eric Engestrom > --- > src/compiler/nir/nir_algebraic.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index 63a7cb5ad1..d53a9869de 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -88,7 +88,7 @@ class Value(object): > static const ${val.c_type} ${val.name} = { > { ${val.type_enum}, ${val.bit_size} }, > % if isinstance(val, Constant): > - ${val.type()}, { ${hex(val)} /* ${val.value} */ }, > + ${val.type()}, { ${val.hex()} /* ${val.value} */ }, > % elif isinstance(val, Variable): > ${val.index}, /* ${val.var_name} */ > ${'true' if val.is_constant else 'false'}, > @@ -142,7 +142,7 @@ class Constant(Value): > assert self.bit_size == 0 or self.bit_size == 32 > self.bit_size = 32 > > - def __hex__(self): > + def hex(self): >if isinstance(self.value, (bool)): > return 'NIR_TRUE' if self.value else 'NIR_FALSE' >if isinstance(self.value, integer_types): > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 20/26] python: Open file in binary mode
On Thursday, 2018-07-05 15:17:51 +0200, Mathieu Bridon wrote: > The XML parser wants byte strings, not unicode strings. > > In both Python 2 and 3, opening a file without specifying the mode will > open it for reading in text mode ('r'). > > On Python 2, the read() method of the file object will return byte > strings, while on Python 3 it will return unicode strings. > > Explicitly specifying the binary mode ('rb') makes the behaviour > identical in both Python 2 and 3, returning what the XML parser > expects. > > Signed-off-by: Mathieu Bridon Reviewed-by: Eric Engestrom > --- > src/intel/genxml/gen_bits_header.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/intel/genxml/gen_bits_header.py > b/src/intel/genxml/gen_bits_header.py > index e31e9ff103..dcd6ccb7d9 100644 > --- a/src/intel/genxml/gen_bits_header.py > +++ b/src/intel/genxml/gen_bits_header.py > @@ -282,7 +282,7 @@ class XmlParser(object): > self.container = None > > def parse(self, filename): > -with open(filename) as f: > +with open(filename, 'rb') as f: > self.parser.ParseFile(f) > > def start_element(self, name, attrs): > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: Apply fragment color clamping to gl_FragData[] as well.
On Thu, Jul 5, 2018 at 12:49 PM, Eric Anholt wrote: > From the ARB_color_buffer_float spec: > >35. Should the clamping of fragment shader output gl_FragData[n] >be controlled by the fragment color clamp. > >RESOLVED: Since the destination of the FragData is a color >buffer, the fragment color clamp control should apply. > > Fixes arb_color_buffer_float-mrt mixed on v3d. Reviewed-by: Rob Clark *possibly* over thinking this, the comment in the gl_frag_result enum implies that FRAG_RESULT_DATAn come at the end of the enum (ie. if there was somehow ever a new value added it should come before FRAG_RESULT_DATA0).. maybe that comment should be less subtle to avoid accidentally breaking the logic below? BR, -R > --- > src/compiler/nir/nir_lower_clamp_color_outputs.c | 9 ++--- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/src/compiler/nir/nir_lower_clamp_color_outputs.c > b/src/compiler/nir/nir_lower_clamp_color_outputs.c > index b76a4d51aaca..32f855624276 100644 > --- a/src/compiler/nir/nir_lower_clamp_color_outputs.c > +++ b/src/compiler/nir/nir_lower_clamp_color_outputs.c > @@ -47,13 +47,8 @@ is_color_output(lower_state *state, nir_variable *out) >} >break; > case MESA_SHADER_FRAGMENT: > - switch (out->data.location) { > - case FRAG_RESULT_COLOR: > - return true; > - default: > - return false; > - } > - break; > + return (out->data.location == FRAG_RESULT_COLOR || > + out->data.location >= FRAG_RESULT_DATA0); > default: >return false; > } > -- > 2.18.0 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 17/26] python: Better check for string types
On Thursday, 2018-07-05 09:33:24 -0700, Dylan Baker wrote: > I've done enough python 2 -> 3 porting to feel very nervous about this, my > experience tells me that mixing bytes and unicode always leads to subtle and > hard to track down bugs. I'd much rather enforce that we're always getting > unicode or bytes, but not mixing them. Agreed, but the mix was already there, this patch doesn't change it. I think it should be cleaned up at some point, but for now this patch is ok with me, although I'd like to replace the exception with a version check like you suggested in 18/26; with that: Reviewed-by: Eric Engestrom > > Quoting Mathieu Bridon (2018-07-05 06:17:48) > > Python 2 byte strings were called "str", and its unicode strings were > > called "unicode". > > > > In Python 3, they are called "bytes" and "str". > > > > This commit makes the script compatible with Python 2 and Python 3, > > checking for the right types on both. > > > > Signed-off-by: Mathieu Bridon > > --- > > src/compiler/nir/nir_algebraic.py | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/src/compiler/nir/nir_algebraic.py > > b/src/compiler/nir/nir_algebraic.py > > index fda72d3c69..e17e2d26b9 100644 > > --- a/src/compiler/nir/nir_algebraic.py > > +++ b/src/compiler/nir/nir_algebraic.py > > @@ -35,6 +35,13 @@ import traceback > > > > from nir_opcodes import opcodes > > > > +try: > > +string_types = (str, unicode) > > + > > +except NameError: > > +# This is Python 3 > > +string_types = (bytes, str) > > + > > _type_re = re.compile(r"(?Pint|uint|bool|float)?(?P\d+)?") > > > > def type_bits(type_str): > > @@ -70,7 +77,7 @@ class Value(object): > > return Expression(val, name_base, varset) > >elif isinstance(val, Expression): > > return val > > - elif isinstance(val, (str, unicode)): > > + elif isinstance(val, string_types): > > return Variable(val, name_base, varset) > >elif isinstance(val, (bool, int, long, float)): > > return Constant(val, name_base) > > -- > > 2.17.1 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radv: fix emitting the view index on GFX9
For merged shaders, VS as HS for example. Signed-off-by: Samuel Pitoiset Cc: --- src/amd/vulkan/radv_cmd_buffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c index b7519dce49..1ea023a811 100644 --- a/src/amd/vulkan/radv_cmd_buffer.c +++ b/src/amd/vulkan/radv_cmd_buffer.c @@ -3079,8 +3079,9 @@ static void radv_emit_view_index(struct radv_cmd_buffer *cmd_buffer, unsigned in { struct radv_pipeline *pipeline = cmd_buffer->state.pipeline; for (unsigned stage = 0; stage < MESA_SHADER_STAGES; ++stage) { - if (!pipeline->shaders[stage]) + if (!radv_get_shader(pipeline, stage)) continue; + struct radv_userdata_info *loc = radv_lookup_user_sgpr(pipeline, stage, AC_UD_VIEW_INDEX); if (loc->sgpr_idx == -1) continue; -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 18/26] python: Better check for integer types
On Thursday, 2018-07-05 09:34:54 -0700, Dylan Baker wrote: > Quoting Mathieu Bridon (2018-07-05 06:17:49) > > Python 3 lost the long type: now everything is an int, with the right > > size. > > > > This commit makes the script compatible with Python 2 (where we check > > for both int and long) and Python 3 (where we only check for int). > > > > Signed-off-by: Mathieu Bridon > > --- > > src/compiler/nir/nir_algebraic.py | 8 +--- > > src/gallium/auxiliary/util/u_format_pack.py | 12 ++-- > > 2 files changed, 15 insertions(+), 5 deletions(-) > > > > diff --git a/src/compiler/nir/nir_algebraic.py > > b/src/compiler/nir/nir_algebraic.py > > index e17e2d26b9..63a7cb5ad1 100644 > > --- a/src/compiler/nir/nir_algebraic.py > > +++ b/src/compiler/nir/nir_algebraic.py > > @@ -36,10 +36,12 @@ import traceback > > from nir_opcodes import opcodes > > > > how about > import sys > > if sys.version < (3, 0): > ... > else: > ... > > Since we expect the exception to be hit at least 50% of the time. Agreed; with that: Reviewed-by: Eric Engestrom > > with that, > Reviewed-by: Dylan Baker > > > try: > > +integer_types = (int, long) > > string_types = (str, unicode) > > > > except NameError: > > # This is Python 3 > > +integer_types = (int, ) > > string_types = (bytes, str) > > > > _type_re = re.compile(r"(?Pint|uint|bool|float)?(?P\d+)?") > > @@ -79,7 +81,7 @@ class Value(object): > > return val > >elif isinstance(val, string_types): > > return Variable(val, name_base, varset) > > - elif isinstance(val, (bool, int, long, float)): > > + elif isinstance(val, (bool, float) + integer_types): > > return Constant(val, name_base) > > > > __template = mako.template.Template(""" > > @@ -143,7 +145,7 @@ class Constant(Value): > > def __hex__(self): > >if isinstance(self.value, (bool)): > > return 'NIR_TRUE' if self.value else 'NIR_FALSE' > > - if isinstance(self.value, (int, long)): > > + if isinstance(self.value, integer_types): > > return hex(self.value) > >elif isinstance(self.value, float): > > return hex(struct.unpack('Q', struct.pack('d', self.value))[0]) > > @@ -153,7 +155,7 @@ class Constant(Value): > > def type(self): > >if isinstance(self.value, (bool)): > > return "nir_type_bool32" > > - elif isinstance(self.value, (int, long)): > > + elif isinstance(self.value, integer_types): > > return "nir_type_int" > >elif isinstance(self.value, float): > > return "nir_type_float" > > diff --git a/src/gallium/auxiliary/util/u_format_pack.py > > b/src/gallium/auxiliary/util/u_format_pack.py > > index 1cfd85fb7f..c753336a84 100644 > > --- a/src/gallium/auxiliary/util/u_format_pack.py > > +++ b/src/gallium/auxiliary/util/u_format_pack.py > > @@ -41,6 +41,14 @@ from __future__ import print_function > > from u_format_parse import * > > > > > > +try: > > +integer_types = (int, long) > > + > > +except NameError: > > +# This is Python 3 > > +integer_types = (int, ) > > + > > + > > def inv_swizzles(swizzles): > > '''Return an array[4] of inverse swizzle terms''' > > '''Only pick the first matching value to avoid l8 getting blue and i8 > > getting alpha''' > > @@ -212,7 +220,7 @@ def truncate_mantissa(x, bits): > > '''Truncate an integer so it can be represented exactly with a floating > > point mantissa''' > > > > -assert isinstance(x, (int, long)) > > +assert isinstance(x, integer_types) > > > > s = 1 > > if x < 0: > > @@ -236,7 +244,7 @@ def value_to_native(type, value): > > '''Get the value of unity for this type.''' > > if type.type == FLOAT: > > if type.size <= 32 \ > > -and isinstance(value, (int, long)): > > +and isinstance(value, integer_types): > > return truncate_mantissa(value, 23) > > return value > > if type.type == FIXED: > > -- > > 2.17.1 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir: Apply fragment color clamping to gl_FragData[] as well.
From the ARB_color_buffer_float spec: 35. Should the clamping of fragment shader output gl_FragData[n] be controlled by the fragment color clamp. RESOLVED: Since the destination of the FragData is a color buffer, the fragment color clamp control should apply. Fixes arb_color_buffer_float-mrt mixed on v3d. --- src/compiler/nir/nir_lower_clamp_color_outputs.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/compiler/nir/nir_lower_clamp_color_outputs.c b/src/compiler/nir/nir_lower_clamp_color_outputs.c index b76a4d51aaca..32f855624276 100644 --- a/src/compiler/nir/nir_lower_clamp_color_outputs.c +++ b/src/compiler/nir/nir_lower_clamp_color_outputs.c @@ -47,13 +47,8 @@ is_color_output(lower_state *state, nir_variable *out) } break; case MESA_SHADER_FRAGMENT: - switch (out->data.location) { - case FRAG_RESULT_COLOR: - return true; - default: - return false; - } - break; + return (out->data.location == FRAG_RESULT_COLOR || + out->data.location >= FRAG_RESULT_DATA0); default: return false; } -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 25/26] python: Explicitly add the 'L' suffix on Python 3
Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:56) > Python 2 had two integer types: int and long. Python 3 dropped the > latter, as it made the int type automatically support bigger numbers. > > As a result, Python 3 lost the 'L' suffix on integer litterals. > > This probably doesn't make much difference when compiling the generated > C code, but adding it explicitly means that both Python 2 and 3 generate > the exact same C code anyway, which makes it easier to compare and check > for discrepencies when moving to Python 3. > > Signed-off-by: Mathieu Bridon > --- > src/compiler/nir/nir_algebraic.py | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index d53a9869de..2081c29034 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -148,7 +148,16 @@ class Constant(Value): >if isinstance(self.value, integer_types): > return hex(self.value) >elif isinstance(self.value, float): > - return hex(struct.unpack('Q', struct.pack('d', self.value))[0]) > + i = struct.unpack('Q', struct.pack('d', self.value))[0] > + h = hex(i) > + > + # On Python 2 this 'L' suffix is automatically added, but not on > Python 3 > + # Adding it explicitly makes the generated file identical, > regardless > + # of the Python version running this script. > + if h[-1] != 'L' and i > sys.maxsize: > +h += 'L' > + > + return h >else: > assert False > > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 24/26] python: Use the unicode_escape codec
Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:55) > Python 2 had string_escape and unicode_escape codecs. Python 3 only has > the latter. These work the same as far as we're concerned, so let's use > the future-proof one. > > However, the reste of the code expects unicode strings, so we need to > decode them again. > > Signed-off-by: Mathieu Bridon > --- > src/amd/common/sid_tables.py | 2 +- > src/gallium/drivers/r600/egd_tables.py | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/amd/common/sid_tables.py b/src/amd/common/sid_tables.py > index 421c2a1335..7b5e626e3e 100644 > --- a/src/amd/common/sid_tables.py > +++ b/src/amd/common/sid_tables.py > @@ -65,7 +65,7 @@ class StringTable: > """ > fragments = [ > '"%s\\0" /* %s */' % ( > -te[0].encode('string_escape'), > +te[0].encode('unicode_escape').decode(), > ', '.join(str(idx) for idx in sorted(te[2])) > ) > for te in self.table > diff --git a/src/gallium/drivers/r600/egd_tables.py > b/src/gallium/drivers/r600/egd_tables.py > index 7489649ec7..8a60a6229a 100644 > --- a/src/gallium/drivers/r600/egd_tables.py > +++ b/src/gallium/drivers/r600/egd_tables.py > @@ -61,7 +61,7 @@ class StringTable: > """ > fragments = [ > '"%s\\0" /* %s */' % ( > -te[0].encode('string_escape'), > +te[0].encode('unicode_escape').decode(), > ', '.join(str(idx) for idx in te[2]) > ) > for te in self.table > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 22/26] python: Use open(), not file()
Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:53) > The latter is a constructor for file objects, but when actually opening > a file, using the former is more idiomatic. > > In addition, file() is not a builtin any more in Python 3, so this makes > the script compatible with both Python 2 and Python 3. > > Signed-off-by: Mathieu Bridon > --- > src/util/xmlpool/gen_xmlpool.py | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/util/xmlpool/gen_xmlpool.py b/src/util/xmlpool/gen_xmlpool.py > index 886c1854f3..b0db183854 100644 > --- a/src/util/xmlpool/gen_xmlpool.py > +++ b/src/util/xmlpool/gen_xmlpool.py > @@ -168,7 +168,7 @@ > print("/***\ > > # Process the options template and generate options.h with all > # translations. > -template = file (template_header_path, "r") > +template = open (template_header_path, "r") > descMatches = [] > for line in template: > if len(descMatches) > 0: > @@ -199,6 +199,8 @@ for line in template: > else: > print(line, end='') > > +template.close() > + > if len(descMatches) > 0: > sys.stderr.write ("Warning: unterminated description at end of file.\n") > expandMatches (descMatches, translations) > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 21/26] python: Use key-functions when sorting containers
Quoting Mathieu Bridon (2018-07-05 06:17:52) > In Python 2, the traditional way to sort containers was to use a > comparison function (which returned either -1, 0 or 1 when passed two > objects) and pass that as the "cmp" argument to the container's sort() > method. > > Python 2.4 introduced key-functions, which instead only operate on a > given item, and return a sorting key for this item. > > In general, this runs faster, because the cmp-function has to get run > multiple times for each item of the container. > > Python 3 removed the cmp-function, enforcing usage of key-functions > instead. > > This change makes the script compatible with Python 2 and Python 3. > > Signed-off-by: Mathieu Bridon > --- > src/mapi/mapi_abi.py | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/mapi/mapi_abi.py b/src/mapi/mapi_abi.py > index 67fdb10650..19fdc4572a 100644 > --- a/src/mapi/mapi_abi.py > +++ b/src/mapi/mapi_abi.py > @@ -32,6 +32,7 @@ import os > GLAPI = os.path.join(".", os.path.dirname(sys.argv[0]), "glapi/gen") > sys.path.append(GLAPI) > > +from operator import attrgetter > import re > from optparse import OptionParser > import gl_XML > @@ -305,7 +306,7 @@ class ABIPrinter(object): > > # sort entries by their names > self.entries_sorted_by_names = self.entries[:] > -self.entries_sorted_by_names.sort(lambda x, y: cmp(x.name, y.name)) > +self.entries_sorted_by_names.sort(key=attrgetter('name')) how about: self.entries_sorted_by_names = list(sorted(self.entries, key=attrgetter('name'))) To avoid the copy and in-place sort. > > self.indent = ' ' * 3 > self.noop_warn = 'noop_warn' > @@ -455,7 +456,7 @@ class ABIPrinter(object): > """Return the string pool for use by stubs.""" > # sort entries by their names > sorted_entries = self.entries[:] > -sorted_entries.sort(lambda x, y: cmp(x.name, y.name)) > +sorted_entries.sort(key=attrgetter('name')) > > pool = [] > offsets = {} > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 19/26] python: Don't abuse hex()
This is nice change in itself, Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:50) > The hex() builtin returns a string containing the hexa-decimal > representation of an integer. > > When the argument is not an integer, then the function calls that > object's __hex__() method, if one is defined. That method is supposed to > return a string. > > While that's not explicitly documented, that string is supposed to be a > valid hexa-decimal representation for a number. Python 2 doesn't enforce > this though, which is why we got away with returning things like > 'NIR_TRUE' which are not numbers. > > In Python 3, the hex() builtin instead calls an object's __index__() > method, which itself must return an integer. That integer is then > automatically converted to a string with its hexa-decimal representation > by the rest of the hex() function. > > As a result, we really can't make this compatible with Python 3 as it > is. > > The solution is to stop using the hex() builtin, and instead use a hex() > object method, which can return whatever we want, in Python 2 and 3. > > Signed-off-by: Mathieu Bridon > --- > src/compiler/nir/nir_algebraic.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index 63a7cb5ad1..d53a9869de 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -88,7 +88,7 @@ class Value(object): > static const ${val.c_type} ${val.name} = { > { ${val.type_enum}, ${val.bit_size} }, > % if isinstance(val, Constant): > - ${val.type()}, { ${hex(val)} /* ${val.value} */ }, > + ${val.type()}, { ${val.hex()} /* ${val.value} */ }, > % elif isinstance(val, Variable): > ${val.index}, /* ${val.var_name} */ > ${'true' if val.is_constant else 'false'}, > @@ -142,7 +142,7 @@ class Constant(Value): > assert self.bit_size == 0 or self.bit_size == 32 > self.bit_size = 32 > > - def __hex__(self): > + def hex(self): >if isinstance(self.value, (bool)): > return 'NIR_TRUE' if self.value else 'NIR_FALSE' >if isinstance(self.value, integer_types): > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 20/26] python: Open file in binary mode
Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:51) > The XML parser wants byte strings, not unicode strings. > > In both Python 2 and 3, opening a file without specifying the mode will > open it for reading in text mode ('r'). > > On Python 2, the read() method of the file object will return byte > strings, while on Python 3 it will return unicode strings. > > Explicitly specifying the binary mode ('rb') makes the behaviour > identical in both Python 2 and 3, returning what the XML parser > expects. > > Signed-off-by: Mathieu Bridon > --- > src/intel/genxml/gen_bits_header.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/intel/genxml/gen_bits_header.py > b/src/intel/genxml/gen_bits_header.py > index e31e9ff103..dcd6ccb7d9 100644 > --- a/src/intel/genxml/gen_bits_header.py > +++ b/src/intel/genxml/gen_bits_header.py > @@ -282,7 +282,7 @@ class XmlParser(object): > self.container = None > > def parse(self, filename): > -with open(filename) as f: > +with open(filename, 'rb') as f: > self.parser.ParseFile(f) > > def start_element(self, name, attrs): > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 18/26] python: Better check for integer types
Quoting Mathieu Bridon (2018-07-05 06:17:49) > Python 3 lost the long type: now everything is an int, with the right > size. > > This commit makes the script compatible with Python 2 (where we check > for both int and long) and Python 3 (where we only check for int). > > Signed-off-by: Mathieu Bridon > --- > src/compiler/nir/nir_algebraic.py | 8 +--- > src/gallium/auxiliary/util/u_format_pack.py | 12 ++-- > 2 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index e17e2d26b9..63a7cb5ad1 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -36,10 +36,12 @@ import traceback > from nir_opcodes import opcodes > how about import sys if sys.version < (3, 0): ... else: ... Since we expect the exception to be hit at least 50% of the time. with that, Reviewed-by: Dylan Baker > try: > +integer_types = (int, long) > string_types = (str, unicode) > > except NameError: > # This is Python 3 > +integer_types = (int, ) > string_types = (bytes, str) > > _type_re = re.compile(r"(?Pint|uint|bool|float)?(?P\d+)?") > @@ -79,7 +81,7 @@ class Value(object): > return val >elif isinstance(val, string_types): > return Variable(val, name_base, varset) > - elif isinstance(val, (bool, int, long, float)): > + elif isinstance(val, (bool, float) + integer_types): > return Constant(val, name_base) > > __template = mako.template.Template(""" > @@ -143,7 +145,7 @@ class Constant(Value): > def __hex__(self): >if isinstance(self.value, (bool)): > return 'NIR_TRUE' if self.value else 'NIR_FALSE' > - if isinstance(self.value, (int, long)): > + if isinstance(self.value, integer_types): > return hex(self.value) >elif isinstance(self.value, float): > return hex(struct.unpack('Q', struct.pack('d', self.value))[0]) > @@ -153,7 +155,7 @@ class Constant(Value): > def type(self): >if isinstance(self.value, (bool)): > return "nir_type_bool32" > - elif isinstance(self.value, (int, long)): > + elif isinstance(self.value, integer_types): > return "nir_type_int" >elif isinstance(self.value, float): > return "nir_type_float" > diff --git a/src/gallium/auxiliary/util/u_format_pack.py > b/src/gallium/auxiliary/util/u_format_pack.py > index 1cfd85fb7f..c753336a84 100644 > --- a/src/gallium/auxiliary/util/u_format_pack.py > +++ b/src/gallium/auxiliary/util/u_format_pack.py > @@ -41,6 +41,14 @@ from __future__ import print_function > from u_format_parse import * > > > +try: > +integer_types = (int, long) > + > +except NameError: > +# This is Python 3 > +integer_types = (int, ) > + > + > def inv_swizzles(swizzles): > '''Return an array[4] of inverse swizzle terms''' > '''Only pick the first matching value to avoid l8 getting blue and i8 > getting alpha''' > @@ -212,7 +220,7 @@ def truncate_mantissa(x, bits): > '''Truncate an integer so it can be represented exactly with a floating > point mantissa''' > > -assert isinstance(x, (int, long)) > +assert isinstance(x, integer_types) > > s = 1 > if x < 0: > @@ -236,7 +244,7 @@ def value_to_native(type, value): > '''Get the value of unity for this type.''' > if type.type == FLOAT: > if type.size <= 32 \ > -and isinstance(value, (int, long)): > +and isinstance(value, integer_types): > return truncate_mantissa(value, 23) > return value > if type.type == FIXED: > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 17/26] python: Better check for string types
I've done enough python 2 -> 3 porting to feel very nervous about this, my experience tells me that mixing bytes and unicode always leads to subtle and hard to track down bugs. I'd much rather enforce that we're always getting unicode or bytes, but not mixing them. Quoting Mathieu Bridon (2018-07-05 06:17:48) > Python 2 byte strings were called "str", and its unicode strings were > called "unicode". > > In Python 3, they are called "bytes" and "str". > > This commit makes the script compatible with Python 2 and Python 3, > checking for the right types on both. > > Signed-off-by: Mathieu Bridon > --- > src/compiler/nir/nir_algebraic.py | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index fda72d3c69..e17e2d26b9 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -35,6 +35,13 @@ import traceback > > from nir_opcodes import opcodes > > +try: > +string_types = (str, unicode) > + > +except NameError: > +# This is Python 3 > +string_types = (bytes, str) > + > _type_re = re.compile(r"(?Pint|uint|bool|float)?(?P\d+)?") > > def type_bits(type_str): > @@ -70,7 +77,7 @@ class Value(object): > return Expression(val, name_base, varset) >elif isinstance(val, Expression): > return val > - elif isinstance(val, (str, unicode)): > + elif isinstance(val, string_types): > return Variable(val, name_base, varset) >elif isinstance(val, (bool, int, long, float)): > return Constant(val, name_base) > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 16/26] python: Explicitly use lists
Quoting Mathieu Bridon (2018-07-05 06:17:47) > On Python 2, the builtin functions filter() and zip() would return > lists. > > On Python 3, they return iterators. > > Since we want to use those objects in contexts where we need lists, we > need to explicitly turn them into lists. > > This makes the code compatible with both Python 2 and Python 3. > > Signed-off-by: Mathieu Bridon > --- > src/compiler/nir/nir_opt_algebraic.py | 2 +- > src/mesa/main/get_hash_generator.py | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/compiler/nir/nir_opt_algebraic.py > b/src/compiler/nir/nir_opt_algebraic.py > index 5e07d662b0..7b2ba56990 100644 > --- a/src/compiler/nir/nir_opt_algebraic.py > +++ b/src/compiler/nir/nir_opt_algebraic.py > @@ -633,7 +633,7 @@ optimizations = [ > > invert = OrderedDict([('feq', 'fne'), ('fne', 'feq'), ('fge', 'flt'), > ('flt', 'fge')]) > > -for left, right in list(itertools.combinations(invert.keys(), 2)) + > zip(invert.keys(), invert.keys()): > +for left, right in list(itertools.combinations(invert.keys(), 2)) + > list(zip(invert.keys(), invert.keys())): Isn't this just a really expenisve re-implementation of: itertools.combinations_with_replacement(invert.keys(), 2) There's really no reason to make this concrete either, so lets not. This will change the order of the output slightly, but I think that's okay. > optimizations.append((('inot', ('ior(is_used_once)', (left, a, b), > (right, c, d))), > ('iand', (invert[left], a, b), (invert[right], c, > d > optimizations.append((('inot', ('iand(is_used_once)', (left, a, b), > (right, c, d))), > diff --git a/src/mesa/main/get_hash_generator.py > b/src/mesa/main/get_hash_generator.py > index facdccd8a5..37dae45e0b 100644 > --- a/src/mesa/main/get_hash_generator.py > +++ b/src/mesa/main/get_hash_generator.py > @@ -117,8 +117,8 @@ def print_tables(tables): > def merge_tables(tables): > merged_tables = [] > for api, indices in sorted(tables.items()): > - matching_table = filter(lambda mt:mt["indices"] == indices, > - merged_tables) > + matching_table = list(filter(lambda mt:mt["indices"] == indices, > + merged_tables)) how about: matching_table = [m for m in merged_tables if m["indicies" == indicies"] since that works in both, is more common python, and avoids the extra function call and the lambda. >if matching_table: > matching_table[0]["apis"].append(api) >else: > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2] Check if the window is non-NULL before setting swap interval.
On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote: > On 5 July 2018 at 10:53, Eric Engestrom wrote: > > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote: > >> This fixes crash due to NULL window when swap interval is set > >> for pbuffer surface. > >> > >> Test: CtsDisplayTestCases pass > >> > >> Signed-off-by: samiuddi > >> --- > >> > >> Kindly ignore this patch > >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html > >> > >> src/egl/drivers/dri2/platform_android.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/src/egl/drivers/dri2/platform_android.c > >> b/src/egl/drivers/dri2/platform_android.c > >> index ca8708a..b5b960a 100644 > >> --- a/src/egl/drivers/dri2/platform_android.c > >> +++ b/src/egl/drivers/dri2/platform_android.c > >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy, > >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); > >> struct ANativeWindow *window = dri2_surf->window; > >> > >> - if (window->setSwapInterval(window, interval)) > >> + if (window && window->setSwapInterval(window, interval)) > >>return EGL_FALSE; > > > > Shouldn't we return false if we couldn't set the swap interval? > > > > I think this should be: > >if (!window || window->setSwapInterval(window, interval)) > > return EGL_FALSE; > > > Changing the patch as above will lead to eglSwapInterval consistently > failing for pbuffer surfaces. I'm not that familiar with pbuffers, but does SwapInterval really make sense for them? I thought you couldn't swap a pbuffer. If so, failing an invalid op seems like the right thing to do. Otherwise, I guess sure, no-op returning success is fine. > Ideally Android will promote SwapInterval from a window to a display > decision. Or app instance at least. > > Until then I think we can live with eglSwapInterval being a no-op in said > case. > > -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 15/26] python: Specify the template output encoding
Does it make more sense to encode, or to use io.open and open the file in text mode? I've gone back and forth on this myself several times. Quoting Mathieu Bridon (2018-07-05 06:17:46) > We're trying to write a unicode string (i.e decoded) to a file opened > in binary (i.e encoded) mode. > > In Python 2 this works, because of the automatic conversion between > byte and unicode strings. > > In Python 3 this fails though, as no automatic conversion is attempted. > > This change makes the scripts compatible with both versions of Python. > > Signed-off-by: Mathieu Bridon > --- > src/compiler/nir/nir_intrinsics_c.py | 2 +- > src/compiler/nir/nir_intrinsics_h.py | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/compiler/nir/nir_intrinsics_c.py > b/src/compiler/nir/nir_intrinsics_c.py > index 98af67c38a..ac45b94d49 100644 > --- a/src/compiler/nir/nir_intrinsics_c.py > +++ b/src/compiler/nir/nir_intrinsics_c.py > @@ -64,7 +64,7 @@ def main(): > > path = os.path.join(args.outdir, 'nir_intrinsics.c') > with open(path, 'wb') as f: > -f.write(Template(template).render(INTR_OPCODES=INTR_OPCODES)) > +f.write(Template(template, > output_encoding='utf-8').render(INTR_OPCODES=INTR_OPCODES)) > > if __name__ == '__main__': > main() > diff --git a/src/compiler/nir/nir_intrinsics_h.py > b/src/compiler/nir/nir_intrinsics_h.py > index 8a4f0d501e..8abc6a8626 100644 > --- a/src/compiler/nir/nir_intrinsics_h.py > +++ b/src/compiler/nir/nir_intrinsics_h.py > @@ -53,7 +53,7 @@ def main(): > > path = os.path.join(args.outdir, 'nir_intrinsics.h') > with open(path, 'wb') as f: > -f.write(Template(template).render(INTR_OPCODES=INTR_OPCODES)) > +f.write(Template(template, > output_encoding='utf-8').render(INTR_OPCODES=INTR_OPCODES)) > > if __name__ == '__main__': > main() > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/26] python: Fix unequality comparisons
Quoting Mathieu Bridon (2018-07-05 06:17:43) > On Python 3, executing `foo != bar` will first try to call > foo.__ne__(bar), and fallback on the opposite result of foo.__eq__(bar). > > Python 2 does not do that. > > As a result, those __eq__ methods were never called, when we were > testing for inequality. > > Expliclty adding the __ne__ methods fixes this issue, in a way that is > compatible with both Python 2 and 3. > > However, this means the __eq__ methods are now called when testing for > `foo != None`, so they need to be guarded correctly. > > Signed-off-by: Mathieu Bridon > --- > src/amd/vulkan/vk_format_parse.py| 6 ++ > src/gallium/auxiliary/util/u_format_parse.py | 6 ++ > src/mesa/main/format_parser.py | 6 ++ > 3 files changed, 18 insertions(+) > > diff --git a/src/amd/vulkan/vk_format_parse.py > b/src/amd/vulkan/vk_format_parse.py > index 00cf1adf5a..778eae61ba 100644 > --- a/src/amd/vulkan/vk_format_parse.py > +++ b/src/amd/vulkan/vk_format_parse.py > @@ -73,8 +73,14 @@ class Channel: > return s > > def __eq__(self, other): > +if other is None: > +return False > + > return self.type == other.type and self.norm == other.norm and > self.pure == other.pure and self.size == other.size and self.scaled == > other.scaled > > +def __ne__(self, other): > +return not self.__eq__(other) This can be written as "not (self == other)", right? > + > def max(self): > '''Maximum representable number.''' > if self.type == FLOAT: > diff --git a/src/gallium/auxiliary/util/u_format_parse.py > b/src/gallium/auxiliary/util/u_format_parse.py > index 315c771081..e60b317e08 100644 > --- a/src/gallium/auxiliary/util/u_format_parse.py > +++ b/src/gallium/auxiliary/util/u_format_parse.py > @@ -69,8 +69,14 @@ class Channel: > return s > > def __eq__(self, other): > +if other is None: > +return False > + > return self.type == other.type and self.norm == other.norm and > self.pure == other.pure and self.size == other.size > > +def __ne__(self, other): > +return not self.__eq__(other) > + > def max(self): > '''Maximum representable number.''' > if self.type == FLOAT: > diff --git a/src/mesa/main/format_parser.py b/src/mesa/main/format_parser.py > index 3321ad33ff..c0d73c9d22 100644 > --- a/src/mesa/main/format_parser.py > +++ b/src/mesa/main/format_parser.py > @@ -61,8 +61,14 @@ class Channel: >return s > > def __eq__(self, other): > + if other is None: > + return False > + >return self.type == other.type and self.norm == other.norm and > self.size == other.size > > + def __ne__(self, other): > + return not self.__eq__(other) > + > def max(self): >"""Returns the maximum representable number.""" >if self.type == FLOAT: > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/26] python: Fix rich comparisons
Quoting Mathieu Bridon (2018-07-05 06:17:42) > Python 3 lost the cmp() builtin, and doesn't call objects __cmp__() > methods any more to compare them. > > Instead, Python 3 requires implementing the rich comparison methods > explicitly: __eq__(), __ne(), __lt__(), __le__(), __gt__() and __ge__(). > > Fortunately those are trivial to implement by just calling the existing > __cmp__() method, which makes the code compatible with both Python 2 and > Python 3. Noo. Kill __cmp__ with fire and death. Python 2 supports rich comparison methods as well, so please, please, please, please just get rid of __cmp__. It also gets rid of all of the try/except madness. > > This commit only implements the comparison methods which are actually > used by the build scripts. > > In addition, this commit brings back to Python 3 the cmp() builtin > method as required. > > Signed-off-by: Mathieu Bridon > --- > src/amd/vulkan/radv_extensions.py | 6 +- > src/intel/vulkan/anv_extensions.py | 6 +- > src/mapi/mapi_abi.py | 13 + > 3 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/src/amd/vulkan/radv_extensions.py > b/src/amd/vulkan/radv_extensions.py > index a0f1038110..7f48d77629 100644 > --- a/src/amd/vulkan/radv_extensions.py > +++ b/src/amd/vulkan/radv_extensions.py > @@ -150,7 +150,11 @@ class VkVersion: > other = copy.copy(other) > other.patch = self.patch > > -return self.__int_ver().__cmp__(other.__int_ver()) > +return self.__int_ver() - other.__int_ver() > + > +def __gt__(self, other): > +return self.__cmp__(other) > 0 > + > > MAX_API_VERSION = VkVersion(MAX_API_VERSION) > > diff --git a/src/intel/vulkan/anv_extensions.py > b/src/intel/vulkan/anv_extensions.py > index 0f99f58ecb..213cfdc551 100644 > --- a/src/intel/vulkan/anv_extensions.py > +++ b/src/intel/vulkan/anv_extensions.py > @@ -162,7 +162,11 @@ class VkVersion: > other = copy.copy(other) > other.patch = self.patch > > -return self.__int_ver().__cmp__(other.__int_ver()) > +return self.__int_ver() - other.__int_ver() > + > +def __gt__(self, other): > +return self.__cmp__(other) > 0 > + > > > MAX_API_VERSION = VkVersion('0.0.0') > diff --git a/src/mapi/mapi_abi.py b/src/mapi/mapi_abi.py > index be1d15d922..67fdb10650 100644 > --- a/src/mapi/mapi_abi.py > +++ b/src/mapi/mapi_abi.py > @@ -38,6 +38,15 @@ import gl_XML > import glX_XML > > > +try: > +cmp > + > +except NameError: > +# Python 3 does not have cmp() > +def cmp(a, b): > +return ((a > b) - (a < b)) > + > + > # number of dynamic entries > ABI_NUM_DYNAMIC_ENTRIES = 256 > > @@ -135,6 +144,10 @@ class ABIEntry(object): > > return res > > +def __lt__(self, other): > +return self.__cmp__(other) < 0 > + > + > def abi_parse_xml(xml): > """Parse a GLAPI XML file for ABI entries.""" > api = gl_XML.parse_GL_API(xml, glX_XML.glx_item_factory()) > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/26] python: Use explicit integer divisions
How about using future division (ala from __future__ import division), which makes division behave like python 3 division, So that >>> 32 / 4 8.0 >>> 32 // 4 8 (I'm really a fan of python 3's explicit integer and float division operators) Quoting Mathieu Bridon (2018-07-05 06:17:41) > In Python 2, divisions return an integer: > > >>> 32 / 4 > 8 > > In Python 3 though, they return floats: > > >>> 32 / 4 > 8.0 > > Explicitly converting to integers make the scripts compatible with both > Python 2 and 3. > > Signed-off-by: Mathieu Bridon > --- > src/gallium/auxiliary/util/u_format_pack.py | 2 +- > src/gallium/auxiliary/util/u_format_parse.py | 4 ++-- > src/mapi/glapi/gen/glX_proto_send.py | 2 +- > src/mesa/main/format_info.py | 2 +- > src/mesa/main/format_pack.py | 6 +++--- > src/mesa/main/format_unpack.py | 6 +++--- > 6 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/src/gallium/auxiliary/util/u_format_pack.py > b/src/gallium/auxiliary/util/u_format_pack.py > index 7a952a48b3..1cfd85fb7f 100644 > --- a/src/gallium/auxiliary/util/u_format_pack.py > +++ b/src/gallium/auxiliary/util/u_format_pack.py > @@ -240,7 +240,7 @@ def value_to_native(type, value): > return truncate_mantissa(value, 23) > return value > if type.type == FIXED: > -return int(value * (1 << (type.size/2))) > +return int(value * (1 << int(type.size/2))) > if not type.norm: > return int(value) > if type.type == UNSIGNED: > diff --git a/src/gallium/auxiliary/util/u_format_parse.py > b/src/gallium/auxiliary/util/u_format_parse.py > index c0456f6d15..315c771081 100644 > --- a/src/gallium/auxiliary/util/u_format_parse.py > +++ b/src/gallium/auxiliary/util/u_format_parse.py > @@ -76,7 +76,7 @@ class Channel: > if self.type == FLOAT: > return VERY_LARGE > if self.type == FIXED: > -return (1 << (self.size/2)) - 1 > +return (1 << int(self.size/2)) - 1 > if self.norm: > return 1 > if self.type == UNSIGNED: > @@ -90,7 +90,7 @@ class Channel: > if self.type == FLOAT: > return -VERY_LARGE > if self.type == FIXED: > -return -(1 << (self.size/2)) > +return -(1 << int(self.size/2)) > if self.type == UNSIGNED: > return 0 > if self.norm: > diff --git a/src/mapi/glapi/gen/glX_proto_send.py > b/src/mapi/glapi/gen/glX_proto_send.py > index a920ecc012..28f8a0d67b 100644 > --- a/src/mapi/glapi/gen/glX_proto_send.py > +++ b/src/mapi/glapi/gen/glX_proto_send.py > @@ -809,7 +809,7 @@ generic_%u_byte( GLint rop, const void * ptr ) > # Dividing by the array size (1 for > # non-arrays) gives us this. > > -s = p.size() / p.get_element_count() > +s = int(p.size() / p.get_element_count()) > print(" %s __glXReadReply(dpy, %s, %s, %s);" % > (return_str, s, p.name, aa)) > got_reply = 1 > > diff --git a/src/mesa/main/format_info.py b/src/mesa/main/format_info.py > index bbecaa2451..285bf13e1b 100644 > --- a/src/mesa/main/format_info.py > +++ b/src/mesa/main/format_info.py > @@ -198,7 +198,7 @@ for fmat in formats: >chan = fmat.array_element() >norm = chan.norm or chan.type == parser.FLOAT >print(' .ArrayFormat = MESA_ARRAY_FORMAT({0}),'.format(', '.join([ > - str(chan.size / 8), > + str(int(chan.size / 8)), > str(int(chan.sign)), > str(int(chan.type == parser.FLOAT)), > str(int(norm)), > diff --git a/src/mesa/main/format_pack.py b/src/mesa/main/format_pack.py > index d3c8d24acd..3b69580a93 100644 > --- a/src/mesa/main/format_pack.py > +++ b/src/mesa/main/format_pack.py > @@ -356,7 +356,7 @@ _mesa_pack_ubyte_rgba_row(mesa_format format, GLuint n, > case ${f.name}: >for (i = 0; i < n; ++i) { > pack_ubyte_${f.short_name()}(src[i], d); > - d += ${f.block_size() / 8}; > + d += ${int(f.block_size() / 8)}; >} >break; > %endfor > @@ -388,7 +388,7 @@ _mesa_pack_uint_rgba_row(mesa_format format, GLuint n, > case ${f.name}: >for (i = 0; i < n; ++i) { > pack_uint_${f.short_name()}(src[i], d); > - d += ${f.block_size() / 8}; > + d += ${int(f.block_size() / 8)}; >} >break; > %endfor > @@ -418,7 +418,7 @@ _mesa_pack_float_rgba_row(mesa_format format, GLuint n, > case ${f.name}: >for (i = 0; i < n; ++i) { > pack_float_${f.short_name()}(src[i], d); > - d += ${f.block_size() / 8}; > + d += ${int(f.block_size() / 8)}; >} >break; > %endfor > diff --git a/src/mesa/main/format_unpack.py b/src/mesa/main/format_unpack.py > index 286c08e621..feddaed5cd 100644 > --- a/src/mesa/main/format_unpack.py >
Re: [Mesa-dev] [PATCH 09/26] python: Use range() instead of xrange()
This has the same python 2 performance issue, so I'd like to either drop python2 support at the end, or do something to make python2 performance not terrible. but, with Eric's comments addressed, Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:40) > Python 2 has a range() function which returns a list, and an xrange() > one which returns an iterator. > > Python 3 lost the function returning a list, and renamed the function > returning an iterator as range(). > > As a result, using range() makes the scripts compatible with both Python > versions 2 and 3. > > Signed-off-by: Mathieu Bridon > --- > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- > src/broadcom/cle/gen_pack_header.py | 2 +- > src/compiler/glsl/ir_expression_operation.py | 2 +- > src/compiler/nir/nir_opcodes.py | 4 ++-- > src/intel/vulkan/anv_entrypoints_gen.py | 2 +- > src/mapi/glapi/gen/glX_proto_send.py | 2 +- > src/mapi/glapi/gen/gl_XML.py | 2 +- > src/mapi/glapi/gen/gl_gentable.py| 4 ++-- > src/mapi/mapi_abi.py | 2 +- > src/mesa/main/format_parser.py | 4 ++-- > 10 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py > b/src/amd/vulkan/radv_entrypoints_gen.py > index 9c4dfd02a0..ca022bcbb0 100644 > --- a/src/amd/vulkan/radv_entrypoints_gen.py > +++ b/src/amd/vulkan/radv_entrypoints_gen.py > @@ -136,7 +136,7 @@ static const struct string_map_entry string_map_entries[] > = { > /* Hash table stats: > * size ${len(strmap.sorted_strings)} entries > * collisions entries: > -% for i in xrange(10): > +% for i in range(10): > * ${i}${'+' if i == 9 else ' '} ${strmap.collisions[i]} > % endfor > */ > diff --git a/src/broadcom/cle/gen_pack_header.py > b/src/broadcom/cle/gen_pack_header.py > index c6e1c564e6..8ad54464cb 100644 > --- a/src/broadcom/cle/gen_pack_header.py > +++ b/src/broadcom/cle/gen_pack_header.py > @@ -216,7 +216,7 @@ class Group(object): > first_byte = field.start // 8 > last_byte = field.end // 8 > > -for b in xrange(first_byte, last_byte + 1): > +for b in range(first_byte, last_byte + 1): > if not b in bytes: > bytes[b] = self.Byte() > > diff --git a/src/compiler/glsl/ir_expression_operation.py > b/src/compiler/glsl/ir_expression_operation.py > index b3dac3da3f..16b98690a6 100644 > --- a/src/compiler/glsl/ir_expression_operation.py > +++ b/src/compiler/glsl/ir_expression_operation.py > @@ -116,7 +116,7 @@ constant_template_common = mako.template.Template("""\ > constant_template_vector_scalar = mako.template.Template("""\ > case ${op.get_enum_name()}: > % if "mixed" in op.flags: > -% for i in xrange(op.num_operands): > +% for i in range(op.num_operands): >assert(op[${i}]->type->base_type == ${op.source_types[0].glsl_type} || > % for src_type in op.source_types[1:-1]: > op[${i}]->type->base_type == ${src_type.glsl_type} || > diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py > index 3c3316dcaa..b03c5da2ea 100644 > --- a/src/compiler/nir/nir_opcodes.py > +++ b/src/compiler/nir/nir_opcodes.py > @@ -367,8 +367,8 @@ for (unsigned bit = 0; bit < bit_size; bit++) { > """) > > > -for i in xrange(1, 5): > - for j in xrange(1, 5): > +for i in range(1, 5): > + for j in range(1, 5): >unop_horiz("fnoise{0}_{1}".format(i, j), i, tfloat, j, tfloat, "0.0f") > > > diff --git a/src/intel/vulkan/anv_entrypoints_gen.py > b/src/intel/vulkan/anv_entrypoints_gen.py > index 8a37336496..5e2cd0740a 100644 > --- a/src/intel/vulkan/anv_entrypoints_gen.py > +++ b/src/intel/vulkan/anv_entrypoints_gen.py > @@ -145,7 +145,7 @@ static const struct string_map_entry string_map_entries[] > = { > /* Hash table stats: > * size ${len(strmap.sorted_strings)} entries > * collisions entries: > -% for i in xrange(10): > +% for i in range(10): > * ${i}${'+' if i == 9 else ' '} ${strmap.collisions[i]} > % endfor > */ > diff --git a/src/mapi/glapi/gen/glX_proto_send.py > b/src/mapi/glapi/gen/glX_proto_send.py > index fba2f0cc1e..a920ecc012 100644 > --- a/src/mapi/glapi/gen/glX_proto_send.py > +++ b/src/mapi/glapi/gen/glX_proto_send.py > @@ -392,7 +392,7 @@ static const struct proc_pair > _glapi_proc proc; > } proc_pairs[%d] = {""" % len(procs)) > names = sorted(procs.keys()) > -for i in xrange(len(names)): > +for i in range(len(names)): > comma = ',' if i < len(names) - 1 else '' > print(' { "%s", (_glapi_proc) gl%s }%s' % (names[i], > procs[names[i]], comma)) > print("""}; > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > index bfbb1ec6e0..96dc1b3c12 100644 > --- a/src/mapi/glapi/gen/gl_XML.py > +++ b/src/mapi/glapi/gen/gl_XML.py > @@ -834,7 +834,7 @@ cla
Re: [Mesa-dev] [PATCH 08/26] python: Better use iterators
Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:39) > In Python 2, iterators had a .next() method. > > In Python 3, instead they have a .__next__() method, which is > automatically called by the next() builtin. > > In addition, it is better to use the iter() builtin to create an > iterator, rather than calling its __iter__() method. > > These were also introduced in Python 2.6, so using it makes the script > compatible with Python 2 and 3. > > Signed-off-by: Mathieu Bridon > --- > src/compiler/glsl/ir_expression_operation.py | 4 +++- > src/compiler/nir/nir_algebraic.py| 4 ++-- > src/mapi/glapi/gen/glX_XML.py| 17 + > src/mapi/glapi/gen/gl_XML.py | 12 ++-- > 4 files changed, 20 insertions(+), 17 deletions(-) > > diff --git a/src/compiler/glsl/ir_expression_operation.py > b/src/compiler/glsl/ir_expression_operation.py > index d8542925a0..b3dac3da3f 100644 > --- a/src/compiler/glsl/ir_expression_operation.py > +++ b/src/compiler/glsl/ir_expression_operation.py > @@ -62,7 +62,7 @@ class type_signature_iter(object): > def __iter__(self): >return self > > - def next(self): > + def __next__(self): >if self.i < len(self.source_types): > i = self.i > self.i += 1 > @@ -76,6 +76,8 @@ class type_signature_iter(object): >else: > raise StopIteration() > > + next = __next__ > + > > uint_type = type("unsigned", "u", "GLSL_TYPE_UINT") > int_type = type("int", "i", "GLSL_TYPE_INT") > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index 8c0b530f69..fda72d3c69 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -56,7 +56,7 @@ class VarSet(object): > def __getitem__(self, name): >if name not in self.names: > assert not self.immutable, "Unknown replacement variable: " + name > - self.names[name] = self.ids.next() > + self.names[name] = next(self.ids) > >return self.names[name] > > @@ -468,7 +468,7 @@ condition_list = ['true'] > > class SearchAndReplace(object): > def __init__(self, transform): > - self.id = _optimization_ids.next() > + self.id = next(_optimization_ids) > >search = transform[0] >replace = transform[1] > diff --git a/src/mapi/glapi/gen/glX_XML.py b/src/mapi/glapi/gen/glX_XML.py > index bbcecd6023..4ec8cd766f 100644 > --- a/src/mapi/glapi/gen/glX_XML.py > +++ b/src/mapi/glapi/gen/glX_XML.py > @@ -296,7 +296,7 @@ class glx_function(gl_XML.gl_function): > parameters.extend( temp[1] ) > if include_variable_parameters: > parameters.extend( temp[2] ) > -return parameters.__iter__() > +return iter(parameters) > > > def parameterIterateCounters(self): > @@ -304,7 +304,7 @@ class glx_function(gl_XML.gl_function): > for name in self.counter_list: > temp.append( self.parameters_by_name[ name ] ) > > -return temp.__iter__() > +return iter(temp) > > > def parameterIterateOutputs(self): > @@ -547,13 +547,14 @@ class glx_function_iterator(object): > return self > > > -def next(self): > -f = self.iterator.next() > +def __next__(self): > +while True: > +f = next(self.iterator) > > -if f.client_supported_for_indirect(): > -return f > -else: > -return self.next() > +if f.client_supported_for_indirect(): > +return f > + > +next = __next__ > > > class glx_api(gl_XML.gl_api): > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > index 3a4f59221e..bfbb1ec6e0 100644 > --- a/src/mapi/glapi/gen/gl_XML.py > +++ b/src/mapi/glapi/gen/gl_XML.py > @@ -782,9 +782,9 @@ class gl_function( gl_item ): > > def parameterIterator(self, name = None): > if name is not None: > -return self.entry_point_parameters[name].__iter__(); > +return iter(self.entry_point_parameters[name]); > else: > -return self.parameters.__iter__(); > +return iter(self.parameters); > > > def get_parameter_string(self, entrypoint = None): > @@ -996,7 +996,7 @@ class gl_api(object): > for name in names: > functions.append(lists[func_cat_type][key][name]) > > -return functions.__iter__() > +return iter(functions) > > > def functionIterateByOffset(self): > @@ -1017,7 +1017,7 @@ class gl_api(object): > if temp[i]: > list.append(temp[i]) > > -return list.__iter__(); > +return iter(list); > > > def functionIterateAll(self): > @@ -1031,7 +1031,7 @@ class gl_api(object): > for enum in keys: > list.append( self.enums_by_name[ enum ] ) > > -return list.__iter__() > +
Re: [Mesa-dev] [PATCH 14/26] python: Better get character ordinals
On Thursday, 2018-07-05 15:17:45 +0200, Mathieu Bridon wrote: > In Python 2, iterating over a byte-string yields single-byte strings, > and we can pass them to ord() to get the corresponding integer. > > In Python 3, iterating over a byte-string directly yields those > integers. > > Transforming the byte string into a bytearray gives us a list of the > integers corresponding to each byte in the string, removing the need to > call ord(). > > This makes the script compatible with both Python 2 and 3. > > Signed-off-by: Mathieu Bridon Reviewed-by: Eric Engestrom > --- > src/intel/genxml/gen_zipped_file.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/intel/genxml/gen_zipped_file.py > b/src/intel/genxml/gen_zipped_file.py > index 6d8daf4d69..616409183f 100644 > --- a/src/intel/genxml/gen_zipped_file.py > +++ b/src/intel/genxml/gen_zipped_file.py > @@ -62,8 +62,8 @@ def main(): > print("") > print("static const uint8_t compress_genxmls[] = {") > print(" ", end='') > -for i, c in enumerate(compressed_data, start=1): > -print("0x%.2x, " % ord(c), end='\n ' if not i % 12 else '') > +for i, c in enumerate(bytearray(compressed_data), start=1): > +print("0x%.2x, " % c, end='\n ' if not i % 12 else '') > print('\n};') > > > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: tools: fix build on old systems
On 5 July 2018 at 16:33, Matt Turner wrote: > On Thu, Jul 5, 2018 at 7:42 AM Lionel Landwerlin > wrote: >> >> Older system might not have support for memfd_create at the kernel >> level. There we won't be able to use aubinator. >> >> We also initially tried to workaround some libc having the >> memfd_create syscall number defined, but not the memfd_create() >> function. >> >> This change fixes the broken build on the travis CI by only compiling >> aubinator if memfd_create() is available as part of the libc. > > memfd_create() was added in glibc-2.27. I think that's too new to rely > on. I know aubinator is a developer tool, but I don't think most > developers have glibc 2.27 yet. > > I'd suggest we rename our "memfd_create" functions to avoid the clash > with glibc-2.27+ and just use them regardless. > I'm a fan on this. Less code and build system magic is always great. I'd imagine the man page meant to say "linux/memfd.h" instead of "sys/memfd.h" -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] meson: Build with Python 3
Dave, Brian, and Jose, IIRC when we discussed migrating piglit to python 3 (and went with a hybrid approach instead), y'all had requirements to run on python 2, and couldn't support python 3. Is that still the case, or would moving to python 3 be acceptable? Quoting Mathieu Bridon (2018-07-05 06:17:31) > This patch series allows building Mesa with Python 3. > > The build scripts are kept compatible with Python 2 as well, for those > platforms which don't have Python 3 yet. > > In fact, only the Meson build system is moved to Python 3, since it is > the only one I'm 100% sure has Python 3 available. (Meson itself > requires it) > > I briefly thought about adding an option to the Meson build system to > control which version of Python to build with, but decided against. I'm > happy to add it if you think it's necessary. > > I checked (with the `diff` command) that all the scripts output the > exact same things when built with Meson on: > > * master (as of f9b6dfd919 which includes my patches to make the build > output reproducible) > * the second to last patch in this series (that is, all the scripts > changed but still building with Python 2) > * the last patch in this series (that is, with Python 3) > > Each patch fixes a single type of problem in multiple > scripts/subsystems. As a result, it should be possible to review and > merge each patch independently (but probably not in order), without > breaking the build. > > It's a lot of changes: > > 86 files changed, 1965 insertions(+), 1833 deletions(-) > > The end goal is to be able to eventually remove Python 2 from future > versions of the Flatpak Freedesktop SDK, Mesa being one of the last few > things still requiring it. > > For those who prefer reviewing a git repo, I have pushed the changes to > a fork on the FDO Gitlab: > > * a branch compatible with both Python 2 and 3, but still building with > Python 2: > > https://gitlab.freedesktop.org/bochecha/mesa/tree/python-2-and-3 > > * a branch building with Python 3: > > https://gitlab.freedesktop.org/bochecha/mesa/tree/python3 > > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 5/5] DEBUG
Hey Emil, On 05/07/18 15:13, Emil Velikov wrote: Hi Rob, On 5 July 2018 at 11:07, Robert Foss wrote: @@ -511,7 +515,7 @@ dri2_open_driver(_EGLDisplay *disp) char path[PATH_MAX], *search_paths, *next, *end; char *get_extensions_name; const __DRIextension **(*get_extensions)(void); - + ALOGE("%s() 1 driver_name=%s", __func__, dri2_dpy->driver_name); Aside: Personally, I try to use "before/after X". Otherwise I find myself always bouncing back and forth, relate the 1, 1.1... with the actual call chain. Hmmph, yeah. Maybe that is a more pleasant way of going about it. I'll have to feel it out :) @@ -1367,10 +1379,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - Even with the issues you mentioned in the cover letter, this could be fleshed out or even squashed with 3/5. Up-to you really. This chunk shouldn't really be in this patch. I had some issues setting the Android property and while figuring out that the property wasn't propagated properly this chunk was removed. The debug patch will be dropped in v1, so this will be removed. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/26] python: Better iterate over dictionaries
I've asked a couple of people who have (in the past at least) had a hard requirement on python 2.x if moving to 3.x will be okay for them. If it's not then we may need to do something else here. I've used six in the past (although I know a lot of other pythonistas don't like six), so that would be an option I think. While this works fine, it's going to create a lot of overhead for anyone using python 2, since some of these data structures are huge, and returning a copy (or copy of a copy) is going to be fairly expensive. If we can drop python 2 support that of course isn't a problem. Assuming that those people are okay with python 3, Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:37) > In Python 2, dictionaries have 2 sets of methods to iterate over their > keys and values: keys()/values()/items() and > iterkeys()/itervalues()/iteritems(). > > The former return lists while the latter return iterators. > > Python 3 dropped the method which return lists, and renamed the methods > returning iterators to keys()/values()/items(). > > Using those names makes the scripts compatible with both Python 2 and 3. > > Signed-off-by: Mathieu Bridon > --- > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- > src/compiler/nir/nir_algebraic.py| 2 +- > src/compiler/nir/nir_builder_opcodes_h.py| 4 ++-- > src/compiler/nir/nir_constant_expressions.py | 4 ++-- > src/compiler/nir/nir_intrinsics_c.py | 2 +- > src/compiler/nir/nir_opcodes_c.py| 2 +- > src/compiler/nir/nir_opcodes_h.py| 2 +- > src/intel/genxml/gen_bits_header.py | 10 +- > src/intel/vulkan/anv_entrypoints_gen.py | 2 +- > src/mapi/glapi/gen/gl_XML.py | 12 ++-- > src/mapi/glapi/gen/gl_gentable.py| 4 ++-- > src/mesa/drivers/dri/i965/brw_oa.py | 4 ++-- > 12 files changed, 25 insertions(+), 25 deletions(-) > > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py > b/src/amd/vulkan/radv_entrypoints_gen.py > index bef0c447f6..9c4dfd02a0 100644 > --- a/src/amd/vulkan/radv_entrypoints_gen.py > +++ b/src/amd/vulkan/radv_entrypoints_gen.py > @@ -433,7 +433,7 @@ def get_entrypoints(doc, entrypoints_to_defines, > start_index): > e_clone.name = e.name > entrypoints[e.name] = e_clone > > -return [e for e in entrypoints.itervalues() if e.enabled] > +return [e for e in entrypoints.values() if e.enabled] > > > def get_entrypoints_defines(doc): > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index 847c59dbd8..8c0b530f69 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -512,7 +512,7 @@ struct transform { > > #endif > > -% for (opcode, xform_list) in xform_dict.iteritems(): > +% for (opcode, xform_list) in xform_dict.items(): > % for xform in xform_list: > ${xform.search.render()} > ${xform.replace.render()} > diff --git a/src/compiler/nir/nir_builder_opcodes_h.py > b/src/compiler/nir/nir_builder_opcodes_h.py > index 72cf5b4549..e600093e9f 100644 > --- a/src/compiler/nir/nir_builder_opcodes_h.py > +++ b/src/compiler/nir/nir_builder_opcodes_h.py > @@ -34,7 +34,7 @@ def src_list(num_srcs): > return ', '.join('src' + str(i) if i < num_srcs else 'NULL' for i in > range(4)) > %> > > -% for name, opcode in sorted(opcodes.iteritems()): > +% for name, opcode in sorted(opcodes.items()): > static inline nir_ssa_def * > nir_${name}(nir_builder *build, ${src_decl_list(opcode.num_inputs)}) > { > @@ -55,7 +55,7 @@ nir_load_system_value(nir_builder *build, nir_intrinsic_op > op, int index) > return &load->dest.ssa; > } > > -% for name, opcode in filter(lambda v: v[1].sysval, > sorted(INTR_OPCODES.iteritems())): > +% for name, opcode in filter(lambda v: v[1].sysval, > sorted(INTR_OPCODES.items())): > static inline nir_ssa_def * > nir_${name}(nir_builder *build) > { > diff --git a/src/compiler/nir/nir_constant_expressions.py > b/src/compiler/nir/nir_constant_expressions.py > index 35dffe70ce..118af9f781 100644 > --- a/src/compiler/nir/nir_constant_expressions.py > +++ b/src/compiler/nir/nir_constant_expressions.py > @@ -387,7 +387,7 @@ struct bool32_vec { > % endif > > > -% for name, op in sorted(opcodes.iteritems()): > +% for name, op in sorted(opcodes.items()): > static nir_const_value > evaluate_${name}(MAYBE_UNUSED unsigned num_components, > ${"UNUSED" if op_bit_sizes(op) is None else ""} unsigned > bit_size, > @@ -420,7 +420,7 @@ nir_eval_const_opcode(nir_op op, unsigned num_components, >unsigned bit_width, nir_const_value *src) > { > switch (op) { > -% for name in sorted(opcodes.iterkeys()): > +% for name in sorted(opcodes.keys()): > case nir_op_${name}: >return evaluate_${name}(num_components, bit_width, src); > % endfor > diff --git a/src/compiler/nir/nir_intrinsics_c.py > b/src/compiler/nir
Re: [Mesa-dev] [RFC 1/5] st/dri: Allow kms_swrast to work without a device FD
On 5 July 2018 at 16:25, Robert Foss wrote: > Hey Tomasz, > > > On 05/07/18 15:07, Tomasz Figa wrote: >> >> Hi Emil, Robert, >> >> On Thu, Jul 5, 2018 at 9:57 PM Emil Velikov >> wrote: >>> >>> >>> On 5 July 2018 at 12:32, Robert Foss wrote: Hey Eric! On 05/07/18 12:35, Eric Engestrom wrote: > > > On Thursday, 2018-07-05 12:07:36 +0200, Robert Foss wrote: >> >> >> From: Tomeu Vizoso >> >> A KMS device isn't strictly needed for the kms_swrast to work, so stop >> failing to init if the FD is -1. Also don't call DRM_IOCTL_GET_CAP in >> that case. >> >> This allows the driver to be used in machines where no KMS device at >> all >> is present. >> >> Signed-off-by: Tomeu Vizoso >> --- >>src/gallium/state_trackers/dri/dri2.c | 6 -- >>1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/src/gallium/state_trackers/dri/dri2.c >> b/src/gallium/state_trackers/dri/dri2.c >> index 58a6757f037..c262c0ca118 100644 >> --- a/src/gallium/state_trackers/dri/dri2.c >> +++ b/src/gallium/state_trackers/dri/dri2.c >> @@ -2189,7 +2189,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) >> sPriv->driverPrivate = (void *)screen; >>- if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, >> 3)) >> < 0) >> + /* We don't really need a device FD if we are soft-rendering */ >> + if (screen->fd >= 0 && (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, >> 3)) < >> 0) >> goto free_screen; >> if (pipe_loader_sw_probe_kms(&screen->dev, fd)) { > > > > Aren't you going to use an uninitialised `fd` here if `screen->fd < 0`? From my understanding during dri2_initialize_android(), droid_open_device[1] will always allocate an FD or fail, before a screen is created in dri2_create_screen[2]. But maybe there is some part I don't quite understand. >>> You're tracking a single instance instead of looking at the function in >>> itself. >>> When screen->fd is invalid, fd will be uninitialised. The >>> pipe_loader_sw_probe_kms() call will then use that uninitialised >>> value. >>> >>> I believe a better solution is to have distinct dri/null/kms backends >>> to swrast, instead of hacking it in this way. >>> Some ideas are listed in here [1]. >>> >>> -Emil >>> >>> [1] >>> https://gitlab.collabora.com/tomeu/mesa/commit/54adda6a4d7b5c783d54dfd37d38d1a5a0f3187f >> >> >> First of all, do we really need this patch at all? If you ended up >> booting Android with Mesa, you must have had a DRM driver for the >> display anyway, so what prevents you from using it as the KMS device >> for kms_swrast? >> > > Just dropping this patch does indeed cause no regressions. > Emil: Do you see any problems with that? > None. The distinct dri/null/ thing mentioned earlier is orthogonal to the Android support. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 13/26] python: Explicitly use byte strings
On Thursday, 2018-07-05 15:17:44 +0200, Mathieu Bridon wrote: > In both Python 2 and 3, zlib.Compress.compress() takes a byte string, > and returns a byte string as well. > > In Python 2, the script was working because: > > 1. string literalls were byte strings; > 2. opening a file in unicode mode, reading from it, then passing the >unicode string to compress() would automatically encode to a byte >string; > > On Python 3, the above two points are not valid any more, so: > > 1. zlib.Compress.compress() refuses the passed unicode string; > 2. compressed_data, defined as an empty unicode string literal, can't be >concatenated with the byte string returned by compress(); > > This commit fixes this by explicitly using byte strings where > appropriate, so that the script works on both Python 2 and 3. > > Signed-off-by: Mathieu Bridon Reviewed-by: Eric Engestrom > --- > src/intel/genxml/gen_zipped_file.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/intel/genxml/gen_zipped_file.py > b/src/intel/genxml/gen_zipped_file.py > index af2008bea0..6d8daf4d69 100644 > --- a/src/intel/genxml/gen_zipped_file.py > +++ b/src/intel/genxml/gen_zipped_file.py > @@ -42,10 +42,10 @@ def main(): > print("} genxml_files_table[] = {") > > xml_offset = 0 > -compressed_data = '' > +compressed_data = b'' > for i in range(1, len(sys.argv)): > filename = sys.argv[i] > -xml = open(filename).read() > +xml = open(filename, "rb").read() > xml_length = len(xml) > root = et.fromstring(xml) > > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/26] python: Stop using the string module
Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:36) > Most functions in the builtin string module also exist as methods of > string objects. > > Since the functions were removed from the string module in Python 3, > using the instance methods directly makes the code compatible with both > Python 2 and Python 3. > > Signed-off-by: Mathieu Bridon > --- > src/mapi/glapi/gen/glX_proto_common.py | 3 +-- > src/mapi/glapi/gen/glX_proto_send.py | 8 > src/mapi/glapi/gen/gl_XML.py | 8 > src/mapi/glapi/gen/typeexpr.py | 4 ++-- > 4 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/src/mapi/glapi/gen/glX_proto_common.py > b/src/mapi/glapi/gen/glX_proto_common.py > index adc20dc9f0..0559dd1609 100644 > --- a/src/mapi/glapi/gen/glX_proto_common.py > +++ b/src/mapi/glapi/gen/glX_proto_common.py > @@ -27,7 +27,6 @@ > from __future__ import print_function > > import gl_XML, glX_XML > -import string > > > class glx_proto_item_factory(glX_XML.glx_item_factory): > @@ -67,7 +66,7 @@ class glx_print_proto(gl_XML.gl_print_base): > return compsize > > elif len(param.count_parameter_list): > -parameters = string.join( param.count_parameter_list, > "," ) > +parameters = ",".join( param.count_parameter_list ) > compsize = "__gl%s_size(%s)" % (func.name, parameters) > > return compsize > diff --git a/src/mapi/glapi/gen/glX_proto_send.py > b/src/mapi/glapi/gen/glX_proto_send.py > index 7ab1eb4a70..c389872044 100644 > --- a/src/mapi/glapi/gen/glX_proto_send.py > +++ b/src/mapi/glapi/gen/glX_proto_send.py > @@ -31,7 +31,7 @@ from __future__ import print_function > import argparse > > import gl_XML, glX_XML, glX_proto_common, license > -import copy, string > +import copy > > def convertStringForXCB(str): > tmp = "" > @@ -39,10 +39,10 @@ def convertStringForXCB(str): > i = 0 > while i < len(str): > if str[i:i+3] in special: > -tmp = '%s_%s' % (tmp, string.lower(str[i:i+3])) > +tmp = '%s_%s' % (tmp, str[i:i+3].lower()) > i = i + 2; > elif str[i].isupper(): > -tmp = '%s_%s' % (tmp, string.lower(str[i])) > +tmp = '%s_%s' % (tmp, str[i].lower()) > else: > tmp = '%s%s' % (tmp, str[i]) > i += 1 > @@ -662,7 +662,7 @@ generic_%u_byte( GLint rop, const void * ptr ) > > if len( condition_list ) > 0: > if len( condition_list ) > 1: > -skip_condition = "(%s)" % (string.join( condition_list, ") > && (" )) > +skip_condition = "(%s)" % ") && (".join( condition_list ) > else: > skip_condition = "%s" % (condition_list.pop(0)) > > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > index 52e2cab6b9..7cb276ec85 100644 > --- a/src/mapi/glapi/gen/gl_XML.py > +++ b/src/mapi/glapi/gen/gl_XML.py > @@ -29,7 +29,7 @@ from __future__ import print_function > from collections import OrderedDict > from decimal import Decimal > import xml.etree.ElementTree as ET > -import re, sys, string > +import re, sys > import os.path > import typeexpr > import static_data > @@ -320,7 +320,7 @@ def create_parameter_string(parameters, include_names): > > if len(list) == 0: list = ["void"] > > -return string.join(list, ", ") > +return ", ".join(list) > > > class gl_item(object): > @@ -578,9 +578,9 @@ class gl_parameter(object): > list.append( str(s) ) > > if len(list) > 1 and use_parens : > -return "safe_mul(%s)" % (string.join(list, ", ")) > +return "safe_mul(%s)" % ", ".join(list) > else: > -return string.join(list, " * ") > +return " * ".join(list) > > elif self.is_image(): > return "compsize" > diff --git a/src/mapi/glapi/gen/typeexpr.py b/src/mapi/glapi/gen/typeexpr.py > index 5ff9798dad..1f710ea9e7 100644 > --- a/src/mapi/glapi/gen/typeexpr.py > +++ b/src/mapi/glapi/gen/typeexpr.py > @@ -26,7 +26,7 @@ > > from __future__ import print_function > > -import string, copy > +import copy > > class type_node(object): > def __init__(self): > @@ -126,7 +126,7 @@ class type_expression(object): > > # Replace '*' with ' * ' in type_string. Then, split the string > # into tokens, separated by spaces. > -tokens = string.split( string.replace( type_string, "*", " * " ) ) > +tokens = type_string.replace("*", " * ").split() > > const = 0 > t = None > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ m
Re: [Mesa-dev] [PATCH 04/26] python: Better check for keys in dicts
With the changes Eric mentioned (moving the two hunks in the wrong patch to the right one), Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:35) > Python 3 lost the dict.has_key() method. Instead it requires using the > "in" operator. > > This is also compatible with Python 2. > > Signed-off-by: Mathieu Bridon > --- > src/mapi/glapi/gen/glX_XML.py| 2 +- > src/mapi/glapi/gen/glX_proto_send.py | 2 +- > src/mapi/glapi/gen/glX_proto_size.py | 18 +- > src/mapi/glapi/gen/gl_XML.py | 6 +++--- > src/mapi/glapi/gen/gl_procs.py | 2 +- > src/mapi/mapi_abi.py | 10 -- > src/util/xmlpool/gen_xmlpool.py | 4 ++-- > 7 files changed, 21 insertions(+), 23 deletions(-) > > diff --git a/src/mapi/glapi/gen/glX_XML.py b/src/mapi/glapi/gen/glX_XML.py > index e1aa1b3e61..bbcecd6023 100644 > --- a/src/mapi/glapi/gen/glX_XML.py > +++ b/src/mapi/glapi/gen/glX_XML.py > @@ -64,7 +64,7 @@ class glx_enum(gl_XML.gl_enum): > else: > mode = 1 > > -if not self.functions.has_key(n): > +if n not in self.functions: > self.functions[ n ] = [c, mode] > > return > diff --git a/src/mapi/glapi/gen/glX_proto_send.py > b/src/mapi/glapi/gen/glX_proto_send.py > index f199e9a0a1..7ab1eb4a70 100644 > --- a/src/mapi/glapi/gen/glX_proto_send.py > +++ b/src/mapi/glapi/gen/glX_proto_send.py > @@ -842,7 +842,7 @@ generic_%u_byte( GLint rop, const void * ptr ) > > > def printPixelFunction(self, f): > -if self.pixel_stubs.has_key( f.name ): > +if f.name in self.pixel_stubs: > # Normally gl_function::get_parameter_string could be > # used. However, this call needs to have the missing > # dimensions (e.g., a fake height value for > diff --git a/src/mapi/glapi/gen/glX_proto_size.py > b/src/mapi/glapi/gen/glX_proto_size.py > index 284ee70e61..18a6f2e502 100644 > --- a/src/mapi/glapi/gen/glX_proto_size.py > +++ b/src/mapi/glapi/gen/glX_proto_size.py > @@ -71,7 +71,7 @@ class glx_enum_function(object): > for enum_name in enum_dict: > e = enum_dict[ enum_name ] > > -if e.functions.has_key( match_name ): > +if match_name in e.functions: > [count, mode] = e.functions[ match_name ] > > if mode_set and mode != self.mode: > @@ -79,11 +79,11 @@ class glx_enum_function(object): > > self.mode = mode > > -if self.enums.has_key( e.value ): > +if e.value in self.enums: > if e.name not in self.enums[ e.value ]: > self.enums[ e.value ].append( e ) > else: > -if not self.count.has_key( count ): > +if count not in self.count: > self.count[ count ] = [] > > self.enums[ e.value ] = [ e ] > @@ -131,7 +131,7 @@ class glx_enum_function(object): > for a in self.enums: > count += 1 > > -if self.count.has_key(-1): > +if -1 in self.count: > return 0 > > # Determine if there is some mask M, such that M = (2^N) - 1, > @@ -356,7 +356,7 @@ class PrintGlxSizeStubs_c(PrintGlxSizeStubs_common): > > if (ef.is_set() and self.emit_set) or (not ef.is_set() and > self.emit_get): > sig = ef.signature() > -if enum_sigs.has_key( sig ): > +if sig in enum_sigs: > aliases.append( [func.name, enum_sigs[ sig ]] ) > else: > enum_sigs[ sig ] = func.name > @@ -477,10 +477,10 @@ class PrintGlxReqSize_c(PrintGlxReqSize_common): > > sig = ef.signature() > > -if not enum_functions.has_key(func.name): > +if func.name not in enum_functions: > enum_functions[ func.name ] = sig > > -if not enum_sigs.has_key( sig ): > +if sig not in enum_sigs: > enum_sigs[ sig ] = ef > > > @@ -496,7 +496,7 @@ class PrintGlxReqSize_c(PrintGlxReqSize_common): > if func.server_handcode: continue > if not func.has_variable_size_request(): continue > > -if enum_functions.has_key(func.name): > +if func.name in enum_functions: > sig = enum_functions[func.name] > ef = enum_sigs[ sig ] > > @@ -621,7 +621,7 @@ class PrintGlxReqSize_c(PrintGlxReqSize_common): > # already be emitted, don't emit this function. Instead, add > # it to the list of function aliases. > > -if self.counter_sigs.has_key(sig): > +if sig in self.counter_sigs: > n = self.counter_sigs[sig]; > alias = [f.name, n] > else: > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl
Re: [Mesa-dev] [PATCH 03/26] python: Use the Python 3 exception syntax
Quoting Eric Engestrom (2018-07-05 06:57:59) > On Thursday, 2018-07-05 15:17:34 +0200, Mathieu Bridon wrote: > > This is compatible with Python versions >= 2.6. > > > > Signed-off-by: Mathieu Bridon > > --- > > src/mapi/glapi/gen/glX_XML.py | 2 +- > > src/mapi/glapi/gen/gl_XML.py| 6 +++--- > > src/mapi/glapi/gen/gl_marshal.py| 2 +- > > src/mapi/glapi/gen/gl_marshal_h.py | 2 +- > > src/mesa/main/get_hash_generator.py | 2 +- > > 5 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/src/mapi/glapi/gen/glX_XML.py b/src/mapi/glapi/gen/glX_XML.py > > index b6d305c879..e1aa1b3e61 100644 > > --- a/src/mapi/glapi/gen/glX_XML.py > > +++ b/src/mapi/glapi/gen/glX_XML.py > > @@ -470,7 +470,7 @@ class glx_function(gl_XML.gl_function): > > def needs_reply(self): > > try: > > x = self._needs_reply > > -except Exception, e: > > +except Exception as e: > > None of these actually use `e`, might as well drop it. > Reviewed-by: Eric Engestrom I was going to say the same thing, with the unused e dropped, Reviewed-by: Dylan Baker > > > x = 0 > > if self.return_type != 'void': > > x = 1 > > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > > index 3a191abe0d..330ca0e5a6 100644 > > --- a/src/mapi/glapi/gen/gl_XML.py > > +++ b/src/mapi/glapi/gen/gl_XML.py > > @@ -284,7 +284,7 @@ def classify_category(name, number): > > > > try: > > core_version = float(name) > > -except Exception,e: > > +except Exception as e: > > core_version = 0.0 > > > > if core_version > 0.0: > > @@ -365,7 +365,7 @@ class gl_enum( gl_item ): > > else: > > try: > > c = int(temp) > > -except Exception,e: > > +except Exception as e: > > raise RuntimeError('Invalid count value "%s" for enum "%s" > > in function "%s" when an integer was expected.' % (temp, self.name, n)) > > > > self.default_count = c > > @@ -426,7 +426,7 @@ class gl_parameter(object): > > count = int(c) > > self.count = count > > self.counter = None > > -except Exception,e: > > +except Exception as e: > > count = 1 > > self.count = 0 > > self.counter = c > > diff --git a/src/mapi/glapi/gen/gl_marshal.py > > b/src/mapi/glapi/gen/gl_marshal.py > > index e9dd2c4f78..18d7a7012b 100644 > > --- a/src/mapi/glapi/gen/gl_marshal.py > > +++ b/src/mapi/glapi/gen/gl_marshal.py > > @@ -353,7 +353,7 @@ if __name__ == '__main__': > > > > try: > > (args, trail) = getopt.getopt(sys.argv[1:], 'm:f:') > > -except Exception,e: > > +except Exception as e: > > show_usage() > > > > for (arg,val) in args: > > diff --git a/src/mapi/glapi/gen/gl_marshal_h.py > > b/src/mapi/glapi/gen/gl_marshal_h.py > > index 6490595a00..a7a9eda573 100644 > > --- a/src/mapi/glapi/gen/gl_marshal_h.py > > +++ b/src/mapi/glapi/gen/gl_marshal_h.py > > @@ -74,7 +74,7 @@ if __name__ == '__main__': > > > > try: > > (args, trail) = getopt.getopt(sys.argv[1:], 'm:f:') > > -except Exception,e: > > +except Exception: > > show_usage() > > > > for (arg,val) in args: > > diff --git a/src/mesa/main/get_hash_generator.py > > b/src/mesa/main/get_hash_generator.py > > index 86c6771066..facdccd8a5 100644 > > --- a/src/mesa/main/get_hash_generator.py > > +++ b/src/mesa/main/get_hash_generator.py > > @@ -201,7 +201,7 @@ def show_usage(): > > if __name__ == '__main__': > > try: > >(opts, args) = getopt.getopt(sys.argv[1:], "f:") > > - except Exception,e: > > + except Exception: > >show_usage() > > > > if len(args) != 0: > > -- > > 2.17.1 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev 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 02/26] python: Use spaces, not tabs
Reviewed-by: Dylan Baker Quoting Mathieu Bridon (2018-07-05 06:17:33) > Python 3 doesn't allow mixing spaces and tabs in a script, contrarily to > Python 2. > > Signed-off-by: Mathieu Bridon > --- > src/mapi/glapi/gen/glX_proto_size.py | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/mapi/glapi/gen/glX_proto_size.py > b/src/mapi/glapi/gen/glX_proto_size.py > index 2b7cefd235..284ee70e61 100644 > --- a/src/mapi/glapi/gen/glX_proto_size.py > +++ b/src/mapi/glapi/gen/glX_proto_size.py > @@ -612,10 +612,10 @@ class PrintGlxReqSize_c(PrintGlxReqSize_common): > if s == 0: s = 1 > > sig += "(%u,%u)" % (f.offset_of(p.counter), s) > - if size == '': > - size = p.size_string() > - else: > - size = "safe_add(%s, %s)" % (size, p.size_string()) > +if size == '': > +size = p.size_string() > +else: > +size = "safe_add(%s, %s)" % (size, p.size_string()) > > # If the calculated signature matches a function that has > # already be emitted, don't emit this function. Instead, add > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/26] python: Use the print function
This is a really big patch that should be mostly mechanical, I skimmed it, but didn't actually read it closely, Acked-by: Dylan Baker 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 1/2] build: Stabilize some script outputs
Quoting Mathieu Bridon (2018-06-27 03:37:38) > In Python, dictionaries and sets are unordered, and as a result their > is no guarantee that running this script twice will produce the same > output. Small nit: in python < 3.7 dicts are unordered. In cpython 3.6 (and pypy since forever?) they are, in python 3.7 dicts are guaranteed to be ordered, and OrderedDict is just an alias for dict. > > Using ordered dicts and explicitly sorting items makes the build more > reproducible, and will make it possible to verify that we're not > breaking anything when we move the build scripts to Python 3. > --- > src/amd/common/sid_tables.py | 2 +- > src/compiler/nir/nir_algebraic.py | 3 ++- > src/compiler/nir/nir_opt_algebraic.py | 3 ++- > src/mapi/glapi/gen/glX_proto_size.py | 2 +- > src/mapi/glapi/gen/gl_XML.py | 3 ++- > 5 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/src/amd/common/sid_tables.py b/src/amd/common/sid_tables.py > index 4e53acefa4..ca90f82535 100644 > --- a/src/amd/common/sid_tables.py > +++ b/src/amd/common/sid_tables.py > @@ -65,7 +65,7 @@ class StringTable: > fragments = [ > '"%s\\0" /* %s */' % ( > te[0].encode('string_escape'), > -', '.join(str(idx) for idx in te[2]) > +', '.join(str(idx) for idx in sorted(te[2])) > ) > for te in self.table > ] > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index d6784df004..847c59dbd8 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -25,6 +25,7 @@ > > from __future__ import print_function > import ast > +from collections import OrderedDict > import itertools > import struct > import sys > @@ -601,7 +602,7 @@ ${pass_name}(nir_shader *shader) > > class AlgebraicPass(object): > def __init__(self, pass_name, transforms): > - self.xform_dict = {} > + self.xform_dict = OrderedDict() >self.pass_name = pass_name > >error = False > diff --git a/src/compiler/nir/nir_opt_algebraic.py > b/src/compiler/nir/nir_opt_algebraic.py > index db907df854..2f1cba398f 100644 > --- a/src/compiler/nir/nir_opt_algebraic.py > +++ b/src/compiler/nir/nir_opt_algebraic.py > @@ -23,6 +23,7 @@ > # Authors: > #Jason Ekstrand (ja...@jlekstrand.net) > > +from collections import OrderedDict > import nir_algebraic > import itertools > > @@ -628,7 +629,7 @@ optimizations = [ > 'options->lower_unpack_snorm_4x8'), > ] > > -invert = {'feq': 'fne', 'fne': 'feq', 'fge': 'flt', 'flt': 'fge' } > +invert = OrderedDict([('feq', 'fne'), ('fne', 'feq'), ('fge', 'flt'), > ('flt', 'fge')]) > > for left, right in list(itertools.combinations(invert.keys(), 2)) + > zip(invert.keys(), invert.keys()): > optimizations.append((('inot', ('ior(is_used_once)', (left, a, b), > (right, c, d))), > diff --git a/src/mapi/glapi/gen/glX_proto_size.py > b/src/mapi/glapi/gen/glX_proto_size.py > index e16dbab3e0..8dbb0af86d 100644 > --- a/src/mapi/glapi/gen/glX_proto_size.py > +++ b/src/mapi/glapi/gen/glX_proto_size.py > @@ -191,7 +191,7 @@ class glx_enum_function(object): > > print 'switch( e ) {' > > -for c in self.count: > +for c in sorted(self.count): > for e in self.count[c]: > first = 1 > > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > index a5320e90a1..1bab5fee51 100644 > --- a/src/mapi/glapi/gen/gl_XML.py > +++ b/src/mapi/glapi/gen/gl_XML.py > @@ -24,6 +24,7 @@ > # Authors: > #Ian Romanick > > +from collections import OrderedDict > from decimal import Decimal > import xml.etree.ElementTree as ET > import re, sys, string > @@ -861,7 +862,7 @@ class gl_item_factory(object): > > class gl_api(object): > def __init__(self, factory): > -self.functions_by_name = {} > +self.functions_by_name = OrderedDict() > self.enums_by_name = {} > self.types_by_name = {} > > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 2/5] egl/android: Add Android property for forcing kms_swrast
Hey Tomasz, On 05/07/18 16:16, Tomasz Figa wrote: On Thu, Jul 5, 2018 at 7:07 PM Robert Foss wrote: In order to simplify Android bringup on new devices, provide the property "drm.gpu.force_software" which forces kms_swrast to be used. Signed-off-by: Robert Foss --- src/egl/main/egldriver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c index 218b3daef25..bb9e90c157d 100644 --- a/src/egl/main/egldriver.c +++ b/src/egl/main/egldriver.c @@ -39,6 +39,10 @@ #include #include "c11/threads.h" +#ifdef HAVE_ANDROID_PLATFORM +#include +#endif + #include "egldefines.h" #include "egldisplay.h" #include "egldriver.h" @@ -87,6 +91,12 @@ _eglMatchDriver(_EGLDisplay *dpy) dpy->Options.ForceSoftware = env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false); +#ifdef HAVE_ANDROID_PLATFORM + char prop_val[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.force_software", prop_val, "0"); + dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX); ForceSoftware is an EGLBoolean, which is just an unsigned int, not stdbool, so no implicit conversion into {0, 1} for us here. I think what we want here is dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX) != 0; Ack Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: tools: fix build on old systems
On Thu, Jul 5, 2018 at 7:42 AM Lionel Landwerlin wrote: > > Older system might not have support for memfd_create at the kernel > level. There we won't be able to use aubinator. > > We also initially tried to workaround some libc having the > memfd_create syscall number defined, but not the memfd_create() > function. > > This change fixes the broken build on the travis CI by only compiling > aubinator if memfd_create() is available as part of the libc. memfd_create() was added in glibc-2.27. I think that's too new to rely on. I know aubinator is a developer tool, but I don't think most developers have glibc 2.27 yet. I'd suggest we rename our "memfd_create" functions to avoid the clash with glibc-2.27+ and just use them regardless. > Annoyingly the man page says which should include but It doesn't exist in upstream glibc either. I think the man page is simply wrong. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 1/5] st/dri: Allow kms_swrast to work without a device FD
Hey Tomasz, On 05/07/18 15:07, Tomasz Figa wrote: Hi Emil, Robert, On Thu, Jul 5, 2018 at 9:57 PM Emil Velikov wrote: On 5 July 2018 at 12:32, Robert Foss wrote: Hey Eric! On 05/07/18 12:35, Eric Engestrom wrote: On Thursday, 2018-07-05 12:07:36 +0200, Robert Foss wrote: From: Tomeu Vizoso A KMS device isn't strictly needed for the kms_swrast to work, so stop failing to init if the FD is -1. Also don't call DRM_IOCTL_GET_CAP in that case. This allows the driver to be used in machines where no KMS device at all is present. Signed-off-by: Tomeu Vizoso --- src/gallium/state_trackers/dri/dri2.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c index 58a6757f037..c262c0ca118 100644 --- a/src/gallium/state_trackers/dri/dri2.c +++ b/src/gallium/state_trackers/dri/dri2.c @@ -2189,7 +2189,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) sPriv->driverPrivate = (void *)screen; - if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) + /* We don't really need a device FD if we are soft-rendering */ + if (screen->fd >= 0 && (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) goto free_screen; if (pipe_loader_sw_probe_kms(&screen->dev, fd)) { Aren't you going to use an uninitialised `fd` here if `screen->fd < 0`? From my understanding during dri2_initialize_android(), droid_open_device[1] will always allocate an FD or fail, before a screen is created in dri2_create_screen[2]. But maybe there is some part I don't quite understand. You're tracking a single instance instead of looking at the function in itself. When screen->fd is invalid, fd will be uninitialised. The pipe_loader_sw_probe_kms() call will then use that uninitialised value. I believe a better solution is to have distinct dri/null/kms backends to swrast, instead of hacking it in this way. Some ideas are listed in here [1]. -Emil [1] https://gitlab.collabora.com/tomeu/mesa/commit/54adda6a4d7b5c783d54dfd37d38d1a5a0f3187f First of all, do we really need this patch at all? If you ended up booting Android with Mesa, you must have had a DRM driver for the display anyway, so what prevents you from using it as the KMS device for kms_swrast? Just dropping this patch does indeed cause no regressions. Emil: Do you see any problems with that? Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] radv/winsys: make use of radeon_emit()
Reviewed-by: Bas Nieuwenhuizen On Thu, Jul 5, 2018 at 5:07 PM, Samuel Pitoiset wrote: > v2: - do not use it in the chained path (because cdw isn't incremented) > > Signed-off-by: Samuel Pitoiset > --- > src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 23 ++- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c > b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c > index 848e81924f..2741fc0f3b 100644 > --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c > +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c > @@ -355,7 +355,7 @@ static void radv_amdgpu_cs_grow(struct radeon_cmdbuf > *_cs, size_t min_size) > ib_size = MIN2(ib_size, 0xf); > > while (!cs->base.cdw || (cs->base.cdw & 7) != 4) > - cs->base.buf[cs->base.cdw++] = 0x1000; > + radeon_emit(&cs->base, 0x1000); > > *cs->ib_size_ptr |= cs->base.cdw + 4; > > @@ -389,11 +389,12 @@ static void radv_amdgpu_cs_grow(struct radeon_cmdbuf > *_cs, size_t min_size) > > cs->ws->base.cs_add_buffer(&cs->base, cs->ib_buffer, 8); > > - cs->base.buf[cs->base.cdw++] = PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0); > - cs->base.buf[cs->base.cdw++] = > radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va; > - cs->base.buf[cs->base.cdw++] = > radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va >> 32; > - cs->ib_size_ptr = cs->base.buf + cs->base.cdw; > - cs->base.buf[cs->base.cdw++] = S_3F2_CHAIN(1) | S_3F2_VALID(1); > + radeon_emit(&cs->base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0)); > + radeon_emit(&cs->base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va); > + radeon_emit(&cs->base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va > >> 32); > + radeon_emit(&cs->base, S_3F2_CHAIN(1) | S_3F2_VALID(1)); > + > + cs->ib_size_ptr = cs->base.buf + cs->base.cdw - 1; > > cs->base.buf = (uint32_t *)cs->ib_mapped; > cs->base.cdw = 0; > @@ -407,7 +408,7 @@ static bool radv_amdgpu_cs_finalize(struct radeon_cmdbuf > *_cs) > > if (cs->ws->use_ib_bos) { > while (!cs->base.cdw || (cs->base.cdw & 7) != 0) > - cs->base.buf[cs->base.cdw++] = 0x1000; > + radeon_emit(&cs->base, 0x1000); > > *cs->ib_size_ptr |= cs->base.cdw; > > @@ -590,10 +591,10 @@ static void radv_amdgpu_cs_execute_secondary(struct > radeon_cmdbuf *_parent, > if (parent->base.cdw + 4 > parent->base.max_dw) > radv_amdgpu_cs_grow(&parent->base, 4); > > - parent->base.buf[parent->base.cdw++] = > PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0); > - parent->base.buf[parent->base.cdw++] = > child->ib.ib_mc_address; > - parent->base.buf[parent->base.cdw++] = > child->ib.ib_mc_address >> 32; > - parent->base.buf[parent->base.cdw++] = child->ib.size; > + radeon_emit(&parent->base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, > 0)); > + radeon_emit(&parent->base, child->ib.ib_mc_address); > + radeon_emit(&parent->base, child->ib.ib_mc_address >> 32); > + radeon_emit(&parent->base, child->ib.size); > } else { > if (parent->base.cdw + child->base.cdw > parent->base.max_dw) > radv_amdgpu_cs_grow(&parent->base, child->base.cdw); > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] vma/tests: Fix compilation if limits.h defines PAGE_SIZE
On Thursday, 2018-07-05 15:52:01 +0100, Jon Turney wrote: > per POSIX, limits.h may define PAGE_SIZE when the value is not indeterminate "may define" -> don't you need #ifdef around #undef? > > Cc: Scott D Phillips > Signed-off-by: Jon Turney > --- > src/util/tests/vma/vma_random_test.cpp | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/util/tests/vma/vma_random_test.cpp > b/src/util/tests/vma/vma_random_test.cpp > index de887fead3..c08f3751a4 100644 > --- a/src/util/tests/vma/vma_random_test.cpp > +++ b/src/util/tests/vma/vma_random_test.cpp > @@ -40,6 +40,7 @@ > > namespace { > > +#undef PAGE_SIZE > static const uint64_t PAGE_SIZE = 4096; > > struct allocation { > -- > 2.17.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: tools: fix build on old systems
On Thursday, 2018-07-05 15:40:52 +0100, Lionel Landwerlin wrote: > Older system might not have support for memfd_create at the kernel > level. There we won't be able to use aubinator. > > We also initially tried to workaround some libc having the > memfd_create syscall number defined, but not the memfd_create() > function. > > This change fixes the broken build on the travis CI by only compiling > aubinator if memfd_create() is available as part of the libc. > Annoyingly the man page says which should include but > that header doesn't exist on my system and memfd_create() is instead > defined in bits/mman-shared.h. Hence the new checks... > > Signed-off-by: Lionel Landwerlin > --- > configure.ac| 8 +++- > meson.build | 2 +- > src/intel/Makefile.tools.am | 6 +- > src/intel/tools/aubinator.c | 13 +++-- > src/intel/tools/meson.build | 33 +++-- > 5 files changed, 39 insertions(+), 23 deletions(-) > > diff --git a/configure.ac b/configure.ac > index f135d057365..939585411f9 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -872,10 +872,16 @@ AC_HEADER_MAJOR > AC_CHECK_HEADER([xlocale.h], [DEFINES="$DEFINES -DHAVE_XLOCALE_H"]) > AC_CHECK_HEADER([sys/sysctl.h], [DEFINES="$DEFINES -DHAVE_SYS_SYSCTL_H"]) > AC_CHECK_HEADERS([endian.h]) > +AC_CHECK_HEADERS([sys/memfd.h], [DEFINES="$DEFINES -DHAVE_SYS_MEMFD_H") > AC_CHECK_FUNC([strtof], [DEFINES="$DEFINES -DHAVE_STRTOF"]) > AC_CHECK_FUNC([mkostemp], [DEFINES="$DEFINES -DHAVE_MKOSTEMP"]) > AC_CHECK_FUNC([timespec_get], [DEFINES="$DEFINES -DHAVE_TIMESPEC_GET"]) > -AC_CHECK_FUNC([memfd_create], [DEFINES="$DEFINES -DHAVE_MEMFD_CREATE"]) > +AC_CHECK_FUNC([memfd_create], [MEMFD_CREATE=yes], [MEMFD_CREATE=no]) > + > +AM_CONDITIONAL(HAVE_MEMFD_CREATE, test "x$MEMFD_CREATE" = xyes) > +if test "x$MEMFD_CREATE" = xyes; then > + DEFINES="$DEFINES -DHAVE_MEMFD_CREATE" > +fi > > AC_MSG_CHECKING([whether strtod has locale support]) > AC_LINK_IFELSE([AC_LANG_SOURCE([[ > diff --git a/meson.build b/meson.build > index b2722c71e5b..89f17128c03 100644 > --- a/meson.build > +++ b/meson.build > @@ -956,7 +956,7 @@ elif cc.has_header_symbol('sys/mkdev.h', 'major') >pre_args += '-DMAJOR_IN_MKDEV' > endif > > -foreach h : ['xlocale.h', 'sys/sysctl.h', 'linux/futex.h', 'endian.h'] > +foreach h : ['xlocale.h', 'sys/sysctl.h', 'linux/futex.h', 'endian.h', > 'sys/memfd.h'] >if cc.compiles('#include <@0@>'.format(h), name : '@0@'.format(h)) > pre_args += '-DHAVE_@0@'.format(h.to_upper().underscorify()) >endif > diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am > index b00cc8cc2cb..16cc1095f62 100644 > --- a/src/intel/Makefile.tools.am > +++ b/src/intel/Makefile.tools.am > @@ -19,8 +19,12 @@ > # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > # IN THE SOFTWARE. > > +if HAVE_MEMFD_CREATE > +noinst_PROGRAMS += \ > + tools/aubinator > +endif > + > noinst_PROGRAMS += \ > - tools/aubinator \ > tools/aubinator_error_decode > > tools_aubinator_SOURCES = \ > diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c > index 8989d558b66..24ec1a276d9 100644 > --- a/src/intel/tools/aubinator.c > +++ b/src/intel/tools/aubinator.c > @@ -36,6 +36,9 @@ > #include > #include > #include > +#ifdef HAVE_SYS_MEMFD_H > +#include > +#endif > > #include "util/list.h" > #include "util/macros.h" > @@ -46,16 +49,6 @@ > #include "common/gen_gem.h" > #include "intel_aub.h" > > -#ifndef HAVE_MEMFD_CREATE > -#include > - > -static inline int > -memfd_create(const char *name, unsigned int flags) > -{ > - return syscall(SYS_memfd_create, name, flags); > -} > -#endif > - > /* Below is the only command missing from intel_aub.h in libdrm > * So, reuse intel_aub.h from libdrm and #define the > * AUB_MI_BATCH_BUFFER_END as below > diff --git a/src/intel/tools/meson.build b/src/intel/tools/meson.build > index 705a353f26a..717f03c9002 100644 > --- a/src/intel/tools/meson.build > +++ b/src/intel/tools/meson.build > @@ -18,16 +18,29 @@ > # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > THE > # SOFTWARE. > > -aubinator = executable( > - 'aubinator', > - files('aubinator.c', 'intel_aub.h'), > - dependencies : [dep_expat, dep_zlib, dep_dl, dep_thread, dep_m], > - include_directories : [inc_common, inc_intel], > - link_with : [libintel_common, libintel_compiler, libintel_dev, > libmesa_util], > - c_args : [c_vis_args, no_override_init_args], > - build_by_default : with_tools.contains('intel'), > - install : with_tools.contains('intel'), > -) > +has_memfd_create = cc.compiles('''#include > + int main() { > + return memfd_create("", 0); > + }''', > + name : 'memfd create') or > + cc.compiles('''#include > +
[Mesa-dev] [PATCH v2] radv/winsys: make use of radeon_emit()
v2: - do not use it in the chained path (because cdw isn't incremented) Signed-off-by: Samuel Pitoiset --- src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 23 ++- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c index 848e81924f..2741fc0f3b 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c @@ -355,7 +355,7 @@ static void radv_amdgpu_cs_grow(struct radeon_cmdbuf *_cs, size_t min_size) ib_size = MIN2(ib_size, 0xf); while (!cs->base.cdw || (cs->base.cdw & 7) != 4) - cs->base.buf[cs->base.cdw++] = 0x1000; + radeon_emit(&cs->base, 0x1000); *cs->ib_size_ptr |= cs->base.cdw + 4; @@ -389,11 +389,12 @@ static void radv_amdgpu_cs_grow(struct radeon_cmdbuf *_cs, size_t min_size) cs->ws->base.cs_add_buffer(&cs->base, cs->ib_buffer, 8); - cs->base.buf[cs->base.cdw++] = PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0); - cs->base.buf[cs->base.cdw++] = radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va; - cs->base.buf[cs->base.cdw++] = radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va >> 32; - cs->ib_size_ptr = cs->base.buf + cs->base.cdw; - cs->base.buf[cs->base.cdw++] = S_3F2_CHAIN(1) | S_3F2_VALID(1); + radeon_emit(&cs->base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0)); + radeon_emit(&cs->base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va); + radeon_emit(&cs->base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va >> 32); + radeon_emit(&cs->base, S_3F2_CHAIN(1) | S_3F2_VALID(1)); + + cs->ib_size_ptr = cs->base.buf + cs->base.cdw - 1; cs->base.buf = (uint32_t *)cs->ib_mapped; cs->base.cdw = 0; @@ -407,7 +408,7 @@ static bool radv_amdgpu_cs_finalize(struct radeon_cmdbuf *_cs) if (cs->ws->use_ib_bos) { while (!cs->base.cdw || (cs->base.cdw & 7) != 0) - cs->base.buf[cs->base.cdw++] = 0x1000; + radeon_emit(&cs->base, 0x1000); *cs->ib_size_ptr |= cs->base.cdw; @@ -590,10 +591,10 @@ static void radv_amdgpu_cs_execute_secondary(struct radeon_cmdbuf *_parent, if (parent->base.cdw + 4 > parent->base.max_dw) radv_amdgpu_cs_grow(&parent->base, 4); - parent->base.buf[parent->base.cdw++] = PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0); - parent->base.buf[parent->base.cdw++] = child->ib.ib_mc_address; - parent->base.buf[parent->base.cdw++] = child->ib.ib_mc_address >> 32; - parent->base.buf[parent->base.cdw++] = child->ib.size; + radeon_emit(&parent->base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0)); + radeon_emit(&parent->base, child->ib.ib_mc_address); + radeon_emit(&parent->base, child->ib.ib_mc_address >> 32); + radeon_emit(&parent->base, child->ib.size); } else { if (parent->base.cdw + child->base.cdw > parent->base.max_dw) radv_amdgpu_cs_grow(&parent->base, child->base.cdw); -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] vma/tests: Fix compilation if limits.h defines PAGE_SIZE
per POSIX, limits.h may define PAGE_SIZE when the value is not indeterminate Cc: Scott D Phillips Signed-off-by: Jon Turney --- src/util/tests/vma/vma_random_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/tests/vma/vma_random_test.cpp b/src/util/tests/vma/vma_random_test.cpp index de887fead3..c08f3751a4 100644 --- a/src/util/tests/vma/vma_random_test.cpp +++ b/src/util/tests/vma/vma_random_test.cpp @@ -40,6 +40,7 @@ namespace { +#undef PAGE_SIZE static const uint64_t PAGE_SIZE = 4096; struct allocation { -- 2.17.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/26] python: Use explicit integer divisions
On Thursday, 2018-07-05 15:17:41 +0200, Mathieu Bridon wrote: > In Python 2, divisions return an integer: > > >>> 32 / 4 > 8 > > In Python 3 though, they return floats: > > >>> 32 / 4 > 8.0 > > Explicitly converting to integers make the scripts compatible with both > Python 2 and 3. I think an explicit `math.floor()` would be better than using the implicit truncation of casting to an int. (and I'm not 100% sure we want floor() on all these) > > Signed-off-by: Mathieu Bridon > --- > src/gallium/auxiliary/util/u_format_pack.py | 2 +- > src/gallium/auxiliary/util/u_format_parse.py | 4 ++-- > src/mapi/glapi/gen/glX_proto_send.py | 2 +- > src/mesa/main/format_info.py | 2 +- > src/mesa/main/format_pack.py | 6 +++--- > src/mesa/main/format_unpack.py | 6 +++--- > 6 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/src/gallium/auxiliary/util/u_format_pack.py > b/src/gallium/auxiliary/util/u_format_pack.py > index 7a952a48b3..1cfd85fb7f 100644 > --- a/src/gallium/auxiliary/util/u_format_pack.py > +++ b/src/gallium/auxiliary/util/u_format_pack.py > @@ -240,7 +240,7 @@ def value_to_native(type, value): > return truncate_mantissa(value, 23) > return value > if type.type == FIXED: > -return int(value * (1 << (type.size/2))) > +return int(value * (1 << int(type.size/2))) > if not type.norm: > return int(value) > if type.type == UNSIGNED: > diff --git a/src/gallium/auxiliary/util/u_format_parse.py > b/src/gallium/auxiliary/util/u_format_parse.py > index c0456f6d15..315c771081 100644 > --- a/src/gallium/auxiliary/util/u_format_parse.py > +++ b/src/gallium/auxiliary/util/u_format_parse.py > @@ -76,7 +76,7 @@ class Channel: > if self.type == FLOAT: > return VERY_LARGE > if self.type == FIXED: > -return (1 << (self.size/2)) - 1 > +return (1 << int(self.size/2)) - 1 > if self.norm: > return 1 > if self.type == UNSIGNED: > @@ -90,7 +90,7 @@ class Channel: > if self.type == FLOAT: > return -VERY_LARGE > if self.type == FIXED: > -return -(1 << (self.size/2)) > +return -(1 << int(self.size/2)) > if self.type == UNSIGNED: > return 0 > if self.norm: > diff --git a/src/mapi/glapi/gen/glX_proto_send.py > b/src/mapi/glapi/gen/glX_proto_send.py > index a920ecc012..28f8a0d67b 100644 > --- a/src/mapi/glapi/gen/glX_proto_send.py > +++ b/src/mapi/glapi/gen/glX_proto_send.py > @@ -809,7 +809,7 @@ generic_%u_byte( GLint rop, const void * ptr ) > # Dividing by the array size (1 for > # non-arrays) gives us this. > > -s = p.size() / p.get_element_count() > +s = int(p.size() / p.get_element_count()) > print(" %s __glXReadReply(dpy, %s, %s, %s);" % > (return_str, s, p.name, aa)) > got_reply = 1 > > diff --git a/src/mesa/main/format_info.py b/src/mesa/main/format_info.py > index bbecaa2451..285bf13e1b 100644 > --- a/src/mesa/main/format_info.py > +++ b/src/mesa/main/format_info.py > @@ -198,7 +198,7 @@ for fmat in formats: >chan = fmat.array_element() >norm = chan.norm or chan.type == parser.FLOAT >print(' .ArrayFormat = MESA_ARRAY_FORMAT({0}),'.format(', '.join([ > - str(chan.size / 8), > + str(int(chan.size / 8)), > str(int(chan.sign)), > str(int(chan.type == parser.FLOAT)), > str(int(norm)), > diff --git a/src/mesa/main/format_pack.py b/src/mesa/main/format_pack.py > index d3c8d24acd..3b69580a93 100644 > --- a/src/mesa/main/format_pack.py > +++ b/src/mesa/main/format_pack.py > @@ -356,7 +356,7 @@ _mesa_pack_ubyte_rgba_row(mesa_format format, GLuint n, > case ${f.name}: >for (i = 0; i < n; ++i) { > pack_ubyte_${f.short_name()}(src[i], d); > - d += ${f.block_size() / 8}; > + d += ${int(f.block_size() / 8)}; >} >break; > %endfor > @@ -388,7 +388,7 @@ _mesa_pack_uint_rgba_row(mesa_format format, GLuint n, > case ${f.name}: >for (i = 0; i < n; ++i) { > pack_uint_${f.short_name()}(src[i], d); > - d += ${f.block_size() / 8}; > + d += ${int(f.block_size() / 8)}; >} >break; > %endfor > @@ -418,7 +418,7 @@ _mesa_pack_float_rgba_row(mesa_format format, GLuint n, > case ${f.name}: >for (i = 0; i < n; ++i) { > pack_float_${f.short_name()}(src[i], d); > - d += ${f.block_size() / 8}; > + d += ${int(f.block_size() / 8)}; >} >break; > %endfor > diff --git a/src/mesa/main/format_unpack.py b/src/mesa/main/format_unpack.py > index 286c08e621..feddaed5cd 100644 > --- a/src/mesa/main/format_unpack.py > +++ b/src/mesa/main/format_unpack.py > @@ -322,7 +322,7 @@ _mes
[Mesa-dev] [PATCH] intel: tools: fix build on old systems
Older system might not have support for memfd_create at the kernel level. There we won't be able to use aubinator. We also initially tried to workaround some libc having the memfd_create syscall number defined, but not the memfd_create() function. This change fixes the broken build on the travis CI by only compiling aubinator if memfd_create() is available as part of the libc. Annoyingly the man page says which should include but that header doesn't exist on my system and memfd_create() is instead defined in bits/mman-shared.h. Hence the new checks... Signed-off-by: Lionel Landwerlin --- configure.ac| 8 +++- meson.build | 2 +- src/intel/Makefile.tools.am | 6 +- src/intel/tools/aubinator.c | 13 +++-- src/intel/tools/meson.build | 33 +++-- 5 files changed, 39 insertions(+), 23 deletions(-) diff --git a/configure.ac b/configure.ac index f135d057365..939585411f9 100644 --- a/configure.ac +++ b/configure.ac @@ -872,10 +872,16 @@ AC_HEADER_MAJOR AC_CHECK_HEADER([xlocale.h], [DEFINES="$DEFINES -DHAVE_XLOCALE_H"]) AC_CHECK_HEADER([sys/sysctl.h], [DEFINES="$DEFINES -DHAVE_SYS_SYSCTL_H"]) AC_CHECK_HEADERS([endian.h]) +AC_CHECK_HEADERS([sys/memfd.h], [DEFINES="$DEFINES -DHAVE_SYS_MEMFD_H") AC_CHECK_FUNC([strtof], [DEFINES="$DEFINES -DHAVE_STRTOF"]) AC_CHECK_FUNC([mkostemp], [DEFINES="$DEFINES -DHAVE_MKOSTEMP"]) AC_CHECK_FUNC([timespec_get], [DEFINES="$DEFINES -DHAVE_TIMESPEC_GET"]) -AC_CHECK_FUNC([memfd_create], [DEFINES="$DEFINES -DHAVE_MEMFD_CREATE"]) +AC_CHECK_FUNC([memfd_create], [MEMFD_CREATE=yes], [MEMFD_CREATE=no]) + +AM_CONDITIONAL(HAVE_MEMFD_CREATE, test "x$MEMFD_CREATE" = xyes) +if test "x$MEMFD_CREATE" = xyes; then + DEFINES="$DEFINES -DHAVE_MEMFD_CREATE" +fi AC_MSG_CHECKING([whether strtod has locale support]) AC_LINK_IFELSE([AC_LANG_SOURCE([[ diff --git a/meson.build b/meson.build index b2722c71e5b..89f17128c03 100644 --- a/meson.build +++ b/meson.build @@ -956,7 +956,7 @@ elif cc.has_header_symbol('sys/mkdev.h', 'major') pre_args += '-DMAJOR_IN_MKDEV' endif -foreach h : ['xlocale.h', 'sys/sysctl.h', 'linux/futex.h', 'endian.h'] +foreach h : ['xlocale.h', 'sys/sysctl.h', 'linux/futex.h', 'endian.h', 'sys/memfd.h'] if cc.compiles('#include <@0@>'.format(h), name : '@0@'.format(h)) pre_args += '-DHAVE_@0@'.format(h.to_upper().underscorify()) endif diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am index b00cc8cc2cb..16cc1095f62 100644 --- a/src/intel/Makefile.tools.am +++ b/src/intel/Makefile.tools.am @@ -19,8 +19,12 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS # IN THE SOFTWARE. +if HAVE_MEMFD_CREATE +noinst_PROGRAMS += \ + tools/aubinator +endif + noinst_PROGRAMS += \ - tools/aubinator \ tools/aubinator_error_decode tools_aubinator_SOURCES = \ diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c index 8989d558b66..24ec1a276d9 100644 --- a/src/intel/tools/aubinator.c +++ b/src/intel/tools/aubinator.c @@ -36,6 +36,9 @@ #include #include #include +#ifdef HAVE_SYS_MEMFD_H +#include +#endif #include "util/list.h" #include "util/macros.h" @@ -46,16 +49,6 @@ #include "common/gen_gem.h" #include "intel_aub.h" -#ifndef HAVE_MEMFD_CREATE -#include - -static inline int -memfd_create(const char *name, unsigned int flags) -{ - return syscall(SYS_memfd_create, name, flags); -} -#endif - /* Below is the only command missing from intel_aub.h in libdrm * So, reuse intel_aub.h from libdrm and #define the * AUB_MI_BATCH_BUFFER_END as below diff --git a/src/intel/tools/meson.build b/src/intel/tools/meson.build index 705a353f26a..717f03c9002 100644 --- a/src/intel/tools/meson.build +++ b/src/intel/tools/meson.build @@ -18,16 +18,29 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. -aubinator = executable( - 'aubinator', - files('aubinator.c', 'intel_aub.h'), - dependencies : [dep_expat, dep_zlib, dep_dl, dep_thread, dep_m], - include_directories : [inc_common, inc_intel], - link_with : [libintel_common, libintel_compiler, libintel_dev, libmesa_util], - c_args : [c_vis_args, no_override_init_args], - build_by_default : with_tools.contains('intel'), - install : with_tools.contains('intel'), -) +has_memfd_create = cc.compiles('''#include + int main() { + return memfd_create("", 0); + }''', + name : 'memfd create') or + cc.compiles('''#include + int main() { + return memfd_create("", 0); + }''', + name : 'memfd create') + +if has_memfd_create + aubinator = executable( +'aubinator', +files('aubinator.c', 'intel_aub.h'), +dependencies : [dep
Re: [Mesa-dev] [PATCH rfc 0/3] Be able to disable EGL/GLX_EXT_buffer_age
Hi Emil, On Thu, Jul 5, 2018 at 9:54 PM Emil Velikov wrote: > > On 5 July 2018 at 14:31, Emil Velikov wrote: > > Hi Qiang Yu > > > > On 5 July 2018 at 03:31, Qiang Yu wrote: > >> For GPU like ARM mali Utgard EGL/GLX_EXT_buffer_age will make > >> performace worse. But mesa has no way to disable it. > >> > >> This patch series make driver be able to disable it and add a > >> gallium pipe cap for gallium driver usage. Due to currently > >> only out of tree lima driver need it, and not sure if this is > >> the right way to disable it, so I send this RFC before lima be > >> able to upstream. > >> > > Pretty sure we already have a way to handle that. Look for > > glx_disable_ext_buffer_age - it was introduced for the VMWare driver. > > Although we should: > > a) tweak the name - kind of split if we need to > > b) add glx/dri2 and egl support > > > Looking at the implementation - it uses a driver query, meaning that > one could enable it at a later stage. > No need to worry if the user has old drirc/etc as with current solution. Yes, use drirc to disable buffer age means it can be enabled by changing the config (driver has to support it). But GPU like mali just don't want to support it at all (need extra code do bad things). Regards, Qiang > > Having two ways of doing the same thing is a bad idea. > Perhaps we can remove the drirc implementation and use this instead? > > Thomas, what do you think? > > Thanks > Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/26] python: Use range() instead of xrange()
On Thursday, 2018-07-05 15:17:40 +0200, Mathieu Bridon wrote: > Python 2 has a range() function which returns a list, and an xrange() > one which returns an iterator. > > Python 3 lost the function returning a list, and renamed the function > returning an iterator as range(). > > As a result, using range() makes the scripts compatible with both Python > versions 2 and 3. > > Signed-off-by: Mathieu Bridon > --- > src/amd/vulkan/radv_entrypoints_gen.py | 2 +- > src/broadcom/cle/gen_pack_header.py | 2 +- > src/compiler/glsl/ir_expression_operation.py | 2 +- > src/compiler/nir/nir_opcodes.py | 4 ++-- > src/intel/vulkan/anv_entrypoints_gen.py | 2 +- > src/mapi/glapi/gen/glX_proto_send.py | 2 +- > src/mapi/glapi/gen/gl_XML.py | 2 +- > src/mapi/glapi/gen/gl_gentable.py| 4 ++-- > src/mapi/mapi_abi.py | 2 +- > src/mesa/main/format_parser.py | 4 ++-- > 10 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/src/amd/vulkan/radv_entrypoints_gen.py > b/src/amd/vulkan/radv_entrypoints_gen.py > index 9c4dfd02a0..ca022bcbb0 100644 > --- a/src/amd/vulkan/radv_entrypoints_gen.py > +++ b/src/amd/vulkan/radv_entrypoints_gen.py > @@ -136,7 +136,7 @@ static const struct string_map_entry string_map_entries[] > = { > /* Hash table stats: > * size ${len(strmap.sorted_strings)} entries > * collisions entries: > -% for i in xrange(10): > +% for i in range(10): > * ${i}${'+' if i == 9 else ' '} ${strmap.collisions[i]} > % endfor > */ > diff --git a/src/broadcom/cle/gen_pack_header.py > b/src/broadcom/cle/gen_pack_header.py > index c6e1c564e6..8ad54464cb 100644 > --- a/src/broadcom/cle/gen_pack_header.py > +++ b/src/broadcom/cle/gen_pack_header.py > @@ -216,7 +216,7 @@ class Group(object): > first_byte = field.start // 8 > last_byte = field.end // 8 > > -for b in xrange(first_byte, last_byte + 1): > +for b in range(first_byte, last_byte + 1): > if not b in bytes: > bytes[b] = self.Byte() > > diff --git a/src/compiler/glsl/ir_expression_operation.py > b/src/compiler/glsl/ir_expression_operation.py > index b3dac3da3f..16b98690a6 100644 > --- a/src/compiler/glsl/ir_expression_operation.py > +++ b/src/compiler/glsl/ir_expression_operation.py > @@ -116,7 +116,7 @@ constant_template_common = mako.template.Template("""\ > constant_template_vector_scalar = mako.template.Template("""\ > case ${op.get_enum_name()}: > % if "mixed" in op.flags: > -% for i in xrange(op.num_operands): > +% for i in range(op.num_operands): >assert(op[${i}]->type->base_type == ${op.source_types[0].glsl_type} || > % for src_type in op.source_types[1:-1]: > op[${i}]->type->base_type == ${src_type.glsl_type} || > diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py > index 3c3316dcaa..b03c5da2ea 100644 > --- a/src/compiler/nir/nir_opcodes.py > +++ b/src/compiler/nir/nir_opcodes.py > @@ -367,8 +367,8 @@ for (unsigned bit = 0; bit < bit_size; bit++) { > """) > > > -for i in xrange(1, 5): > - for j in xrange(1, 5): > +for i in range(1, 5): > + for j in range(1, 5): >unop_horiz("fnoise{0}_{1}".format(i, j), i, tfloat, j, tfloat, "0.0f") > > > diff --git a/src/intel/vulkan/anv_entrypoints_gen.py > b/src/intel/vulkan/anv_entrypoints_gen.py > index 8a37336496..5e2cd0740a 100644 > --- a/src/intel/vulkan/anv_entrypoints_gen.py > +++ b/src/intel/vulkan/anv_entrypoints_gen.py > @@ -145,7 +145,7 @@ static const struct string_map_entry string_map_entries[] > = { > /* Hash table stats: > * size ${len(strmap.sorted_strings)} entries > * collisions entries: > -% for i in xrange(10): > +% for i in range(10): > * ${i}${'+' if i == 9 else ' '} ${strmap.collisions[i]} > % endfor > */ > diff --git a/src/mapi/glapi/gen/glX_proto_send.py > b/src/mapi/glapi/gen/glX_proto_send.py > index fba2f0cc1e..a920ecc012 100644 > --- a/src/mapi/glapi/gen/glX_proto_send.py > +++ b/src/mapi/glapi/gen/glX_proto_send.py > @@ -392,7 +392,7 @@ static const struct proc_pair > _glapi_proc proc; > } proc_pairs[%d] = {""" % len(procs)) > names = sorted(procs.keys()) > -for i in xrange(len(names)): > +for i in range(len(names)): > comma = ',' if i < len(names) - 1 else '' > print(' { "%s", (_glapi_proc) gl%s }%s' % (names[i], > procs[names[i]], comma)) > print("""}; > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > index bfbb1ec6e0..96dc1b3c12 100644 > --- a/src/mapi/glapi/gen/gl_XML.py > +++ b/src/mapi/glapi/gen/gl_XML.py > @@ -834,7 +834,7 @@ class gl_function( gl_item ): > versions. > """ > result = [] > -for entry_point, api_to_ver in self.entry_point_api_map.iteritems(): > +for entry_point, api_to_ver in
Re: [Mesa-dev] [PATCH 08/26] python: Better use iterators
On Thursday, 2018-07-05 15:17:39 +0200, Mathieu Bridon wrote: > In Python 2, iterators had a .next() method. > > In Python 3, instead they have a .__next__() method, which is > automatically called by the next() builtin. > > In addition, it is better to use the iter() builtin to create an > iterator, rather than calling its __iter__() method. Not all that familiar with that stuff, so Cc'ing Dylan who I think has dealt with __next__() et al in the past. > > These were also introduced in Python 2.6, so using it makes the script > compatible with Python 2 and 3. > > Signed-off-by: Mathieu Bridon > --- > src/compiler/glsl/ir_expression_operation.py | 4 +++- > src/compiler/nir/nir_algebraic.py| 4 ++-- > src/mapi/glapi/gen/glX_XML.py| 17 + > src/mapi/glapi/gen/gl_XML.py | 12 ++-- > 4 files changed, 20 insertions(+), 17 deletions(-) > > diff --git a/src/compiler/glsl/ir_expression_operation.py > b/src/compiler/glsl/ir_expression_operation.py > index d8542925a0..b3dac3da3f 100644 > --- a/src/compiler/glsl/ir_expression_operation.py > +++ b/src/compiler/glsl/ir_expression_operation.py > @@ -62,7 +62,7 @@ class type_signature_iter(object): > def __iter__(self): >return self > > - def next(self): > + def __next__(self): >if self.i < len(self.source_types): > i = self.i > self.i += 1 > @@ -76,6 +76,8 @@ class type_signature_iter(object): >else: > raise StopIteration() > > + next = __next__ > + > > uint_type = type("unsigned", "u", "GLSL_TYPE_UINT") > int_type = type("int", "i", "GLSL_TYPE_INT") > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index 8c0b530f69..fda72d3c69 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -56,7 +56,7 @@ class VarSet(object): > def __getitem__(self, name): >if name not in self.names: > assert not self.immutable, "Unknown replacement variable: " + name > - self.names[name] = self.ids.next() > + self.names[name] = next(self.ids) > >return self.names[name] > > @@ -468,7 +468,7 @@ condition_list = ['true'] > > class SearchAndReplace(object): > def __init__(self, transform): > - self.id = _optimization_ids.next() > + self.id = next(_optimization_ids) > >search = transform[0] >replace = transform[1] > diff --git a/src/mapi/glapi/gen/glX_XML.py b/src/mapi/glapi/gen/glX_XML.py > index bbcecd6023..4ec8cd766f 100644 > --- a/src/mapi/glapi/gen/glX_XML.py > +++ b/src/mapi/glapi/gen/glX_XML.py > @@ -296,7 +296,7 @@ class glx_function(gl_XML.gl_function): > parameters.extend( temp[1] ) > if include_variable_parameters: > parameters.extend( temp[2] ) > -return parameters.__iter__() > +return iter(parameters) > > > def parameterIterateCounters(self): > @@ -304,7 +304,7 @@ class glx_function(gl_XML.gl_function): > for name in self.counter_list: > temp.append( self.parameters_by_name[ name ] ) > > -return temp.__iter__() > +return iter(temp) > > > def parameterIterateOutputs(self): > @@ -547,13 +547,14 @@ class glx_function_iterator(object): > return self > > > -def next(self): > -f = self.iterator.next() > +def __next__(self): > +while True: > +f = next(self.iterator) > > -if f.client_supported_for_indirect(): > -return f > -else: > -return self.next() > +if f.client_supported_for_indirect(): > +return f > + > +next = __next__ > > > class glx_api(gl_XML.gl_api): > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > index 3a4f59221e..bfbb1ec6e0 100644 > --- a/src/mapi/glapi/gen/gl_XML.py > +++ b/src/mapi/glapi/gen/gl_XML.py > @@ -782,9 +782,9 @@ class gl_function( gl_item ): > > def parameterIterator(self, name = None): > if name is not None: > -return self.entry_point_parameters[name].__iter__(); > +return iter(self.entry_point_parameters[name]); > else: > -return self.parameters.__iter__(); > +return iter(self.parameters); > > > def get_parameter_string(self, entrypoint = None): > @@ -996,7 +996,7 @@ class gl_api(object): > for name in names: > functions.append(lists[func_cat_type][key][name]) > > -return functions.__iter__() > +return iter(functions) > > > def functionIterateByOffset(self): > @@ -1017,7 +1017,7 @@ class gl_api(object): > if temp[i]: > list.append(temp[i]) > > -return list.__iter__(); > +return iter(list); > > > def functionIterateAll(self): > @@ -1031,7 +1031,7 @@ class gl_api(object): > for enum in keys
Re: [Mesa-dev] [PATCH 07/26] python: Better sort dictionary keys/values
On Thursday, 2018-07-05 15:17:38 +0200, Mathieu Bridon wrote: > In Python 2, dict.keys() and dict.values() both return a list, which can > be sorted in two ways: > > * l.sort() modifies the list in-place; > * sorted(l) returns a new, sorted list; > > In Python 3, dict.keys() and dict.values() do not return lists any more, > but iterators. Iterators do not have a .sort() method. > > This commit moves the build scripts to using sorted() on dict keys and > values, which makes them compatible with both Python 2 and Python 3. > > Signed-off-by: Mathieu Bridon With the two hunks from patch 4 moved here, Reviewed-by: Eric Engestrom > --- > src/mapi/glapi/gen/glX_proto_send.py | 3 +-- > src/mapi/glapi/gen/glX_proto_size.py | 6 ++ > src/mapi/glapi/gen/gl_XML.py | 12 > src/mapi/glapi/gen/gl_procs.py | 3 +-- > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/src/mapi/glapi/gen/glX_proto_send.py > b/src/mapi/glapi/gen/glX_proto_send.py > index c389872044..fba2f0cc1e 100644 > --- a/src/mapi/glapi/gen/glX_proto_send.py > +++ b/src/mapi/glapi/gen/glX_proto_send.py > @@ -391,8 +391,7 @@ static const struct proc_pair > const char *name; > _glapi_proc proc; > } proc_pairs[%d] = {""" % len(procs)) > -names = procs.keys() > -names.sort() > +names = sorted(procs.keys()) > for i in xrange(len(names)): > comma = ',' if i < len(names) - 1 else '' > print(' { "%s", (_glapi_proc) gl%s }%s' % (names[i], > procs[names[i]], comma)) > diff --git a/src/mapi/glapi/gen/glX_proto_size.py > b/src/mapi/glapi/gen/glX_proto_size.py > index 18a6f2e502..2a843c3e24 100644 > --- a/src/mapi/glapi/gen/glX_proto_size.py > +++ b/src/mapi/glapi/gen/glX_proto_size.py > @@ -208,8 +208,7 @@ class glx_enum_function(object): > for enum_obj in self.enums[e]: > list[ enum_obj.priority() ] = enum_obj.name > > -keys = list.keys() > -keys.sort() > +keys = sorted(list.keys()) > for k in keys: > j = list[k] > if first: > @@ -275,8 +274,7 @@ class glx_server_enum_function(glx_enum_function): > o = f.offset_of( param_name ) > foo[o] = param_name > > -keys = foo.keys() > -keys.sort() > +keys = sorted(foo.keys()) > for o in keys: > p = f.parameters_by_name[ foo[o] ] > > diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py > index fcf8e62d3f..3a4f59221e 100644 > --- a/src/mapi/glapi/gen/gl_XML.py > +++ b/src/mapi/glapi/gen/gl_XML.py > @@ -988,12 +988,10 @@ class gl_api(object): > > functions = [] > for func_cat_type in range(0,4): > -keys = lists[func_cat_type].keys() > -keys.sort() > +keys = sorted(lists[func_cat_type].keys()) > > for key in keys: > -names = lists[func_cat_type][key].keys() > -names.sort() > +names = sorted(lists[func_cat_type][key].keys()) > > for name in names: > functions.append(lists[func_cat_type][key][name]) > @@ -1027,8 +1025,7 @@ class gl_api(object): > > > def enumIterateByName(self): > -keys = self.enums_by_name.keys() > -keys.sort() > +keys = sorted(self.enums_by_name.keys()) > > list = [] > for enum in keys: > @@ -1047,8 +1044,7 @@ class gl_api(object): > > list = [] > for cat_type in range(0,4): > -keys = self.categories[cat_type].keys() > -keys.sort() > +keys = sorted(self.categories[cat_type].keys()) > > for key in keys: > list.append(self.categories[cat_type][key]) > diff --git a/src/mapi/glapi/gen/gl_procs.py b/src/mapi/glapi/gen/gl_procs.py > index 5718f42ab6..4bd3321610 100644 > --- a/src/mapi/glapi/gen/gl_procs.py > +++ b/src/mapi/glapi/gen/gl_procs.py > @@ -144,8 +144,7 @@ typedef struct { > print('') > print('/* OpenGL ES specific prototypes */') > print('') > -keys = categories.keys() > -keys.sort() > +keys = sorted(categories.keys()) > for key in keys: > print('/* category %s */' % key) > print("\n".join(categories[key])) > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 3/5] platform/android: Enable kms_swrast fallback
Hi Rob, On Thu, Jul 5, 2018 at 7:07 PM Robert Foss wrote: > > Add support for the ForceSoftware option, which is togglable > on the Android platform through setting the "drm.gpu.force_software" > property to a non-zero value. > > kms_swrast is also enabled as a fallback for when a driver is not > able to be loaded for for a drm node that was opened. > > Signed-off-by: Robert Foss > --- > src/egl/drivers/dri2/platform_android.c | 26 + > 1 file changed, 18 insertions(+), 8 deletions(-) Thanks for the patch. Please see my comments inline. > > diff --git a/src/egl/drivers/dri2/platform_android.c > b/src/egl/drivers/dri2/platform_android.c > index 92b2d2b343e..bc644c25bf9 100644 > --- a/src/egl/drivers/dri2/platform_android.c > +++ b/src/egl/drivers/dri2/platform_android.c > @@ -1193,17 +1193,21 @@ static const __DRIextension > *droid_image_loader_extensions[] = { > }; > > EGLBoolean > -droid_load_driver(_EGLDisplay *disp) > +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) > { > struct dri2_egl_display *dri2_dpy = disp->DriverData; > const char *err; > > - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); > + if (force_software) { > + dri2_dpy->driver_name = strdup("kms_swrast"); > + } else { > + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); > + } > + > if (dri2_dpy->driver_name == NULL) >return false; > > dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == > DRM_NODE_RENDER; > - Unrelated whitespace change. > if (!dri2_dpy->is_render_node) { > #ifdef HAVE_DRM_GRALLOC > /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM > names > @@ -1359,6 +1363,7 @@ EGLBoolean > dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) > { > struct dri2_egl_display *dri2_dpy; > + int force_software = disp->Options.ForceSoftware; EGLBoolean > const char *err; > int ret; > > @@ -1384,13 +1389,18 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay > *disp) > > dri2_dpy->fd = droid_open_device(disp); > if (dri2_dpy->fd < 0) { > - err = "DRI2: failed to open device"; > - goto cleanup; > + err = "DRI2: failed to open device, trying software device"; > } > > - if (!droid_load_driver(disp)) { > - err = "DRI2: failed to load driver"; > - goto cleanup; > +load_driver: > + if (!droid_load_driver(disp, force_software)) { > + if (force_software) { > + err = "DRI2: failed to load driver"; > + goto cleanup; > + } else { > + force_software = true; > + goto load_driver; > + } I believe we don't need this retry here, because if we fail here once, _eglMatchDriver() would retry us with ForceSoftware = EGL_TRUE [1]. [1] https://cgit.freedesktop.org/mesa/mesa/tree/src/egl/main/egldriver.c#n80 Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH rfc 0/3] Be able to disable EGL/GLX_EXT_buffer_age
On 07/05/2018 03:54 PM, Emil Velikov wrote: On 5 July 2018 at 14:31, Emil Velikov wrote: Hi Qiang Yu On 5 July 2018 at 03:31, Qiang Yu wrote: For GPU like ARM mali Utgard EGL/GLX_EXT_buffer_age will make performace worse. But mesa has no way to disable it. This patch series make driver be able to disable it and add a gallium pipe cap for gallium driver usage. Due to currently only out of tree lima driver need it, and not sure if this is the right way to disable it, so I send this RFC before lima be able to upstream. Pretty sure we already have a way to handle that. Look for glx_disable_ext_buffer_age - it was introduced for the VMWare driver. Although we should: a) tweak the name - kind of split if we need to b) add glx/dri2 and egl support Looking at the implementation - it uses a driver query, meaning that one could enable it at a later stage. No need to worry if the user has old drirc/etc as with current solution. Having two ways of doing the same thing is a bad idea. Perhaps we can remove the drirc implementation and use this instead? Thomas, what do you think? The thing is that we only notice a performance regression with GL compositors like gnome-shell and compiz, and that's typically because of side effects of these compositors. They're using full buffer swaps when the extension is enabled rather than damage regions. For other applications we want it enabled. So we need a way to turn it off for certain apps. /Thomas Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 2/5] egl/android: Add Android property for forcing kms_swrast
On Thu, Jul 5, 2018 at 7:07 PM Robert Foss wrote: > > In order to simplify Android bringup on new devices, > provide the property "drm.gpu.force_software" which > forces kms_swrast to be used. > > Signed-off-by: Robert Foss > --- > src/egl/main/egldriver.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c > index 218b3daef25..bb9e90c157d 100644 > --- a/src/egl/main/egldriver.c > +++ b/src/egl/main/egldriver.c > @@ -39,6 +39,10 @@ > #include > #include "c11/threads.h" > > +#ifdef HAVE_ANDROID_PLATFORM > +#include > +#endif > + > #include "egldefines.h" > #include "egldisplay.h" > #include "egldriver.h" > @@ -87,6 +91,12 @@ _eglMatchDriver(_EGLDisplay *dpy) > dpy->Options.ForceSoftware = >env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false); > > +#ifdef HAVE_ANDROID_PLATFORM > + char prop_val[PROPERTY_VALUE_MAX]; > + property_get("drm.gpu.force_software", prop_val, "0"); > + dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX); ForceSoftware is an EGLBoolean, which is just an unsigned int, not stdbool, so no implicit conversion into {0, 1} for us here. I think what we want here is dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX) != 0; Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev