Re: [Mesa-dev] [PATCH 00/11] nir: Add a pass management framework

[Kenneth Graunke]

On Wednesday, October 28, 2015 02:32:00 PM Jason Ekstrand wrote:
> This series adds a nir_pass datastructure and some helpers for managing
> optimization and lowering passes.  I've been meaning to get around to this
> for some time.  There are a couple of primary benifits to this:
> 
> First, this gives us a central place to put things such as validating the
> shader, printing it if it changes, etc.  Right now, the i965 backend calls
> nir_validate_shader after each pass.  We would also like to add something
> like we have in the i965 backend where it can be set to dump the IR to a
> file after every pass that changess it.
> 
> Mor importantly, though, it moves metadata out of the passes them selves
> and into the runner.  In the process of putting this series together, I
> found at least 3 or 4 optimization passes that don't properly invalidate
> metadata.  By putting a metadata_preserved field in nir_pass and handling
> metadata in the pass runner, we make it much less likely that a pass will
> get this wrong.  LLVM has a similar optimization pass architecture for
> precicely this reason.
> 
> As a nice little side-benifit, we no longer have to iterate over all of the
> overloads with non-NULL impl pointers in each pass.

I read through the series today.  Most of it seemed pretty reasonable,
but when I got to patch 11 and saw the final result...I just wasn't a
huge fan of how it turned out.

There's something really nice about having passes simply be functions
that manipulate the shader.  They can easily take arguments.  You can
easily call them conditionally - which we do, based on scalar-vs-vec4,
generation number, or shader stage.  You can assemble passes into groups
by using helper functions (i.e. brw_nir_lower_inputs).  You can easily
add breakpoints at arbitrary locations.

While you can technically still do these things with the flat array of
structs, I think it would be awkward in practice:

- Packing a bunch of function arguments into a struct, passing them as
  a void *, and unpacking them is a bunch of boilerplate.  Especially
  for passes that have a single parameter, i.e. "true"/"false" or "32".

- For passes that don't have any arguments, adding the extra unused
  (void *closure) parameter also adds more boilerplate.  Plus, it just
  seems unfortunate - they already had a nice clean API...

- Conditionally calling passes means assembling a list of passes on the
  fly, dynamically, rather than using a static array.  Would probably
  get messy.  Hope the array sizes are right.

Also, in order for INTEL_DEBUG=optimizer style output to work, *all*
lowering/optimization passes need to participate.  This lets you diff
successive code dumps, seeing what each pass did.  If some passes don't
get printed, then the output is unintelligable.  (I've had to fix that.)

Notably, patch 11 doesn't convert any passes with arguments or
conditional passes to the infrastructure - only the simple cases.
Will this work okay for the more complex ones, or will we want
something different?  Is this a step toward that?

I also tend to agree with Rob's reluctance toward meta-programming,
FWIW...I'd at least like to be wowed by some nice benefits.

It sounds like Jason and Connor have some ideas about how a pass
manager could be helpful in the future.  But for now, I only see
two concrete benefits:

1. It ensures nir_metadata_preserve gets called appropriately.
   (this is *really* important!)

2. It removes some boilerplate for looping over impls.

I have an alternate approach to suggest for #1.  I don't think #2 is
terribly important for now, since Jason may end up reworking functions
a bit as part of his SPIR-V work...and it seems like we should defer
tidying until we have more than one function.

Also, we do need to fix missing nir_metadata_preserve calls on the
stable branch, so we should fix those independently of implementing
a new pass management scheme.  I have patches for that.

Sorry :(

--Ken


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] MSVC (2015) builds

[Mohamed Mediouni]

Did you care about WoA support, or not ? (ARM target, added in VC2012), 
compatible with Windows RT and Windows 10 IoT.

 

De : mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] De la part de 
Janusz Ganczarski
Envoyé : mardi 3 novembre 2015 04:43
À : mesa-dev@lists.freedesktop.org
Objet : [Mesa-dev] MSVC (2015) builds

 

Hello,

 

In attachment fixed Visual C++ 2015 (VC 14) builds for Mesa.

Currently only gallium softpipe driver support. Gallium llvmpipe

driver support work in progress.

 

Janusz Ganczarski

-

www.januszg.hg.pl   

 

 

  _  


  

Ta wiadomość została sprawdzona na obecność wirusów przez oprogramowanie 
antywirusowe Avast. 
www.avast.com   

 

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

[Mesa-dev] [PATCH 3/8] nir: Properly invalidate metadata in nir_split_var_copies().

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir_split_var_copies.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/glsl/nir/nir_split_var_copies.c 
b/src/glsl/nir/nir_split_var_copies.c
index d2ea58a..d463f7b 100644
--- a/src/glsl/nir/nir_split_var_copies.c
+++ b/src/glsl/nir/nir_split_var_copies.c
@@ -271,6 +271,11 @@ split_var_copies_impl(nir_function_impl *impl)
 
ralloc_free(state.dead_ctx);
 
+   if (state.progress) {
+  nir_metadata_preserve(impl, nir_metadata_block_index |
+  nir_metadata_dominance);
+   }
+
return state.progress;
 }
 
-- 
2.6.2

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

[Mesa-dev] [PATCH 1/8] i965/nir: Add OPT() and OPT_V() macros for invoking NIR passes.

[Kenneth Graunke]

OPT() is the normal macro for passes that return booleans, while OPT_V()
is a variant that works for passes that don't properly report progress.
(Such passes should be fixed to return a boolean, eventually.)

These macros take care of calling nir_validate_shader() and setting
progress appropriately.  In the future, it would be easy to add shader
dumping similar to INTEL_DEBUG=optimizer by extending the macro.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_nir.c | 123 
 1 file changed, 53 insertions(+), 70 deletions(-)

Ideally we'll delete OPT_V eventually.  I added progress to a bunch of
passes a while back; we'll have to hit up the rest of them...

diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
b/src/mesa/drivers/dri/i965/brw_nir.c
index 11f1113..e71a7d1 100644
--- a/src/mesa/drivers/dri/i965/brw_nir.c
+++ b/src/mesa/drivers/dri/i965/brw_nir.c
@@ -136,46 +136,49 @@ brw_nir_lower_outputs(nir_shader *nir, bool is_scalar)
}
 }
 
+#define _OPT(do_pass) (({ \
+   bool this_progress = true; \
+   do_pass\
+   nir_validate_shader(nir);  \
+   this_progress; \
+}))
+
+#define OPT(pass, ...) _OPT( \
+   this_progress = pass(nir ,##__VA_ARGS__); \
+   progress = progress || this_progress; \
+)
+
+#define OPT_V(pass, ...) _OPT( \
+   pass(nir, ##__VA_ARGS__);   \
+)
+
 static void
 nir_optimize(nir_shader *nir, bool is_scalar)
 {
bool progress;
do {
   progress = false;
-  nir_lower_vars_to_ssa(nir);
-  nir_validate_shader(nir);
+  OPT_V(nir_lower_vars_to_ssa);
 
   if (is_scalar) {
- nir_lower_alu_to_scalar(nir);
- nir_validate_shader(nir);
+ OPT_V(nir_lower_alu_to_scalar);
   }
 
-  progress |= nir_copy_prop(nir);
-  nir_validate_shader(nir);
+  OPT(nir_copy_prop);
 
   if (is_scalar) {
- nir_lower_phis_to_scalar(nir);
- nir_validate_shader(nir);
+ OPT_V(nir_lower_phis_to_scalar);
   }
 
-  progress |= nir_copy_prop(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_dce(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_cse(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_peephole_select(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_algebraic(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_constant_folding(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_dead_cf(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_remove_phis(nir);
-  nir_validate_shader(nir);
-  progress |= nir_opt_undef(nir);
-  nir_validate_shader(nir);
+  OPT(nir_copy_prop);
+  OPT(nir_opt_dce);
+  OPT(nir_opt_cse);
+  OPT(nir_opt_peephole_select);
+  OPT(nir_opt_algebraic);
+  OPT(nir_opt_constant_folding);
+  OPT(nir_opt_dead_cf);
+  OPT(nir_opt_remove_phis);
+  OPT(nir_opt_undef);
} while (progress);
 }
 
@@ -193,6 +196,7 @@ brw_create_nir(struct brw_context *brw,
   .lower_txp = ~0,
};
bool debug_enabled = INTEL_DEBUG & intel_debug_flag_for_shader_stage(stage);
+   bool progress = false;
nir_shader *nir;
 
/* First, lower the GLSL IR or Mesa IR to NIR */
@@ -200,80 +204,63 @@ brw_create_nir(struct brw_context *brw,
   nir = glsl_to_nir(shader_prog, stage, options);
} else {
   nir = prog_to_nir(prog, options);
-  nir_convert_to_ssa(nir); /* turn registers into SSA */
+  OPT_V(nir_convert_to_ssa); /* turn registers into SSA */
}
nir_validate_shader(nir);
 
if (stage == MESA_SHADER_GEOMETRY) {
-  nir_lower_gs_intrinsics(nir);
-  nir_validate_shader(nir);
+  OPT(nir_lower_gs_intrinsics);
}
 
-   nir_lower_global_vars_to_local(nir);
-   nir_validate_shader(nir);
+   OPT(nir_lower_global_vars_to_local);
 
-   nir_lower_tex(nir, &tex_options);
-   nir_validate_shader(nir);
+   OPT_V(nir_lower_tex, &tex_options);
 
-   nir_normalize_cubemap_coords(nir);
-   nir_validate_shader(nir);
+   OPT(nir_normalize_cubemap_coords);
 
-   nir_split_var_copies(nir);
-   nir_validate_shader(nir);
+   OPT(nir_split_var_copies);
 
nir_optimize(nir, is_scalar);
 
/* Lower a bunch of stuff */
-   nir_lower_var_copies(nir);
-   nir_validate_shader(nir);
+   OPT_V(nir_lower_var_copies);
 
/* Get rid of split copies */
nir_optimize(nir, is_scalar);
 
-   brw_nir_lower_inputs(nir, is_scalar);
-   brw_nir_lower_outputs(nir, is_scalar);
+   OPT_V(brw_nir_lower_inputs, is_scalar);
+   OPT_V(brw_nir_lower_outputs, is_scalar);
nir_assign_var_locations(&nir->uniforms,
 &nir->num_uniforms,
 is_scalar ? type_size_scalar : type_size_vec4);
-   nir_lower_io(nir, -1, is_scalar ? type_size_scalar : type_size_vec4);
-   nir_validate_shader(nir);
+   OPT_V(nir_lower_io, -1, is_scalar ? type_size_scalar : type_size_vec4);
 
-   nir_remove_dead_variables(nir

[Mesa-dev] [PATCH 4/8] nir: Properly invalidate metadata in nir_remove_dead_variables().

[Kenneth Graunke]

We can't preserve dominance or live variable information.

This also begs the question: what about globals?  Metadata only exists
at the nir_function_impl level, so it would seem there is no metadata
about global variables for us to invalidate.

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir_remove_dead_variables.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/glsl/nir/nir_remove_dead_variables.c 
b/src/glsl/nir/nir_remove_dead_variables.c
index d6783e7..77b6c13 100644
--- a/src/glsl/nir/nir_remove_dead_variables.c
+++ b/src/glsl/nir/nir_remove_dead_variables.c
@@ -126,8 +126,13 @@ nir_remove_dead_variables(nir_shader *shader)
progress = remove_dead_vars(&shader->globals, live) || progress;
 
nir_foreach_overload(shader, overload) {
-  if (overload->impl)
- progress = remove_dead_vars(&overload->impl->locals, live) || 
progress;
+  if (overload->impl) {
+ if (remove_dead_vars(&overload->impl->locals, live)) {
+nir_metadata_preserve(overload->impl, nir_metadata_block_index |
+  nir_metadata_dominance);
+progress = true;
+ }
+  }
}
 
_mesa_set_destroy(live, NULL);
-- 
2.6.2

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

[Mesa-dev] [PATCH 0/8] Alternative to the NIR pass manager idea

[Kenneth Graunke]

Hello,

Here's my alternative suggestion to Jason's pass manager series.

First, it implements an OPT() macro in the i965 NIR backend, and
uses it for ~all passes.  (Other drivers are obviously free to do likewise!)
I chose to group up some operations (such as input lowering) which
technically use a few passes but are logically one operation.

Although these use GCC statement expressions, they're not extensively
tied to them, as suggested.  We only use them to allow putting OPT()
directly in if-conditions.  We could call it outside of an expression
and refer to this_progress instead - this is just a bit nicer.

Secondly, it adds missing nir_metadata_preserve calls in various
passes.  If accepted, I plan to mark these as Cc: mesa-stable.
Even if we go with Jason's idea, these patches should at least be
useful for that :)

Finally, it provides a simple mechanism for ensuring passes properly
call nir_metadata_preserve().  Before calling a pass, the OPT() macro
sets a new meaningless metadata flag on all impls.  If the pass calls
nir_metadata_preserve, this bogus flag will be cleared.  After the
pass, we assert that either !this_progress or the flag was cleared.

This works pretty well and allows us to preserve our function-style
pass interface.  It doesn't work for passes that return void, as we
allow passes to skip calling nir_metadata_preserve when not making
any changes at all.  But we should convert those to return progress
anyway.

Thoughts?  Flames?

--Ken

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

[Mesa-dev] [PATCH 2/8] nir: Properly invalidate metadata in nir_lower_global_vars_to_local().

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir_lower_global_vars_to_local.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/glsl/nir/nir_lower_global_vars_to_local.c 
b/src/glsl/nir/nir_lower_global_vars_to_local.c
index fab2366..9fa64ed 100644
--- a/src/glsl/nir/nir_lower_global_vars_to_local.c
+++ b/src/glsl/nir/nir_lower_global_vars_to_local.c
@@ -100,6 +100,8 @@ nir_lower_global_vars_to_local(nir_shader *shader)
  exec_node_remove(&var->node);
  var->data.mode = nir_var_local;
  exec_list_push_tail(&impl->locals, &var->node);
+ nir_metadata_preserve(impl, nir_metadata_block_index |
+ nir_metadata_dominance);
  progress = true;
   }
}
-- 
2.6.2

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

[Mesa-dev] [PATCH 5/8] nir: Properly invalidate metadata in nir_opt_copy_prop().

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir_opt_copy_propagate.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/glsl/nir/nir_opt_copy_propagate.c 
b/src/glsl/nir/nir_opt_copy_propagate.c
index 96520f8..7d8bdd7 100644
--- a/src/glsl/nir/nir_opt_copy_propagate.c
+++ b/src/glsl/nir/nir_opt_copy_propagate.c
@@ -262,6 +262,12 @@ nir_copy_prop_impl(nir_function_impl *impl)
bool progress = false;
 
nir_foreach_block(impl, copy_prop_block, &progress);
+
+   if (progress) {
+  nir_metadata_preserve(impl, nir_metadata_block_index |
+  nir_metadata_dominance);
+   }
+
return progress;
 }
 
-- 
2.6.2

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

[Mesa-dev] [PATCH 6/8] nir: Properly invalidate metadata in nir_lower_vec_to_movs().

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir_lower_vec_to_movs.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/glsl/nir/nir_lower_vec_to_movs.c 
b/src/glsl/nir/nir_lower_vec_to_movs.c
index c08b721..736a66c 100644
--- a/src/glsl/nir/nir_lower_vec_to_movs.c
+++ b/src/glsl/nir/nir_lower_vec_to_movs.c
@@ -288,6 +288,11 @@ nir_lower_vec_to_movs_impl(nir_function_impl *impl)
 
nir_foreach_block(impl, lower_vec_to_movs_block, &state);
 
+   if (state.progress) {
+  nir_metadata_preserve(impl, nir_metadata_block_index |
+  nir_metadata_dominance);
+   }
+
return state.progress;
 }
 
-- 
2.6.2

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

[Mesa-dev] [PATCH 8/8] i965/nir: Validate that NIR passes call nir_metadata_preserve().

[Kenneth Graunke]

Failing to call nir_metadata_preserve() can have nasty consequences:
some pass breaks dominance information, but leaves it marked as valid,
causing some subsequent pass to go haywire and probably crash.

This pass adds a simple validation mechanism to ensure passes handle
this properly.  We add a new bogus metadata flag that isn't used for
anything in particular, set it before each pass, and ensure it *isn't*
still set after the pass.  nir_metadata_preserve will reset the flag,
so correct passes will work, and bad passes will assert fail.

(I would have made these functions static inline, but nir.h is included
in C++, so we can't bit-or enums without lots of casting...)

Thanks to Dylan Baker for the idea.

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir.h  |  5 +
 src/glsl/nir/nir_metadata.c | 36 
 src/mesa/drivers/dri/i965/brw_nir.c | 10 +++---
 3 files changed, 48 insertions(+), 3 deletions(-)

One slightly ugly aspect is that we leave the bogus flag set if a pass
doesn't make any progress (i.e. doesn't alter the tree), as it won't
necessarily call nir_metadata_preserve in that case.  This should be
pretty harmless, though...

diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index 874a039..1ab740a 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -1311,6 +1311,7 @@ typedef enum {
nir_metadata_block_index = 0x1,
nir_metadata_dominance = 0x2,
nir_metadata_live_variables = 0x4,
+   nir_metadata_not_properly_reset = 0x8,
 } nir_metadata;
 
 typedef struct {
@@ -1880,8 +1881,12 @@ void nir_print_instr(const nir_instr *instr, FILE *fp);
 
 #ifdef DEBUG
 void nir_validate_shader(nir_shader *shader);
+void nir_metadata_set_validation_flag(nir_shader *shader);
+void nir_metadata_check_validation_flag(nir_shader *shader);
 #else
 static inline void nir_validate_shader(nir_shader *shader) { (void) shader; }
+static inline void nir_metadata_set_validation_flag(nir_shader *shader) { 
(void) shader; }
+static inline void nir_metadata_check_validation_flag(nir_shader *shader) { 
(void) shader; }
 #endif /* DEBUG */
 
 void nir_calc_dominance_impl(nir_function_impl *impl);
diff --git a/src/glsl/nir/nir_metadata.c b/src/glsl/nir/nir_metadata.c
index a03e124..0b0159f 100644
--- a/src/glsl/nir/nir_metadata.c
+++ b/src/glsl/nir/nir_metadata.c
@@ -52,3 +52,39 @@ nir_metadata_preserve(nir_function_impl *impl, nir_metadata 
preserved)
 {
impl->valid_metadata &= preserved;
 }
+
+#ifdef DEBUG
+/**
+ * Make sure passes properly invalidate metadata (part 1).
+ *
+ * Call this before running a pass to set a bogus metadata flag, which will
+ * only be preserved if the pass forgets to call nir_metadata_preserve().
+ */
+void
+nir_metadata_set_validation_flag(nir_shader *shader)
+{
+   nir_foreach_overload(shader, overload) {
+  if (overload->impl) {
+ overload->impl->valid_metadata |= nir_metadata_not_properly_reset;
+  }
+   }
+}
+
+/**
+ * Make sure passes properly invalidate metadata (part 2).
+ *
+ * Call this after a pass makes progress to verify that the bogus metadata set 
by
+ * the earlier function was properly thrown away.  Note that passes may not 
call
+ * nir_metadata_preserve() if they don't actually make any changes at all.
+ */
+void
+nir_metadata_check_validation_flag(nir_shader *shader)
+{
+   nir_foreach_overload(shader, overload) {
+  if (overload->impl) {
+ assert(!(overload->impl->valid_metadata &
+  nir_metadata_not_properly_reset));
+  }
+   }
+}
+#endif
diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
b/src/mesa/drivers/dri/i965/brw_nir.c
index e71a7d1..7b72d36 100644
--- a/src/mesa/drivers/dri/i965/brw_nir.c
+++ b/src/mesa/drivers/dri/i965/brw_nir.c
@@ -143,9 +143,13 @@ brw_nir_lower_outputs(nir_shader *nir, bool is_scalar)
this_progress; \
 }))
 
-#define OPT(pass, ...) _OPT( \
-   this_progress = pass(nir ,##__VA_ARGS__); \
-   progress = progress || this_progress; \
+#define OPT(pass, ...) _OPT(   \
+   nir_metadata_set_validation_flag(nir);  \
+   this_progress = pass(nir ,##__VA_ARGS__);   \
+   if (this_progress) {\
+  progress = true; \
+  nir_metadata_check_validation_flag(nir); \
+   }   \
 )
 
 #define OPT_V(pass, ...) _OPT( \
-- 
2.6.2

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

[Mesa-dev] [PATCH 7/8] nir: Properly invalidate metadata in nir_opt_remove_phis().

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir_opt_remove_phis.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/glsl/nir/nir_opt_remove_phis.c 
b/src/glsl/nir/nir_opt_remove_phis.c
index 5bdf7ef..66d3754 100644
--- a/src/glsl/nir/nir_opt_remove_phis.c
+++ b/src/glsl/nir/nir_opt_remove_phis.c
@@ -108,6 +108,11 @@ remove_phis_impl(nir_function_impl *impl)
 
nir_foreach_block(impl, remove_phis_block, &progress);
 
+   if (progress) {
+  nir_metadata_preserve(impl, nir_metadata_block_index |
+  nir_metadata_dominance);
+   }
+
return progress;
 }
 
-- 
2.6.2

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

[Mesa-dev] [PATCH] llvmpipe: use simple coeffs calc for 128bit vectors

[Oded Gabbay]

There are currently two methods in llvmpipe code to calculate coeffs to
be used as inputs for the fragment shader. The two methods use slightly
different ways to do the floating point calculations and thus produce
slightly different results.

The decision which method to use is determined by the size of the vector
that is used by the platform.

For vectors with size of more than 128bit, a single-step method is used,
in which coeffs_init_simple() + attribs_update_simple() are called.

For vectors with size of 128bit or less, a two-step method is used, in
which coeffs_init() + attribs_update() are called.

This causes some piglit tests (clip-distance-bulk-copy,
interface-vs-unnamed-to-fs-unnamed) to fail when using platforms with
128bit vectors (such as ppc64le or x86-64 without AVX).

This patch makes platforms with 128bit vectors use the single-step
method (aka "simple" method) instead of the two-step method.
This would make the resulting coeffs identical between more platforms,
make sure the piglit tests passes, and make debugging and maintainability
a bit easier as the generated LLVM IR will be the same for more platforms.

The performance impact is negligible for x86-64 without AVX, and
basically non-existent for ppc64le, as it can be seen from the following
benchmarking results:

- glxspheres, on ppc64le:

   - original code:  4.892745317 frames/sec 5.460303857 Mpixels/sec
   - with the patch: 4.932083873 frames/sec 5.504205571 Mpixels/sec
   - Additional 0.8% performance boost

- glxspheres, on x86-64 without AVX:

   - original code:  20.16418809 frames/sec 22.50323395 Mpixels/sec
   - with the patch: 20.31328989 frames/sec 22.66963152 Mpixels/sec
   - Additional 0.74% performance boost

- glmark2, on ppc64le:

  - original code:  score of 58
  - with my change: score of 57

- glmark2, on x86-64 without AVX:

  - original code:  score of 175
  - with the patch: score of 167
  - Impact of of -4.5% on performance

- OpenArena, on ppc64le:

  - original code:  3398 frames 1719.0 seconds 2.0 fps
255.0/505.9/2773.0/0.0 ms

  - with the patch: 3398 frames 1690.4 seconds 2.0 fps
241.0/497.5/2563.0/0.2 ms

  - 29 seconds faster with the patch, which is about 2%

- OpenArena, on x86-64 without AVX:

  - original code:  3398 frames 239.6 seconds 14.2 fps
38.0/70.5/719.0/14.6 ms

  - with the patch: 3398 frames 244.4 seconds 13.9 fps
38.0/71.9/697.0/14.3 ms

  - 0.3 fps slower with the patch (about 2%)

Additional details can be found at:
http://lists.freedesktop.org/archives/mesa-dev/2015-October/098635.html

Signed-off-by: Oded Gabbay 
---
 src/gallium/drivers/llvmpipe/lp_bld_interp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_bld_interp.c 
b/src/gallium/drivers/llvmpipe/lp_bld_interp.c
index df262fa..a2055d2 100644
--- a/src/gallium/drivers/llvmpipe/lp_bld_interp.c
+++ b/src/gallium/drivers/llvmpipe/lp_bld_interp.c
@@ -746,7 +746,7 @@ lp_build_interp_soa_init(struct lp_build_interp_soa_context 
*bld,
 
pos_init(bld, x0, y0);
 
-   if (coeff_type.length > 4) {
+   if (coeff_type.length >= 4) {
   bld->simple_interp = TRUE;
   {
  /* XXX this should use a global static table */
-- 
2.4.3

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

Re: [Mesa-dev] [PATCH 1/3] mesa: restrict ES2 from 32-bit blending, add GL_EXT_float_blend

[Marek Olšák]

On Tue, Nov 3, 2015 at 2:14 AM, Ilia Mirkin  wrote:
> On Mon, Nov 2, 2015 at 8:07 PM, Ian Romanick  wrote:
>> On 11/02/2015 04:50 PM, Ilia Mirkin wrote:
>>> GL_EXT_color_buffer_float adds support for float buffers in ES3.0+, but
>>> explicitly disallows 32-bit blending. However this restriction was never
>>> implemented in mesa.
>>>
>>> Add the restriction, and also allow a driver to expose
>>> GL_EXT_float_blend which re-enables the functionality.
>>>
>>> Signed-off-by: Ilia Mirkin 
>>> ---
>>>
>>> Untested... looking for confirmation that this is the right thing to
>>> do. Will write a piglit if it is.
>>
>> Some flavor of this probably is the right thing to do.  The question is
>> whether or not any hardware supported by Mesa can do
>> GL_EXT_color_buffer_float but not GL_EXT_float_blend... if everyone can
>> do both or neither, this patch series could be even simpler. :)
>
> Adreno A3xx can't do 32-bit blending, only 16-bit. Sorry!

r500 (r300g) can't do 32-bit blending either.

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

Re: [Mesa-dev] [PATCH 1/3] mesa: restrict ES2 from 32-bit blending, add GL_EXT_float_blend

[Marek Olšák]

On Tue, Nov 3, 2015 at 9:51 AM, Marek Olšák  wrote:
> On Tue, Nov 3, 2015 at 2:14 AM, Ilia Mirkin  wrote:
>> On Mon, Nov 2, 2015 at 8:07 PM, Ian Romanick  wrote:
>>> On 11/02/2015 04:50 PM, Ilia Mirkin wrote:
 GL_EXT_color_buffer_float adds support for float buffers in ES3.0+, but
 explicitly disallows 32-bit blending. However this restriction was never
 implemented in mesa.

 Add the restriction, and also allow a driver to expose
 GL_EXT_float_blend which re-enables the functionality.

 Signed-off-by: Ilia Mirkin 
 ---

 Untested... looking for confirmation that this is the right thing to
 do. Will write a piglit if it is.
>>>
>>> Some flavor of this probably is the right thing to do.  The question is
>>> whether or not any hardware supported by Mesa can do
>>> GL_EXT_color_buffer_float but not GL_EXT_float_blend... if everyone can
>>> do both or neither, this patch series could be even simpler. :)
>>
>> Adreno A3xx can't do 32-bit blending, only 16-bit. Sorry!
>
> r500 (r300g) can't do 32-bit blending either.

r500 can't do ES3, so nevermind.

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

Re: [Mesa-dev] [PATCH 00/10] i965: always mark used surfaces in the visitors

[Iago Toral]

On Fri, 2015-10-30 at 16:19 +0200, Francisco Jerez wrote:
> Iago Toral Quiroga  writes:
> 
> > Right now some opcodes that only use constant surface indexing mark them as
> > used in the generator while others do it in the visitor. When the opcode can
> > handle both direct and indirect surface indexing then some opcodes handle
> > only the constant part in the generator and leave the indirect case to the
> > caller. It is all very inconsistent and leads to confusion, since one has to
> > go and look into the generator code in each case to check if it marks 
> > surfaces
> > as used or not, and in which cases.
> >
> > when I was working on SSBOs I was tempted to try and fix this but then I
> > forgot. Jordan bumped into this recently too when comparing visitor
> > code paths for similar opcodes (ubos and ssbos) that need to handle this
> > differently because they use different generator opcodes.
> >
> > Since the generator opcodes never handle marking of indirect surfaces, just
> > leave surface marking to the caller completely, since callers always have
> > all the information needed for this. It also makes things more consistent
> > and clear for everyone: marking surfaces as used is always on the side
> > of the visitor, never the generator.
> >
> > No piglit regressions observed in my IVB laptop. Would be nice to have
> > someone giving this a try with Jenkins though, to make sure I did not miss
> > anything in paths specific to other gens.
> 
> Jenkins seems to be mostly happy about the series except for three
> apparent regressions:
> 
> piglit.spec.arb_fragment_layer_viewport.layer-gs-writes-in-range.bdwm64

Mmmm... not sure what could be going on with this, at first glance, it
does not look like this test should be affected by this series. The test
does not use any UBOs, does not really check what is written to the FBO,
does not emit texture accesses... Also, there are other tests in
piglit.spec.arb_fragment_layer_viewport that do very similar stuff
(specially the our-of-range test) and those are passing fine.

> 
> piglit.spec.arb_shader_texture_lod.compiler.tex_grad-texture2dproj-2d-vec4.frag.g965m64
> 
> piglit.spec.glsl-es-1_00.compiler.structure-and-array-operations.sampler-array-index.frag.g965m64
> 
> The latter two die with a crash so you may be able to look into them
> even if you don't have the original i965 by using INTEL_DEVID_OVERRIDE. ;)

Unfortunately it seems that these don't break for me with the DEVID
override, that's weird I guess, since they are compiler tests that I can
fully run without INTEL_NO_HW set:

$ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest
generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag
 pass 1.10 GL_ARB_shader_texture_lod
Successfully compiled fragment shader
generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag:
 
PIGLIT: {"result": "pass" }

$ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest_gles2
tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag
 pass 1.00
Successfully compiled fragment shader
tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag:
 0:21(21): warning: sampler arrays indexed with non-constant expressions will 
be forbidden in GLSL 3.00 and later
PIGLIT: {"result": "pass" }

I tried with most chipsets in between PCI_CHIP_I965_G (0x29A2) and
PCI_CHIP_G41_G (0x2E32).

Iago

> >
> > Iago Toral Quiroga (10):
> >   i965/fs: Do not mark direct used surfaces in
> > VARYING_PULL_CONSTANT_LOAD
> >   i965/fs: Do not mark used direct surfaces in
> > UNIFORM_PULL_CONSTANT_LOAD
> >   i965/fs: Do not mark used direct surfaces in the generator for texture
> > opcodes
> >   i965/vec4: Do not mark used direct surfaces in
> > VS_OPCODE_PULL_CONSTANT_LOAD
> >   i965/vec4: Do not mark used direct surfaces in the generator for
> > texture opcodes
> >   i965/vec4: Do not mark used surfaces in SHADER_OPCODE_SHADER_TIME_ADD
> >   i965/fs: Do not mark used surfaces in SHADER_OPCODE_SHADER_TIME_ADD
> >   i965/vec4: Do not mark used surfaces in VS_OPCODE_GET_BUFFER_SIZE
> >   i965/fs: Do not mark used surfaces in FS_OPCODE_GET_BUFFER_SIZE
> >   i965/fs: Do not mark used surfaces in
> > FS_OPCODE_FB_WRITE/FS_OPCODE_REP_FB_WRITE
> >
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 12 ++-
> >  src/mesa/drivers/dri/i965/brw_fs.h   |  3 +-
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 31 -
> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 35 +--
> >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 24 -
> >  src/mesa/drivers/dri/i965/brw_vec4.cpp   |  3 ++
> >  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 19 --
> >  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp   | 44 
> > +++-
> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   |  7 ++--
> >  9 files changed, 87 inser

Re: [Mesa-dev] Unused (?) duplicated GLSL IR state in NIR

[Emil Velikov]

On 2 November 2015 at 19:41, Jason Ekstrand  wrote:
> On Mon, Nov 2, 2015 at 9:33 AM, Connor Abbott  wrote:
>> On Mon, Nov 2, 2015 at 8:35 AM, Emil Velikov  
>> wrote:
>>> Hi all,
>>>
>>> From a quick look, it seems that NIR copies (almost ?) all the state
>>> from GLSL IR even if it doesn't use it.
>>>
>>> The particular piece that I'm thinking about is nir_variable::data.
>>> Afaict this is a remnant from the early days, when the intent was to
>>> kill off the GLSL IR and use NIR directly. If so should we just nuke
>>> it, or (if there is someone working on it) add some comments "Keepme:
>>> WIP work by XXX to use this and kill the glsl IR one".
>>>
>>> The (not that distant) GLSL IR memory optimisations by Ian, seems to
>>> have missed NIR. Was that intentional or were those worked upon before
>>> NIR got merged ? Perhaps it's worth porting them over ?
>>>
>>>
>>> Thanks
>>> Emil
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> Hi Emil,
>>
>> Indeed, nir_variable was copied from ir_variable before Ian's memory
>> optimizations landed. Also, as you noticed, there are a number of
>> fields only used by the GLSL linker, so they're unused until/unless we
>> do linking in NIR. I wouldn't be opposed to nuking those, since we can
>> always revert the commit if/when we get to that.
>
> Agreed.  If you want to nuke some of them, feel free to send the
> patch.  If you do, please CC me.  There are some things that we're
> using for SPIR-V so I might request that you keep a field or two.
> However, cleaning it up would definitely be a good idea.
>
> One thing I will note though is that you should consider this a
> cleanup and not an optimization.  The reason why Ian's stuff helped
> GLSL IR so much is that it uses variables extensively.  The average
> NIR shader only has about half a dozen variables by the time you get
> done optimizing so it's not nearly as much of a problem.
Ack. Thanks guys. So currently no-one is pursuing move the linking to NIR ?

On the optimisation vs cleanup note - it'll be tiny bit of both.

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

Re: [Mesa-dev] [PATCH 04/24] i965: Make 'dw1' and 'bits' unnamed structures in brw_reg.

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:
> Generated by
>
>sed -i -e 's/\.bits\././g' *.c *.h *.cpp
>sed -i -e 's/dw1\.//g' *.c *.h *.cpp
>
> and then reverting changes to comments in gen7_blorp.cpp and
> brw_fs_generator.cpp.
>
> There wasn't any utility offered by forcing the programmer to list these
> to access their fields. Removing them will reduce churn in future
> commits.
>
> This is C11 (and gcc has apparently supported it for sometime
> "compatibility with other compilers")
>
> See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
FYI on old versions of GCC where this extension is not enabled by
default, we flip in on. We're using -std=gnu99, although perhaps we
should use -fms-extensions - not a big deal either way.

Btw I had an almost identical patch locally :-)

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

Re: [Mesa-dev] [PATCH 05/24] i965: Reorganize brw_reg fields.

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:
> Put fields that are meaningless with an immediate in the same storage
> with the immediate.
There is something funky here. Should it be

"Put fields that are meaningless _without_ an immediate in the same storage
_as_ the immediate..."

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

Re: [Mesa-dev] gallium/hud: control visibility at startup and runtime.

[Jimmy Berry]

Some I managed to not end up with [PATCH] in subject or my coverletter.


This is my first patch to mesa-dev. Hopefully, I have done everything correctly.

I posed the idea about a week ago:
  Manipulate GALLIUM_HUD post-launch (interactively)
  http://lists.freedesktop.org/archives/mesa-dev/2015-October/098544.html

I decided to keep it simple for my first patch and implement the bare
requirements needed to fulfill the proposed usecase. Further improvements like
updating the make up of the hud I have considered and may proceed with, but for
the time being I decided to start with this and see what reaction I received.

My initial goal: toggle GALLIUM_HUD so it can be hidden when not desired, but
toggled without restarting application.

Two new environment variables are provided (from envvars.html):

- GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
- GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified signal.
Especially useful to toggle hud at specific points of application and
disable for unencumbered viewing the rest of the time. For example, set
GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10 (SIGUSR1).
Use kill -10  to toggle the hud as desired.

Based on the documentation an example usecase (perhaps in .bashrc or similar).

GALLIUM_HUD=fps
GALLIUM_HUD_VISIBLE=false
GALLIUM_HUD_TOGGLE_SIGNAL=10

kill -10 

This provides a toggleable fps display which is of potential use to end-users
instead of just debugging purposes. Due to a lack of standard/easy tools for
displaying fps on Linux, Steam added an option to display fps which obviously
only works for applications running alongside Steam, but something like this
could be used more generally. Since the hud is not always visible the variables
can be set more globally rather than requiring one-off tweaks to startup scripts
for games instead of Steam (following with the example).

Combined with a keyboard shortcut one could rather simply toggle such a display
without a lot of fuss.

X example:

kill -10 $(xprop -id `xdotool getwindowfocus` | \
  grep '_NET_WM_PID' | \
  grep -oE '[[:digit:]]*$')

Obviously, this example will not always work, but rather close to what I wanted
to achieve.

For development use this can be used with a much more complex GALLIUM_HUD setup.

The initial implementation is designed with the idea of updating the environment
variables as an alternative to the signal and hence the name GALLIUM_HUD_VISIBLE
which works for representing the current state as well as initial state (all
that is currently supported). An update mechanism (inotify, signal, etc) might
be used to re-read environment variables from one of various sources (as
discussed) which could also toggle the hud by changing GALLIUM_HUD_VISIBLE.

It does not seem that there is a posix mechanism for taking a string
representation of a signal and determining the number. Otherwise, it would seem
reasonable to support string input for GALLIUM_HUD_TOGGLE_SIGNAL as well.

I look forward to your thoughts.

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

[Mesa-dev] gallium/hud: control visibility at startup and runtime.

[boombatower]

- env GALLIUM_HUD_VISIBLE: control default visibility
- env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
---
 docs/envvars.html   |  6 ++
 src/gallium/auxiliary/hud/hud_context.c | 23 +++
 2 files changed, 29 insertions(+)

diff --git a/docs/envvars.html b/docs/envvars.html
index bdfe999..530bbb7 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -179,6 +179,12 @@ Mesa EGL supports different sets of environment variables. 
 See the
 GALLIUM_HUD - draws various information on the screen, like framerate,
 cpu load, driver statistics, performance counters, etc.
 Set GALLIUM_HUD=help and run e.g. glxgears for more info.
+GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
+GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified signal.
+Especially useful to toggle hud at specific points of application and
+disable for unencumbered viewing the rest of the time. For example, set
+GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10 (SIGUSR1).
+Use kill -10  to toggle the hud as desired.
 GALLIUM_LOG_FILE - specifies a file for logging all errors, warnings, etc.
 rather than stderr.
 GALLIUM_PRINT_OPTIONS - if non-zero, print all the Gallium environment
diff --git a/src/gallium/auxiliary/hud/hud_context.c 
b/src/gallium/auxiliary/hud/hud_context.c
index ffe30b8..f6bfa80 100644
--- a/src/gallium/auxiliary/hud/hud_context.c
+++ b/src/gallium/auxiliary/hud/hud_context.c
@@ -33,6 +33,7 @@
  * Set GALLIUM_HUD=help for more info.
  */
 
+#include 
 #include 
 
 #include "hud/hud_context.h"
@@ -51,8 +52,10 @@
 #include "tgsi/tgsi_text.h"
 #include "tgsi/tgsi_dump.h"
 
+static boolean __visible_toggle = false;
 
 struct hud_context {
+   boolean visible;
struct pipe_context *pipe;
struct cso_context *cso;
struct u_upload_mgr *uploader;
@@ -95,6 +98,11 @@ struct hud_context {
} text, bg, whitelines;
 };
 
+static void
+signal_visible_handler(int signo)
+{
+   __visible_toggle = true;
+}
 
 static void
 hud_draw_colored_prims(struct hud_context *hud, unsigned prim,
@@ -431,8 +439,15 @@ hud_alloc_vertices(struct hud_context *hud, struct 
vertex_queue *v,
 void
 hud_draw(struct hud_context *hud, struct pipe_resource *tex)
 {
+   if (__visible_toggle) {
+  hud->visible = !hud->visible;
+  __visible_toggle = false;
+   }
+   if (!hud->visible) return;
+
struct cso_context *cso = hud->cso;
struct pipe_context *pipe = hud->pipe;
+
struct pipe_framebuffer_state fb;
struct pipe_surface surf_templ, *surf;
struct pipe_viewport_state viewport;
@@ -1124,7 +1139,9 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)
struct hud_context *hud;
struct pipe_sampler_view view_templ;
unsigned i;
+   boolean visible = debug_get_bool_option("GALLIUM_HUD_VISIBLE", true);
const char *env = debug_get_option("GALLIUM_HUD", NULL);
+   long signo = debug_get_num_option("GALLIUM_HUD_TOGGLE_SIGNAL", 0);
 
if (!env || !*env)
   return NULL;
@@ -1138,6 +1155,7 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)
if (!hud)
   return NULL;
 
+   hud->visible = visible;
hud->pipe = pipe;
hud->cso = cso;
hud->uploader = u_upload_create(pipe, 256 * 1024, 16,
@@ -1267,6 +1285,11 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)
 
LIST_INITHEAD(&hud->pane_list);
 
+   if (signo < 1 || signo >= NSIG)
+  fprintf(stderr, "gallium_hud: invalid signal %ld\n", signo);
+   else if (signal(signo, signal_visible_handler) == SIG_ERR)
+  fprintf(stderr, "gallium_hud: unable to set handler for signal %ld\n", 
signo);
+
hud_parse_env_var(hud, env);
return hud;
 }
-- 
2.6.2

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

[Mesa-dev] [PATCH] gallium/hud: control visibility at startup and runtime.

[boombatower]

This is my first patch to mesa-dev. Hopefully, I have done everything correctly.

I posed the idea about a week ago:
  Manipulate GALLIUM_HUD post-launch (interactively)
  http://lists.freedesktop.org/archives/mesa-dev/2015-October/098544.html

I decided to keep it simple for my first patch and implement the bare
requirements needed to fulfill the proposed usecase. Further improvements like
updating the make up of the hud I have considered and may proceed with, but for
the time being I decided to start with this and see what reaction I received.

My initial goal: toggle GALLIUM_HUD so it can be hidden when not desired, but
toggled without restarting application.

Two new environment variables are provided (from envvars.html):

- GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
- GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified signal.
Especially useful to toggle hud at specific points of application and
disable for unencumbered viewing the rest of the time. For example, set
GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10 (SIGUSR1).
Use kill -10  to toggle the hud as desired.

Based on the documentation an example usecase (perhaps in .bashrc or similar).

GALLIUM_HUD=fps
GALLIUM_HUD_VISIBLE=false
GALLIUM_HUD_TOGGLE_SIGNAL=10

kill -10 

This provides a toggleable fps display which is of potential use to end-users
instead of just debugging purposes. Due to a lack of standard/easy tools for
displaying fps on Linux, Steam added an option to display fps which obviously
only works for applications running alongside Steam, but something like this
could be used more generally. Since the hud is not always visible the variables
can be set more globally rather than requiring one-off tweaks to startup scripts
for games instead of Steam (following with the example).

Combined with a keyboard shortcut one could rather simply toggle such a display
without a lot of fuss.

X example:

kill -10 $(xprop -id `xdotool getwindowfocus` | \
  grep '_NET_WM_PID' | \
  grep -oE '[[:digit:]]*$')

Obviously, this example will not always work, but rather close to what I wanted
to achieve.

For development use this can be used with a much more complex GALLIUM_HUD setup.

The initial implementation is designed with the idea of updating the environment
variables as an alternative to the signal and hence the name GALLIUM_HUD_VISIBLE
which works for representing the current state as well as initial state (all
that is currently supported). An update mechanism (inotify, signal, etc) might
be used to re-read environment variables from one of various sources (as
discussed) which could also toggle the hud by changing GALLIUM_HUD_VISIBLE.

It does not seem that there is a posix mechanism for taking a string
representation of a signal and determining the number. Otherwise, it would seem
reasonable to support string input for GALLIUM_HUD_TOGGLE_SIGNAL as well.

I look forward to your thoughts.

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

Re: [Mesa-dev] [PATCH 06/24] i965: Add and use enum brw_reg_file.

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_defines.h| 10 ++
>  src/mesa/drivers/dri/i965/brw_eu_emit.c|  2 +-
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  5 +++--
>  src/mesa/drivers/dri/i965/brw_reg.h| 25 +
>  4 files changed, 23 insertions(+), 19 deletions(-)
>

There are a couple of places that can also use the same treatment:

brw_disasm.c:

static int reg(... unsigned _reg_file ...)

static int dest_3src(...)
...
uint32_t reg_file;
...


brw_inst.h:
Worth tweaking the FF/F8 macros (wrt *_reg_file) ?


> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index e980003..ed3e335 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -33,7 +33,8 @@
>  #include "brw_fs.h"
>  #include "brw_cfg.h"
>
> -static uint32_t brw_file_from_reg(fs_reg *reg)
> +static enum brw_reg_file
> +brw_file_from_reg(fs_reg *reg)
>  {
> switch (reg->file) {
> case GRF:
> @@ -48,7 +49,7 @@ static uint32_t brw_file_from_reg(fs_reg *reg)
> case UNIFORM:
>unreachable("not reached");
> }
> -   return 0;
> +   return GRF;
Although unreachable (and gcc being silly) GRF looks wrong. Use
BRW_ARCHITECTURE_REGISTER_FILE perhaps ?

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

Re: [Mesa-dev] [PATCH 20/24] Revert "i965: Have brw_imm_vf4() take the vector components as integer values."

[Francisco Jerez]

Matt Turner  writes:

> This reverts commit bbf8239f92ecd79431dfa41402e1c85318e7267f.
>
> I didn't like that commit to begin with -- computing things at compile
> time is fine -- but for purposes of verifying that the resulting values
> are correct, looking up 0x00 and 0x30 in a table is a lot better than
> evaluating a recursive function.
>
FYI the function only ever recurses once in order to handle the sign bit
orthogonally from the exponent and mantissa.

> Anyway, by making brw_imm_vf4() take the actual 8-bit restricted floats
> directly (instead of only integral values that would be converted to
> restricted float), we can use this function as a replacement for the
> vector float src_reg/fs_reg constructors.

Seems awful to me, it replaces a formula that can be verified correct by
reading the first paragraph of the description of the restricted float
format (Gen Graphics » BSpec » 3D-Media-GPGPU Engine » EU Overview » ISA
Introduction » EU Data Types » Numeric Data Types) with a table of magic
constants that don't give the slightest insight into the structure of
the restricted float format and therefore have to be verified
individually.

It also makes it impossible to use brw_imm_vf4() with
dynamically-specified integer constants which was the original
motivation of this change.

If the obfuscation is meant as an optimization, I don't think it helps.
GCC compiles the four int_to_float8() calls from brw_imm_vf4() into a
single instruction already when the arguments are constant expressions:

  4a9dc9:   48 b8 00 00 00 00 30movabs $0x30,%rax

(assembly taken from brw_clip_interp_vertex())

> ---
>  src/mesa/drivers/dri/i965/brw_clip_util.c |  2 +-
>  src/mesa/drivers/dri/i965/brw_reg.h   | 40 
> ---
>  2 files changed, 11 insertions(+), 31 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c 
> b/src/mesa/drivers/dri/i965/brw_clip_util.c
> index 40ad144..0253d52 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_util.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
> @@ -224,7 +224,7 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
>vec1(t_nopersp),
>brw_imm_f(0));
>brw_IF(p, BRW_EXECUTE_1);
> -  brw_MOV(p, t_nopersp, brw_imm_vf4(1, 0, 0, 0));
> +  brw_MOV(p, t_nopersp, brw_imm_vf4(VF_ONE, VF_ZERO, VF_ZERO, VF_ZERO));
>brw_ENDIF(p);
>  
>/* Now compute t_nopersp = t_nopersp.y/t_nopersp.x and broadcast it. */
> diff --git a/src/mesa/drivers/dri/i965/brw_reg.h 
> b/src/mesa/drivers/dri/i965/brw_reg.h
> index b906892..a4c1901 100644
> --- a/src/mesa/drivers/dri/i965/brw_reg.h
> +++ b/src/mesa/drivers/dri/i965/brw_reg.h
> @@ -43,7 +43,6 @@
>  #define BRW_REG_H
>  
>  #include 
> -#include "main/imports.h"
>  #include "main/compiler.h"
>  #include "main/macros.h"
>  #include "program/prog_instruction.h"
> @@ -638,38 +637,19 @@ brw_imm_vf(unsigned v)
> return imm;
>  }
>  
> -/**
> - * Convert an integer into a "restricted" 8-bit float, used in vector
> - * immediates.  The 8-bit floating point format has a sign bit, an
> - * excess-3 3-bit exponent, and a 4-bit mantissa.  All integer values
> - * from -31 to 31 can be represented exactly.
> - */
> -static inline uint8_t
> -int_to_float8(int x)
> -{
> -   if (x == 0) {
> -  return 0;
> -   } else if (x < 0) {
> -  return 1 << 7 | int_to_float8(-x);
> -   } else {
> -  const unsigned exponent = _mesa_logbase2(x);
> -  const unsigned mantissa = (x - (1 << exponent)) << (4 - exponent);
> -  assert(exponent <= 4);
> -  return (exponent + 3) << 4 | mantissa;
> -   }
> -}
> +#define VF_ZERO 0x0
> +#define VF_ONE  0x30
> +#define VF_NEG  (1<<7)
>  
> -/**
> - * Construct a floating-point packed vector immediate from its integer
> - * values. \sa int_to_float8()
> - */
>  static inline struct brw_reg
> -brw_imm_vf4(int v0, int v1, int v2, int v3)
> +brw_imm_vf4(unsigned v0, unsigned v1, unsigned v2, unsigned v3)
>  {
> -   return brw_imm_vf((int_to_float8(v0) << 0) |
> - (int_to_float8(v1) << 8) |
> - (int_to_float8(v2) << 16) |
> - (int_to_float8(v3) << 24));
> +   struct brw_reg imm = brw_imm_reg(BRW_REGISTER_TYPE_VF);
> +   imm.vstride = BRW_VERTICAL_STRIDE_0;
> +   imm.width = BRW_WIDTH_4;
> +   imm.hstride = BRW_HORIZONTAL_STRIDE_1;
> +   imm.ud = ((v0 << 0) | (v1 << 8) | (v2 << 16) | (v3 << 24));
> +   return imm;
>  }
>  
>  
> -- 
> 2.4.9
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] gallium/hud: control visibility at startup and runtime.

