Re: [Mesa-dev] [PATCH rfc 0/3] Be able to disable EGL/GLX_EXT_buffer_age

2018-07-05 Thread Qiang Yu
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

2018-07-05 Thread Caio Marcelo de Oliveira Filho
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, );

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

2018-07-05 Thread Caio Marcelo de Oliveira Filho
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 , 
> 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

2018-07-05 Thread Caio Marcelo de Oliveira Filho
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(, nir_before_cf_list(>else_list),
 nir_after_cf_list(>else_list));
nir_cf_reinsert(, nir_before_cf_list(>then_list));
-   nir_cf_delete();
 
return true;
 }
@@ -345,7 +344,6 @@ opt_if_loop_terminator(nir_if *nif)
nir_cf_extract(, nir_before_block(first_continue_from_blk),
 nir_after_block(continue_from_blk));
nir_cf_reinsert(, nir_after_cf_node(>cf_node));
-   nir_cf_delete();
 
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

2018-07-05 Thread bugzilla-daemon
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

2018-07-05 Thread Caio Marcelo de Oliveira Filho
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

2018-07-05 Thread Caio Marcelo de Oliveira Filho
> > 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

2018-07-05 Thread Caio Marcelo de Oliveira Filho
(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

2018-07-05 Thread bugzilla-daemon
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

2018-07-05 Thread bugzilla-daemon
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

2018-07-05 Thread Eric Anholt
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)