[Eero Tamminen]

Hi,

This seems otherwise OK, but you should use sigaction() instead of 
signal() for portability (see NOTES section in the manual page).



- Eero

On 11/03/2015 12:43 PM, boombatower wrote:

- env GALLIUM_HUD_VISIBLE: control default visibility
- env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
---
  docs/envvars.html   |  6 ++
  src/gallium/auxiliary/hud/hud_context.c | 23 +++
  2 files changed, 29 insertions(+)

diff --git a/docs/envvars.html b/docs/envvars.html
index bdfe999..530bbb7 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -179,6 +179,12 @@ Mesa EGL supports different sets of environment variables. 
 See the
  GALLIUM_HUD - draws various information on the screen, like framerate,
  cpu load, driver statistics, performance counters, etc.
  Set GALLIUM_HUD=help and run e.g. glxgears for more info.
+GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
+GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified signal.
+Especially useful to toggle hud at specific points of application and
+disable for unencumbered viewing the rest of the time. For example, set
+GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10 (SIGUSR1).
+Use kill -10  to toggle the hud as desired.
  GALLIUM_LOG_FILE - specifies a file for logging all errors, warnings, etc.
  rather than stderr.
  GALLIUM_PRINT_OPTIONS - if non-zero, print all the Gallium environment
diff --git a/src/gallium/auxiliary/hud/hud_context.c 
b/src/gallium/auxiliary/hud/hud_context.c
index ffe30b8..f6bfa80 100644
--- a/src/gallium/auxiliary/hud/hud_context.c
+++ b/src/gallium/auxiliary/hud/hud_context.c
@@ -33,6 +33,7 @@
   * Set GALLIUM_HUD=help for more info.
   */

+#include 
  #include 

  #include "hud/hud_context.h"
@@ -51,8 +52,10 @@
  #include "tgsi/tgsi_text.h"
  #include "tgsi/tgsi_dump.h"

+static boolean __visible_toggle = false;

  struct hud_context {
+   boolean visible;
 struct pipe_context *pipe;
 struct cso_context *cso;
 struct u_upload_mgr *uploader;
@@ -95,6 +98,11 @@ struct hud_context {
 } text, bg, whitelines;
  };

+static void
+signal_visible_handler(int signo)
+{
+   __visible_toggle = true;
+}

  static void
  hud_draw_colored_prims(struct hud_context *hud, unsigned prim,
@@ -431,8 +439,15 @@ hud_alloc_vertices(struct hud_context *hud, struct 
vertex_queue *v,
  void
  hud_draw(struct hud_context *hud, struct pipe_resource *tex)
  {
+   if (__visible_toggle) {
+  hud->visible = !hud->visible;
+  __visible_toggle = false;
+   }
+   if (!hud->visible) return;
+
 struct cso_context *cso = hud->cso;
 struct pipe_context *pipe = hud->pipe;
+
 struct pipe_framebuffer_state fb;
 struct pipe_surface surf_templ, *surf;
 struct pipe_viewport_state viewport;
@@ -1124,7 +1139,9 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)
 struct hud_context *hud;
 struct pipe_sampler_view view_templ;
 unsigned i;
+   boolean visible = debug_get_bool_option("GALLIUM_HUD_VISIBLE", true);
 const char *env = debug_get_option("GALLIUM_HUD", NULL);
+   long signo = debug_get_num_option("GALLIUM_HUD_TOGGLE_SIGNAL", 0);

 if (!env || !*env)
return NULL;
@@ -1138,6 +1155,7 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)
 if (!hud)
return NULL;

+   hud->visible = visible;
 hud->pipe = pipe;
 hud->cso = cso;
 hud->uploader = u_upload_create(pipe, 256 * 1024, 16,
@@ -1267,6 +1285,11 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)

 LIST_INITHEAD(&hud->pane_list);

+   if (signo < 1 || signo >= NSIG)
+  fprintf(stderr, "gallium_hud: invalid signal %ld\n", signo);
+   else if (signal(signo, signal_visible_handler) == SIG_ERR)
+  fprintf(stderr, "gallium_hud: unable to set handler for signal %ld\n", 
signo);
+
 hud_parse_env_var(hud, env);
 return hud;
  }



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

Re: [Mesa-dev] [PATCH 00/10] i965: always mark used surfaces in the visitors

[Francisco Jerez]

Iago Toral  writes:

> On Fri, 2015-10-30 at 16:19 +0200, Francisco Jerez wrote:
>> Iago Toral Quiroga  writes:
>> 
>> > Right now some opcodes that only use constant surface indexing mark them as
>> > used in the generator while others do it in the visitor. When the opcode 
>> > can
>> > handle both direct and indirect surface indexing then some opcodes handle
>> > only the constant part in the generator and leave the indirect case to the
>> > caller. It is all very inconsistent and leads to confusion, since one has 
>> > to
>> > go and look into the generator code in each case to check if it marks 
>> > surfaces
>> > as used or not, and in which cases.
>> >
>> > when I was working on SSBOs I was tempted to try and fix this but then I
>> > forgot. Jordan bumped into this recently too when comparing visitor
>> > code paths for similar opcodes (ubos and ssbos) that need to handle this
>> > differently because they use different generator opcodes.
>> >
>> > Since the generator opcodes never handle marking of indirect surfaces, just
>> > leave surface marking to the caller completely, since callers always have
>> > all the information needed for this. It also makes things more consistent
>> > and clear for everyone: marking surfaces as used is always on the side
>> > of the visitor, never the generator.
>> >
>> > No piglit regressions observed in my IVB laptop. Would be nice to have
>> > someone giving this a try with Jenkins though, to make sure I did not miss
>> > anything in paths specific to other gens.
>> 
>> Jenkins seems to be mostly happy about the series except for three
>> apparent regressions:
>> 
>> piglit.spec.arb_fragment_layer_viewport.layer-gs-writes-in-range.bdwm64
>
> Mmmm... not sure what could be going on with this, at first glance, it
> does not look like this test should be affected by this series. The test
> does not use any UBOs, does not really check what is written to the FBO,
> does not emit texture accesses... Also, there are other tests in
> piglit.spec.arb_fragment_layer_viewport that do very similar stuff
> (specially the our-of-range test) and those are passing fine.
>
Odd, I guess it may have been an intermittent failing test that just
happens to have failed during your run.  Mark, have you seen any of
these fail intermittently by any chance?

>> 
>> piglit.spec.arb_shader_texture_lod.compiler.tex_grad-texture2dproj-2d-vec4.frag.g965m64
>> 
>> piglit.spec.glsl-es-1_00.compiler.structure-and-array-operations.sampler-array-index.frag.g965m64
>> 
>> The latter two die with a crash so you may be able to look into them
>> even if you don't have the original i965 by using INTEL_DEVID_OVERRIDE. ;)
>
> Unfortunately it seems that these don't break for me with the DEVID
> override, that's weird I guess, since they are compiler tests that I can
> fully run without INTEL_NO_HW set:
>
> $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest
> generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag
>  pass 1.10 GL_ARB_shader_texture_lod
> Successfully compiled fragment shader
> generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag:
>  
> PIGLIT: {"result": "pass" }
>
> $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest_gles2
> tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag
>  pass 1.00
> Successfully compiled fragment shader
> tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag:
>  0:21(21): warning: sampler arrays indexed with non-constant expressions will 
> be forbidden in GLSL 3.00 and later
> PIGLIT: {"result": "pass" }
>
Looking at the logs it actually seems to have been an assertion failure:

glslparsertest_gles2:
/mnt/space/jenkins/jobs/Leeroy/workspace/repos/mesa/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp:112:
void brw::fs_live_variables::setup_one_write(brw::block_data*, fs_inst*,
int, const fs_reg&): Assertion `var < num_vars' failed.

Did you test it on a debug build?

> I tried with most chipsets in between PCI_CHIP_I965_G (0x29A2) and
> PCI_CHIP_G41_G (0x2E32).
>
> Iago
>
>> >
>> > Iago Toral Quiroga (10):
>> >   i965/fs: Do not mark direct used surfaces in
>> > VARYING_PULL_CONSTANT_LOAD
>> >   i965/fs: Do not mark used direct surfaces in
>> > UNIFORM_PULL_CONSTANT_LOAD
>> >   i965/fs: Do not mark used direct surfaces in the generator for texture
>> > opcodes
>> >   i965/vec4: Do not mark used direct surfaces in
>> > VS_OPCODE_PULL_CONSTANT_LOAD
>> >   i965/vec4: Do not mark used direct surfaces in the generator for
>> > texture opcodes
>> >   i965/vec4: Do not mark used surfaces in SHADER_OPCODE_SHADER_TIME_ADD
>> >   i965/fs: Do not mark used surfaces in SHADER_OPCODE_SHADER_TIME_ADD
>> >   i965/vec4: Do not mark used surfaces in VS_OPCODE_GET_BUFFER_SIZE
>> >   i965/fs: Do not mark used surfaces in FS_OPCODE_GET_BUFFER_SIZE
>> >   i965/fs: Do not mark used surfaces in
>> > FS_OPCODE_FB_WRITE

Re: [Mesa-dev] [PATCH 04/24] i965: Make 'dw1' and 'bits' unnamed structures in brw_reg.

[Francisco Jerez]

Matt Turner  writes:

> Generated by
>
>sed -i -e 's/\.bits\././g' *.c *.h *.cpp
>sed -i -e 's/dw1\.//g' *.c *.h *.cpp
>
> and then reverting changes to comments in gen7_blorp.cpp and
> brw_fs_generator.cpp.
>
> There wasn't any utility offered by forcing the programmer to list these
> to access their fields. Removing them will reduce churn in future
> commits.
>
> This is C11 (and gcc has apparently supported it for sometime
> "compatibility with other compilers")
>
> See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html

This is also used from C++ source where anonymous structs are not part
of any released standard.  I guess in C++ it would be preferable to
define accessor methods instead of relying on a language extension --
That would also allow you to introduce checks making sure that the
register is of the correct type in order to catch cases in which the
wrong field of the union is accessed easily.

> ---
>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 52 -
>  src/mesa/drivers/dri/i965/brw_ff_gs_emit.c |  2 +-
>  src/mesa/drivers/dri/i965/brw_fs.cpp   | 68 
> +++---
>  .../drivers/dri/i965/brw_fs_combine_constants.cpp  |  6 +-
>  .../drivers/dri/i965/brw_fs_copy_propagation.cpp   |  8 +--
>  src/mesa/drivers/dri/i965/brw_fs_cse.cpp   | 16 ++---
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 42 ++---
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp   |  2 +-
>  src/mesa/drivers/dri/i965/brw_reg.h| 38 ++--
>  src/mesa/drivers/dri/i965/brw_shader.cpp   | 30 +-
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 40 ++---
>  .../drivers/dri/i965/brw_vec4_copy_propagation.cpp |  6 +-
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   | 40 ++---
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
>  14 files changed, 176 insertions(+), 176 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index a6fbb54..775027d 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -169,10 +169,10 @@ brw_set_dest(struct brw_codegen *p, brw_inst *inst, 
> struct brw_reg dest)
>   brw_inst_set_dst_hstride(devinfo, inst, dest.hstride);
>} else {
>   brw_inst_set_dst_da16_subreg_nr(devinfo, inst, dest.subnr / 16);
> - brw_inst_set_da16_writemask(devinfo, inst, dest.dw1.bits.writemask);
> + brw_inst_set_da16_writemask(devinfo, inst, dest.writemask);
>   if (dest.file == BRW_GENERAL_REGISTER_FILE ||
>   dest.file == BRW_MESSAGE_REGISTER_FILE) {
> -assert(dest.dw1.bits.writemask != 0);
> +assert(dest.writemask != 0);
>   }
>/* From the Ivybridge PRM, Vol 4, Part 3, Section 5.2.4.1:
> *Although Dst.HorzStride is a don't care for Align16, HW needs
> @@ -187,13 +187,13 @@ brw_set_dest(struct brw_codegen *p, brw_inst *inst, 
> struct brw_reg dest)
> */
>if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) {
>   brw_inst_set_dst_ia1_addr_imm(devinfo, inst,
> -   dest.dw1.bits.indirect_offset);
> +   dest.indirect_offset);
>if (dest.hstride == BRW_HORIZONTAL_STRIDE_0)
>   dest.hstride = BRW_HORIZONTAL_STRIDE_1;
>   brw_inst_set_dst_hstride(devinfo, inst, dest.hstride);
>} else {
>   brw_inst_set_dst_ia16_addr_imm(devinfo, inst,
> -dest.dw1.bits.indirect_offset);
> +dest.indirect_offset);
>/* even ignored in da16, still need to set as '01' */
>   brw_inst_set_dst_hstride(devinfo, inst, 1);
>}
> @@ -243,7 +243,7 @@ validate_reg(const struct brw_device_info *devinfo,
>  */
> if (reg.file == BRW_ARCHITECTURE_REGISTER_FILE &&
> reg.nr == BRW_ARF_ACCUMULATOR)
> -  assert(reg.dw1.bits.swizzle == BRW_SWIZZLE_XYZW);
> +  assert(reg.swizzle == BRW_SWIZZLE_XYZW);
>  
> assert(reg.hstride >= 0 && reg.hstride < ARRAY_SIZE(hstride_for_reg));
> hstride = hstride_for_reg[reg.hstride];
> @@ -338,7 +338,7 @@ brw_set_src0(struct brw_codegen *p, brw_inst *inst, 
> struct brw_reg reg)
> brw_inst_set_src0_address_mode(devinfo, inst, reg.address_mode);
>  
> if (reg.file == BRW_IMMEDIATE_VALUE) {
> -  brw_inst_set_imm_ud(devinfo, inst, reg.dw1.ud);
> +  brw_inst_set_imm_ud(devinfo, inst, reg.ud);
>  
>/* The Bspec's section titled "Non-present Operands" claims that if 
> src0
> * is an immediate that src1's type must be the same as that of src0.
> @@ -408,9 +408,9 @@ brw_set_src0(struct brw_codegen *p, brw_inst *inst, 
> struct brw_reg reg)
>   brw_inst_set_src0_ia_subreg_nr(devinfo, inst, reg.subnr);
>  
>   if (brw_inst_access_mode(devinfo, 

Re: [Mesa-dev] gallium/hud: control visibility at startup and runtime.

[Marek Olšák]

On Tue, Nov 3, 2015 at 11:43 AM, boombatower  wrote:
> - env GALLIUM_HUD_VISIBLE: control default visibility
> - env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
> ---
>  docs/envvars.html   |  6 ++
>  src/gallium/auxiliary/hud/hud_context.c | 23 +++
>  2 files changed, 29 insertions(+)
>
> diff --git a/docs/envvars.html b/docs/envvars.html
> index bdfe999..530bbb7 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -179,6 +179,12 @@ Mesa EGL supports different sets of environment 
> variables.  See the
>  GALLIUM_HUD - draws various information on the screen, like framerate,
>  cpu load, driver statistics, performance counters, etc.
>  Set GALLIUM_HUD=help and run e.g. glxgears for more info.
> +GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
> +GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified signal.
> +Especially useful to toggle hud at specific points of application and
> +disable for unencumbered viewing the rest of the time. For example, set
> +GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10 
> (SIGUSR1).
> +Use kill -10  to toggle the hud as desired.
>  GALLIUM_LOG_FILE - specifies a file for logging all errors, warnings, 
> etc.
>  rather than stderr.
>  GALLIUM_PRINT_OPTIONS - if non-zero, print all the Gallium environment
> diff --git a/src/gallium/auxiliary/hud/hud_context.c 
> b/src/gallium/auxiliary/hud/hud_context.c
> index ffe30b8..f6bfa80 100644
> --- a/src/gallium/auxiliary/hud/hud_context.c
> +++ b/src/gallium/auxiliary/hud/hud_context.c
> @@ -33,6 +33,7 @@
>   * Set GALLIUM_HUD=help for more info.
>   */
>
> +#include 
>  #include 
>
>  #include "hud/hud_context.h"
> @@ -51,8 +52,10 @@
>  #include "tgsi/tgsi_text.h"
>  #include "tgsi/tgsi_dump.h"
>
> +static boolean __visible_toggle = false;
>
>  struct hud_context {
> +   boolean visible;
> struct pipe_context *pipe;
> struct cso_context *cso;
> struct u_upload_mgr *uploader;
> @@ -95,6 +98,11 @@ struct hud_context {
> } text, bg, whitelines;
>  };
>
> +static void
> +signal_visible_handler(int signo)
> +{
> +   __visible_toggle = true;
> +}
>
>  static void
>  hud_draw_colored_prims(struct hud_context *hud, unsigned prim,
> @@ -431,8 +439,15 @@ hud_alloc_vertices(struct hud_context *hud, struct 
> vertex_queue *v,
>  void
>  hud_draw(struct hud_context *hud, struct pipe_resource *tex)
>  {
> +   if (__visible_toggle) {
> +  hud->visible = !hud->visible;
> +  __visible_toggle = false;

Could we either
- not use global variables
or
- not modify global variables from any context functions.

There can be several contexts and therefore several HUDs per process.
This patch doesn't take that into account.

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

Re: [Mesa-dev] [PATCH 00/10] i965: always mark used surfaces in the visitors

[Iago Toral]

On Tue, 2015-11-03 at 15:28 +0200, Francisco Jerez wrote:
> Iago Toral  writes:
> 
> > On Fri, 2015-10-30 at 16:19 +0200, Francisco Jerez wrote:
> >> Iago Toral Quiroga  writes:
> >> 
> >> > Right now some opcodes that only use constant surface indexing mark them 
> >> > as
> >> > used in the generator while others do it in the visitor. When the opcode 
> >> > can
> >> > handle both direct and indirect surface indexing then some opcodes handle
> >> > only the constant part in the generator and leave the indirect case to 
> >> > the
> >> > caller. It is all very inconsistent and leads to confusion, since one 
> >> > has to
> >> > go and look into the generator code in each case to check if it marks 
> >> > surfaces
> >> > as used or not, and in which cases.
> >> >
> >> > when I was working on SSBOs I was tempted to try and fix this but then I
> >> > forgot. Jordan bumped into this recently too when comparing visitor
> >> > code paths for similar opcodes (ubos and ssbos) that need to handle this
> >> > differently because they use different generator opcodes.
> >> >
> >> > Since the generator opcodes never handle marking of indirect surfaces, 
> >> > just
> >> > leave surface marking to the caller completely, since callers always have
> >> > all the information needed for this. It also makes things more consistent
> >> > and clear for everyone: marking surfaces as used is always on the side
> >> > of the visitor, never the generator.
> >> >
> >> > No piglit regressions observed in my IVB laptop. Would be nice to have
> >> > someone giving this a try with Jenkins though, to make sure I did not 
> >> > miss
> >> > anything in paths specific to other gens.
> >> 
> >> Jenkins seems to be mostly happy about the series except for three
> >> apparent regressions:
> >> 
> >> piglit.spec.arb_fragment_layer_viewport.layer-gs-writes-in-range.bdwm64
> >
> > Mmmm... not sure what could be going on with this, at first glance, it
> > does not look like this test should be affected by this series. The test
> > does not use any UBOs, does not really check what is written to the FBO,
> > does not emit texture accesses... Also, there are other tests in
> > piglit.spec.arb_fragment_layer_viewport that do very similar stuff
> > (specially the our-of-range test) and those are passing fine.
> >
> Odd, I guess it may have been an intermittent failing test that just
> happens to have failed during your run.  Mark, have you seen any of
> these fail intermittently by any chance?
> 
> >> 
> >> piglit.spec.arb_shader_texture_lod.compiler.tex_grad-texture2dproj-2d-vec4.frag.g965m64
> >> 
> >> piglit.spec.glsl-es-1_00.compiler.structure-and-array-operations.sampler-array-index.frag.g965m64
> >> 
> >> The latter two die with a crash so you may be able to look into them
> >> even if you don't have the original i965 by using INTEL_DEVID_OVERRIDE. ;)
> >
> > Unfortunately it seems that these don't break for me with the DEVID
> > override, that's weird I guess, since they are compiler tests that I can
> > fully run without INTEL_NO_HW set:
> >
> > $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest
> > generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag
> >  pass 1.10 GL_ARB_shader_texture_lod
> > Successfully compiled fragment shader
> > generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag:
> >  
> > PIGLIT: {"result": "pass" }
> >
> > $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest_gles2
> > tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag
> >  pass 1.00
> > Successfully compiled fragment shader
> > tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag:
> >  0:21(21): warning: sampler arrays indexed with non-constant expressions 
> > will be forbidden in GLSL 3.00 and later
> > PIGLIT: {"result": "pass" }
> >
> Looking at the logs it actually seems to have been an assertion failure:
> 
> glslparsertest_gles2:
> /mnt/space/jenkins/jobs/Leeroy/workspace/repos/mesa/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp:112:
> void brw::fs_live_variables::setup_one_write(brw::block_data*, fs_inst*,
> int, const fs_reg&): Assertion `var < num_vars' failed.
> 
> Did you test it on a debug build?

Yes, it was a debug build.

I also don't see how these patches could break that assertion. The
assert is related liveness analysis and the number of variables in the
shader code, but these patches do not change the code we emit (or even
the number of vgrfs we instantiate), they only move calls to
brw_mark_surface_used around, which has no relation with the shader code
at all :-/

> > I tried with most chipsets in between PCI_CHIP_I965_G (0x29A2) and
> > PCI_CHIP_G41_G (0x2E32).
> >
> > Iago
> >
> >> >
> >> > Iago Toral Quiroga (10):
> >> >   i965/fs: Do not mark direct used surfaces in
> >> > VARYING_PULL_CONSTANT_LOAD
> >> >   i965/fs: Do not mark used direct surfaces in
> >> > UNIFORM_PULL_CON

Re: [Mesa-dev] Unused (?) duplicated GLSL IR state in NIR

[Connor Abbott]

On Tue, Nov 3, 2015 at 5:08 AM, Emil Velikov  wrote:
> On 2 November 2015 at 19:41, Jason Ekstrand  wrote:
>> On Mon, Nov 2, 2015 at 9:33 AM, Connor Abbott  wrote:
>>> On Mon, Nov 2, 2015 at 8:35 AM, Emil Velikov  
>>> wrote:
 Hi all,

 From a quick look, it seems that NIR copies (almost ?) all the state
 from GLSL IR even if it doesn't use it.

 The particular piece that I'm thinking about is nir_variable::data.
 Afaict this is a remnant from the early days, when the intent was to
 kill off the GLSL IR and use NIR directly. If so should we just nuke
 it, or (if there is someone working on it) add some comments "Keepme:
 WIP work by XXX to use this and kill the glsl IR one".

 The (not that distant) GLSL IR memory optimisations by Ian, seems to
 have missed NIR. Was that intentional or were those worked upon before
 NIR got merged ? Perhaps it's worth porting them over ?


 Thanks
 Emil
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>> Hi Emil,
>>>
>>> Indeed, nir_variable was copied from ir_variable before Ian's memory
>>> optimizations landed. Also, as you noticed, there are a number of
>>> fields only used by the GLSL linker, so they're unused until/unless we
>>> do linking in NIR. I wouldn't be opposed to nuking those, since we can
>>> always revert the commit if/when we get to that.
>>
>> Agreed.  If you want to nuke some of them, feel free to send the
>> patch.  If you do, please CC me.  There are some things that we're
>> using for SPIR-V so I might request that you keep a field or two.
>> However, cleaning it up would definitely be a good idea.
>>
>> One thing I will note though is that you should consider this a
>> cleanup and not an optimization.  The reason why Ian's stuff helped
>> GLSL IR so much is that it uses variables extensively.  The average
>> NIR shader only has about half a dozen variables by the time you get
>> done optimizing so it's not nearly as much of a problem.
> Ack. Thanks guys. So currently no-one is pursuing move the linking to NIR ?

No, not currently, since first it requires a lot of optimization
passes that GLSL IR has but not NIR to be translated to NIR (loop
unrolling, function inlining, tree balancing etc.) before it's both
possible and not a regression.

>
> On the optimisation vs cleanup note - it'll be tiny bit of both.
>
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/10] i965: always mark used surfaces in the visitors

[Francisco Jerez]

Iago Toral  writes:

> On Tue, 2015-11-03 at 15:28 +0200, Francisco Jerez wrote:
>> Iago Toral  writes:
>> 
>> > On Fri, 2015-10-30 at 16:19 +0200, Francisco Jerez wrote:
>> >> Iago Toral Quiroga  writes:
>> >> 
>> >> > Right now some opcodes that only use constant surface indexing mark 
>> >> > them as
>> >> > used in the generator while others do it in the visitor. When the 
>> >> > opcode can
>> >> > handle both direct and indirect surface indexing then some opcodes 
>> >> > handle
>> >> > only the constant part in the generator and leave the indirect case to 
>> >> > the
>> >> > caller. It is all very inconsistent and leads to confusion, since one 
>> >> > has to
>> >> > go and look into the generator code in each case to check if it marks 
>> >> > surfaces
>> >> > as used or not, and in which cases.
>> >> >
>> >> > when I was working on SSBOs I was tempted to try and fix this but then I
>> >> > forgot. Jordan bumped into this recently too when comparing visitor
>> >> > code paths for similar opcodes (ubos and ssbos) that need to handle this
>> >> > differently because they use different generator opcodes.
>> >> >
>> >> > Since the generator opcodes never handle marking of indirect surfaces, 
>> >> > just
>> >> > leave surface marking to the caller completely, since callers always 
>> >> > have
>> >> > all the information needed for this. It also makes things more 
>> >> > consistent
>> >> > and clear for everyone: marking surfaces as used is always on the side
>> >> > of the visitor, never the generator.
>> >> >
>> >> > No piglit regressions observed in my IVB laptop. Would be nice to have
>> >> > someone giving this a try with Jenkins though, to make sure I did not 
>> >> > miss
>> >> > anything in paths specific to other gens.
>> >> 
>> >> Jenkins seems to be mostly happy about the series except for three
>> >> apparent regressions:
>> >> 
>> >> 
>> >> piglit.spec.arb_fragment_layer_viewport.layer-gs-writes-in-range.bdwm64
>> >
>> > Mmmm... not sure what could be going on with this, at first glance, it
>> > does not look like this test should be affected by this series. The test
>> > does not use any UBOs, does not really check what is written to the FBO,
>> > does not emit texture accesses... Also, there are other tests in
>> > piglit.spec.arb_fragment_layer_viewport that do very similar stuff
>> > (specially the our-of-range test) and those are passing fine.
>> >
>> Odd, I guess it may have been an intermittent failing test that just
>> happens to have failed during your run.  Mark, have you seen any of
>> these fail intermittently by any chance?
>> 
>> >> 
>> >> piglit.spec.arb_shader_texture_lod.compiler.tex_grad-texture2dproj-2d-vec4.frag.g965m64
>> >> 
>> >> piglit.spec.glsl-es-1_00.compiler.structure-and-array-operations.sampler-array-index.frag.g965m64
>> >> 
>> >> The latter two die with a crash so you may be able to look into them
>> >> even if you don't have the original i965 by using INTEL_DEVID_OVERRIDE. ;)
>> >
>> > Unfortunately it seems that these don't break for me with the DEVID
>> > override, that's weird I guess, since they are compiler tests that I can
>> > fully run without INTEL_NO_HW set:
>> >
>> > $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest
>> > generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag
>> >  pass 1.10 GL_ARB_shader_texture_lod
>> > Successfully compiled fragment shader
>> > generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag:
>> >  
>> > PIGLIT: {"result": "pass" }
>> >
>> > $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest_gles2
>> > tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag
>> >  pass 1.00
>> > Successfully compiled fragment shader
>> > tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag:
>> >  0:21(21): warning: sampler arrays indexed with non-constant expressions 
>> > will be forbidden in GLSL 3.00 and later
>> > PIGLIT: {"result": "pass" }
>> >
>> Looking at the logs it actually seems to have been an assertion failure:
>> 
>> glslparsertest_gles2:
>> /mnt/space/jenkins/jobs/Leeroy/workspace/repos/mesa/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp:112:
>> void brw::fs_live_variables::setup_one_write(brw::block_data*, fs_inst*,
>> int, const fs_reg&): Assertion `var < num_vars' failed.
>> 
>> Did you test it on a debug build?
>
> Yes, it was a debug build.
>
> I also don't see how these patches could break that assertion. The
> assert is related liveness analysis and the number of variables in the
> shader code, but these patches do not change the code we emit (or even
> the number of vgrfs we instantiate), they only move calls to
> brw_mark_surface_used around, which has no relation with the shader code
> at all :-/
>
Yeah, sounds like an unrelated intermittent failure.  Mark, had you seen
this already?

>> > I tried with most chipsets in between PCI_CHIP_I965_G (0x29A2) and
>> > PC

Re: [Mesa-dev] gallium/hud: control visibility at startup and runtime.

[Brian Paul]

On 11/03/2015 03:43 AM, boombatower wrote:

- env GALLIUM_HUD_VISIBLE: control default visibility
- env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
---
  docs/envvars.html   |  6 ++
  src/gallium/auxiliary/hud/hud_context.c | 23 +++
  2 files changed, 29 insertions(+)

diff --git a/docs/envvars.html b/docs/envvars.html
index bdfe999..530bbb7 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -179,6 +179,12 @@ Mesa EGL supports different sets of environment variables. 
 See the
  GALLIUM_HUD - draws various information on the screen, like framerate,
  cpu load, driver statistics, performance counters, etc.
  Set GALLIUM_HUD=help and run e.g. glxgears for more info.
+GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
+GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified signal.
+Especially useful to toggle hud at specific points of application and
+disable for unencumbered viewing the rest of the time. For example, set
+GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10 (SIGUSR1).
+Use kill -10  to toggle the hud as desired.
  GALLIUM_LOG_FILE - specifies a file for logging all errors, warnings, etc.
  rather than stderr.
  GALLIUM_PRINT_OPTIONS - if non-zero, print all the Gallium environment
diff --git a/src/gallium/auxiliary/hud/hud_context.c 
b/src/gallium/auxiliary/hud/hud_context.c
index ffe30b8..f6bfa80 100644
--- a/src/gallium/auxiliary/hud/hud_context.c
+++ b/src/gallium/auxiliary/hud/hud_context.c
@@ -33,6 +33,7 @@
   * Set GALLIUM_HUD=help for more info.
   */

+#include 
  #include 

  #include "hud/hud_context.h"
@@ -51,8 +52,10 @@
  #include "tgsi/tgsi_text.h"
  #include "tgsi/tgsi_dump.h"

+static boolean __visible_toggle = false;


There's no need for the double underscore prefix.  Maybe "toggle_hud" 
would be a better name.  And there should probably be a comment on it 
explaining how it works.  It's not obvious.


Let's either use gallium's boolean/TRUE/FALSE or stdbool's 
bool/true/false rather than mix them.





  struct hud_context {
+   boolean visible;
 struct pipe_context *pipe;
 struct cso_context *cso;
 struct u_upload_mgr *uploader;
@@ -95,6 +98,11 @@ struct hud_context {
 } text, bg, whitelines;
  };

+static void
+signal_visible_handler(int signo)
+{
+   __visible_toggle = true;
+}

  static void
  hud_draw_colored_prims(struct hud_context *hud, unsigned prim,
@@ -431,8 +439,15 @@ hud_alloc_vertices(struct hud_context *hud, struct 
vertex_queue *v,
  void
  hud_draw(struct hud_context *hud, struct pipe_resource *tex)
  {
+   if (__visible_toggle) {
+  hud->visible = !hud->visible;
+  __visible_toggle = false;
+   }
+   if (!hud->visible) return;


Please put 'return' on the next line.

I keep forgetting, but I think some compilers complain about 
declarations after code, so this should be moved after the declarations 
below.




+
 struct cso_context *cso = hud->cso;
 struct pipe_context *pipe = hud->pipe;
+


No need to introduce a new empty line here.


 struct pipe_framebuffer_state fb;
 struct pipe_surface surf_templ, *surf;
 struct pipe_viewport_state viewport;
@@ -1124,7 +1139,9 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)
 struct hud_context *hud;
 struct pipe_sampler_view view_templ;
 unsigned i;
+   boolean visible = debug_get_bool_option("GALLIUM_HUD_VISIBLE", true);
 const char *env = debug_get_option("GALLIUM_HUD", NULL);
+   long signo = debug_get_num_option("GALLIUM_HUD_TOGGLE_SIGNAL", 0);


Can we avoid calling debug_get_bool/num_option() for every draw call? 
Wouldn't hud_create() be a better place for this?





 if (!env || !*env)
return NULL;
@@ -1138,6 +1155,7 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)
 if (!hud)
return NULL;

+   hud->visible = visible;
 hud->pipe = pipe;
 hud->cso = cso;
 hud->uploader = u_upload_create(pipe, 256 * 1024, 16,
@@ -1267,6 +1285,11 @@ hud_create(struct pipe_context *pipe, struct cso_context 
*cso)

 LIST_INITHEAD(&hud->pane_list);

+   if (signo < 1 || signo >= NSIG)
+  fprintf(stderr, "gallium_hud: invalid signal %ld\n", signo);
+   else if (signal(signo, signal_visible_handler) == SIG_ERR)
+  fprintf(stderr, "gallium_hud: unable to set handler for signal %ld\n", 
signo);


Can that go into hud_create() too?

Finally, I'll need to check if any of this can work on Windows.  If not, 
we'll need some #ifndef PIPE_OS_WINDOWS tests...


-Brian


+
 hud_parse_env_var(hud, env);
 return hud;
  }



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

Re: [Mesa-dev] MSVC (2015) builds

[Brian Paul]

On 11/02/2015 08:42 PM, Janusz Ganczarski wrote:

Hello,
In attachment fixed Visual C++ 2015 (VC 14) builds for Mesa.
Currently only gallium softpipe driver support. Gallium llvmpipe
driver support work in progress.


I'm not sure we're interested in MSVC project files (I'm certainly not). 
 Unless we had several people who wanted to actively develop Mesa with 
MSVC and promise to maintain MSVC support, I don't think this would be 
used much and would eventually be dropped.  We've gone through this 
several times in the past.


-Brian

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

Re: [Mesa-dev] [PATCH 08/24] i965: Remove fixed_hw_reg field from backend_reg.

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:

Please add a bit of commit message - "Mostly unused as of last commit.
Fold the remaining cases (GRF only?) to use the base brw_reg struct."
or anything else that you feel is appropriate.

> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  93 +-
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |   9 +-
>  src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp  |   4 +-
>  src/mesa/drivers/dri/i965/brw_ir_fs.h  |   4 +-
>  src/mesa/drivers/dri/i965/brw_ir_vec4.h|   4 +-
>  .../drivers/dri/i965/brw_schedule_instructions.cpp |  54 +--
>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +-
>  src/mesa/drivers/dri/i965/brw_shader.h |   5 +-
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 108 
> +
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  12 +--
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   2 -
>  11 files changed, 141 insertions(+), 162 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 6b9e979..243a4ac 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -422,13 +422,15 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
> uint8_t vf3)
> (vf3 << 24);
>  }
>
> -/** Fixed brw_reg. */
> -fs_reg::fs_reg(struct brw_reg fixed_hw_reg)
> +fs_reg::fs_reg(struct brw_reg reg) :
> +   backend_reg(reg)
>  {
> -   init();
Keep this for now ?

> this->file = HW_REG;
> -   this->fixed_hw_reg = fixed_hw_reg;
> -   this->type = fixed_hw_reg.type;

> +   this->reg = 0;
> +   this->reg_offset = 0;
> +   this->subreg_offset = 0;
> +   this->reladdr = NULL;
> +   this->stride = 1;
.. and drop these ?


>  fs_reg::fs_reg(enum register_file file, int reg, enum brw_reg_type type)
>  {
> init();
> @@ -1400,10 +1399,11 @@ fs_visitor::assign_curb_setup()
> struct brw_reg brw_reg = brw_vec1_grf(payload.num_regs +
>   constant_nr / 8,
>   constant_nr % 8);
> +brw_reg.abs = inst->src[i].abs;
> +brw_reg.negate = inst->src[i].negate;
>
This looks like a bugfix which might contribute to an occasional
random behaviour ?

>  assert(inst->src[i].stride == 0);
> -   inst->src[i].file = HW_REG;
> -   inst->src[i].fixed_hw_reg = byte_offset(
> +inst->src[i] = byte_offset(
Something looks odd here. We will likely create a brw_reg and the
remaining bits of inst->src[i] will be uninitialised as the old object
will be torn down. Although I could be missing something.

> @@ -1556,12 +1556,15 @@ fs_visitor::assign_vs_urb_setup()
>inst->src[i].reg +
>inst->src[i].reg_offset;
>
> -inst->src[i].file = HW_REG;
> -inst->src[i].fixed_hw_reg =
> +struct brw_reg reg =
> stride(byte_offset(retype(brw_vec8_grf(grf, 0), 
> inst->src[i].type),
>inst->src[i].subreg_offset),
>inst->exec_size * inst->src[i].stride,
>inst->exec_size, inst->src[i].stride);
> +reg.abs = inst->src[i].abs;
> +reg.negate = inst->src[i].negate;
> +
> +inst->src[i] = reg;
Similar concerns as above.

> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h 
> b/src/mesa/drivers/dri/i965/brw_shader.h
> index 9a7a2d5..4d9a946 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.h
> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> @@ -51,6 +51,9 @@ enum PACKED register_file {
>  #ifdef __cplusplus
>  struct backend_reg : public brw_reg
>  {
> +   backend_reg() {}
> +   backend_reg(struct brw_reg reg) : brw_reg(reg) {}
> +
As brw_reg is a normal struct, wouldn't it be better if we initialize
it and backend_reg's members in the above two ? Ideally this could be
a separate commit (7.1) and patch 7.2 would in turn drop the init()
and use the above ctors. If you want to keep it as is, I won't push
it.

> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 74d26da..ad52c9f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -119,25 +119,24 @@ src_reg::src_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
> uint8_t vf3)
> (vf3 << 24);
>  }
>
> -src_reg::src_reg(struct brw_reg reg)
> +src_reg::src_reg(struct brw_reg reg) :
> +   backend_reg(reg)
>  {
> -   init();
> -
Might as well keep the initI() for now (here and below)...

> this->file = HW_REG;
> -   this->fixed_hw_reg = reg;
> -   this->type = reg.type;

> +   this->reg = 0;
> +   this->reg_offset = 0;
> +   this->swizzle = BRW_SWIZZLE_;
> +   this->reladdr = NULL;
... and drop these (here and b

Re: [Mesa-dev] [PATCH 12/24] i965: Initialize registers' file to BAD_FILE.

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:
> The test (file == BAD_FILE) works on registers for which the constructor
> has not run because BAD_FILE is zero.  The next commit will move
> BAD_FILE in the enum so that it's no longer zero.

Doesn't the DECLARE_RALLOC_CXX_OPERATORS macro and fs_reg::fs_reg()
kick in ? If not things look quite fragile and perhaps we should wire
them up.

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

Re: [Mesa-dev] [PATCH 17/24] i965: Replace HW_REG with ARF/GRF.

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:

> @@ -422,7 +423,7 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
> uint8_t vf3)
>  fs_reg::fs_reg(struct brw_reg reg) :
> backend_reg(reg)
>  {
> -   this->file = HW_REG;
> +   this->file = (enum register_file)reg.file;
You're not adding the cast in the rest of the ctors (be that fs_reg,
src_reg or dst_reg) so might as well drop this one ?

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

Re: [Mesa-dev] [PATCH 18/24] i965: Combine register file field.

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:

> index 6eeafd5..3d2b051 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -423,7 +423,6 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
> uint8_t vf3)
>  fs_reg::fs_reg(struct brw_reg reg) :
> backend_reg(reg)
>  {
> -   this->file = (enum register_file)reg.file;
Should we fold the remaining this->file = foo into the backend_reg() ctors ?

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

Re: [Mesa-dev] [PATCH 20/24] Revert "i965: Have brw_imm_vf4() take the vector components as integer values."

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:
> This reverts commit bbf8239f92ecd79431dfa41402e1c85318e7267f.
>
> I didn't like that commit to begin with -- computing things at compile
> time is fine -- but for purposes of verifying that the resulting values
> are correct, looking up 0x00 and 0x30 in a table is a lot better than
> evaluating a recursive function.
>
> Anyway, by making brw_imm_vf4() take the actual 8-bit restricted floats
> directly (instead of only integral values that would be converted to
> restricted float), we can use this function as a replacement for the
> vector float src_reg/fs_reg constructors.
Yes please. If we have any floats we can easily convert them with the
existing brw_float_to_vf().

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

Re: [Mesa-dev] [PATCH 23/24] i965/vec4: Replace src_reg(imm) constructors with brw_imm_*().

[Emil Velikov]

On 3 November 2015 at 00:29, Matt Turner  wrote:

> @@ -387,7 +342,9 @@ vec4_visitor::opt_vector_float()
>
>remaining_channels &= ~inst->dst.writemask;
>if (remaining_channels == 0) {
> - vec4_instruction *mov = MOV(inst->dst, imm);
> + unsigned vf;
> + memcpy(&vf, imm, sizeof(vf));
> + vec4_instruction *mov = MOV(inst->dst, brw_imm_vf(vf));
You can drop the temp variable + memcpy call and use brw_imm_vf4(imm[x],)

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

Re: [Mesa-dev] gallium/hud: control visibility at startup and runtime.

[Eero Tamminen]

On 11/03/2015 05:43 PM, Brian Paul wrote:

On 11/03/2015 03:43 AM, boombatower wrote:

- env GALLIUM_HUD_VISIBLE: control default visibility
- env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
---
  docs/envvars.html   |  6 ++
  src/gallium/auxiliary/hud/hud_context.c | 23 +++
  2 files changed, 29 insertions(+)

diff --git a/docs/envvars.html b/docs/envvars.html
index bdfe999..530bbb7 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -179,6 +179,12 @@ Mesa EGL supports different sets of environment
variables.  See the
  GALLIUM_HUD - draws various information on the screen, like
framerate,
  cpu load, driver statistics, performance counters, etc.
  Set GALLIUM_HUD=help and run e.g. glxgears for more info.
+GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
+GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified
signal.
+Especially useful to toggle hud at specific points of application
and
+disable for unencumbered viewing the rest of the time. For
example, set
+GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10
(SIGUSR1).
+Use kill -10  to toggle the hud as desired.
  GALLIUM_LOG_FILE - specifies a file for logging all errors,
warnings, etc.
  rather than stderr.
  GALLIUM_PRINT_OPTIONS - if non-zero, print all the Gallium
environment
diff --git a/src/gallium/auxiliary/hud/hud_context.c
b/src/gallium/auxiliary/hud/hud_context.c
index ffe30b8..f6bfa80 100644
--- a/src/gallium/auxiliary/hud/hud_context.c
+++ b/src/gallium/auxiliary/hud/hud_context.c
@@ -33,6 +33,7 @@
   * Set GALLIUM_HUD=help for more info.
   */

+#include 
  #include 

  #include "hud/hud_context.h"
@@ -51,8 +52,10 @@
  #include "tgsi/tgsi_text.h"
  #include "tgsi/tgsi_dump.h"

+static boolean __visible_toggle = false;


There's no need for the double underscore prefix.  Maybe "toggle_hud"
would be a better name.  And there should probably be a comment on it
explaining how it works.  It's not obvious.


Regarding Marek's comment, IMHO it's better to have this as 
"huds_visible", see below...




Let's either use gallium's boolean/TRUE/FALSE or stdbool's
bool/true/false rather than mix them.




  struct hud_context {
+   boolean visible;
 struct pipe_context *pipe;
 struct cso_context *cso;
 struct u_upload_mgr *uploader;
@@ -95,6 +98,11 @@ struct hud_context {
 } text, bg, whitelines;
  };

+static void
+signal_visible_handler(int signo)
+{
+   __visible_toggle = true;
+}

  static void
  hud_draw_colored_prims(struct hud_context *hud, unsigned prim,
@@ -431,8 +439,15 @@ hud_alloc_vertices(struct hud_context *hud,
struct vertex_queue *v,
  void
  hud_draw(struct hud_context *hud, struct pipe_resource *tex)
  {
+   if (__visible_toggle) {
+  hud->visible = !hud->visible;
+  __visible_toggle = false;
+   }
+   if (!hud->visible) return;


if (hud->visible != huds_visible)
hud->visible = huds_visible;
if (!hud->visible)
return



Please put 'return' on the next line.

I keep forgetting, but I think some compilers complain about
declarations after code, so this should be moved after the declarations
below.

>

+
 struct cso_context *cso = hud->cso;
 struct pipe_context *pipe = hud->pipe;
+


No need to introduce a new empty line here.


 struct pipe_framebuffer_state fb;
 struct pipe_surface surf_templ, *surf;
 struct pipe_viewport_state viewport;
@@ -1124,7 +1139,9 @@ hud_create(struct pipe_context *pipe, struct
cso_context *cso)
 struct hud_context *hud;
 struct pipe_sampler_view view_templ;
 unsigned i;
+   boolean visible = debug_get_bool_option("GALLIUM_HUD_VISIBLE", true);
 const char *env = debug_get_option("GALLIUM_HUD", NULL);
+   long signo = debug_get_num_option("GALLIUM_HUD_TOGGLE_SIGNAL", 0);


Can we avoid calling debug_get_bool/num_option() for every draw call?
Wouldn't hud_create() be a better place for this?


Signal checking and setup should be done only once in program startup. 
 Is hud_create() run for each context (hud) separately?



- Eero




 if (!env || !*env)
return NULL;
@@ -1138,6 +1155,7 @@ hud_create(struct pipe_context *pipe, struct
cso_context *cso)
 if (!hud)
return NULL;

+   hud->visible = visible;
 hud->pipe = pipe;
 hud->cso = cso;
 hud->uploader = u_upload_create(pipe, 256 * 1024, 16,
@@ -1267,6 +1285,11 @@ hud_create(struct pipe_context *pipe, struct
cso_context *cso)

 LIST_INITHEAD(&hud->pane_list);

+   if (signo < 1 || signo >= NSIG)
+  fprintf(stderr, "gallium_hud: invalid signal %ld\n", signo);
+   else if (signal(signo, signal_visible_handler) == SIG_ERR)
+  fprintf(stderr, "gallium_hud: unable to set handler for signal
%ld\n", signo);


Can that go into hud_create() too?

Finally, I'll need to check if any of this can work on Windows.  If not,
we'll need some #ifndef PIPE_OS_WINDOWS tests...

-Brian


+

Re: [Mesa-dev] [PATCH 00/24] i965: Refactor register classes

[Emil Velikov]

Hi Matt,

On 3 November 2015 at 00:29, Matt Turner  wrote:
> backend_reg (from which fs_reg, src_reg, and dst_reg inherit) includes a
> brw_reg that's used for "hardware regs" -- precolored registers or 
> architecture
> registers. This leads to properties like source modifiers, the register type,
> swizzles, and writemasks being duplicated between the derived classes and the
> brw_reg and of course often being out of sync.
>
Great work ! While looking around the *_reg::init() I noticed a few
places where we don't derive (clone all the member variables) new
*_reg correctly. Plus I believe I've seen a few initialised member
variables - mostly on the abs/negate front.

I'm curious of there could attribute to the intermittent piglit
failures that people occasionally see.

> This series removes the "fixed_hw_reg" field from backend_reg by just making
> backend_reg inherit from brw_reg, and then removes fields duplicated in the
> derived classes. In the process, it gets rid of HW_REG.
>
I remember initially looking at fixed_hw_reg and having a few 'wtf'
moments. Inheriting from brw_reg will certainly help other (fs/vec4
IR) new-comers.

> This in turn simplifies a lot of code -- no longer do you have to check a
> number of subfields if file == HW_REG.
>
> The last few patches begin some clean ups -- since the base of our register
> classes is now brw_reg we don't need to do as many conversions. I've only
> handled immediates so far and more is planned, but the series is growing large
> and is a lot of churn already.
>
> The sizes of the register classes all shrink by 8 bytes:
>
>backend_reg   20 -> 12
>fs_reg40 -> 32
>src_reg   32 -> 24?
>dst_reg   32 -> 24?
>
> The remaining fields in the classes are
>
>backend_reg: reg_offset
>fs_reg:  reladdr, subreg_offset, stride
>src_reg  reladdr
>dst_reg  reladdr
>
I'm thinking that we can also move reladdr to backend_reg. Although
that'll be obviously follow up patches.

Fwiw I really like the whole series and barring a few small comments I
would love to see it land.


Big thanks for doing this.

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

Re: [Mesa-dev] MSVC (2015) builds

[Emil Velikov]

On 3 November 2015 at 15:48, Brian Paul  wrote:
> On 11/02/2015 08:42 PM, Janusz Ganczarski wrote:
>>
>> Hello,
>> In attachment fixed Visual C++ 2015 (VC 14) builds for Mesa.
>> Currently only gallium softpipe driver support. Gallium llvmpipe
>> driver support work in progress.
>
>
> I'm not sure we're interested in MSVC project files (I'm certainly not).
> Unless we had several people who wanted to actively develop Mesa with MSVC
> and promise to maintain MSVC support, I don't think this would be used much
> and would eventually be dropped.  We've gone through this several times in
> the past.
>
Out of curiosity: doesn't the existing scons build allow you to do all
sort of builds or there is something special that can be done with the
project files ?

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] gallium/hud: control visibility at startup and runtime.

[Emil Velikov]

Hello Jimmy,

Please add your name to your git config.

On 3 November 2015 at 10:43, boombatower  wrote:
> - env GALLIUM_HUD_VISIBLE: control default visibility
> - env GALLIUM_HUD_SIGNAL_TOGGLE: toggle visibility via signal
> ---
>  docs/envvars.html   |  6 ++
>  src/gallium/auxiliary/hud/hud_context.c | 23 +++
>  2 files changed, 29 insertions(+)
>
> diff --git a/docs/envvars.html b/docs/envvars.html
> index bdfe999..530bbb7 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -179,6 +179,12 @@ Mesa EGL supports different sets of environment 
> variables.  See the
>  GALLIUM_HUD - draws various information on the screen, like framerate,
>  cpu load, driver statistics, performance counters, etc.
>  Set GALLIUM_HUD=help and run e.g. glxgears for more info.
> +GALLIUM_HUD_VISIBLE - control default visibility, defaults to true.
> +GALLIUM_HUD_TOGGLE_SIGNAL - toggle visibility via user specified signal.
> +Especially useful to toggle hud at specific points of application and
> +disable for unencumbered viewing the rest of the time. For example, set
> +GALLIUM_HUD_VISIBLE to false and GALLIUM_HUD_SIGNAL_TOGGLE to 10 
> (SIGUSR1).
> +Use kill -10  to toggle the hud as desired.
A couple of open questions:

Wouldn't it be better to hardcode the signal to SIGUSR1 (otherwise one
can attempt to use SIGKILL/TERM/other funny cases) for now ?
Additionally dropping the GALLIUM_HUD_VISIBLE will eliminate some of
the issues pointed out so far. One can easily toggle off as the
application is up.

Regards,
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Unused (?) duplicated GLSL IR state in NIR

[Jason Ekstrand]

On Tue, Nov 3, 2015 at 2:08 AM, Emil Velikov  wrote:
> On 2 November 2015 at 19:41, Jason Ekstrand  wrote:
>> On Mon, Nov 2, 2015 at 9:33 AM, Connor Abbott  wrote:
>>> On Mon, Nov 2, 2015 at 8:35 AM, Emil Velikov  
>>> wrote:
 Hi all,

 From a quick look, it seems that NIR copies (almost ?) all the state
 from GLSL IR even if it doesn't use it.

 The particular piece that I'm thinking about is nir_variable::data.
 Afaict this is a remnant from the early days, when the intent was to
 kill off the GLSL IR and use NIR directly. If so should we just nuke
 it, or (if there is someone working on it) add some comments "Keepme:
 WIP work by XXX to use this and kill the glsl IR one".

 The (not that distant) GLSL IR memory optimisations by Ian, seems to
 have missed NIR. Was that intentional or were those worked upon before
 NIR got merged ? Perhaps it's worth porting them over ?


 Thanks
 Emil
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>> Hi Emil,
>>>
>>> Indeed, nir_variable was copied from ir_variable before Ian's memory
>>> optimizations landed. Also, as you noticed, there are a number of
>>> fields only used by the GLSL linker, so they're unused until/unless we
>>> do linking in NIR. I wouldn't be opposed to nuking those, since we can
>>> always revert the commit if/when we get to that.
>>
>> Agreed.  If you want to nuke some of them, feel free to send the
>> patch.  If you do, please CC me.  There are some things that we're
>> using for SPIR-V so I might request that you keep a field or two.
>> However, cleaning it up would definitely be a good idea.
>>
>> One thing I will note though is that you should consider this a
>> cleanup and not an optimization.  The reason why Ian's stuff helped
>> GLSL IR so much is that it uses variables extensively.  The average
>> NIR shader only has about half a dozen variables by the time you get
>> done optimizing so it's not nearly as much of a problem.
> Ack. Thanks guys. So currently no-one is pursuing move the linking to NIR ?

I have no plans to do full-blown GLSL-style linking in NIR.  Those
linking rules are nuts and that's what we have a GLSL compiler for.
If we do any linking in NIR, it'll be much simpler.  Like I said,
delete at will and, if I want you to keep anything, I'll say so when
you send the patches.
--Jason

> On the optimisation vs cleanup note - it'll be tiny bit of both.
>
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 1/6] gallium: expose a debug message callback settable by context owner

[Brian Paul]

On 10/31/2015 11:45 AM, Ilia Mirkin wrote:

On Sat, Oct 31, 2015 at 10:23 AM, Brian Paul  wrote:

On 10/30/2015 11:15 PM, Ilia Mirkin wrote:


This will allow gallium drivers to send messages to KHR_debug endpoints

Signed-off-by: Ilia Mirkin 
---
   src/gallium/auxiliary/util/u_debug.c | 16 
   src/gallium/auxiliary/util/u_debug.h | 24 
   src/gallium/docs/source/context.rst  |  3 +++
   src/gallium/include/pipe/p_context.h |  4 
   src/gallium/include/pipe/p_defines.h | 35
+++
   src/gallium/include/pipe/p_state.h   | 29 +
   6 files changed, 111 insertions(+)

diff --git a/src/gallium/auxiliary/util/u_debug.c
b/src/gallium/auxiliary/util/u_debug.c
index 7388a49..81280ea 100644
--- a/src/gallium/auxiliary/util/u_debug.c
+++ b/src/gallium/auxiliary/util/u_debug.c
@@ -70,6 +70,22 @@ void _debug_vprintf(const char *format, va_list ap)
   #endif
   }

+void
+_pipe_debug_message(
+   struct pipe_debug_info *info,
+   unsigned *id,
+   enum pipe_debug_source source,
+   enum pipe_debug_type type,
+   enum pipe_debug_severity severity,
+   const char *fmt, ...)
+{
+   va_list args;
+   va_start(args, fmt);
+   if (info && info->debug_message)
+  info->debug_message(info->data, id, source, type, severity, fmt,
args);
+   va_end(args);
+}
+

   void
   debug_disable_error_message_boxes(void)
diff --git a/src/gallium/auxiliary/util/u_debug.h
b/src/gallium/auxiliary/util/u_debug.h
index 926063a..a4ce88b 100644
--- a/src/gallium/auxiliary/util/u_debug.h
+++ b/src/gallium/auxiliary/util/u_debug.h
@@ -42,6 +42,7 @@
   #include "os/os_misc.h"

   #include "pipe/p_format.h"
+#include "pipe/p_defines.h"


   #ifdef__cplusplus
@@ -262,6 +263,29 @@ void _debug_assert_fail(const char *expr,
  _debug_printf("error: %s\n", __msg)
   #endif

+/**
+ * Output a debug log message to the debug info callback.
+ */
+#define pipe_debug_message(info, source, type, severity, fmt, ...) do { \
+   static unsigned id = 0; \
+   _pipe_debug_message(info, &id, \
+   PIPE_DEBUG_SOURCE_ ## source,\
+   PIPE_DEBUG_TYPE_ ## type, \
+   PIPE_DEBUG_SEVERITY_ ## severity, \
+   fmt, __VA_ARGS__); \
+} while (0)
+
+struct pipe_debug_info;
+
+void
+_pipe_debug_message(
+   struct pipe_debug_info *info,
+   unsigned *id,
+   enum pipe_debug_source source,
+   enum pipe_debug_type type,
+   enum pipe_debug_severity severity,
+   const char *fmt, ...) _util_printf_format(6, 7);
+

   /**
* Used by debug_dump_enum and debug_dump_flags to describe symbols.
diff --git a/src/gallium/docs/source/context.rst
b/src/gallium/docs/source/context.rst
index a7d08d2..5cae4d6 100644
--- a/src/gallium/docs/source/context.rst
+++ b/src/gallium/docs/source/context.rst
@@ -84,6 +84,9 @@ objects. They all follow simple, one-method binding
calls, e.g.
   levels. This corresponds to GL's ``PATCH_DEFAULT_OUTER_LEVEL``.
 * ``default_inner_level`` is the default value for the inner
tessellation
   levels. This corresponds to GL's ``PATCH_DEFAULT_INNER_LEVEL``.
+* ``set_debug_info`` sets the callback to be used for reporting
+  various debug messages, eventually reported via KHR_debug and
+  similar mechanisms.


   Sampler Views
diff --git a/src/gallium/include/pipe/p_context.h
b/src/gallium/include/pipe/p_context.h
index 6f9fe76..0d5eeab 100644
--- a/src/gallium/include/pipe/p_context.h
+++ b/src/gallium/include/pipe/p_context.h
@@ -45,6 +45,7 @@ struct pipe_blit_info;
   struct pipe_box;
   struct pipe_clip_state;
   struct pipe_constant_buffer;
+struct pipe_debug_info;
   struct pipe_depth_stencil_alpha_state;
   struct pipe_draw_info;
   struct pipe_fence_handle;
@@ -238,6 +239,9 @@ struct pipe_context {
 const float default_outer_level[4],
 const float default_inner_level[2]);

+   void (*set_debug_info)(struct pipe_context *,
+  struct pipe_debug_info *);



Evidently, the implementation of this function must make a copy of the
pipe_debug_info and can't just save the pointer.  Could you add a comment
about that?  'info' could be const-qualified too.



Will do. I believe that's how all the set_foo's work though, no?


I think so but would have to check to be sure.



+
  /**
   * Bind an array of shader buffers that will be used by a shader.
   * Any buffers that were previously bound to the specified range
diff --git a/src/gallium/include/pipe/p_defines.h
b/src/gallium/include/pipe/p_defines.h
index b15c880..860ebc6 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -868,6 +868,41 @@ struct pipe_driver_query_group_info
  unsigned num_queries;
   };

+enum pipe_debug_source
+{
+   PIPE_DEBUG_SOURCE_API,
+   PIPE_DEBUG_SOURCE_WINDOW_SYSTEM,
+   PIPE_DEBUG_SOURCE_SHADER_COMPILER,
+   PIPE_DEBUG_SOURCE_THIRD_PARTY,
+   PI

Re: [Mesa-dev] [PATCH 00/10] i965: always mark used surfaces in the visitors

[Mark Janes]

Francisco Jerez  writes:

> Iago Toral  writes:
>
>> On Tue, 2015-11-03 at 15:28 +0200, Francisco Jerez wrote:
>>> Iago Toral  writes:
>>> 
>>> > On Fri, 2015-10-30 at 16:19 +0200, Francisco Jerez wrote:
>>> >> Iago Toral Quiroga  writes:
>>> >> 
>>> >> > Right now some opcodes that only use constant surface indexing mark 
>>> >> > them as
>>> >> > used in the generator while others do it in the visitor. When the 
>>> >> > opcode can
>>> >> > handle both direct and indirect surface indexing then some opcodes 
>>> >> > handle
>>> >> > only the constant part in the generator and leave the indirect case to 
>>> >> > the
>>> >> > caller. It is all very inconsistent and leads to confusion, since one 
>>> >> > has to
>>> >> > go and look into the generator code in each case to check if it marks 
>>> >> > surfaces
>>> >> > as used or not, and in which cases.
>>> >> >
>>> >> > when I was working on SSBOs I was tempted to try and fix this but then 
>>> >> > I
>>> >> > forgot. Jordan bumped into this recently too when comparing visitor
>>> >> > code paths for similar opcodes (ubos and ssbos) that need to handle 
>>> >> > this
>>> >> > differently because they use different generator opcodes.
>>> >> >
>>> >> > Since the generator opcodes never handle marking of indirect surfaces, 
>>> >> > just
>>> >> > leave surface marking to the caller completely, since callers always 
>>> >> > have
>>> >> > all the information needed for this. It also makes things more 
>>> >> > consistent
>>> >> > and clear for everyone: marking surfaces as used is always on the side
>>> >> > of the visitor, never the generator.
>>> >> >
>>> >> > No piglit regressions observed in my IVB laptop. Would be nice to have
>>> >> > someone giving this a try with Jenkins though, to make sure I did not 
>>> >> > miss
>>> >> > anything in paths specific to other gens.
>>> >> 
>>> >> Jenkins seems to be mostly happy about the series except for three
>>> >> apparent regressions:
>>> >> 
>>> >> 
>>> >> piglit.spec.arb_fragment_layer_viewport.layer-gs-writes-in-range.bdwm64
>>> >
>>> > Mmmm... not sure what could be going on with this, at first glance, it
>>> > does not look like this test should be affected by this series. The test
>>> > does not use any UBOs, does not really check what is written to the FBO,
>>> > does not emit texture accesses... Also, there are other tests in
>>> > piglit.spec.arb_fragment_layer_viewport that do very similar stuff
>>> > (specially the our-of-range test) and those are passing fine.
>>> >
>>> Odd, I guess it may have been an intermittent failing test that just
>>> happens to have failed during your run.  Mark, have you seen any of
>>> these fail intermittently by any chance?

This was fixed in https://bugs.freedesktop.org/show_bug.cgi?id=92744

Ordinarily, all failures/fixes are tracked by commit, and failures due
to an old branchpoint are filtered out.  Unfortunately, these failures
were intermittent.

If you rebase, you shouldn't see those failures anymore.

>>> 
>>> >> 
>>> >> piglit.spec.arb_shader_texture_lod.compiler.tex_grad-texture2dproj-2d-vec4.frag.g965m64
>>> >> 
>>> >> piglit.spec.glsl-es-1_00.compiler.structure-and-array-operations.sampler-array-index.frag.g965m64
>>> >> 
>>> >> The latter two die with a crash so you may be able to look into them
>>> >> even if you don't have the original i965 by using INTEL_DEVID_OVERRIDE. 
>>> >> ;)
>>> >
>>> > Unfortunately it seems that these don't break for me with the DEVID
>>> > override, that's weird I guess, since they are compiler tests that I can
>>> > fully run without INTEL_NO_HW set:
>>> >
>>> > $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest
>>> > generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag
>>> >  pass 1.10 GL_ARB_shader_texture_lod
>>> > Successfully compiled fragment shader
>>> > generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag:
>>> >  
>>> > PIGLIT: {"result": "pass" }
>>> >
>>> > $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest_gles2
>>> > tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag
>>> >  pass 1.00
>>> > Successfully compiled fragment shader
>>> > tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag:
>>> >  0:21(21): warning: sampler arrays indexed with non-constant expressions 
>>> > will be forbidden in GLSL 3.00 and later
>>> > PIGLIT: {"result": "pass" }
>>> >
>>> Looking at the logs it actually seems to have been an assertion failure:
>>> 
>>> glslparsertest_gles2:
>>> /mnt/space/jenkins/jobs/Leeroy/workspace/repos/mesa/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp:112:
>>> void brw::fs_live_variables::setup_one_write(brw::block_data*, fs_inst*,
>>> int, const fs_reg&): Assertion `var < num_vars' failed.
>>> 
>>> Did you test it on a debug build?
>>
>> Yes, it was a debug build.
>>
>> I also don't see how these patches could break that assertion. The
>> assert is related 

Re: [Mesa-dev] [PATCH 05/24] i965: Reorganize brw_reg fields.

[Matt Turner]

On Tue, Nov 3, 2015 at 2:38 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>> Put fields that are meaningless with an immediate in the same storage
>> with the immediate.
> There is something funky here. Should it be
>
> "Put fields that are meaningless _without_ an immediate in the same storage
> _as_ the immediate..."


I think what I said (though "with" -> "as" does sound better). To
elaborate: some things like writemask, swizzle, vstride, width,
hstride are only meaningful when the register is not an immediate --
that is, they're mutually exclusive.

So, use the same storage as the immediate to store those fields.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/24] i965: Add and use enum brw_reg_file.

[Matt Turner]

On Tue, Nov 3, 2015 at 3:11 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>> ---
>>  src/mesa/drivers/dri/i965/brw_defines.h| 10 ++
>>  src/mesa/drivers/dri/i965/brw_eu_emit.c|  2 +-
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  5 +++--
>>  src/mesa/drivers/dri/i965/brw_reg.h| 25 
>> +
>>  4 files changed, 23 insertions(+), 19 deletions(-)
>>
>
> There are a couple of places that can also use the same treatment:
>
> brw_disasm.c:
>
> static int reg(... unsigned _reg_file ...)
>
> static int dest_3src(...)
> ...
> uint32_t reg_file;
> ...
>
> brw_inst.h:
> Worth tweaking the FF/F8 macros (wrt *_reg_file) ?