2018-07-05 Thread Eric Anholt
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, , );
>  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 = _context(cPriv)->Base;
>  
> - draw = dri_drawable(driDrawPriv);
> - read = dri_drawable(driReadPriv);
> - mesaCtx = >Base;
> - mesaDraw = >Base;
> - mesaRead = >Base;
> + if (driDrawPriv && driReadPriv) {
> +   draw = dri_drawable(driDrawPriv);
> +   read = dri_drawable(driReadPriv);
> +   mesaDraw = >Base;
> +   mesaRead = >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

2018-07-05 Thread Eric Anholt
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

2018-07-05 Thread Marek Olšák
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

2018-07-05 Thread Eric Anholt
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, >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

2018-07-05 Thread Eric Anholt
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

2018-07-05 Thread Jason Ekstrand
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

2018-07-05 Thread Marek Olšák
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

2018-07-05 Thread Eric Anholt
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(>condition);
>  
> -   handle_if_block(>then_instructions);
> -   handle_if_block(>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(>then_instructions, new_kills, _killed_all);
> +   handle_if_block(>else_instructions, new_kills, _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

2018-07-05 Thread Jason Ekstrand
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

2018-07-05 Thread Jason Ekstrand
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

2018-07-05 Thread Marek Olšák
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 = >surf_index_fmask;
-   else if (!(flags & RADEON_SURF_Z_OR_SBUFFER))
-  config.info.surf_index = >surf_index_color;
-   else
-  config.info.surf_index = NULL;
-
+   config.info.surf_index = >surf_index_color;
config.info.fmask_surf_index = >surf_index_fmask;
 
+   if (flags & RADEON_SURF_Z_OR_SBUFFER)
+  config.info.surf_index = NULL;
+
return ac_compute_surface(ws->addrlib, >info, , 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

2018-07-05 Thread Marek Olšák
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

2018-07-05 Thread Eric Anholt
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

2018-07-05 Thread Eric Anholt
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

2018-07-05 Thread Caio Marcelo de Oliveira Filho
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.

2018-07-05 Thread Eric Anholt
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.

2018-07-05 Thread Eric Anholt
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)

2018-07-05 Thread Adam Jackson
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, , );
 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 = _context(cPriv)->Base;
 
-   draw = dri_drawable(driDrawPriv);
-   read = dri_drawable(driReadPriv);
-   mesaCtx = >Base;
-   mesaDraw = >Base;
-   mesaRead = >Base;
+   if (driDrawPriv && driReadPriv) {
+   draw = dri_drawable(driDrawPriv);
+   read = dri_drawable(driReadPriv);
+   mesaDraw = >Base;
+   mesaRead = >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

2018-07-05 Thread Mathieu Bridon
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

2018-07-05 Thread Dongwon Kim
'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.

2018-07-05 Thread Marek Olšák
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

2018-07-05 Thread Francisco Jerez
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 

Re: [Mesa-dev] [PATCH 16/26] python: Explicitly use lists

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Jon Turney

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)

2018-07-05 Thread Jon Turney
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(, MEM_START_PAGE * PAGE_SIZE, MEM_SIZE);
+  util_vma_heap_init(, 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(, 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(, a.start_page * PAGE_SIZE,
- a.num_pages * PAGE_SIZE);
+  util_vma_heap_free(, 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

2018-07-05 Thread Bas Nieuwenhuizen
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

2018-07-05 Thread bugzilla-daemon
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

2018-07-05 Thread bugzilla-daemon
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=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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Gert Wollny
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(, 0, sizeof(struct r600_bytecode_output));
+   get_spilled_array_base_and_size(ctx, 
tgsi_dst->Register.Index,
+   _base, _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 = 
>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(, 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,
-   _base, _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_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_WRITE;
+   

Re: [Mesa-dev] [PATCH 26/26] meson: Build with Python 3

2018-07-05 Thread Eric Engestrom
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', 

[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

2018-07-05 Thread bugzilla-daemon
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()

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Eric Engestrom
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.

2018-07-05 Thread Emil Velikov
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()

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Eric Engestrom
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.

2018-07-05 Thread Rob Clark
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

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Samuel Pitoiset
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

2018-07-05 Thread Eric Engestrom
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.

2018-07-05 Thread Eric Anholt
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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()

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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 20/26] python: Open file in binary mode

2018-07-05 Thread Dylan Baker
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 19/26] python: Don't abuse hex()

2018-07-05 Thread Dylan Baker
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 18/26] python: Better check for integer types

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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.

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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()

2018-07-05 Thread Dylan Baker
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 @@ 

Re: [Mesa-dev] [PATCH 08/26] python: Better use iterators

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Emil Velikov
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Robert Foss

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

2018-07-05 Thread Dylan Baker
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 >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 
> 

Re: [Mesa-dev] [RFC 1/5] st/dri: Allow kms_swrast to work without a device FD

2018-07-05 Thread Emil Velikov
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(>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

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Dylan Baker
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
___

Re: [Mesa-dev] [PATCH 04/26] python: Better check for keys in dicts

2018-07-05 Thread Dylan Baker
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 

Re: [Mesa-dev] [PATCH 03/26] python: Use the Python 3 exception syntax

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Dylan Baker
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

2018-07-05 Thread Robert Foss

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

2018-07-05 Thread Matt Turner
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

2018-07-05 Thread Robert Foss

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

2018-07-05 Thread Bas Nieuwenhuizen
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(>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(>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(>base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0));
> +   radeon_emit(>base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va);
> +   radeon_emit(>base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va 
> >> 32);
> +   radeon_emit(>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(>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(>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(>base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 
> 0));
> +   radeon_emit(>base, child->ib.ib_mc_address);
> +   radeon_emit(>base, child->ib.ib_mc_address >> 32);
> +   radeon_emit(>base, child->ib.size);
> } else {
> if (parent->base.cdw + child->base.cdw > parent->base.max_dw)
> radv_amdgpu_cs_grow(>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

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Eric Engestrom
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()

2018-07-05 Thread Samuel Pitoiset
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(>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(>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(>base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 0));
+   radeon_emit(>base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va);
+   radeon_emit(>base, radv_amdgpu_winsys_bo(cs->ib_buffer)->base.va >> 
32);
+   radeon_emit(>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(>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(>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(>base, PKT3(PKT3_INDIRECT_BUFFER_CIK, 2, 
0));
+   radeon_emit(>base, child->ib.ib_mc_address);
+   radeon_emit(>base, child->ib.ib_mc_address >> 32);
+   radeon_emit(>base, child->ib.size);
} else {
if (parent->base.cdw + child->base.cdw > parent->base.max_dw)
radv_amdgpu_cs_grow(>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

2018-07-05 Thread Jon Turney
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

2018-07-05 Thread Eric Engestrom
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 @@ 

[Mesa-dev] [PATCH] intel: tools: fix build on old systems

2018-07-05 Thread Lionel Landwerlin
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 : 

Re: [Mesa-dev] [PATCH rfc 0/3] Be able to disable EGL/GLX_EXT_buffer_age

2018-07-05 Thread Qiang Yu
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()

2018-07-05 Thread Eric Engestrom
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 

Re: [Mesa-dev] [PATCH 08/26] python: Better use iterators

2018-07-05 Thread Eric Engestrom
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 

Re: [Mesa-dev] [PATCH 07/26] python: Better sort dictionary keys/values

2018-07-05 Thread Eric Engestrom
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

2018-07-05 Thread Tomasz Figa
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

2018-07-05 Thread Thomas Hellstrom

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

2018-07-05 Thread Tomasz Figa
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


Re: [Mesa-dev] [PATCH 04/26] python: Better check for keys in dicts

2018-07-05 Thread Eric Engestrom
On Thursday, 2018-07-05 15:17:35 +0200, Mathieu Bridon wrote:
> 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(-)
> 
[snip]
> diff --git a/src/mapi/mapi_abi.py b/src/mapi/mapi_abi.py
> index 0a49c06ff2..826721479d 100644
> --- a/src/mapi/mapi_abi.py
> +++ b/src/mapi/mapi_abi.py
> @@ -168,7 +168,7 @@ def abi_parse_xml(xml):
>  else:
>  attrs['handcode'] = None
>  
> -if entry_dict.has_key(name):
> +if name in entry_dict:
>  raise Exception('%s is duplicated' % (name))
>  
>  cols = []

I guess the two hunks below actually belong in path 7 :)

> @@ -180,8 +180,7 @@ def abi_parse_xml(xml):
>  ent = ABIEntry(cols, attrs, func)
>  entry_dict[ent.name] = ent
>  
> -entries = entry_dict.values()
> -entries.sort()
> +entries = sorted(entry_dict.values())
>  
>  return entries
>  
> @@ -246,12 +245,11 @@ def abi_parse(filename):
>  raise Exception('invalid slot in %s' % (line))
>  
>  ent = ABIEntry(cols, attrs)
> -if entry_dict.has_key(ent.name):
> +if ent.name in entry_dict:
>  raise Exception('%s is duplicated' % (ent.name))
>  entry_dict[ent.name] = ent
>  
> -entries = entry_dict.values()
> -entries.sort()
> +entries = sorted(entry_dict.values())
>  
>  return entries
>  
[snip]
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


  1   2   >