Yeah, potentially. The main value you get from using enums is better
debugger support and I've never had to debug the disassembler
(fortunately :). It's not uncommon to reach a brw_inst.h function in
gdb, but you just go up a frame to print its argument, so those aren't
a big deal either. I also don't know how much work it'd be to start
supporting different parameter/return types in brw_inst.h

I'm happy to leave both of those projects for later :)

>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index e980003..ed3e335 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -33,7 +33,8 @@
>>  #include "brw_fs.h"
>>  #include "brw_cfg.h"
>>
>> -static uint32_t brw_file_from_reg(fs_reg *reg)
>> +static enum brw_reg_file
>> +brw_file_from_reg(fs_reg *reg)
>>  {
>> switch (reg->file) {
>> case GRF:
>> @@ -48,7 +49,7 @@ static uint32_t brw_file_from_reg(fs_reg *reg)
>> case UNIFORM:
>>unreachable("not reached");
>> }
>> -   return 0;
>> +   return GRF;
> Although unreachable (and gcc being silly) GRF looks wrong. Use
> BRW_ARCHITECTURE_REGISTER_FILE perhaps ?

*shrug*

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

Re: [Mesa-dev] [PATCH 1/8] i965/nir: Add OPT() and OPT_V() macros for invoking NIR passes.

[Rob Clark]

On Tue, Nov 3, 2015 at 3:31 AM, Kenneth Graunke  wrote:
> OPT() is the normal macro for passes that return booleans, while OPT_V()
> is a variant that works for passes that don't properly report progress.
> (Such passes should be fixed to return a boolean, eventually.)
>
> These macros take care of calling nir_validate_shader() and setting
> progress appropriately.  In the future, it would be easy to add shader
> dumping similar to INTEL_DEBUG=optimizer by extending the macro.
>

a couple of fwiw's

1) imo, we should put these macros in nir.h.. no need to redefine them
in each driver
2) v2 of my NIR_PASS macros (assuming I actually remembered to resend
it) switched to not rely on the gnu c extension.. not quite as pretty
but tolerable
3) could be an idea to standardize on all the passes returning a bool,
it would simplify the macros for running passes

BR,
-R

> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_nir.c | 123 
> 
>  1 file changed, 53 insertions(+), 70 deletions(-)
>
> Ideally we'll delete OPT_V eventually.  I added progress to a bunch of
> passes a while back; we'll have to hit up the rest of them...
>
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
> b/src/mesa/drivers/dri/i965/brw_nir.c
> index 11f1113..e71a7d1 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -136,46 +136,49 @@ brw_nir_lower_outputs(nir_shader *nir, bool is_scalar)
> }
>  }
>
> +#define _OPT(do_pass) (({ \
> +   bool this_progress = true; \
> +   do_pass\
> +   nir_validate_shader(nir);  \
> +   this_progress; \
> +}))
> +
> +#define OPT(pass, ...) _OPT( \
> +   this_progress = pass(nir ,##__VA_ARGS__); \
> +   progress = progress || this_progress; \
> +)
> +
> +#define OPT_V(pass, ...) _OPT( \
> +   pass(nir, ##__VA_ARGS__);   \
> +)
> +
>  static void
>  nir_optimize(nir_shader *nir, bool is_scalar)
>  {
> bool progress;
> do {
>progress = false;
> -  nir_lower_vars_to_ssa(nir);
> -  nir_validate_shader(nir);
> +  OPT_V(nir_lower_vars_to_ssa);
>
>if (is_scalar) {
> - nir_lower_alu_to_scalar(nir);
> - nir_validate_shader(nir);
> + OPT_V(nir_lower_alu_to_scalar);
>}
>
> -  progress |= nir_copy_prop(nir);
> -  nir_validate_shader(nir);
> +  OPT(nir_copy_prop);
>
>if (is_scalar) {
> - nir_lower_phis_to_scalar(nir);
> - nir_validate_shader(nir);
> + OPT_V(nir_lower_phis_to_scalar);
>}
>
> -  progress |= nir_copy_prop(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_dce(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_cse(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_peephole_select(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_algebraic(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_constant_folding(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_dead_cf(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_remove_phis(nir);
> -  nir_validate_shader(nir);
> -  progress |= nir_opt_undef(nir);
> -  nir_validate_shader(nir);
> +  OPT(nir_copy_prop);
> +  OPT(nir_opt_dce);
> +  OPT(nir_opt_cse);
> +  OPT(nir_opt_peephole_select);
> +  OPT(nir_opt_algebraic);
> +  OPT(nir_opt_constant_folding);
> +  OPT(nir_opt_dead_cf);
> +  OPT(nir_opt_remove_phis);
> +  OPT(nir_opt_undef);
> } while (progress);
>  }
>
> @@ -193,6 +196,7 @@ brw_create_nir(struct brw_context *brw,
>.lower_txp = ~0,
> };
> bool debug_enabled = INTEL_DEBUG & 
> intel_debug_flag_for_shader_stage(stage);
> +   bool progress = false;
> nir_shader *nir;
>
> /* First, lower the GLSL IR or Mesa IR to NIR */
> @@ -200,80 +204,63 @@ brw_create_nir(struct brw_context *brw,
>nir = glsl_to_nir(shader_prog, stage, options);
> } else {
>nir = prog_to_nir(prog, options);
> -  nir_convert_to_ssa(nir); /* turn registers into SSA */
> +  OPT_V(nir_convert_to_ssa); /* turn registers into SSA */
> }
> nir_validate_shader(nir);
>
> if (stage == MESA_SHADER_GEOMETRY) {
> -  nir_lower_gs_intrinsics(nir);
> -  nir_validate_shader(nir);
> +  OPT(nir_lower_gs_intrinsics);
> }
>
> -   nir_lower_global_vars_to_local(nir);
> -   nir_validate_shader(nir);
> +   OPT(nir_lower_global_vars_to_local);
>
> -   nir_lower_tex(nir, &tex_options);
> -   nir_validate_shader(nir);
> +   OPT_V(nir_lower_tex, &tex_options);
>
> -   nir_normalize_cubemap_coords(nir);
> -   nir_validate_shader(nir);
> +   OPT(nir_normalize_cubemap_coords);
>
> -   nir_split_var_copies(nir);
> -   nir_validate_shader(nir);
> +   OPT(nir_split_var_copies);
>
> nir_optimize(nir, is_scalar);
>
> /* Lower a bunch of stuff */
> -   nir_lower_var_copies(ni

Re: [Mesa-dev] [PATCH 08/24] i965: Remove fixed_hw_reg field from backend_reg.

[Matt Turner]

On Tue, Nov 3, 2015 at 8:04 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>
> Please add a bit of commit message - "Mostly unused as of last commit.
> Fold the remaining cases (GRF only?) to use the base brw_reg struct."
> or anything else that you feel is appropriate.

How about

   Since backend_reg now inherits brw_reg, we can use it in place of the
   fixed_hw_reg field.

>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  93 +-
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |   9 +-
>>  src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp  |   4 +-
>>  src/mesa/drivers/dri/i965/brw_ir_fs.h  |   4 +-
>>  src/mesa/drivers/dri/i965/brw_ir_vec4.h|   4 +-
>>  .../drivers/dri/i965/brw_schedule_instructions.cpp |  54 +--
>>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +-
>>  src/mesa/drivers/dri/i965/brw_shader.h |   5 +-
>>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 108 
>> +
>>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  12 +--
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   2 -
>>  11 files changed, 141 insertions(+), 162 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 6b9e979..243a4ac 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -422,13 +422,15 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
>> uint8_t vf3)
>> (vf3 << 24);
>>  }
>>
>> -/** Fixed brw_reg. */
>> -fs_reg::fs_reg(struct brw_reg fixed_hw_reg)
>> +fs_reg::fs_reg(struct brw_reg reg) :
>> +   backend_reg(reg)
>>  {
>> -   init();
> Keep this for now ?

I can't -- since this function now uses an initializer list, init()
would happen after the backend_reg() constructor has run, thereby
memsetting the object to zero.

>
>> this->file = HW_REG;
>> -   this->fixed_hw_reg = fixed_hw_reg;
>> -   this->type = fixed_hw_reg.type;
>
>> +   this->reg = 0;
>> +   this->reg_offset = 0;
>> +   this->subreg_offset = 0;
>> +   this->reladdr = NULL;
>> +   this->stride = 1;
> .. and drop these ?

Can't, without readding init().

>
>>  fs_reg::fs_reg(enum register_file file, int reg, enum brw_reg_type type)
>>  {
>> init();
>> @@ -1400,10 +1399,11 @@ fs_visitor::assign_curb_setup()
>> struct brw_reg brw_reg = brw_vec1_grf(payload.num_regs +
>>   constant_nr / 8,
>>   constant_nr % 8);
>> +brw_reg.abs = inst->src[i].abs;
>> +brw_reg.negate = inst->src[i].negate;
>>
> This looks like a bugfix which might contribute to an occasional
> random behaviour ?

Actually, this is necessary (and now that I think about it might
indicate an intermediate breakage earlier in the series).

The problem is that we're reconstructing the fs_reg using the
fs_reg(brw_reg) constructor (see "inst->src[i] = byte_offset(" below).
The default abs/negate values for brw_reg are false, so we have to
copy them over from our fs_reg.

I'll check whether this is fixing a problem introduced by removing the
abs/negate fields from backend_reg. If it is, some of this will need
to be moved to that commit.

>>  assert(inst->src[i].stride == 0);
>> -   inst->src[i].file = HW_REG;
>> -   inst->src[i].fixed_hw_reg = byte_offset(
>> +inst->src[i] = byte_offset(
> Something looks odd here. We will likely create a brw_reg and the
> remaining bits of inst->src[i] will be uninitialised as the old object
> will be torn down. Although I could be missing something.

It shouldn't matter matter. Once we have a brw_reg (with file ==
HW_REG), none of those fields are meaningful.

>> @@ -1556,12 +1556,15 @@ fs_visitor::assign_vs_urb_setup()
>>inst->src[i].reg +
>>inst->src[i].reg_offset;
>>
>> -inst->src[i].file = HW_REG;
>> -inst->src[i].fixed_hw_reg =
>> +struct brw_reg reg =
>> stride(byte_offset(retype(brw_vec8_grf(grf, 0), 
>> inst->src[i].type),
>>inst->src[i].subreg_offset),
>>inst->exec_size * inst->src[i].stride,
>>inst->exec_size, inst->src[i].stride);
>> +reg.abs = inst->src[i].abs;
>> +reg.negate = inst->src[i].negate;
>> +
>> +inst->src[i] = reg;
> Similar concerns as above.
>
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h 
>> b/src/mesa/drivers/dri/i965/brw_shader.h
>> index 9a7a2d5..4d9a946 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.h
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
>> @@ -51,6 +51,9 @@ enum PACKED register_file {
>>  #ifdef __cplusplus
>>  struct backend_reg : public brw_reg
>>  {
>> +   backend_reg() {}
>> +   backend_reg(struct brw_re

Re: [Mesa-dev] [PATCH v3 1/6] gallium: expose a debug message callback settable by context owner

[Ilia Mirkin]

On Tue, Nov 3, 2015 at 11:58 AM, Brian Paul  wrote:
> On 10/31/2015 11:45 AM, Ilia Mirkin wrote:
>>
>> On Sat, Oct 31, 2015 at 10:23 AM, Brian Paul  wrote:
>>>
>>> On 10/30/2015 11:15 PM, Ilia Mirkin wrote:


 This will allow gallium drivers to send messages to KHR_debug endpoints

 Signed-off-by: Ilia Mirkin 
 ---
src/gallium/auxiliary/util/u_debug.c | 16 
src/gallium/auxiliary/util/u_debug.h | 24 
src/gallium/docs/source/context.rst  |  3 +++
src/gallium/include/pipe/p_context.h |  4 
src/gallium/include/pipe/p_defines.h | 35
 +++
src/gallium/include/pipe/p_state.h   | 29
 +
6 files changed, 111 insertions(+)

 diff --git a/src/gallium/auxiliary/util/u_debug.c
 b/src/gallium/auxiliary/util/u_debug.c
 index 7388a49..81280ea 100644
 --- a/src/gallium/auxiliary/util/u_debug.c
 +++ b/src/gallium/auxiliary/util/u_debug.c
 @@ -70,6 +70,22 @@ void _debug_vprintf(const char *format, va_list ap)
#endif
}

 +void
 +_pipe_debug_message(
 +   struct pipe_debug_info *info,
 +   unsigned *id,
 +   enum pipe_debug_source source,
 +   enum pipe_debug_type type,
 +   enum pipe_debug_severity severity,
 +   const char *fmt, ...)
 +{
 +   va_list args;
 +   va_start(args, fmt);
 +   if (info && info->debug_message)
 +  info->debug_message(info->data, id, source, type, severity, fmt,
 args);
 +   va_end(args);
 +}
 +

void
debug_disable_error_message_boxes(void)
 diff --git a/src/gallium/auxiliary/util/u_debug.h
 b/src/gallium/auxiliary/util/u_debug.h
 index 926063a..a4ce88b 100644
 --- a/src/gallium/auxiliary/util/u_debug.h
 +++ b/src/gallium/auxiliary/util/u_debug.h
 @@ -42,6 +42,7 @@
#include "os/os_misc.h"

#include "pipe/p_format.h"
 +#include "pipe/p_defines.h"


#ifdef__cplusplus
 @@ -262,6 +263,29 @@ void _debug_assert_fail(const char *expr,
   _debug_printf("error: %s\n", __msg)
#endif

 +/**
 + * Output a debug log message to the debug info callback.
 + */
 +#define pipe_debug_message(info, source, type, severity, fmt, ...) do {
 \
 +   static unsigned id = 0; \
 +   _pipe_debug_message(info, &id, \
 +   PIPE_DEBUG_SOURCE_ ## source,\
 +   PIPE_DEBUG_TYPE_ ## type, \
 +   PIPE_DEBUG_SEVERITY_ ## severity, \
 +   fmt, __VA_ARGS__); \
 +} while (0)
 +
 +struct pipe_debug_info;
 +
 +void
 +_pipe_debug_message(
 +   struct pipe_debug_info *info,
 +   unsigned *id,
 +   enum pipe_debug_source source,
 +   enum pipe_debug_type type,
 +   enum pipe_debug_severity severity,
 +   const char *fmt, ...) _util_printf_format(6, 7);
 +

/**
 * Used by debug_dump_enum and debug_dump_flags to describe symbols.
 diff --git a/src/gallium/docs/source/context.rst
 b/src/gallium/docs/source/context.rst
 index a7d08d2..5cae4d6 100644
 --- a/src/gallium/docs/source/context.rst
 +++ b/src/gallium/docs/source/context.rst
 @@ -84,6 +84,9 @@ objects. They all follow simple, one-method binding
 calls, e.g.
levels. This corresponds to GL's ``PATCH_DEFAULT_OUTER_LEVEL``.
  * ``default_inner_level`` is the default value for the inner
 tessellation
levels. This corresponds to GL's ``PATCH_DEFAULT_INNER_LEVEL``.
 +* ``set_debug_info`` sets the callback to be used for reporting
 +  various debug messages, eventually reported via KHR_debug and
 +  similar mechanisms.


Sampler Views
 diff --git a/src/gallium/include/pipe/p_context.h
 b/src/gallium/include/pipe/p_context.h
 index 6f9fe76..0d5eeab 100644
 --- a/src/gallium/include/pipe/p_context.h
 +++ b/src/gallium/include/pipe/p_context.h
 @@ -45,6 +45,7 @@ struct pipe_blit_info;
struct pipe_box;
struct pipe_clip_state;
struct pipe_constant_buffer;
 +struct pipe_debug_info;
struct pipe_depth_stencil_alpha_state;
struct pipe_draw_info;
struct pipe_fence_handle;
 @@ -238,6 +239,9 @@ struct pipe_context {
  const float default_outer_level[4],
  const float default_inner_level[2]);

 +   void (*set_debug_info)(struct pipe_context *,
 +  struct pipe_debug_info *);
>>>
>>>
>>>
>>> Evidently, the implementation of this function must make a copy of the
>>> pipe_debug_info and can't just save the pointer.  Could you add a comment
>>> about that?  'info' could be const-qualified too.
>>>
>>
>> Will do. I believe that's how all th

Re: [Mesa-dev] [PATCH 12/24] i965: Initialize registers' file to BAD_FILE.

[Matt Turner]

On Tue, Nov 3, 2015 at 8:05 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>> The test (file == BAD_FILE) works on registers for which the constructor
>> has not run because BAD_FILE is zero.  The next commit will move
>> BAD_FILE in the enum so that it's no longer zero.
>
> Doesn't the DECLARE_RALLOC_CXX_OPERATORS macro and fs_reg::fs_reg()
> kick in ? If not things look quite fragile and perhaps we should wire
> them up.

I don't know.

I'm certainly in favor of actually constructing fs_regs...

Ken was pretty horrified when I told him that we were just
ralloc_arrary()ing fs_regs without calling the constructor, but he's
partly to blame (commit commit 6928bea :)

What's worse, ralloc_array() isn't even supposed to zero-initialize
things. That's just an accident that's kind of hard to fix now.

I could try to clean that stuff up, but again, this series is already
cleaning plenty of stuff. If I stopped to fix every bug I saw...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/24] i965: Reorganize brw_reg fields.

[Emil Velikov]

On 3 November 2015 at 17:27, Matt Turner  wrote:
> On Tue, Nov 3, 2015 at 2:38 AM, Emil Velikov  wrote:
>> On 3 November 2015 at 00:29, Matt Turner  wrote:
>>> Put fields that are meaningless with an immediate in the same storage
>>> with the immediate.
>> There is something funky here. Should it be
>>
>> "Put fields that are meaningless _without_ an immediate in the same storage
>> _as_ the immediate..."
>
>
> I think what I said (though "with" -> "as" does sound better). To
> elaborate: some things like writemask, swizzle, vstride, width,
> hstride are only meaningful when the register is not an immediate --
> that is, they're mutually exclusive.
>
> So, use the same storage as the immediate to store those fields.
Dough... you're right I've spilled my marbles there.

Sorry about that.
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 17/24] i965: Replace HW_REG with ARF/GRF.

[Matt Turner]

On Tue, Nov 3, 2015 at 8:06 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>
>> @@ -422,7 +423,7 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
>> uint8_t vf3)
>>  fs_reg::fs_reg(struct brw_reg reg) :
>> backend_reg(reg)
>>  {
>> -   this->file = HW_REG;
>> +   this->file = (enum register_file)reg.file;
> You're not adding the cast in the rest of the ctors (be that fs_reg,
> src_reg or dst_reg) so might as well drop this one ?

Actually, that's a problem with src_reg/dst_reg. I'll add them there.
(The whole line gets removed in a few patches).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 18/24] i965: Combine register file field.

[Matt Turner]

On Tue, Nov 3, 2015 at 8:07 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>
>> index 6eeafd5..3d2b051 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -423,7 +423,6 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
>> uint8_t vf3)
>>  fs_reg::fs_reg(struct brw_reg reg) :
>> backend_reg(reg)
>>  {
>> -   this->file = (enum register_file)reg.file;
> Should we fold the remaining this->file = foo into the backend_reg() ctors ?

Not necessary. The only remaining file field after this commit is in
brw_reg. So the backend_reg(reg) constructor is already doing that.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 92783] MESA_DEBUG=incomplete_tex prints warnings from glClear which doesn't use the state

[bugzilla-daemon]

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

Ben Widawsky  changed:

   What|Removed |Added

 CC||b...@bwidawsk.net

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

Re: [Mesa-dev] [PATCH 23/24] i965/vec4: Replace src_reg(imm) constructors with brw_imm_*().

[Matt Turner]

On Tue, Nov 3, 2015 at 8:09 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>
>> @@ -387,7 +342,9 @@ vec4_visitor::opt_vector_float()
>>
>>remaining_channels &= ~inst->dst.writemask;
>>if (remaining_channels == 0) {
>> - vec4_instruction *mov = MOV(inst->dst, imm);
>> + unsigned vf;
>> + memcpy(&vf, imm, sizeof(vf));
>> + vec4_instruction *mov = MOV(inst->dst, brw_imm_vf(vf));
> You can drop the temp variable + memcpy call and use brw_imm_vf4(imm[x],)

Ah, yes, I can. And it even generates identical code.

Unfortunately, gcc isn't smart enough to understand that imm[] is not
uninitialized. Wonder if it just doesn't warn about src arguments to
memcpy()?

../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp: In
member function ‘bool brw::vec4_visitor::opt_vector_float()’:
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[3]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  vec4_instruction *mov = MOV(inst->dst, brw_imm_vf4(imm[0],
imm[1], imm[2], imm[3]));

 ^
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[2]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[1]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[0]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]

Changing it to brw_imm_vf(*(unsigned *)&imm) generates the same code
as well, showing that the memcpy isn't really happening. It's just
avoiding breaking strict aliasing rules.

Given the (incorrect) warnings, I'm inclined to keep the code as it is.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] nvc0: add missing compute parameters required by clover

[Samuel Pitoiset]

This fixes crashes with some piglit OpenCL tests.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index ea317a5..ccaab44 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -353,7 +353,8 @@ static int
 nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   enum pipe_compute_cap param, void *data)
 {
-   const uint16_t obj_class = nvc0_screen(pscreen)->compute->oclass;
+   struct nvc0_screen *screen = nvc0_screen(pscreen);
+   const uint16_t obj_class = screen->compute->oclass;
 
 #define RET(x) do {  \
if (data) \
@@ -384,6 +385,14 @@ nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   RET((uint64_t []) { 4096ul });
case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
   RET((uint32_t []) { 32u });
+   case PIPE_COMPUTE_CAP_MAX_MEM_ALLOC_SIZE:
+  RET((uint64_t []) { 1ul << 40 });
+   case PIPE_COMPUTE_CAP_IMAGES_SUPPORTED:
+  RET((uint32_t []) { 0u });
+   case PIPE_COMPUTE_CAP_MAX_COMPUTE_UNITS:
+  RET((uint32_t []) { screen->mp_count_compute });
+   case PIPE_COMPUTE_CAP_MAX_CLOCK_FREQUENCY:
+  RET((uint32_t []) { 512u }); /* FIXME: arbitrary limit */
default:
   return 0;
}
-- 
2.5.3

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

[Mesa-dev] [PATCH 1/2] nvc0: handle NULL pointer in nvc0_get_compute_param()

[Samuel Pitoiset]

To get the size (in bytes) of a compute parameter, clover first calls
get_compute_param() with a NULL data pointer. The RET() macro is based
on nv50.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 45 --
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 6aa4f0b..ea317a5 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -353,45 +353,42 @@ static int
 nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   enum pipe_compute_cap param, void *data)
 {
-   uint64_t *data64 = (uint64_t *)data;
-   uint32_t *data32 = (uint32_t *)data;
const uint16_t obj_class = nvc0_screen(pscreen)->compute->oclass;
 
+#define RET(x) do {  \
+   if (data) \
+  memcpy(data, x, sizeof(x));\
+   return sizeof(x); \
+} while (0)
+
switch (param) {
case PIPE_COMPUTE_CAP_GRID_DIMENSION:
-  data64[0] = 3;
-  return 8;
+  RET((uint64_t []) { 3ul });
case PIPE_COMPUTE_CAP_MAX_GRID_SIZE:
-  data64[0] = (obj_class >= NVE4_COMPUTE_CLASS) ? 0x7fff : 65535;
-  data64[1] = 65535;
-  data64[2] = 65535;
-  return 24;
+  if (obj_class >= NVE4_COMPUTE_CLASS) {
+ RET(((uint64_t []) { 0x7fff, 65535ul, 65535ul }));
+  } else {
+ RET(((uint64_t []) { 65535ul, 65535ul, 65535ul }));
+  }
case PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE:
-  data64[0] = 1024;
-  data64[1] = 1024;
-  data64[2] = 64;
-  return 24;
+  RET(((uint64_t []) { 1024ul, 1024ul, 64ul }));
case PIPE_COMPUTE_CAP_MAX_THREADS_PER_BLOCK:
-  data64[0] = 1024;
-  return 8;
+  RET((uint64_t []) { 1024ul });
case PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE: /* g[] */
-  data64[0] = (uint64_t)1 << 40;
-  return 8;
+  RET((uint64_t []) { 1ul << 40 });
case PIPE_COMPUTE_CAP_MAX_LOCAL_SIZE: /* s[] */
-  data64[0] = 48 << 10;
-  return 8;
+  RET((uint64_t []) { 48ul << 10 });
case PIPE_COMPUTE_CAP_MAX_PRIVATE_SIZE: /* l[] */
-  data64[0] = 512 << 10;
-  return 8;
+  RET((uint64_t []) { 512ul << 10 });
case PIPE_COMPUTE_CAP_MAX_INPUT_SIZE: /* c[], arbitrary limit */
-  data64[0] = 4096;
-  return 8;
+  RET((uint64_t []) { 4096ul });
case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
-  data32[0] = 32;
-  return 4;
+  RET((uint32_t []) { 32u });
default:
   return 0;
}
+
+#undef RET
 }
 
 static void
-- 
2.5.3

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

Re: [Mesa-dev] [PATCH 1/2] nvc0: handle NULL pointer in nvc0_get_compute_param()

[Ilia Mirkin]

On Tue, Nov 3, 2015 at 1:35 PM, Samuel Pitoiset
 wrote:
> To get the size (in bytes) of a compute parameter, clover first calls
> get_compute_param() with a NULL data pointer. The RET() macro is based
> on nv50.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 45 
> --
>  1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> index 6aa4f0b..ea317a5 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> @@ -353,45 +353,42 @@ static int
>  nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
>enum pipe_compute_cap param, void *data)
>  {
> -   uint64_t *data64 = (uint64_t *)data;
> -   uint32_t *data32 = (uint32_t *)data;
> const uint16_t obj_class = nvc0_screen(pscreen)->compute->oclass;
>
> +#define RET(x) do {  \
> +   if (data) \
> +  memcpy(data, x, sizeof(x));\
> +   return sizeof(x); \
> +} while (0)
> +
> switch (param) {
> case PIPE_COMPUTE_CAP_GRID_DIMENSION:
> -  data64[0] = 3;
> -  return 8;
> +  RET((uint64_t []) { 3ul });
> case PIPE_COMPUTE_CAP_MAX_GRID_SIZE:
> -  data64[0] = (obj_class >= NVE4_COMPUTE_CLASS) ? 0x7fff : 65535;
> -  data64[1] = 65535;
> -  data64[2] = 65535;
> -  return 24;
> +  if (obj_class >= NVE4_COMPUTE_CLASS) {
> + RET(((uint64_t []) { 0x7fff, 65535ul, 65535ul }));

Why the ul's everywhere? And why not on the 0x7 ?

> +  } else {
> + RET(((uint64_t []) { 65535ul, 65535ul, 65535ul }));
> +  }
> case PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE:
> -  data64[0] = 1024;
> -  data64[1] = 1024;
> -  data64[2] = 64;
> -  return 24;
> +  RET(((uint64_t []) { 1024ul, 1024ul, 64ul }));
> case PIPE_COMPUTE_CAP_MAX_THREADS_PER_BLOCK:
> -  data64[0] = 1024;
> -  return 8;
> +  RET((uint64_t []) { 1024ul });
> case PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE: /* g[] */
> -  data64[0] = (uint64_t)1 << 40;
> -  return 8;
> +  RET((uint64_t []) { 1ul << 40 });
> case PIPE_COMPUTE_CAP_MAX_LOCAL_SIZE: /* s[] */
> -  data64[0] = 48 << 10;
> -  return 8;
> +  RET((uint64_t []) { 48ul << 10 });
> case PIPE_COMPUTE_CAP_MAX_PRIVATE_SIZE: /* l[] */
> -  data64[0] = 512 << 10;
> -  return 8;
> +  RET((uint64_t []) { 512ul << 10 });
> case PIPE_COMPUTE_CAP_MAX_INPUT_SIZE: /* c[], arbitrary limit */
> -  data64[0] = 4096;
> -  return 8;
> +  RET((uint64_t []) { 4096ul });
> case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
> -  data32[0] = 32;
> -  return 4;
> +  RET((uint32_t []) { 32u });
> default:
>return 0;
> }
> +
> +#undef RET
>  }
>
>  static void
> --
> 2.5.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] nvc0: handle NULL pointer in nvc0_get_compute_param()

[Samuel Pitoiset]

On 11/03/2015 07:26 PM, Ilia Mirkin wrote:

On Tue, Nov 3, 2015 at 1:35 PM, Samuel Pitoiset
 wrote:

To get the size (in bytes) of a compute parameter, clover first calls
get_compute_param() with a NULL data pointer. The RET() macro is based
on nv50.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 45 --
  1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 6aa4f0b..ea317a5 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -353,45 +353,42 @@ static int
  nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
enum pipe_compute_cap param, void *data)
  {
-   uint64_t *data64 = (uint64_t *)data;
-   uint32_t *data32 = (uint32_t *)data;
 const uint16_t obj_class = nvc0_screen(pscreen)->compute->oclass;

+#define RET(x) do {  \
+   if (data) \
+  memcpy(data, x, sizeof(x));\
+   return sizeof(x); \
+} while (0)
+
 switch (param) {
 case PIPE_COMPUTE_CAP_GRID_DIMENSION:
-  data64[0] = 3;
-  return 8;
+  RET((uint64_t []) { 3ul });
 case PIPE_COMPUTE_CAP_MAX_GRID_SIZE:
-  data64[0] = (obj_class >= NVE4_COMPUTE_CLASS) ? 0x7fff : 65535;
-  data64[1] = 65535;
-  data64[2] = 65535;
-  return 24;
+  if (obj_class >= NVE4_COMPUTE_CLASS) {
+ RET(((uint64_t []) { 0x7fff, 65535ul, 65535ul }));


Why the ul's everywhere? And why not on the 0x7 ?


Based on curro's branch for nv50 compute support, but I assume I can get 
rid of this.





+  } else {
+ RET(((uint64_t []) { 65535ul, 65535ul, 65535ul }));
+  }
 case PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE:
-  data64[0] = 1024;
-  data64[1] = 1024;
-  data64[2] = 64;
-  return 24;
+  RET(((uint64_t []) { 1024ul, 1024ul, 64ul }));
 case PIPE_COMPUTE_CAP_MAX_THREADS_PER_BLOCK:
-  data64[0] = 1024;
-  return 8;
+  RET((uint64_t []) { 1024ul });
 case PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE: /* g[] */
-  data64[0] = (uint64_t)1 << 40;
-  return 8;
+  RET((uint64_t []) { 1ul << 40 });
 case PIPE_COMPUTE_CAP_MAX_LOCAL_SIZE: /* s[] */
-  data64[0] = 48 << 10;
-  return 8;
+  RET((uint64_t []) { 48ul << 10 });
 case PIPE_COMPUTE_CAP_MAX_PRIVATE_SIZE: /* l[] */
-  data64[0] = 512 << 10;
-  return 8;
+  RET((uint64_t []) { 512ul << 10 });
 case PIPE_COMPUTE_CAP_MAX_INPUT_SIZE: /* c[], arbitrary limit */
-  data64[0] = 4096;
-  return 8;
+  RET((uint64_t []) { 4096ul });
 case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
-  data32[0] = 32;
-  return 4;
+  RET((uint32_t []) { 32u });
 default:
return 0;
 }
+
+#undef RET
  }

  static void
--
2.5.3

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

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

Re: [Mesa-dev] [PATCH 12/24] i965: Initialize registers' file to BAD_FILE.

[Emil Velikov]

On 3 November 2015 at 18:02, Matt Turner  wrote:
> On Tue, Nov 3, 2015 at 8:05 AM, Emil Velikov  wrote:
>> On 3 November 2015 at 00:29, Matt Turner  wrote:
>>> The test (file == BAD_FILE) works on registers for which the constructor
>>> has not run because BAD_FILE is zero.  The next commit will move
>>> BAD_FILE in the enum so that it's no longer zero.
>>
>> Doesn't the DECLARE_RALLOC_CXX_OPERATORS macro and fs_reg::fs_reg()
>> kick in ? If not things look quite fragile and perhaps we should wire
>> them up.
>
> I don't know.
>
> I'm certainly in favor of actually constructing fs_regs...
>
> Ken was pretty horrified when I told him that we were just
> ralloc_arrary()ing fs_regs without calling the constructor, but he's
> partly to blame (commit commit 6928bea :)
>
> What's worse, ralloc_array() isn't even supposed to zero-initialize
> things. That's just an accident that's kind of hard to fix now.
>
Yay ralloc :)

> I could try to clean that stuff up, but again, this series is already
> cleaning plenty of stuff. If I stopped to fix every bug I saw...
Did not mean it as "drop everything and fix that bug", but more of
"add a note about this strange behaviour". Come to think of it I'm not
sure which is the better place for it :'(

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

Re: [Mesa-dev] [PATCH 1/2] intel: Add SKL GT4 PCI IDs

[Jordan Justen]

Series Reviewed-by: Jordan Justen 

For the 2de4e8fdbae1e1909ce35f8ba15608a124686fb0 version on your drm
gt4 branch.

On 2015-10-23 10:56:33, Ben Widawsky wrote:
> Cc: Kristian Høgsberg 
> Cc: Damien Lespiau 
> Signed-off-by: Ben Widawsky 
> ---
>  intel/intel_chipset.h | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
> index 253ea71..6c8dc73 100644
> --- a/intel/intel_chipset.h
> +++ b/intel/intel_chipset.h
> @@ -180,6 +180,10 @@
>  #define PCI_CHIP_SKYLAKE_SRV_GT3   0x192A
>  #define PCI_CHIP_SKYLAKE_SRV_GT1   0x190A
>  #define PCI_CHIP_SKYLAKE_WKS_GT2   0x191D
> +#define PCI_CHIP_SKYLAKE_DT_GT40x1932
> +#define PCI_CHIP_SKYLAKE_SRV_GT4   0x193A
> +#define PCI_CHIP_SKYLAKE_H_GT4 0x193B
> +#define PCI_CHIP_SKYLAKE_WKS_GT4   0x193D
>  
>  #define PCI_CHIP_BROXTON_0 0x0A84
>  #define PCI_CHIP_BROXTON_1 0x1A84
> @@ -362,9 +366,15 @@
>  (devid) == PCI_CHIP_SKYLAKE_HALO_GT3   || \
>  (devid) == PCI_CHIP_SKYLAKE_SRV_GT3)
>  
> +#define IS_SKL_GT4(devid)  ((devid) == PCI_CHIP_SKYLAKE_DT_GT4 || \
> +(devid) == PCI_CHIP_SKYLAKE_SRV_GT4|| \
> +(devid) == PCI_CHIP_SKYLAKE_H_GT4  || \
> +(devid) == PCI_CHIP_SKYLAKE_WKS_GT4)
> +
>  #define IS_SKYLAKE(devid)  (IS_SKL_GT1(devid) || \
>  IS_SKL_GT2(devid) || \
> -IS_SKL_GT3(devid))
> +IS_SKL_GT3(devid) || \
> +IS_SKL_GT4(devid))
>  
>  #define IS_BROXTON(devid)  ((devid) == PCI_CHIP_BROXTON_0  || \
>  (devid) == PCI_CHIP_BROXTON_1  || \
> -- 
> 2.6.1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/24] i965: Remove fixed_hw_reg field from backend_reg.

[Emil Velikov]

On 3 November 2015 at 17:55, Matt Turner  wrote:
> On Tue, Nov 3, 2015 at 8:04 AM, Emil Velikov  wrote:
>> On 3 November 2015 at 00:29, Matt Turner  wrote:
>>
>> Please add a bit of commit message - "Mostly unused as of last commit.
>> Fold the remaining cases (GRF only?) to use the base brw_reg struct."
>> or anything else that you feel is appropriate.
>
> How about
>
>Since backend_reg now inherits brw_reg, we can use it in place of the
>fixed_hw_reg field.
>
Quite better. Thanks.

>>> ---
>>>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  93 +-
>>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |   9 +-
>>>  src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp  |   4 +-
>>>  src/mesa/drivers/dri/i965/brw_ir_fs.h  |   4 +-
>>>  src/mesa/drivers/dri/i965/brw_ir_vec4.h|   4 +-
>>>  .../drivers/dri/i965/brw_schedule_instructions.cpp |  54 +--
>>>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +-
>>>  src/mesa/drivers/dri/i965/brw_shader.h |   5 +-
>>>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 108 
>>> +
>>>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  12 +--
>>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   2 -
>>>  11 files changed, 141 insertions(+), 162 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> index 6b9e979..243a4ac 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> @@ -422,13 +422,15 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
>>> uint8_t vf3)
>>> (vf3 << 24);
>>>  }
>>>
>>> -/** Fixed brw_reg. */
>>> -fs_reg::fs_reg(struct brw_reg fixed_hw_reg)
>>> +fs_reg::fs_reg(struct brw_reg reg) :
>>> +   backend_reg(reg)
>>>  {
>>> -   init();
>> Keep this for now ?
>
> I can't -- since this function now uses an initializer list, init()
> would happen after the backend_reg() constructor has run, thereby
> memsetting the object to zero.
>
>>
>>> this->file = HW_REG;
>>> -   this->fixed_hw_reg = fixed_hw_reg;
>>> -   this->type = fixed_hw_reg.type;
>>
>>> +   this->reg = 0;
>>> +   this->reg_offset = 0;
>>> +   this->subreg_offset = 0;
>>> +   this->reladdr = NULL;
>>> +   this->stride = 1;
>> .. and drop these ?
>
> Can't, without readding init().
>
Yes scratch these comments. Brain froze between "how will things look
if we've initialize things in backend_reg(?) and drop the init()" and
this series.

>>
>>>  fs_reg::fs_reg(enum register_file file, int reg, enum brw_reg_type type)
>>>  {
>>> init();
>>> @@ -1400,10 +1399,11 @@ fs_visitor::assign_curb_setup()
>>> struct brw_reg brw_reg = brw_vec1_grf(payload.num_regs +
>>>   constant_nr / 8,
>>>   constant_nr % 8);
>>> +brw_reg.abs = inst->src[i].abs;
>>> +brw_reg.negate = inst->src[i].negate;
>>>
>> This looks like a bugfix which might contribute to an occasional
>> random behaviour ?
>
> Actually, this is necessary (and now that I think about it might
> indicate an intermediate breakage earlier in the series).
>
> The problem is that we're reconstructing the fs_reg using the
> fs_reg(brw_reg) constructor (see "inst->src[i] = byte_offset(" below).
> The default abs/negate values for brw_reg are false, so we have to
> copy them over from our fs_reg.
>
> I'll check whether this is fixing a problem introduced by removing the
> abs/negate fields from backend_reg. If it is, some of this will need
> to be moved to that commit.
>
Glad I could help :)

>>>  assert(inst->src[i].stride == 0);
>>> -   inst->src[i].file = HW_REG;
>>> -   inst->src[i].fixed_hw_reg = byte_offset(
>>> +inst->src[i] = byte_offset(
>> Something looks odd here. We will likely create a brw_reg and the
>> remaining bits of inst->src[i] will be uninitialised as the old object
>> will be torn down. Although I could be missing something.
>
> It shouldn't matter matter. Once we have a brw_reg (with file ==
> HW_REG), none of those fields are meaningful.
>
Indeed.

>>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h 
>>> b/src/mesa/drivers/dri/i965/brw_shader.h
>>> index 9a7a2d5..4d9a946 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_shader.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
>>> @@ -51,6 +51,9 @@ enum PACKED register_file {
>>>  #ifdef __cplusplus
>>>  struct backend_reg : public brw_reg
>>>  {
>>> +   backend_reg() {}
>>> +   backend_reg(struct brw_reg reg) : brw_reg(reg) {}
>>> +
>> As brw_reg is a normal struct, wouldn't it be better if we initialize
>> it and backend_reg's members in the above two ? Ideally this could be
>> a separate commit (7.1) and patch 7.2 would in turn drop the init()
>> and use the above ctors. If you want to keep it as is, I won't push
>> it.
>
> The thing is, most of 

Re: [Mesa-dev] [PATCH 1/2] [v3] i965/skl: Add GT4 PCI IDs

[Jordan Justen]

Seried Reviewed-by: Jordan Justen 

For the dde33fc23c4ef8b8e02fb5768161fdaa078847d5 version on your mesa
gt4 branch.

On 2015-10-29 17:30:35, Ben Widawsky wrote:
> Like other gen8+ hardware, the hardware automatically scales up thread counts
> and URB sizes, so there is no need to do anything but add the PCI IDs.
> 
> FINISHME: This patch still needs testing before merge.
> 
> v2: Remove the PCI ID removal. That should be done as part of the next patch.
> 
> v3: Update the wm thread count to support GT4.
> 
> Cc: mesa-sta...@lists.freedesktop.org
> Signed-off-by: Ben Widawsky 
> ---
>  include/pci_ids/i965_pci_ids.h  | 4 
>  src/mesa/drivers/dri/i965/brw_device_info.c | 6 +-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_ids.h
> index 8a42599..626064a 100644
> --- a/include/pci_ids/i965_pci_ids.h
> +++ b/include/pci_ids/i965_pci_ids.h
> @@ -124,6 +124,10 @@ CHIPSET(0x1921, skl_gt2, "Intel(R) Skylake ULT GT2F")
>  CHIPSET(0x1926, skl_gt3, "Intel(R) Skylake ULT GT3")
>  CHIPSET(0x192A, skl_gt3, "Intel(R) Skylake SRV GT3")
>  CHIPSET(0x192B, skl_gt3, "Intel(R) Skylake Halo GT3")
> +CHIPSET(0x1932, skl_gt4, "Intel(R) Skylake GT4")
> +CHIPSET(0x193A, skl_gt4, "Intel(R) Skylake GT4")
> +CHIPSET(0x193B, skl_gt4, "Intel(R) Skylake GT4")
> +CHIPSET(0x193D, skl_gt4, "Intel(R) Skylake GT4")
>  CHIPSET(0x22B0, chv, "Intel(R) HD Graphics (Cherryview)")
>  CHIPSET(0x22B1, chv, "Intel(R) HD Graphics (Cherryview)")
>  CHIPSET(0x22B2, chv, "Intel(R) HD Graphics (Cherryview)")
> diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c 
> b/src/mesa/drivers/dri/i965/brw_device_info.c
> index e86b530..2ebc084 100644
> --- a/src/mesa/drivers/dri/i965/brw_device_info.c
> +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
> @@ -311,7 +311,7 @@ static const struct brw_device_info brw_device_info_chv = 
> {
> .max_gs_threads = 336,   \
> .max_hs_threads = 336,   \
> .max_ds_threads = 336,   \
> -   .max_wm_threads = 64 * 6,\
> +   .max_wm_threads = 64 * 9,\
> .max_cs_threads = 56,\
> .urb = { \
>.size = 384,  \
> @@ -335,6 +335,10 @@ static const struct brw_device_info 
> brw_device_info_skl_gt3 = {
> GEN9_FEATURES, .gt = 3,
>  };
>  
> +static const struct brw_device_info brw_device_info_skl_gt4 = {
> +   GEN9_FEATURES, .gt = 4,
> +};
> +
>  static const struct brw_device_info brw_device_info_bxt = {
> GEN9_FEATURES,
> .is_broxton = 1,
> -- 
> 2.6.2
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/24] i965: Remove fixed_hw_reg field from backend_reg.

[Matt Turner]

On Tue, Nov 3, 2015 at 10:45 AM, Emil Velikov  wrote:
> I already have a few patches for what. I've even started exploring
> which constructors we can nuke :-)
> Can you post a branch somewhere so that I can rebase + send before you
> get onto the next batch.

Sure. It's the brw_reg branch of my tree.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 18/24] i965: Combine register file field.

[Emil Velikov]

On 3 November 2015 at 18:10, Matt Turner  wrote:
> On Tue, Nov 3, 2015 at 8:07 AM, Emil Velikov  wrote:
>> On 3 November 2015 at 00:29, Matt Turner  wrote:
>>
>>> index 6eeafd5..3d2b051 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> @@ -423,7 +423,6 @@ fs_reg::fs_reg(uint8_t vf0, uint8_t vf1, uint8_t vf2, 
>>> uint8_t vf3)
>>>  fs_reg::fs_reg(struct brw_reg reg) :
>>> backend_reg(reg)
>>>  {
>>> -   this->file = (enum register_file)reg.file;
>> Should we fold the remaining this->file = foo into the backend_reg() ctors ?
>
> Not necessary. The only remaining file field after this commit is in
> brw_reg. So the backend_reg(reg) constructor is already doing that.
I might be missing a patch but I think that the default
fs/src/dst_reg() ctors still have it ?
Either way it's not a show stopper.

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

Re: [Mesa-dev] MSVC (2015) builds

[Jose Fonseca]

On 03/11/15 15:48, Brian Paul wrote:

On 11/02/2015 08:42 PM, Janusz Ganczarski wrote:

Hello,
In attachment fixed Visual C++ 2015 (VC 14) builds for Mesa.
Currently only gallium softpipe driver support. Gallium llvmpipe
driver support work in progress.


I'm not sure we're interested in MSVC project files (I'm certainly not).
  Unless we had several people who wanted to actively develop Mesa with
MSVC and promise to maintain MSVC support, I don't think this would be
used much and would eventually be dropped.  We've gone through this
several times in the past.

-Brian


I agree with Brian.  Maitaining MSVC projects would be a waste of time 
for us.  Even when we use MSVC we just use scons and MSVC command line 
compilers.  I never use MSVC IDE to develop or debug Mesa -- I do most 
of development in Linux, mostly with MinGW, and just build Mesa w/ MSVC 
when necessary.


Jose

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

Re: [Mesa-dev] Avoid segfault in omx deconstructor

[StDenis, Tom]

Now with the correct email address ...


Cheers,

Tom



From: StDenis, Tom
Sent: Tuesday, November 3, 2015 13:03
To: mesa-dev@lists.freedesktop.org
Cc: gpudriverdevsupport
Subject: Avoid segfault in omx deconstructor


If the constructor fails before the lists are initialized the deconstructor 
will fail.


This patch is a bit of a kludge fix to just avoid the undefined behaviour.  The 
real fix down the road would be to avoid calling the deconstructor if the 
constructor failed at all.


Tom
From 35d811dd83237f5feff91d1e237f517f5e39316b Mon Sep 17 00:00:00 2001
From: Tom St Denis 
Date: Tue, 3 Nov 2015 12:41:54 -0500
Subject: [PATCH] Avoid segfault in deconstructor if constructor fails

If the constructor fails before the LIST_INIT calls the pointers
will be null and the deconstructor will segfault.

Signed-off-by: Tom St Denis 
Reviewed-by: Leo Liu 
---
 src/gallium/state_trackers/omx/vid_enc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/gallium/state_trackers/omx/vid_enc.c b/src/gallium/state_trackers/omx/vid_enc.c
index 2bd0194189f1..f1f187ce9f7c 100644
--- a/src/gallium/state_trackers/omx/vid_enc.c
+++ b/src/gallium/state_trackers/omx/vid_enc.c
@@ -869,6 +869,9 @@ static void enc_ReleaseTasks(struct list_head *head)
 {
struct encode_task *i, *next;
 
+   if (!head)
+	   return;
+
LIST_FOR_EACH_ENTRY_SAFE(i, next, head, list) {
   pipe_resource_reference(&i->bitstream, NULL);
   i->buf->destroy(i->buf);
-- 
2.6.2

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

Re: [Mesa-dev] [PATCH 04/24] i965: Make 'dw1' and 'bits' unnamed structures in brw_reg.

[Matt Turner]

On Tue, Nov 3, 2015 at 2:33 AM, Emil Velikov  wrote:
> On 3 November 2015 at 00:29, Matt Turner  wrote:
>> Generated by
>>
>>sed -i -e 's/\.bits\././g' *.c *.h *.cpp
>>sed -i -e 's/dw1\.//g' *.c *.h *.cpp
>>
>> and then reverting changes to comments in gen7_blorp.cpp and
>> brw_fs_generator.cpp.
>>
>> There wasn't any utility offered by forcing the programmer to list these
>> to access their fields. Removing them will reduce churn in future
>> commits.
>>
>> This is C11 (and gcc has apparently supported it for sometime
>> "compatibility with other compilers")
>>
>> See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
> FYI on old versions of GCC where this extension is not enabled by
> default, we flip in on. We're using -std=gnu99, although perhaps we
> should use -fms-extensions - not a big deal either way.

I tested gcc-4.4.7 (the version RHEL 5 uses, IIRC) and it produced a
Mesa capable of running glxgears :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 23/24] i965/vec4: Replace src_reg(imm) constructors with brw_imm_*().

[Emil Velikov]

On 3 November 2015 at 18:20, Matt Turner  wrote:
> On Tue, Nov 3, 2015 at 8:09 AM, Emil Velikov  wrote:
>> On 3 November 2015 at 00:29, Matt Turner  wrote:
>>
>>> @@ -387,7 +342,9 @@ vec4_visitor::opt_vector_float()
>>>
>>>remaining_channels &= ~inst->dst.writemask;
>>>if (remaining_channels == 0) {
>>> - vec4_instruction *mov = MOV(inst->dst, imm);
>>> + unsigned vf;
>>> + memcpy(&vf, imm, sizeof(vf));
>>> + vec4_instruction *mov = MOV(inst->dst, brw_imm_vf(vf));
>> You can drop the temp variable + memcpy call and use brw_imm_vf4(imm[x],)
>
> Ah, yes, I can. And it even generates identical code.
>
> Unfortunately, gcc isn't smart enough to understand that imm[] is not
> uninitialized. Wonder if it just doesn't warn about src arguments to
> memcpy()?
>
Actually it seems like a genuine warning.


  if (inst->opcode != BRW_OPCODE_MOV ||
  inst->dst.writemask == WRITEMASK_XYZW ||
  inst->src[0].file != IMM)
 continue;

  int vf = brw_float_to_vf(inst->src[0].fixed_hw_reg.dw1.f);
  if (vf == -1)
 continue;

  if ((inst->dst.writemask & WRITEMASK_X) != 0)
 imm[0] = vf;
  if ((inst->dst.writemask & WRITEMASK_Y) != 0)
 imm[1] = vf;
  if ((inst->dst.writemask & WRITEMASK_Z) != 0)
 imm[2] = vf;
  if ((inst->dst.writemask & WRITEMASK_W) != 0)
 imm[3] = vf;

  imm_inst[inst_count++] = inst;

  remaining_channels &= ~inst->dst.writemask;
  if (remaining_channels == 0) {
 vec4_instruction *mov = MOV(inst->dst, imm);
 mov->dst.type = BRW_REGISTER_TYPE_F;
 mov->dst.writemask = WRITEMASK_XYZW;


From the above, if all the X Y Z and W writemasks are set, we just
jump to the next instruction. Thus, at least one of the imm values is
uninitialised. We might have found ourselves a bug :-)

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

[Mesa-dev] The i965 vec4 backend, exec_masks, and 64-bit types

[Connor Abbott]

Hi all,

While working on FP64 for i965, there's an issue that I thought of
with the vec4 backend that I'm not sure how to resolve. From what I
understand, the execmask works the same way in Align16 mode as Align1
mode, except that you only use the first 8 channels in practice for
SIMD4x2, and the first four channels are always the same as well as
the last 4 channels. But this doesn't work for 64-bit things, since
there we only operate on 4 components at the same time, so it's more
like SIMD2x2. For example, imagine that only the second vertex is
currently enabled at the moment. Then the execmask looks like
, and if we do something like:

mul(4)  g24<1>DF g12<4,4,1>DF g13<4,4,1>DF { align16 };

then all 4 channels will be disabled, which is not what we want.

I think the first thing to do is to write a piglit test that tests
this case, since currently all the arb_gpu_shader_fp64 tests only use
uniforms. We need a test that uses non-uniform control flow that
triggers the case described above. Once we do that, and if we
determine there's actually a problem, then we need to figure out how
to solve it.. The ideas I had were:

1. make every FP64 thing use WE_all. This isn't actually too bad at
the moment, since our notion of interference already assumes
(more-or-less) that everything is WE_all, but it prevents us from
improving it in the future with FP64 things. Unfortunately, it also
means that we can't use writemasks since setting WE_all makes the EU
ignore the writemask, so we'll have to do some trickery to get things
with only 1 channel enabled to work correctly.

2. Use the NibCtrl field, and split each FP64 operation into 2.
Unfortunately, this field only appeared on gen8, and the PRM only says
it works for SIMD4 operations, whereas we need it to work for SIMD2
operations, although there's a chance it'll actually work for SIMD2 as
well. This lets us potentially do better register allocation, but it
might not work and even if it does it won't work for gen7.

#1 sounds like the better solution for now, but who knows... maybe the
HW people magically made it work already, and I'm not aware or they
didn't document it.

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

Re: [Mesa-dev] Avoid segfault in omx deconstructor

[Christian König]

The subject line should have something like "st/omx: ...".

Apart from that the patch is Reviewed-by: Christian König 



Regards,
Christian.

On 03.11.2015 19:06, StDenis, Tom wrote:


Now with the correct email address ...


Cheers,

Tom




*From:* StDenis, Tom
*Sent:* Tuesday, November 3, 2015 13:03
*To:* mesa-dev@lists.freedesktop.org
*Cc:* gpudriverdevsupport
*Subject:* Avoid segfault in omx deconstructor

If the constructor fails before the lists are initialized the 
deconstructor will fail.



This patch is a bit of a kludge fix to just avoid the undefined 
behaviour.  The real fix down the road would be to avoid calling the 
deconstructor if the constructor failed at all.



Tom



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

Re: [Mesa-dev] memoryBarrier + SSBO

[Ilia Mirkin]

Ian, any comment on this?

On Fri, Sep 25, 2015 at 1:32 PM, Ilia Mirkin  wrote:
> Hi Ian (and other spec experts),
>
> The ARB_ssbo spec mentions the following:
>
> OpenGL 4.0 (either core or compatibility profile) is required.
>
> ...
>
> Additionally, the shading language provides the memoryBarrier() function
> to control the relative order of memory accesses within individual shader
> invocations and provides various memory qualifiers controlling how the
> memory corresponding to individual variables is accessed.
>
> However the memoryBarrier() function only becomes available in GLSL
> 4.20 [along with the glMemoryBarrier() function that the spec also
> refers to] or with ARB_shader_image_load_store.
>
> Is the implication that such functionality should be auto-exposed in
> image-less drivers that support ssbo? Or that this functionality
> should just not exist unless images are supported?
>
> This is relevant to me as I plan on adding ssbo support to gallium
> before images. Also, is there really anything in there that would
> prevent this from being exposed on a GL 3.1 driver (I'm thinking of
> freedreno which doesn't support GS yet and is my initial target for
> this, since I can make atomics actually work on there, unlike NVIDIA
> where the damn things just won't budge without some magic
> yet-to-be-found make-it-work bit).
>
> Cheers,
>
>   -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] The i965 vec4 backend, exec_masks, and 64-bit types

[Francisco Jerez]

Connor Abbott  writes:

> Hi all,
>
> While working on FP64 for i965, there's an issue that I thought of
> with the vec4 backend that I'm not sure how to resolve. From what I
> understand, the execmask works the same way in Align16 mode as Align1
> mode, except that you only use the first 8 channels in practice for
> SIMD4x2, and the first four channels are always the same as well as
> the last 4 channels. But this doesn't work for 64-bit things, since
> there we only operate on 4 components at the same time, so it's more
> like SIMD2x2. For example, imagine that only the second vertex is
> currently enabled at the moment. Then the execmask looks like
> , and if we do something like:
>
> mul(4)  g24<1>DF g12<4,4,1>DF g13<4,4,1>DF { align16 };
>
> then all 4 channels will be disabled, which is not what we want.
>
AFAIUI this shouldn't be a problem.  In align16 mode each component of
an instruction with double-precision execution type maps to *two* bits
of the execmask instead of one (one for each 32-bit half), which is
compensated by each logical thread having two components instead of
four, so in your example [assuming  is little-endian notation
and you actually do 'mul(8)' ;)] the x and y components of the first
logical thread will be disabled while the x and y components of the
second logical thread will be enabled.

> I think the first thing to do is to write a piglit test that tests
> this case, since currently all the arb_gpu_shader_fp64 tests only use
> uniforms. We need a test that uses non-uniform control flow that
> triggers the case described above. Once we do that, and if we
> determine there's actually a problem, then we need to figure out how
> to solve it.. The ideas I had were:
>

I guess a piglit test would be nice, but you're unlikely to have to do
much about it. ;)

> 1. make every FP64 thing use WE_all. This isn't actually too bad at
> the moment, since our notion of interference already assumes
> (more-or-less) that everything is WE_all, but it prevents us from
> improving it in the future with FP64 things. Unfortunately, it also
> means that we can't use writemasks since setting WE_all makes the EU
> ignore the writemask, so we'll have to do some trickery to get things
> with only 1 channel enabled to work correctly.
>
> 2. Use the NibCtrl field, and split each FP64 operation into 2.
> Unfortunately, this field only appeared on gen8, and the PRM only says
> it works for SIMD4 operations, whereas we need it to work for SIMD2
> operations, although there's a chance it'll actually work for SIMD2 as
> well. This lets us potentially do better register allocation, but it
> might not work and even if it does it won't work for gen7.
>
NibCtrl is Gen7+ actually.  I believe that indeed has a good chance of
working for Align16 2-wide DF instructions but I don't know for sure
offhand.

> #1 sounds like the better solution for now, but who knows... maybe the
> HW people magically made it work already, and I'm not aware or they
> didn't document it.
>
> Connor
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 04/24] i965: Make 'dw1' and 'bits' unnamed structures in brw_reg.

[Matt Turner]

On Tue, Nov 3, 2015 at 5:48 AM, Francisco Jerez  wrote:
> Matt Turner  writes:
>
>> Generated by
>>
>>sed -i -e 's/\.bits\././g' *.c *.h *.cpp
>>sed -i -e 's/dw1\.//g' *.c *.h *.cpp
>>
>> and then reverting changes to comments in gen7_blorp.cpp and
>> brw_fs_generator.cpp.
>>
>> There wasn't any utility offered by forcing the programmer to list these
>> to access their fields. Removing them will reduce churn in future
>> commits.
>>
>> This is C11 (and gcc has apparently supported it for sometime
>> "compatibility with other compilers")
>>
>> See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
>
> This is also used from C++ source where anonymous structs are not part
> of any released standard.

That is true. I have built this series with both clang-3.6 and
gcc-4.4.7. I don't think it's a problem.

> I guess in C++ it would be preferable to
> define accessor methods instead of relying on a language extension --
> That would also allow you to introduce checks making sure that the
> register is of the correct type in order to catch cases in which the
> wrong field of the union is accessed easily.

Maybe. Since I was changing so much code in this series, I wouldn't
want to do that here. Also, having commits that use the brw_reg fields
separately from any accessors seems beneficial.

We could also simply mark fields private with using declarations. That
would get almost all of any potential benefit (of which I'm not sure
how much there is, really).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 04/24] i965: Make 'dw1' and 'bits' unnamed structures in brw_reg.

[Ilia Mirkin]

On Tue, Nov 3, 2015 at 3:00 PM, Matt Turner  wrote:
> On Tue, Nov 3, 2015 at 5:48 AM, Francisco Jerez  wrote:
>> Matt Turner  writes:
>>
>>> Generated by
>>>
>>>sed -i -e 's/\.bits\././g' *.c *.h *.cpp
>>>sed -i -e 's/dw1\.//g' *.c *.h *.cpp
>>>
>>> and then reverting changes to comments in gen7_blorp.cpp and
>>> brw_fs_generator.cpp.
>>>
>>> There wasn't any utility offered by forcing the programmer to list these
>>> to access their fields. Removing them will reduce churn in future
>>> commits.
>>>
>>> This is C11 (and gcc has apparently supported it for sometime
>>> "compatibility with other compilers")
>>>
>>> See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
>>
>> This is also used from C++ source where anonymous structs are not part
>> of any released standard.
>
> That is true. I have built this series with both clang-3.6 and
> gcc-4.4.7. I don't think it's a problem.

FWIW the min supported compiler by mesa is GCC 4.2. I believe this is
the last pre-GPLv3 version, and used by the BSD's.

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

Re: [Mesa-dev] [PATCH 04/24] i965: Make 'dw1' and 'bits' unnamed structures in brw_reg.

[Matt Turner]

On Tue, Nov 3, 2015 at 12:09 PM, Ilia Mirkin  wrote:
> On Tue, Nov 3, 2015 at 3:00 PM, Matt Turner  wrote:
>> On Tue, Nov 3, 2015 at 5:48 AM, Francisco Jerez  
>> wrote:
>>> Matt Turner  writes:
>>>
 Generated by

sed -i -e 's/\.bits\././g' *.c *.h *.cpp
sed -i -e 's/dw1\.//g' *.c *.h *.cpp

 and then reverting changes to comments in gen7_blorp.cpp and
 brw_fs_generator.cpp.

 There wasn't any utility offered by forcing the programmer to list these
 to access their fields. Removing them will reduce churn in future
 commits.

 This is C11 (and gcc has apparently supported it for sometime
 "compatibility with other compilers")

 See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
>>>
>>> This is also used from C++ source where anonymous structs are not part
>>> of any released standard.
>>
>> That is true. I have built this series with both clang-3.6 and
>> gcc-4.4.7. I don't think it's a problem.
>
> FWIW the min supported compiler by mesa is GCC 4.2. I believe this is
> the last pre-GPLv3 version, and used by the BSD's.
>
>   -ilia

That doesn't compile i965 anyway because we use boolean literals, and
also because Mesa uses cpuid.h. But, hacking around those things, I
don't see any evidence that gcc-4.2 isn't able to handle this code
anyway.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 04/24] i965: Make 'dw1' and 'bits' unnamed structures in brw_reg.

[Matt Turner]

On Tue, Nov 3, 2015 at 12:13 PM, Matt Turner  wrote:
> On Tue, Nov 3, 2015 at 12:09 PM, Ilia Mirkin  wrote:
>> On Tue, Nov 3, 2015 at 3:00 PM, Matt Turner  wrote:
>>> On Tue, Nov 3, 2015 at 5:48 AM, Francisco Jerez  
>>> wrote:
 Matt Turner  writes:

> Generated by
>
>sed -i -e 's/\.bits\././g' *.c *.h *.cpp
>sed -i -e 's/dw1\.//g' *.c *.h *.cpp
>
> and then reverting changes to comments in gen7_blorp.cpp and
> brw_fs_generator.cpp.
>
> There wasn't any utility offered by forcing the programmer to list these
> to access their fields. Removing them will reduce churn in future
> commits.
>
> This is C11 (and gcc has apparently supported it for sometime
> "compatibility with other compilers")
>
> See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html

 This is also used from C++ source where anonymous structs are not part
 of any released standard.
>>>
>>> That is true. I have built this series with both clang-3.6 and
>>> gcc-4.4.7. I don't think it's a problem.
>>
>> FWIW the min supported compiler by mesa is GCC 4.2. I believe this is
>> the last pre-GPLv3 version, and used by the BSD's.
>>
>>   -ilia
>
> That doesn't compile i965 anyway because we use boolean literals, and
> also because Mesa uses cpuid.h. But, hacking around those things, I
> don't see any evidence that gcc-4.2 isn't able to handle this code
> anyway.

Sorry -- meant "binary literals" like 0b101.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 7/8] nir: Properly invalidate metadata in nir_opt_remove_phis().

[Eduardo Lima Mitev]

On 11/03/2015 09:31 AM, Kenneth Graunke wrote:
> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/nir_opt_remove_phis.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/glsl/nir/nir_opt_remove_phis.c 
> b/src/glsl/nir/nir_opt_remove_phis.c
> index 5bdf7ef..66d3754 100644
> --- a/src/glsl/nir/nir_opt_remove_phis.c
> +++ b/src/glsl/nir/nir_opt_remove_phis.c
> @@ -108,6 +108,11 @@ remove_phis_impl(nir_function_impl *impl)
>  
> nir_foreach_block(impl, remove_phis_block, &progress);
>  
> +   if (progress) {
> +  nir_metadata_preserve(impl, nir_metadata_block_index |
> +  nir_metadata_dominance);
> +   }
> +
> return progress;
>  }
>  
> 

Patches 2 to 7 (inclusive) are,

Reviewed-by: Eduardo Lima Mitev 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/8] nir: Properly invalidate metadata in nir_remove_dead_variables().

[Eduardo Lima Mitev]

On 11/03/2015 09:31 AM, Kenneth Graunke wrote:
> We can't preserve dominance or live variable information.
> 
> This also begs the question: what about globals?  Metadata only exists
> at the nir_function_impl level, so it would seem there is no metadata
> about global variables for us to invalidate.
> 

Shouldn't globals be always lowered to locals prior to invoking this
pass? That's what i965 does. Are there other drivers using nir, that
deal with global vars?

> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/nir_remove_dead_variables.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/glsl/nir/nir_remove_dead_variables.c 
> b/src/glsl/nir/nir_remove_dead_variables.c
> index d6783e7..77b6c13 100644
> --- a/src/glsl/nir/nir_remove_dead_variables.c
> +++ b/src/glsl/nir/nir_remove_dead_variables.c
> @@ -126,8 +126,13 @@ nir_remove_dead_variables(nir_shader *shader)
> progress = remove_dead_vars(&shader->globals, live) || progress;

At least in i965, where nir_lower_global_vars_to_local() runs prior to
this pass, only dead global variables can reach this point. So metadata
seems irrelevant here.

I tested this quickly with the following patch, and saw no new crashes
running several tests:

diff --git a/src/glsl/nir/nir_remove_dead_variables.c
b/src/glsl/nir/nir_remove_dead_variables.c
index d6783e7..7db48eb 100644
--- a/src/glsl/nir/nir_remove_dead_variables.c
+++ b/src/glsl/nir/nir_remove_dead_variables.c
@@ -124,6 +124,7 @@ nir_remove_dead_variables(nir_shader *shader)
add_var_use_shader(shader, live);

progress = remove_dead_vars(&shader->globals, live) || progress;
+   assert(exec_list_length(&shader->globals) == 0);

nir_foreach_overload(shader, overload) {
   if (overload->impl)

Eduardo

>  
> nir_foreach_overload(shader, overload) {
> -  if (overload->impl)
> - progress = remove_dead_vars(&overload->impl->locals, live) || 
> progress;
> +  if (overload->impl) {
> + if (remove_dead_vars(&overload->impl->locals, live)) {
> +nir_metadata_preserve(overload->impl, nir_metadata_block_index |
> +  nir_metadata_dominance);
> +progress = true;
> + }
> +  }
> }
>  
> _mesa_set_destroy(live, NULL);
> 

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

Re: [Mesa-dev] [PATCH 20/24] Revert "i965: Have brw_imm_vf4() take the vector components as integer values."

[Matt Turner]

On Tue, Nov 3, 2015 at 5:16 AM, Francisco Jerez  wrote:
> Matt Turner  writes:
>
>> This reverts commit bbf8239f92ecd79431dfa41402e1c85318e7267f.
>>
>> I didn't like that commit to begin with -- computing things at compile
>> time is fine -- but for purposes of verifying that the resulting values
>> are correct, looking up 0x00 and 0x30 in a table is a lot better than
>> evaluating a recursive function.
>>
> FYI the function only ever recurses once in order to handle the sign bit
> orthogonally from the exponent and mantissa.
>
>> Anyway, by making brw_imm_vf4() take the actual 8-bit restricted floats
>> directly (instead of only integral values that would be converted to
>> restricted float), we can use this function as a replacement for the
>> vector float src_reg/fs_reg constructors.
>
> Seems awful to me, it replaces a formula that can be verified correct by
> reading the first paragraph of the description of the restricted float
> format (Gen Graphics » BSpec » 3D-Media-GPGPU Engine » EU Overview » ISA
> Introduction » EU Data Types » Numeric Data Types) with a table of magic
> constants that don't give the slightest insight into the structure of
> the restricted float format and therefore have to be verified
> individually.

I don't think integer-only conversion functions are useful. The float
<-> vf functions that I added subsume int_to_float8.

But that's going towards a repeat of that discussion -- with you
saying that we should have vf constructors that take a float or an int
and does the conversion to restricted float for you, and me saying
having to know a priori whether the inputs can be represented as
restricted float makes that untenable.

Since the process of checking whether the input is representable is
basically identical to doing the actual conversion, if  there has to
be a function that tells you if a value is representable, and it might
as well return the actual restricted float if it is.


If your concern is the hardcoded constants for VF_ZERO/ONE, I can get
rid of them and change the code in brw_clip_util.c to

   brw_MOV(p, t_nopersp, brw_imm_vf4(brw_float_to_vf(1.0),
 brw_float_to_vf(0.0),
 brw_float_to_vf(0.0),
 brw_float_to_vf(0.0)));

(with appropriate indentation, in case gmail messes this up)

That of course should all compile away into code that is identical to
what we have now.

I'd be happy to change other instances of hard-coded restricted floats
to do this as well.

> It also makes it impossible to use brw_imm_vf4() with
> dynamically-specified integer constants which was the original
> motivation of this change.

Unless "integer constants" is really the stressed part of this
sentence, I don't think it's true given the suggestion I just made.

> If the obfuscation is meant as an optimization, I don't think it helps.
> GCC compiles the four int_to_float8() calls from brw_imm_vf4() into a
> single instruction already when the arguments are constant expressions:
>
>   4a9dc9:   48 b8 00 00 00 00 30movabs $0x30,%rax
>
> (assembly taken from brw_clip_interp_vertex())

No, it's not meant as an optimization -- I understand that
int_to_float8(const) compiles away. The main purpose is to make
brw_imm_vf4() usable in the following commits.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 2/2] nvc0: add missing compute parameters required by clover

[Samuel Pitoiset]

This fixes crashes with some piglit OpenCL tests.

Changes since v2:
- get rid of ul suffixes when they are unnecessary

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 52ce2d5..6ad3980 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -353,7 +353,8 @@ static int
 nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   enum pipe_compute_cap param, void *data)
 {
-   const uint16_t obj_class = nvc0_screen(pscreen)->compute->oclass;
+   struct nvc0_screen *screen = nvc0_screen(pscreen);
+   const uint16_t obj_class = screen->compute->oclass;
 
 #define RET(x) do {  \
if (data) \
@@ -384,6 +385,14 @@ nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   RET((uint64_t []) { 4096 });
case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
   RET((uint32_t []) { 32 });
+   case PIPE_COMPUTE_CAP_MAX_MEM_ALLOC_SIZE:
+  RET((uint64_t []) { 1ULL << 40 });
+   case PIPE_COMPUTE_CAP_IMAGES_SUPPORTED:
+  RET((uint32_t []) { 0 });
+   case PIPE_COMPUTE_CAP_MAX_COMPUTE_UNITS:
+  RET((uint32_t []) { screen->mp_count_compute });
+   case PIPE_COMPUTE_CAP_MAX_CLOCK_FREQUENCY:
+  RET((uint32_t []) { 512 }); /* FIXME: arbitrary limit */
default:
   return 0;
}
-- 
2.5.3

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

[Mesa-dev] [PATCH v2 1/2] nvc0: handle NULL pointer in nvc0_get_compute_param()

[Samuel Pitoiset]

To get the size (in bytes) of a compute parameter, clover first calls
get_compute_param() with a NULL data pointer. The RET() macro is based
on nv50.

Changes since v2:
- get rid of ul suffixes when they are unnecessary

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 45 --
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 6aa4f0b..52ce2d5 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -353,45 +353,42 @@ static int
 nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   enum pipe_compute_cap param, void *data)
 {
-   uint64_t *data64 = (uint64_t *)data;
-   uint32_t *data32 = (uint32_t *)data;
const uint16_t obj_class = nvc0_screen(pscreen)->compute->oclass;
 
+#define RET(x) do {  \
+   if (data) \
+  memcpy(data, x, sizeof(x));\
+   return sizeof(x); \
+} while (0)
+
switch (param) {
case PIPE_COMPUTE_CAP_GRID_DIMENSION:
-  data64[0] = 3;
-  return 8;
+  RET((uint64_t []) { 3 });
case PIPE_COMPUTE_CAP_MAX_GRID_SIZE:
-  data64[0] = (obj_class >= NVE4_COMPUTE_CLASS) ? 0x7fff : 65535;
-  data64[1] = 65535;
-  data64[2] = 65535;
-  return 24;
+  if (obj_class >= NVE4_COMPUTE_CLASS) {
+ RET(((uint64_t []) { 0x7fff, 65535, 65535 }));
+  } else {
+ RET(((uint64_t []) { 65535, 65535, 65535 }));
+  }
case PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE:
-  data64[0] = 1024;
-  data64[1] = 1024;
-  data64[2] = 64;
-  return 24;
+  RET(((uint64_t []) { 1024, 1024, 64 }));
case PIPE_COMPUTE_CAP_MAX_THREADS_PER_BLOCK:
-  data64[0] = 1024;
-  return 8;
+  RET((uint64_t []) { 1024 });
case PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE: /* g[] */
-  data64[0] = (uint64_t)1 << 40;
-  return 8;
+  RET((uint64_t []) { 1ULL << 40 });
case PIPE_COMPUTE_CAP_MAX_LOCAL_SIZE: /* s[] */
-  data64[0] = 48 << 10;
-  return 8;
+  RET((uint64_t []) { 48 << 10 });
case PIPE_COMPUTE_CAP_MAX_PRIVATE_SIZE: /* l[] */
-  data64[0] = 512 << 10;
-  return 8;
+  RET((uint64_t []) { 512 << 10 });
case PIPE_COMPUTE_CAP_MAX_INPUT_SIZE: /* c[], arbitrary limit */
-  data64[0] = 4096;
-  return 8;
+  RET((uint64_t []) { 4096 });
case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
-  data32[0] = 32;
-  return 4;
+  RET((uint32_t []) { 32 });
default:
   return 0;
}
+
+#undef RET
 }
 
 static void
-- 
2.5.3

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

Re: [Mesa-dev] [PATCH] i965/vec4: Send from GRF in atomic operations.

[Kenneth Graunke]

On Saturday, October 31, 2015 02:22:47 PM Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 30 
> +++---
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index b8f90f2..606fbd0 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1183,24 +1183,27 @@ vec4_visitor::gs_end_primitive()
>  
>  void
>  vec4_visitor::emit_untyped_atomic(unsigned atomic_op, unsigned surf_index,
> -  dst_reg dst, src_reg offset,
> +  dst_reg dst, src_reg surf_offset,
>src_reg src0, src_reg src1)
>  {
> -   unsigned mlen = 0;
> +   unsigned mlen = 1 + (src0.file != BAD_FILE) + (src1.file != BAD_FILE);
> +   src_reg src_payload(this, glsl_type::uint_type, mlen);
> +   dst_reg payload(src_payload);
> +   payload.writemask = WRITEMASK_X;
>  
> /* Set the atomic operation offset. */
> -   emit(MOV(brw_writemask(brw_uvec_mrf(8, mlen, 0), WRITEMASK_X), offset));
> -   mlen++;
> +   emit(MOV(offset(payload, 0), surf_offset));
> +   unsigned i = 1;
>  
> /* Set the atomic operation arguments. */
> if (src0.file != BAD_FILE) {
> -  emit(MOV(brw_writemask(brw_uvec_mrf(8, mlen, 0), WRITEMASK_X), src0));
> -  mlen++;
> +  emit(MOV(offset(payload, i), src0));
> +  i++;
> }
>  
> if (src1.file != BAD_FILE) {
> -  emit(MOV(brw_writemask(brw_uvec_mrf(8, mlen, 0), WRITEMASK_X), src1));
> -  mlen++;
> +  emit(MOV(offset(payload, i), src1));
> +  i++;
> }
>  
> /* Emit the instruction.  Note that this maps to the normal SIMD8
> @@ -1208,24 +1211,27 @@ vec4_visitor::emit_untyped_atomic(unsigned atomic_op, 
> unsigned surf_index,
>  * unused channels will be masked out.
>  */
> vec4_instruction *inst = emit(SHADER_OPCODE_UNTYPED_ATOMIC, dst,
> - brw_message_reg(0),
> + src_payload,
>   src_reg(surf_index), src_reg(atomic_op));
> inst->mlen = mlen;
>  }
>  
>  void
>  vec4_visitor::emit_untyped_surface_read(unsigned surf_index, dst_reg dst,
> -src_reg offset)
> +src_reg surf_offset)
>  {
> +   dst_reg offset(this, glsl_type::uint_type);
> +   offset.writemask = WRITEMASK_X;
> +
> /* Set the surface read offset. */
> -   emit(MOV(brw_writemask(brw_uvec_mrf(8, 0, 0), WRITEMASK_X), offset));
> +   emit(MOV(offset, surf_offset));
>  
> /* Emit the instruction.  Note that this maps to the normal SIMD8
>  * untyped surface read message, but that's OK because unused
>  * channels will be masked out.
>  */
> vec4_instruction *inst = emit(SHADER_OPCODE_UNTYPED_SURFACE_READ, dst,
> - brw_message_reg(0),
> + src_reg(offset),
>   src_reg(surf_index), src_reg(1));
> inst->mlen = 1;
>  }
> 

Looks good to me - it's always great to see more Gen7+ only MRF code
going away.

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 1/2] nvc0: handle NULL pointer in nvc0_get_compute_param()

[Ilia Mirkin]

Series is:

Reviewed-by: Ilia Mirkin 

On Tue, Nov 3, 2015 at 4:04 PM, Samuel Pitoiset
 wrote:
> To get the size (in bytes) of a compute parameter, clover first calls
> get_compute_param() with a NULL data pointer. The RET() macro is based
> on nv50.
>
> Changes since v2:
> - get rid of ul suffixes when they are unnecessary
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 45 
> --
>  1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> index 6aa4f0b..52ce2d5 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> @@ -353,45 +353,42 @@ static int
>  nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
>enum pipe_compute_cap param, void *data)
>  {
> -   uint64_t *data64 = (uint64_t *)data;
> -   uint32_t *data32 = (uint32_t *)data;
> const uint16_t obj_class = nvc0_screen(pscreen)->compute->oclass;
>
> +#define RET(x) do {  \
> +   if (data) \
> +  memcpy(data, x, sizeof(x));\
> +   return sizeof(x); \
> +} while (0)
> +
> switch (param) {
> case PIPE_COMPUTE_CAP_GRID_DIMENSION:
> -  data64[0] = 3;
> -  return 8;
> +  RET((uint64_t []) { 3 });
> case PIPE_COMPUTE_CAP_MAX_GRID_SIZE:
> -  data64[0] = (obj_class >= NVE4_COMPUTE_CLASS) ? 0x7fff : 65535;
> -  data64[1] = 65535;
> -  data64[2] = 65535;
> -  return 24;
> +  if (obj_class >= NVE4_COMPUTE_CLASS) {
> + RET(((uint64_t []) { 0x7fff, 65535, 65535 }));
> +  } else {
> + RET(((uint64_t []) { 65535, 65535, 65535 }));
> +  }
> case PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE:
> -  data64[0] = 1024;
> -  data64[1] = 1024;
> -  data64[2] = 64;
> -  return 24;
> +  RET(((uint64_t []) { 1024, 1024, 64 }));
> case PIPE_COMPUTE_CAP_MAX_THREADS_PER_BLOCK:
> -  data64[0] = 1024;
> -  return 8;
> +  RET((uint64_t []) { 1024 });
> case PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE: /* g[] */
> -  data64[0] = (uint64_t)1 << 40;
> -  return 8;
> +  RET((uint64_t []) { 1ULL << 40 });
> case PIPE_COMPUTE_CAP_MAX_LOCAL_SIZE: /* s[] */
> -  data64[0] = 48 << 10;
> -  return 8;
> +  RET((uint64_t []) { 48 << 10 });
> case PIPE_COMPUTE_CAP_MAX_PRIVATE_SIZE: /* l[] */
> -  data64[0] = 512 << 10;
> -  return 8;
> +  RET((uint64_t []) { 512 << 10 });
> case PIPE_COMPUTE_CAP_MAX_INPUT_SIZE: /* c[], arbitrary limit */
> -  data64[0] = 4096;
> -  return 8;
> +  RET((uint64_t []) { 4096 });
> case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
> -  data32[0] = 32;
> -  return 4;
> +  RET((uint32_t []) { 32 });
> default:
>return 0;
> }
> +
> +#undef RET
>  }
>
>  static void
> --
> 2.5.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/9] i965: Combine assembly annotations if possible.

[Matt Turner]

On Mon, Oct 26, 2015 at 5:08 AM, Pohjolainen, Topi
 wrote:
> On Wed, Oct 21, 2015 at 03:58:13PM -0700, Matt Turner wrote:
>> Often annotations are identical between sets of consecutive
>> instructions. We can perhaps avoid some memory allocations by reusing
>> the previous annotation.
>> ---
>>  src/mesa/drivers/dri/i965/intel_asm_annotation.c | 19 ++-
>>  1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_asm_annotation.c 
>> b/src/mesa/drivers/dri/i965/intel_asm_annotation.c
>> index f87a9bb..58830db 100644
>> --- a/src/mesa/drivers/dri/i965/intel_asm_annotation.c
>> +++ b/src/mesa/drivers/dri/i965/intel_asm_annotation.c
>> @@ -112,6 +112,20 @@ void annotate(const struct brw_device_info *devinfo,
>>ann->block_start = cfg->blocks[annotation->cur_block];
>> }
>>
>> +   if (bblock_end(cfg->blocks[annotation->cur_block]) == inst) {
>> +  ann->block_end = cfg->blocks[annotation->cur_block];
>> +  annotation->cur_block++;
>> +   }
>> +
>> +   /* Merge this annotation with the previous if possible. */
>> +   struct annotation *prev = &annotation->ann[annotation->ann_count - 2];
>
> What guarantees that annotation->ann_count is always at least two at this
> point?

Nothing! Good catch :)

I'll add "annotation->ann_count >= 2 &&" as the first part of the
conditional immediately below.

>> +   if (ann->ir == prev->ir &&
>> +   ann->annotation == prev->annotation &&
>> +   ann->block_start == NULL) {
>> +  annotation->ann_count--;
>> +  return;
>> +   }
>> +
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [RFC 1/5] nir: Separate texture from sampler in nir_tex_instr

[Jason Ekstrand]

This commit adds the capability to NIR to support separate textures and
samplers.  As it currently stands, glsl_to_nir only sets the sampler and
leaves the texture alone as it did before and nir_lower_samplers assumes
this.  However, backends can, if they wish, assume that they are separate
because nir_lower_samplers sets both texture and sampler index (they are
the same in this case).
---
 src/glsl/nir/nir.c |  8 +++-
 src/glsl/nir/nir.h | 28 
 src/glsl/nir/nir_instr_set.c   | 13 -
 src/glsl/nir/nir_lower_samplers.c  | 15 +--
 src/glsl/nir/nir_print.c   | 14 +++---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp   |  2 +-
 src/mesa/drivers/dri/i965/brw_vec4_nir.cpp |  2 +-
 src/mesa/program/prog_to_nir.c |  1 +
 8 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 5f03095..d7f5909 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -488,8 +488,10 @@ nir_tex_instr_create(nir_shader *shader, unsigned num_srcs)
for (unsigned i = 0; i < num_srcs; i++)
   src_init(&instr->src[i].src);
 
+   instr->texture_index = 0;
+   instr->texture_array_size = 0;
+   instr->texture = NULL;
instr->sampler_index = 0;
-   instr->sampler_array_size = 0;
instr->sampler = NULL;
 
return instr;
@@ -1007,6 +1009,10 @@ visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb 
cb, void *state)
   if (!visit_src(&instr->src[i].src, cb, state))
  return false;
 
+   if (instr->texture != NULL)
+  if (!visit_deref_src(instr->texture, cb, state))
+ return false;
+
if (instr->sampler != NULL)
   if (!visit_deref_src(instr->sampler, cb, state))
  return false;
diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index ac42251..682c4ed 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -933,6 +933,7 @@ typedef enum {
nir_tex_src_ms_index, /* MSAA sample index */
nir_tex_src_ddx,
nir_tex_src_ddy,
+   nir_tex_src_texture_offset, /* < dynamically uniform indirect offset */
nir_tex_src_sampler_offset, /* < dynamically uniform indirect offset */
nir_num_tex_src_types
 } nir_tex_src_type;
@@ -980,6 +981,24 @@ typedef struct {
/* gather component selector */
unsigned component : 2;
 
+   /** The texture index
+*
+* If this texture instruction has a nir_tex_src_texture_offset source,
+* then the texture index is given by texture_index + texture_offset.
+*/
+   unsigned texture_index;
+
+   /** The size of the texture array or 0 if it's not an array */
+   unsigned texture_array_size;
+
+   /** The texture deref
+*
+* If both this and `sampler` are both NULL, use texture_index instead.
+* If `texture` is NULL, but `sampler` is non-NULL, then the texture is
+* implied from the sampler.
+*/
+   nir_deref_var *texture;
+
/** The sampler index
 *
 * If this texture instruction has a nir_tex_src_sampler_offset source,
@@ -987,10 +1006,11 @@ typedef struct {
 */
unsigned sampler_index;
 
-   /** The size of the sampler array or 0 if it's not an array */
-   unsigned sampler_array_size;
-
-   nir_deref_var *sampler; /* if this is NULL, use sampler_index instead */
+   /** The sampler deref
+*
+* If this is null, use sampler_index instead.
+*/
+   nir_deref_var *sampler;
 } nir_tex_instr;
 
 static inline unsigned
diff --git a/src/glsl/nir/nir_instr_set.c b/src/glsl/nir/nir_instr_set.c
index d3f939f..eb02132 100644
--- a/src/glsl/nir/nir_instr_set.c
+++ b/src/glsl/nir/nir_instr_set.c
@@ -155,8 +155,9 @@ hash_tex(uint32_t hash, const nir_tex_instr *instr)
hash = HASH(hash, instr->const_offset);
unsigned component = instr->component;
hash = HASH(hash, component);
+   hash = HASH(hash, instr->texture_index);
+   hash = HASH(hash, instr->texture_array_size);
hash = HASH(hash, instr->sampler_index);
-   hash = HASH(hash, instr->sampler_array_size);
 
assert(!instr->sampler);
 
@@ -305,13 +306,15 @@ nir_instrs_equal(const nir_instr *instr1, const nir_instr 
*instr2)
   memcmp(tex1->const_offset, tex2->const_offset,
  sizeof(tex1->const_offset)) != 0 ||
   tex1->component != tex2->component ||
- tex1->sampler_index != tex2->sampler_index ||
- tex1->sampler_array_size != tex2->sampler_array_size) {
+ tex1->texture_index != tex2->texture_index ||
+ tex1->texture_array_size != tex2->texture_array_size ||
+ tex1->sampler_index != tex2->sampler_index) {
  return false;
   }
 
   /* Don't support un-lowered sampler derefs currently. */
-  assert(!tex1->sampler && !tex2->sampler);
+  assert(!tex1->texture && !tex1->sampler &&
+ !tex2->texture && !tex2->sampler);
 
   return true;
}
@@ -422,7 +425,7 @@ instr_can_rewrite(nir_instr *instr)
   nir_tex_instr *te

[Mesa-dev] [RFC 4/5] i965/vec4: Separate the sampler from the surface in generate_tex

[Jason Ekstrand]

---
 src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
index 8bc21df..6155274 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
@@ -108,6 +108,7 @@ generate_tex(struct brw_codegen *p,
  vec4_instruction *inst,
  struct brw_reg dst,
  struct brw_reg src,
+ struct brw_reg surface_index,
  struct brw_reg sampler_index)
 {
const struct brw_device_info *devinfo = p->devinfo;
@@ -259,14 +260,16 @@ generate_tex(struct brw_codegen *p,
  ? prog_data->base.binding_table.gather_texture_start
  : prog_data->base.binding_table.texture_start;
 
-   if (sampler_index.file == BRW_IMMEDIATE_VALUE) {
+   if (surface_index.file == BRW_IMMEDIATE_VALUE &&
+   sampler_index.file == BRW_IMMEDIATE_VALUE) {
+  uint32_t surface = surface_index.dw1.ud;
   uint32_t sampler = sampler_index.dw1.ud;
 
   brw_SAMPLE(p,
  dst,
  inst->base_mrf,
  src,
- sampler + base_binding_table_index,
+ surface + base_binding_table_index,
  sampler % 16,
  msg_type,
  1, /* response length */
@@ -280,14 +283,19 @@ generate_tex(struct brw_codegen *p,
   /* Non-constant sampler index. */
 
   struct brw_reg addr = vec1(retype(brw_address_reg(0), 
BRW_REGISTER_TYPE_UD));
+  struct brw_reg surface_reg = vec1(retype(surface_index, 
BRW_REGISTER_TYPE_UD));
   struct brw_reg sampler_reg = vec1(retype(sampler_index, 
BRW_REGISTER_TYPE_UD));
 
   brw_push_insn_state(p);
   brw_set_default_mask_control(p, BRW_MASK_DISABLE);
   brw_set_default_access_mode(p, BRW_ALIGN_1);
 
-  /* addr = ((sampler * 0x101) + base_binding_table_index) & 0xfff */
-  brw_MUL(p, addr, sampler_reg, brw_imm_uw(0x101));
+  if (memcmp(&surface_reg, &sampler_reg, sizeof(surface_reg)) == 0) {
+ brw_MUL(p, addr, sampler_reg, brw_imm_uw(0x101));
+  } else {
+ brw_SHL(p, addr, sampler_reg, brw_imm_ud(8));
+ brw_OR(p, addr, addr, surface_reg);
+  }
   if (base_binding_table_index)
  brw_ADD(p, addr, addr, brw_imm_ud(base_binding_table_index));
   brw_AND(p, addr, addr, brw_imm_ud(0xfff));
@@ -1319,7 +1327,7 @@ generate_code(struct brw_codegen *p,
   case SHADER_OPCODE_TG4:
   case SHADER_OPCODE_TG4_OFFSET:
   case SHADER_OPCODE_SAMPLEINFO:
- generate_tex(p, prog_data, inst, dst, src[0], src[1]);
+ generate_tex(p, prog_data, inst, dst, src[0], src[1], src[1]);
  break;
 
   case VS_OPCODE_URB_WRITE:
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [RFC 3/5] i965/fs: Plumb separate surfaces and samplers through from NIR

[Jason Ekstrand]

---
 src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp |  2 +-
 src/mesa/drivers/dri/i965/brw_fs.cpp| 47 +++--
 src/mesa/drivers/dri/i965/brw_fs.h  |  4 ++-
 src/mesa/drivers/dri/i965/brw_fs_generator.cpp  |  2 +-
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp| 25 +
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp| 16 +
 6 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
index 5308d17..7fa4ce8 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
@@ -85,7 +85,7 @@ brw_blorp_eu_emitter::emit_texture_lookup(const struct 
brw_reg &dst,
   unsigned msg_length)
 {
fs_inst *inst = new (mem_ctx) fs_inst(op, 16, dst, 
brw_message_reg(base_mrf),
- fs_reg(0u));
+ fs_reg(0u), fs_reg(0u));
 
inst->base_mrf = base_mrf;
inst->mlen = msg_length;
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 2d0acb9..92fa02f 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -723,15 +723,15 @@ fs_inst::components_read(unsigned i) const
case SHADER_OPCODE_LOD_LOGICAL:
case SHADER_OPCODE_TG4_LOGICAL:
case SHADER_OPCODE_TG4_OFFSET_LOGICAL:
-  assert(src[8].file == IMM && src[9].file == IMM);
+  assert(src[9].file == IMM && src[10].file == IMM);
   /* Texture coordinates. */
   if (i == 0)
- return src[8].fixed_hw_reg.dw1.ud;
+ return src[9].fixed_hw_reg.dw1.ud;
   /* Texture derivatives. */
   else if ((i == 2 || i == 3) && opcode == SHADER_OPCODE_TXD_LOGICAL)
- return src[9].fixed_hw_reg.dw1.ud;
+ return src[10].fixed_hw_reg.dw1.ud;
   /* Texture offset. */
-  else if (i == 7)
+  else if (i == 8)
  return 2;
   else
  return 1;
@@ -3505,6 +3505,7 @@ lower_sampler_logical_send_gen4(const fs_builder &bld, 
fs_inst *inst, opcode op,
 const fs_reg &coordinate,
 const fs_reg &shadow_c,
 const fs_reg &lod, const fs_reg &lod2,
+const fs_reg &surface,
 const fs_reg &sampler,
 unsigned coord_components,
 unsigned grad_components)
@@ -3597,8 +3598,9 @@ lower_sampler_logical_send_gen4(const fs_builder &bld, 
fs_inst *inst, opcode op,
 
inst->opcode = op;
inst->src[0] = reg_undef;
-   inst->src[1] = sampler;
-   inst->resize_sources(2);
+   inst->src[1] = surface;
+   inst->src[2] = sampler;
+   inst->resize_sources(3);
inst->base_mrf = msg_begin.reg;
inst->mlen = msg_end.reg - msg_begin.reg;
inst->header_size = 1;
@@ -3610,6 +3612,7 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, 
fs_inst *inst, opcode op,
 const fs_reg &shadow_c,
 fs_reg lod, fs_reg lod2,
 const fs_reg &sample_index,
+const fs_reg &surface,
 const fs_reg &sampler,
 const fs_reg &offset_value,
 unsigned coord_components,
@@ -3692,8 +3695,9 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, 
fs_inst *inst, opcode op,
 
inst->opcode = op;
inst->src[0] = reg_undef;
-   inst->src[1] = sampler;
-   inst->resize_sources(2);
+   inst->src[1] = surface;
+   inst->src[2] = sampler;
+   inst->resize_sources(3);
inst->base_mrf = message.reg;
inst->mlen = msg_end.reg - message.reg;
inst->header_size = header_size;
@@ -3717,7 +3721,9 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, 
fs_inst *inst, opcode op,
 const fs_reg &shadow_c,
 fs_reg lod, fs_reg lod2,
 const fs_reg &sample_index,
-const fs_reg &mcs, const fs_reg &sampler,
+const fs_reg &mcs,
+const fs_reg &surface,
+const fs_reg &sampler,
 fs_reg offset_value,
 unsigned coord_components,
 unsigned grad_components)
@@ -3906,8 +3912,9 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, 
fs_inst *inst, opcode op,
/* Generate the SEND. */
inst->opcode = op;
inst->src[0] = src_payload;
-   inst->src[1] = sampler;
-   inst->resize_sources(2);
+   inst->src[1] = surface;
+   inst->src[2] = sampler;
+   inst->resize_sources(3);
inst->base_mrf = -1;

[Mesa-dev] [RFC 0/5] nir: Separate Textures and Samplers

[Jason Ekstrand]

Separate textures and samplers are something that a lot of hardware
supports.  Our hardware in particular has done this ever since the original
i965 chips.  Part of this is because DX has made it a requirement for some
time now.  GL allows you to expose it sort-of but weasel-words it enough
that you can do it entirely at the API level and that's how we handle it
today.  I've been doing some thinking lately about how we could expose this
(if we wanted to) in the compiler.  This patch series is a first crack at
doing that.

Key design points

 1) There is no "combine" instruction; nir_tex_instr just takes a deref for
each of sampler and texture.
 2) It's optional (just make the texture deref NULL)
 3) The lowering pass sets both sampler and texture indices so that
backends don't have to make a distinction between whether you have a
separate texture or not

There is no API stuff here, just compiler stuff for the people who may or
may not be interested.

Happy Bikeshedding!

--Jason

Jason Ekstrand (5):
  nir: Separate texture from sampler in nir_tex_instr
  i965/fs: Separate the sampler from the surface in generate_tex
  i965/fs: Plumb separate surfaces and samplers through from NIR
  i965/vec4: Separate the sampler from the surface in generate_tex
  i965/vec4: Plumb separate surfaces and samplers through from NIR

 src/glsl/nir/nir.c   |  8 +++-
 src/glsl/nir/nir.h   | 28 --
 src/glsl/nir/nir_instr_set.c | 13 ---
 src/glsl/nir/nir_lower_samplers.c| 15 +++-
 src/glsl/nir/nir_print.c | 14 +--
 src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp  |  2 +-
 src/mesa/drivers/dri/i965/brw_fs.cpp | 47 ++--
 src/mesa/drivers/dri/i965/brw_fs.h   |  5 ++-
 src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 20 +++---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 25 +
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 16 
 src/mesa/drivers/dri/i965/brw_vec4.h |  4 +-
 src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 18 ++---
 src/mesa/drivers/dri/i965/brw_vec4_nir.cpp   | 27 ++
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 12 --
 src/mesa/program/prog_to_nir.c   |  1 +
 16 files changed, 181 insertions(+), 74 deletions(-)

-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [RFC 2/5] i965/fs: Separate the sampler from the surface in generate_tex

[Jason Ekstrand]

---
 src/mesa/drivers/dri/i965/brw_fs.h |  1 +
 src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 20 ++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
b/src/mesa/drivers/dri/i965/brw_fs.h
index 8058b34..b06a069 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -437,6 +437,7 @@ private:
void generate_linterp(fs_inst *inst, struct brw_reg dst,
 struct brw_reg *src);
void generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src,
+ struct brw_reg surface_index,
  struct brw_reg sampler_index);
void generate_get_buffer_size(fs_inst *inst, struct brw_reg dst,
  struct brw_reg src,
diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index 58bd23f..14a8f29 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -667,6 +667,7 @@ fs_generator::generate_get_buffer_size(fs_inst *inst,
 
 void
 fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg 
src,
+   struct brw_reg surface_index,
struct brw_reg sampler_index)
 {
int msg_type = -1;
@@ -899,14 +900,16 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg 
dst, struct brw_reg src
  ? prog_data->binding_table.gather_texture_start
  : prog_data->binding_table.texture_start;
 
-   if (sampler_index.file == BRW_IMMEDIATE_VALUE) {
+   if (surface_index.file == BRW_IMMEDIATE_VALUE &&
+   sampler_index.file == BRW_IMMEDIATE_VALUE) {
+  uint32_t surface = surface_index.dw1.ud;
   uint32_t sampler = sampler_index.dw1.ud;
 
   brw_SAMPLE(p,
  retype(dst, BRW_REGISTER_TYPE_UW),
  inst->base_mrf,
  src,
- sampler + base_binding_table_index,
+ surface + base_binding_table_index,
  sampler % 16,
  msg_type,
  rlen,
@@ -915,19 +918,24 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg 
dst, struct brw_reg src
  simd_mode,
  return_format);
 
-  brw_mark_surface_used(prog_data, sampler + base_binding_table_index);
+  brw_mark_surface_used(prog_data, surface + base_binding_table_index);
} else {
   /* Non-const sampler index */
 
   struct brw_reg addr = vec1(retype(brw_address_reg(0), 
BRW_REGISTER_TYPE_UD));
+  struct brw_reg surface_reg = vec1(retype(surface_index, 
BRW_REGISTER_TYPE_UD));
   struct brw_reg sampler_reg = vec1(retype(sampler_index, 
BRW_REGISTER_TYPE_UD));
 
   brw_push_insn_state(p);
   brw_set_default_mask_control(p, BRW_MASK_DISABLE);
   brw_set_default_access_mode(p, BRW_ALIGN_1);
 
-  /* addr = ((sampler * 0x101) + base_binding_table_index) & 0xfff */
-  brw_MUL(p, addr, sampler_reg, brw_imm_uw(0x101));
+  if (memcmp(&surface_reg, &sampler_reg, sizeof(surface_reg)) == 0) {
+ brw_MUL(p, addr, sampler_reg, brw_imm_uw(0x101));
+  } else {
+ brw_SHL(p, addr, sampler_reg, brw_imm_ud(8));
+ brw_OR(p, addr, addr, surface_reg);
+  }
   if (base_binding_table_index)
  brw_ADD(p, addr, addr, brw_imm_ud(base_binding_table_index));
   brw_AND(p, addr, addr, brw_imm_ud(0xfff));
@@ -2053,7 +2061,7 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
   case SHADER_OPCODE_TG4:
   case SHADER_OPCODE_TG4_OFFSET:
   case SHADER_OPCODE_SAMPLEINFO:
-generate_tex(inst, dst, src[0], src[1]);
+generate_tex(inst, dst, src[0], src[1], src[1]);
 break;
   case FS_OPCODE_DDX_COARSE:
   case FS_OPCODE_DDX_FINE:
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [RFC 5/5] i965/vec4: Plumb separate surfaces and samplers through from NIR

[Jason Ekstrand]

---
 src/mesa/drivers/dri/i965/brw_vec4.h   |  4 +++-
 src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 27 ++
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 12 
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h 
b/src/mesa/drivers/dri/i965/brw_vec4.h
index ec8abf4..52d68c5 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -273,9 +273,11 @@ public:
  src_reg offset_value,
  src_reg mcs,
  bool is_cube_array,
+ uint32_t surface, src_reg surface_reg,
  uint32_t sampler, src_reg sampler_reg);
 
-   uint32_t gather_channel(unsigned gather_component, uint32_t sampler);
+   uint32_t gather_channel(unsigned gather_component,
+   uint32_t surface, uint32_t sampler);
src_reg emit_mcs_fetch(const glsl_type *coordinate_type, src_reg coordinate,
   src_reg sampler);
void emit_gen6_gather_wa(uint8_t wa, dst_reg dst);
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
index 7a6372b..86e3dec 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
@@ -1585,7 +1585,9 @@ glsl_type_for_nir_alu_type(nir_alu_type alu_type,
 void
 vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
 {
+   unsigned texture = instr->texture_index;
unsigned sampler = instr->sampler_index;
+   src_reg texture_reg = src_reg(texture);
src_reg sampler_reg = src_reg(sampler);
src_reg coordinate;
const glsl_type *coord_type = NULL;
@@ -1666,8 +1668,8 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
  sample_index = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_D, 1);
  assert(coord_type != NULL);
  if (devinfo->gen >= 7 &&
- key_tex->compressed_multisample_layout_mask & (1 << sampler)) {
-mcs = emit_mcs_fetch(coord_type, coordinate, sampler_reg);
+ key_tex->compressed_multisample_layout_mask & (1 << texture)) {
+mcs = emit_mcs_fetch(coord_type, coordinate, texture_reg);
  } else {
 mcs = src_reg(0u);
  }
@@ -1679,13 +1681,12 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
  offset_value = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_D, 2);
  break;
 
-  case nir_tex_src_sampler_offset: {
- /* The highest sampler which may be used by this operation is
+  case nir_tex_src_texture_offset: {
+ /* The highest texture which may be used by this operation is
   * the last element of the array. Mark it here, because the generator
   * doesn't have enough information to determine the bound.
   */
- uint32_t array_size = instr->texture_array_size;
- uint32_t max_used = sampler + array_size - 1;
+ uint32_t max_used = texture + instr->texture_array_size - 1;
  if (instr->op == nir_texop_tg4) {
 max_used += prog_data->base.binding_table.gather_texture_start;
  } else {
@@ -1697,6 +1698,15 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
  /* Emit code to evaluate the actual indexing expression */
  src_reg src = get_nir_src(instr->src[i].src, 1);
  src_reg temp(this, glsl_type::uint_type);
+ emit(ADD(dst_reg(temp), src, src_reg(texture)));
+ texture_reg = emit_uniformize(temp);
+ break;
+  }
+
+  case nir_tex_src_sampler_offset: {
+ /* Emit code to evaluate the actual indexing expression */
+ src_reg src = get_nir_src(instr->src[i].src, 1);
+ src_reg temp(this, glsl_type::uint_type);
  emit(ADD(dst_reg(temp), src, src_reg(sampler)));
  sampler_reg = emit_uniformize(temp);
  break;
@@ -1723,7 +1733,7 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
 
/* Stuff the channel select bits in the top of the texture offset */
if (instr->op == nir_texop_tg4)
-  constant_offset |= gather_channel(instr->component, sampler) << 16;
+  constant_offset |= gather_channel(instr->component, texture, sampler) << 
16;
 
ir_texture_opcode op = ir_texture_opcode_for_nir_texop(instr->op);
 
@@ -1736,7 +1746,8 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
 shadow_comparitor,
 lod, lod2, sample_index,
 constant_offset, offset_value,
-mcs, is_cube_array, sampler, sampler_reg);
+mcs, is_cube_array,
+texture, texture_reg, sampler, sampler_reg);
 }
 
 void
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index b8f90f2..5eb2f7b 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -879,6 +879,8

Re: [Mesa-dev] [PATCH 5/8] nir: Properly invalidate metadata in nir_opt_copy_prop().

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Tue, Nov 3, 2015 at 12:31 AM, Kenneth Graunke  wrote:
> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/nir_opt_copy_propagate.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/glsl/nir/nir_opt_copy_propagate.c 
> b/src/glsl/nir/nir_opt_copy_propagate.c
> index 96520f8..7d8bdd7 100644
> --- a/src/glsl/nir/nir_opt_copy_propagate.c
> +++ b/src/glsl/nir/nir_opt_copy_propagate.c
> @@ -262,6 +262,12 @@ nir_copy_prop_impl(nir_function_impl *impl)
> bool progress = false;
>
> nir_foreach_block(impl, copy_prop_block, &progress);
> +
> +   if (progress) {
> +  nir_metadata_preserve(impl, nir_metadata_block_index |
> +  nir_metadata_dominance);
> +   }
> +
> return progress;
>  }
>
> --
> 2.6.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] nir: Properly invalidate metadata in nir_lower_global_vars_to_local().

[Jason Ekstrand]

On Tue, Nov 3, 2015 at 12:31 AM, Kenneth Graunke  wrote:
> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/nir_lower_global_vars_to_local.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/glsl/nir/nir_lower_global_vars_to_local.c 
> b/src/glsl/nir/nir_lower_global_vars_to_local.c
> index fab2366..9fa64ed 100644
> --- a/src/glsl/nir/nir_lower_global_vars_to_local.c
> +++ b/src/glsl/nir/nir_lower_global_vars_to_local.c
> @@ -100,6 +100,8 @@ nir_lower_global_vars_to_local(nir_shader *shader)
>   exec_node_remove(&var->node);
>   var->data.mode = nir_var_local;
>   exec_list_push_tail(&impl->locals, &var->node);
> + nir_metadata_preserve(impl, nir_metadata_block_index |
> + nir_metadata_dominance);

This doesn't invalidate live_variables (which should be renamed live_ssa_values)
--Jason

>   progress = true;
>}
> }
> --
> 2.6.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] nir: Properly invalidate metadata in nir_split_var_copies().

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Tue, Nov 3, 2015 at 12:31 AM, Kenneth Graunke  wrote:
> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/nir_split_var_copies.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/glsl/nir/nir_split_var_copies.c 
> b/src/glsl/nir/nir_split_var_copies.c
> index d2ea58a..d463f7b 100644
> --- a/src/glsl/nir/nir_split_var_copies.c
> +++ b/src/glsl/nir/nir_split_var_copies.c
> @@ -271,6 +271,11 @@ split_var_copies_impl(nir_function_impl *impl)
>
> ralloc_free(state.dead_ctx);
>
> +   if (state.progress) {
> +  nir_metadata_preserve(impl, nir_metadata_block_index |
> +  nir_metadata_dominance);
> +   }
> +
> return state.progress;
>  }
>
> --
> 2.6.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/8] nir: Properly invalidate metadata in nir_remove_dead_variables().

[Jason Ekstrand]

On Tue, Nov 3, 2015 at 12:31 AM, Kenneth Graunke  wrote:
> We can't preserve dominance or live variable information.
>
> This also begs the question: what about globals?  Metadata only exists
> at the nir_function_impl level, so it would seem there is no metadata
> about global variables for us to invalidate.
>
> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/nir_remove_dead_variables.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/glsl/nir/nir_remove_dead_variables.c 
> b/src/glsl/nir/nir_remove_dead_variables.c
> index d6783e7..77b6c13 100644
> --- a/src/glsl/nir/nir_remove_dead_variables.c
> +++ b/src/glsl/nir/nir_remove_dead_variables.c
> @@ -126,8 +126,13 @@ nir_remove_dead_variables(nir_shader *shader)
> progress = remove_dead_vars(&shader->globals, live) || progress;
>
> nir_foreach_overload(shader, overload) {
> -  if (overload->impl)
> - progress = remove_dead_vars(&overload->impl->locals, live) || 
> progress;
> +  if (overload->impl) {
> + if (remove_dead_vars(&overload->impl->locals, live)) {
> +nir_metadata_preserve(overload->impl, nir_metadata_block_index |
> +  nir_metadata_dominance);

This doesn't invalidate live_variables

> +progress = true;
> + }
> +  }
> }
>
> _mesa_set_destroy(live, NULL);
> --
> 2.6.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/8] nir: Properly invalidate metadata in nir_lower_vec_to_movs().

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Tue, Nov 3, 2015 at 12:31 AM, Kenneth Graunke  wrote:
> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/nir_lower_vec_to_movs.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/glsl/nir/nir_lower_vec_to_movs.c 
> b/src/glsl/nir/nir_lower_vec_to_movs.c
> index c08b721..736a66c 100644
> --- a/src/glsl/nir/nir_lower_vec_to_movs.c
> +++ b/src/glsl/nir/nir_lower_vec_to_movs.c
> @@ -288,6 +288,11 @@ nir_lower_vec_to_movs_impl(nir_function_impl *impl)
>
> nir_foreach_block(impl, lower_vec_to_movs_block, &state);
>
> +   if (state.progress) {
> +  nir_metadata_preserve(impl, nir_metadata_block_index |
> +  nir_metadata_dominance);
> +   }
> +
> return state.progress;
>  }
>
> --
> 2.6.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 7/8] nir: Properly invalidate metadata in nir_opt_remove_phis().

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Tue, Nov 3, 2015 at 12:19 PM, Eduardo Lima Mitev  wrote:
> On 11/03/2015 09:31 AM, Kenneth Graunke wrote:
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/glsl/nir/nir_opt_remove_phis.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/src/glsl/nir/nir_opt_remove_phis.c 
>> b/src/glsl/nir/nir_opt_remove_phis.c
>> index 5bdf7ef..66d3754 100644
>> --- a/src/glsl/nir/nir_opt_remove_phis.c
>> +++ b/src/glsl/nir/nir_opt_remove_phis.c
>> @@ -108,6 +108,11 @@ remove_phis_impl(nir_function_impl *impl)
>>
>> nir_foreach_block(impl, remove_phis_block, &progress);
>>
>> +   if (progress) {
>> +  nir_metadata_preserve(impl, nir_metadata_block_index |
>> +  nir_metadata_dominance);
>> +   }
>> +
>> return progress;
>>  }
>>
>>
>
> Patches 2 to 7 (inclusive) are,
>
> Reviewed-by: Eduardo Lima Mitev 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 01/10] nir: Move nir_metadata.c to nir_pass.c

[Jason Ekstrand]

We're about to put more pass management stuff in here so it needs a better
name.

Reviewed-by: Kristian Høgsberg 
---
 src/glsl/Makefile.sources   |  2 +-
 src/glsl/nir/nir_metadata.c | 54 -
 src/glsl/nir/nir_pass.c | 54 +
 3 files changed, 55 insertions(+), 55 deletions(-)
 delete mode 100644 src/glsl/nir/nir_metadata.c
 create mode 100644 src/glsl/nir/nir_pass.c

diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index ca87036..98708f7 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -57,7 +57,6 @@ NIR_FILES = \
nir/nir_lower_vars_to_ssa.c \
nir/nir_lower_var_copies.c \
nir/nir_lower_vec_to_movs.c \
-   nir/nir_metadata.c \
nir/nir_move_vec_src_uses_to_dest.c \
nir/nir_normalize_cubemap_coords.c \
nir/nir_opt_constant_folding.c \
@@ -71,6 +70,7 @@ NIR_FILES = \
nir/nir_opt_peephole_select.c \
nir/nir_opt_remove_phis.c \
nir/nir_opt_undef.c \
+   nir/nir_pass.c \
nir/nir_print.c \
nir/nir_remove_dead_variables.c \
nir/nir_search.c \
diff --git a/src/glsl/nir/nir_metadata.c b/src/glsl/nir/nir_metadata.c
deleted file mode 100644
index a03e124..000
--- a/src/glsl/nir/nir_metadata.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * Copyright © 2014 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- *Jason Ekstrand (ja...@jlekstrand.net)
- */
-
-#include "nir.h"
-
-/*
- * Handles management of the metadata.
- */
-
-void
-nir_metadata_require(nir_function_impl *impl, nir_metadata required)
-{
-#define NEEDS_UPDATE(X) ((required & ~impl->valid_metadata) & (X))
-
-   if (NEEDS_UPDATE(nir_metadata_block_index))
-  nir_index_blocks(impl);
-   if (NEEDS_UPDATE(nir_metadata_dominance))
-  nir_calc_dominance_impl(impl);
-   if (NEEDS_UPDATE(nir_metadata_live_variables))
-  nir_live_variables_impl(impl);
-
-#undef NEEDS_UPDATE
-
-   impl->valid_metadata |= required;
-}
-
-void
-nir_metadata_preserve(nir_function_impl *impl, nir_metadata preserved)
-{
-   impl->valid_metadata &= preserved;
-}
diff --git a/src/glsl/nir/nir_pass.c b/src/glsl/nir/nir_pass.c
new file mode 100644
index 000..a03e124
--- /dev/null
+++ b/src/glsl/nir/nir_pass.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Jason Ekstrand (ja...@jlekstrand.net)
+ */
+
+#include "nir.h"
+
+/*
+ * Handles management of the metadata.
+ */
+
+void
+nir_metadata_require(nir_function_impl *impl, nir_metadata required)
+{
+#define NEEDS_UPDATE(X) ((required & ~impl->valid_metadata) & (X))
+
+   if (NEEDS_UPDATE(nir_metadata_block_index))
+  nir_index_blocks(impl);
+   if (NEEDS_UPDA

[Mesa-dev] [PATCH v2 03/10] nir: Unexpose _impl versions of copy_prop and dce

[Jason Ekstrand]

Reviewed-by: Kristian Høgsberg 
---
 src/glsl/nir/nir.h| 2 --
 src/glsl/nir/nir_opt_copy_propagate.c | 2 +-
 src/glsl/nir/nir_opt_dce.c| 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index 1d3e281..f2d01e9 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -2021,12 +2021,10 @@ bool nir_opt_constant_folding(nir_shader *shader);
 
 bool nir_opt_global_to_local(nir_shader *shader);
 
-bool nir_copy_prop_impl(nir_function_impl *impl);
 bool nir_copy_prop(nir_shader *shader);
 
 bool nir_opt_cse(nir_shader *shader);
 
-bool nir_opt_dce_impl(nir_function_impl *impl);
 bool nir_opt_dce(nir_shader *shader);
 
 bool nir_opt_dead_cf(nir_shader *shader);
diff --git a/src/glsl/nir/nir_opt_copy_propagate.c 
b/src/glsl/nir/nir_opt_copy_propagate.c
index 71367d0..96520f8 100644
--- a/src/glsl/nir/nir_opt_copy_propagate.c
+++ b/src/glsl/nir/nir_opt_copy_propagate.c
@@ -256,7 +256,7 @@ copy_prop_block(nir_block *block, void *_state)
return true;
 }
 
-bool
+static bool
 nir_copy_prop_impl(nir_function_impl *impl)
 {
bool progress = false;
diff --git a/src/glsl/nir/nir_opt_dce.c b/src/glsl/nir/nir_opt_dce.c
index e0ebdc6..6032528 100644
--- a/src/glsl/nir/nir_opt_dce.c
+++ b/src/glsl/nir/nir_opt_dce.c
@@ -145,7 +145,7 @@ delete_block_cb(nir_block *block, void *_state)
return true;
 }
 
-bool
+static bool
 nir_opt_dce_impl(nir_function_impl *impl)
 {
struct exec_list *worklist = ralloc(NULL, struct exec_list);
-- 
2.5.0.400.gff86faf

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