[Mesa-dev] [PATCH 8/7] i965: Accurately bail on SIMD16 compiles.

[Kenneth Graunke]

Ideally, we'd like to never even attempt the SIMD16 compile if we could
know ahead of time that it won't succeed---it's purely a waste of time.
This is especially important for state-based recompiles, which happen at
draw time.

The fragment shader compiler has a number of checks like:

   if (dispatch_width == 16)
  fail("...some reason...");

This patch introduces a new no16() function which replaces the above
pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
SIMD16 mode, no16() calls fail(), for safety's sake.)

The great part is that this is not a heuristic---if the flag is set, we
know with 100% certainty that the SIMD16 compile would fail.  (It might
fail anyway if we run out of registers, but it's always worth trying.)

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_fs.cpp | 69 +++-
 src/mesa/drivers/dri/i965/brw_fs.h   |  4 ++
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +-
 3 files changed, 83 insertions(+), 34 deletions(-)

I forgot to send this one out...it applies on top of the previous 7 patches.

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 62848be..9ad80c5 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum 
shader_time_shader_type type,
 }
 
 void
-fs_visitor::fail(const char *format, ...)
+fs_visitor::vfail(const char *format, va_list va)
 {
-   va_list va;
char *msg;
 
if (failed)
@@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
 
failed = true;
 
-   va_start(va, format);
msg = ralloc_vasprintf(mem_ctx, format, va);
-   va_end(va);
msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
 
this->fail_msg = msg;
@@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
}
 }
 
+void
+fs_visitor::fail(const char *format, ...)
+{
+   va_list va;
+
+   va_start(va, format);
+   vfail(format, va);
+   va_end(va);
+}
+
+/**
+ * Mark this program as impossible to compile in SIMD16 mode.
+ *
+ * During the SIMD8 compile (which happens first), we can detect and flag
+ * things that are unsupported in SIMD16 mode, so the compiler can skip
+ * the SIMD16 compile altogether.
+ *
+ * During a SIMD16 compile (if one happens anyway), this just calls fail().
+ */
+void
+fs_visitor::no16(const char *format, ...)
+{
+   va_list va;
+
+   va_start(va, format);
+
+   if (dispatch_width == 16) {
+  vfail(format, va);
+  return;
+   }
+
+   simd16_unsupported = true;
+
+   if (INTEL_DEBUG & DEBUG_PERF) {
+  if (no16_msg)
+ ralloc_vasprintf_append(&no16_msg, format, va);
+  else
+ no16_msg = ralloc_vasprintf(mem_ctx, format, va);
+   }
+
+   va_end(va);
+}
+
 fs_inst *
 fs_visitor::emit(enum opcode opcode)
 {
@@ -1356,8 +1396,8 @@ fs_visitor::emit_math(enum opcode opcode, fs_reg dst, 
fs_reg src0, fs_reg src1)
switch (opcode) {
case SHADER_OPCODE_INT_QUOTIENT:
case SHADER_OPCODE_INT_REMAINDER:
-  if (brw->gen >= 7 && dispatch_width == 16)
-fail("SIMD16 INTDIV unsupported\n");
+  if (brw->gen >= 7)
+no16("SIMD16 INTDIV unsupported\n");
   break;
case SHADER_OPCODE_POW:
   break;
@@ -3504,13 +3544,18 @@ brw_wm_fs_emit(struct brw_context *brw, struct 
brw_wm_compile *c,
exec_list *simd16_instructions = NULL;
fs_visitor v2(brw, c, prog, fp, 16);
if (brw->gen >= 5 && likely(!(INTEL_DEBUG & DEBUG_NO16))) {
-  /* Try a SIMD16 compile */
-  v2.import_uniforms(&v);
-  if (!v2.run()) {
- perf_debug("SIMD16 shader failed to compile, falling back to "
-"SIMD8 at a 10-20%% performance cost: %s", v2.fail_msg);
+  if (!v.simd16_unsupported) {
+ /* Try a SIMD16 compile */
+ v2.import_uniforms(&v);
+ if (!v2.run()) {
+perf_debug("SIMD16 shader failed to compile, falling back to "
+   "SIMD8 at a 10-20%% performance cost: %s", v2.fail_msg);
+ } else {
+simd16_instructions = &v2.instructions;
+ }
   } else {
- simd16_instructions = &v2.instructions;
+ perf_debug("SIMD16 shader unsupported, falling back to "
+"SIMD8 at a 10-20%% performance cost: %s", v.no16_msg);
   }
}
 
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
b/src/mesa/drivers/dri/i965/brw_fs.h
index 9de1f3a..0d064f6 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -381,7 +381,9 @@ public:
void insert_gen4_send_dependency_workarounds();
void insert_gen4_pre_send_dependency_workarounds(fs_inst *inst);
void insert_gen4_post_send_dependency_workarounds(fs_inst *inst);
+   void vfail(const char *msg, va_list a

Re: [Mesa-dev] [PATCH 1/6] glsl: Optimize pow(x, 2) into x * x.

[Erik Faye-Lund]

On Wed, Mar 12, 2014 at 1:32 AM, Eric Anholt  wrote:
> Erik Faye-Lund  writes:
>
>> On Wed, Mar 12, 2014 at 12:00 AM, Eric Anholt  wrote:
>>> Erik Faye-Lund  writes:
>>>
 On Tue, Mar 11, 2014 at 7:27 PM, Eric Anholt  wrote:
> Erik Faye-Lund  writes:
>
>> On Tue, Mar 11, 2014 at 2:50 PM, Erik Faye-Lund  
>> wrote:
>>> On Mon, Mar 10, 2014 at 11:54 PM, Matt Turner  
>>> wrote:
 Cuts two instructions out of SynMark's Gl32VSInstancing benchmark.
 ---
  src/glsl/opt_algebraic.cpp | 8 
  1 file changed, 8 insertions(+)

 diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
 index 5c49a78..8494bd9 100644
 --- a/src/glsl/opt_algebraic.cpp
 +++ b/src/glsl/opt_algebraic.cpp
 @@ -528,6 +528,14 @@ 
 ir_algebraic_visitor::handle_expression(ir_expression *ir)
if (is_vec_two(op_const[0]))
   return expr(ir_unop_exp2, ir->operands[1]);

 +  if (is_vec_two(op_const[1])) {
 + ir_variable *x = new(ir) ir_variable(ir->operands[1]->type, 
 "x",
 +  ir_var_temporary);
 + base_ir->insert_before(x);
 + base_ir->insert_before(assign(x, ir->operands[0]));
 + return mul(x, x);
 +  }
 +
>>>
>>> Is this safe? Since many GPUs implement pow(x, y) as exp2(log2(x) *
>>> y), this will give different results for if y comes from a uniform vs
>>> if it's a constant, no?
>
> Yes, but that wouldn't be covered by the "invariant" keyword.
>
>> To be a bit more clear: I don't think this is valid for expressions
>> writing to variables marked as invariant (or expressions taking part
>> in the calculations that leads up to invariant variable writes).
>>
>> I can't find anything allowing variance like this in the invariance
>> section of the GLSL 3.30 specifications. In particular, the list
>> following "To guarantee invariance of a particular output variable
>> across two programs, the following must also be true" doesn't seem to
>> require the values to be passed from the same source, only that the
>> same values are passed. And in this case, the value 2.0 is usually
>> exactly representable no matter what path it took there.
>>
>> Perhaps I'm being a bit too pedantic here, though.
>
> This file would do the same thing on the same expression tree in two
> different programs, so "invariant" is fine (we've probably got other
> problems with invariant, though).  The keyword you're probably thinking
> of is "precise", which isn't in GLSL we implement yet.

 Are you saying that this only rewrites "x = pow(y, 2.0)" and not
 "const float e = 2.0; x = pow(y, e);"? If so, my point is moot,
 indeed. But if that's *not* the case, then I think we're in trouble
 still.
>>>
>>> The second would also get rewritten, because other passes will move the
>>> 2.0 into the pow.
>>>
>>> I thought I understood your objection, but now I don't.  I think you'll
>>> have to lay out the pair of shaders involving the invariant keyword that
>>> you think that would be broken by this pass.
>>
>> My understanding is that
>> ---8<---
>> invariant varying float v;
>> attribute float a;
>> const float e = 2.0;
>> void main()
>> {
>> v = pow(a, e);
>> }
>> ---8<---
>> and
>> ---8<---
>> invariant varying float v;
>> attribute float a;
>> uniform float e;
>> void main()
>> {
>> v = pow(a, e);
>> }
>> ---8<---
>> ...should produce the exact same result, as long as the latter is
>> passed 2.0 as the uniform e.
>>
>> Because v is marked as invariant, the expressions writing to v are the
>> same, and the values passed in are the same.
>>
>> If we rewrite the first one to do "a * a", we get a different result
>> on implementations that do "exp2(log2(a) * 2.0)" for the latter, due
>> to floating-point normalization in the intermediate steps.
>
> I don't think that's what the spec authors intended from the keyword.  I
> think what they intended was that if you had uniform float e in both
> cases, but different code for setting *other* lvalues, that you'd still
> get the same result in v.

I think that *might* be correct, but it doesn't seem to be what's
actually defined. Invariance comes from ESSL, and FWIW, I was one of
the spec authors in this case. But I don't remember the details down
to this level, and the spec doesn't seem to clarify either.

However, since constant expressions are given some slack wrt how it's
evaluated, I'm inclined to think that you're right about the spirit of
the spec.

AFAIR, we introduced "invariant" to get rid of ftransform (since we'd
already gotten rid of fixed-function state), so that multi-pass
rendering algorthms could be guaranteed that all passes ended up
covering the exact same fragments at the exact same d

[Mesa-dev] [PATCH] i965: Don't enable reset notification support on Gen4-5.

[Kenneth Graunke]

arekm reported that using Chrome with GPU acceleration enabled on GM45
triggered the hw_ctx != NULL assertion in brw_get_graphics_reset_status.

We definitely do not want to advertise reset notification support on
Gen4-5 systems, since it needs hardware contexts, and we never even
request a hardware context on those systems.

Cc: Ian Romanick 
Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/intel_screen.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

This may not be the best solution; if someone would like to send patches for
a better one, I'm more than happy to go with something else.  Checking for
hardware contexts is a bit awkward - we need to decide whether to advertise
support here in the screen, but we don't create hardware contexts until
context creation time, which is a fair bit later.  So, I'm not sure how to
do better than the generation check, for now.

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index 464cebf..12006be 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1342,13 +1342,18 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
 * no error.  If the ioctl is not supported, it always generate EINVAL.
 * Use this to determine whether to advertise the __DRI2_ROBUSTNESS
 * extension to the loader.
+*
+* Don't even try on pre-Gen6, since we don't attempt to use contexts there.
 */
-   struct drm_i915_reset_stats stats;
-   memset(&stats, 0, sizeof(stats));
+   if (intelScreen->devinfo->gen >= 6) {
+  struct drm_i915_reset_stats stats;
+  memset(&stats, 0, sizeof(stats));
 
-   const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS, &stats);
+  const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS, 
&stats);
 
-   intelScreen->has_context_reset_notification = (ret != -1 || errno != 
EINVAL);
+  intelScreen->has_context_reset_notification =
+ (ret != -1 || errno != EINVAL);
+   }
 
psp->extensions = !intelScreen->has_context_reset_notification
   ? intelScreenExtensions : intelRobustScreenExtensions;
-- 
1.9.0

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

Re: [Mesa-dev] [PATCH 8/7] i965: Accurately bail on SIMD16 compiles.

[Chris Forbes]

This patch is:

Reviewed-by: Chris Forbes 

On Wed, Mar 12, 2014 at 9:28 PM, Kenneth Graunke  wrote:
> Ideally, we'd like to never even attempt the SIMD16 compile if we could
> know ahead of time that it won't succeed---it's purely a waste of time.
> This is especially important for state-based recompiles, which happen at
> draw time.
>
> The fragment shader compiler has a number of checks like:
>
>if (dispatch_width == 16)
>   fail("...some reason...");
>
> This patch introduces a new no16() function which replaces the above
> pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
> Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
> issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
> SIMD16 mode, no16() calls fail(), for safety's sake.)
>
> The great part is that this is not a heuristic---if the flag is set, we
> know with 100% certainty that the SIMD16 compile would fail.  (It might
> fail anyway if we run out of registers, but it's always worth trying.)
>
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 69 
> +++-
>  src/mesa/drivers/dri/i965/brw_fs.h   |  4 ++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +-
>  3 files changed, 83 insertions(+), 34 deletions(-)
>
> I forgot to send this one out...it applies on top of the previous 7 patches.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 62848be..9ad80c5 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum 
> shader_time_shader_type type,
>  }
>
>  void
> -fs_visitor::fail(const char *format, ...)
> +fs_visitor::vfail(const char *format, va_list va)
>  {
> -   va_list va;
> char *msg;
>
> if (failed)
> @@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
>
> failed = true;
>
> -   va_start(va, format);
> msg = ralloc_vasprintf(mem_ctx, format, va);
> -   va_end(va);
> msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
>
> this->fail_msg = msg;
> @@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
> }
>  }
>
> +void
> +fs_visitor::fail(const char *format, ...)
> +{
> +   va_list va;
> +
> +   va_start(va, format);
> +   vfail(format, va);
> +   va_end(va);
> +}
> +
> +/**
> + * Mark this program as impossible to compile in SIMD16 mode.
> + *
> + * During the SIMD8 compile (which happens first), we can detect and flag
> + * things that are unsupported in SIMD16 mode, so the compiler can skip
> + * the SIMD16 compile altogether.
> + *
> + * During a SIMD16 compile (if one happens anyway), this just calls fail().
> + */
> +void
> +fs_visitor::no16(const char *format, ...)
> +{
> +   va_list va;
> +
> +   va_start(va, format);
> +
> +   if (dispatch_width == 16) {
> +  vfail(format, va);
> +  return;
> +   }
> +
> +   simd16_unsupported = true;
> +
> +   if (INTEL_DEBUG & DEBUG_PERF) {
> +  if (no16_msg)
> + ralloc_vasprintf_append(&no16_msg, format, va);
> +  else
> + no16_msg = ralloc_vasprintf(mem_ctx, format, va);
> +   }
> +
> +   va_end(va);
> +}
> +
>  fs_inst *
>  fs_visitor::emit(enum opcode opcode)
>  {
> @@ -1356,8 +1396,8 @@ fs_visitor::emit_math(enum opcode opcode, fs_reg dst, 
> fs_reg src0, fs_reg src1)
> switch (opcode) {
> case SHADER_OPCODE_INT_QUOTIENT:
> case SHADER_OPCODE_INT_REMAINDER:
> -  if (brw->gen >= 7 && dispatch_width == 16)
> -fail("SIMD16 INTDIV unsupported\n");
> +  if (brw->gen >= 7)
> +no16("SIMD16 INTDIV unsupported\n");
>break;
> case SHADER_OPCODE_POW:
>break;
> @@ -3504,13 +3544,18 @@ brw_wm_fs_emit(struct brw_context *brw, struct 
> brw_wm_compile *c,
> exec_list *simd16_instructions = NULL;
> fs_visitor v2(brw, c, prog, fp, 16);
> if (brw->gen >= 5 && likely(!(INTEL_DEBUG & DEBUG_NO16))) {
> -  /* Try a SIMD16 compile */
> -  v2.import_uniforms(&v);
> -  if (!v2.run()) {
> - perf_debug("SIMD16 shader failed to compile, falling back to "
> -"SIMD8 at a 10-20%% performance cost: %s", v2.fail_msg);
> +  if (!v.simd16_unsupported) {
> + /* Try a SIMD16 compile */
> + v2.import_uniforms(&v);
> + if (!v2.run()) {
> +perf_debug("SIMD16 shader failed to compile, falling back to "
> +   "SIMD8 at a 10-20%% performance cost: %s", 
> v2.fail_msg);
> + } else {
> +simd16_instructions = &v2.instructions;
> + }
>} else {
> - simd16_instructions = &v2.instructions;
> + perf_debug("SIMD16 shader unsupported, falling back to "
> +"SIMD8 at a 10-20%% performance cost: %s", v.no16_msg);
>}
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
> b/src/mesa/drivers/dri/i965/brw

Re: [Mesa-dev] [PATCH 8/7] i965: Accurately bail on SIMD16 compiles.

[Ilia Mirkin]

On Wed, Mar 12, 2014 at 4:28 AM, Kenneth Graunke  wrote:
> Ideally, we'd like to never even attempt the SIMD16 compile if we could
> know ahead of time that it won't succeed---it's purely a waste of time.
> This is especially important for state-based recompiles, which happen at
> draw time.
>
> The fragment shader compiler has a number of checks like:
>
>if (dispatch_width == 16)
>   fail("...some reason...");
>
> This patch introduces a new no16() function which replaces the above
> pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
> Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
> issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
> SIMD16 mode, no16() calls fail(), for safety's sake.)
>
> The great part is that this is not a heuristic---if the flag is set, we
> know with 100% certainty that the SIMD16 compile would fail.  (It might
> fail anyway if we run out of registers, but it's always worth trying.)
>
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 69 
> +++-
>  src/mesa/drivers/dri/i965/brw_fs.h   |  4 ++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +-
>  3 files changed, 83 insertions(+), 34 deletions(-)
>
> I forgot to send this one out...it applies on top of the previous 7 patches.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 62848be..9ad80c5 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum 
> shader_time_shader_type type,
>  }
>
>  void
> -fs_visitor::fail(const char *format, ...)
> +fs_visitor::vfail(const char *format, va_list va)
>  {
> -   va_list va;
> char *msg;
>
> if (failed)
> @@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
>
> failed = true;
>
> -   va_start(va, format);
> msg = ralloc_vasprintf(mem_ctx, format, va);
> -   va_end(va);
> msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
>
> this->fail_msg = msg;
> @@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
> }
>  }
>
> +void
> +fs_visitor::fail(const char *format, ...)
> +{
> +   va_list va;
> +
> +   va_start(va, format);
> +   vfail(format, va);
> +   va_end(va);
> +}
> +
> +/**
> + * Mark this program as impossible to compile in SIMD16 mode.
> + *
> + * During the SIMD8 compile (which happens first), we can detect and flag
> + * things that are unsupported in SIMD16 mode, so the compiler can skip
> + * the SIMD16 compile altogether.
> + *
> + * During a SIMD16 compile (if one happens anyway), this just calls fail().
> + */
> +void
> +fs_visitor::no16(const char *format, ...)
> +{
> +   va_list va;
> +
> +   va_start(va, format);
> +
> +   if (dispatch_width == 16) {
> +  vfail(format, va);

I think there's a va_end() missing in this path. Not sure what the
end-effect of that is, but I'm pretty sure that the recommendation is
to always end the list before returning.

> +  return;
> +   }
> +
> +   simd16_unsupported = true;
> +
> +   if (INTEL_DEBUG & DEBUG_PERF) {
> +  if (no16_msg)
> + ralloc_vasprintf_append(&no16_msg, format, va);
> +  else
> + no16_msg = ralloc_vasprintf(mem_ctx, format, va);
> +   }
> +
> +   va_end(va);
> +}
> +
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/7] i965/fs: Don't renumber UNIFORM registers.

[Pohjolainen, Topi]

On Tue, Mar 11, 2014 at 11:48:54PM -0700, Kenneth Graunke wrote:
> Previously, remove_dead_constants() would renumber the UNIFORM registers
> to be sequential starting from zero, and the resulting register number
> would be used directly as an index into the params[] array.
> 
> This renumbering made it difficult to collect and save information about
> pull constant locations, since setup_pull_constants() and
> move_uniform_array_access_to_pull_constants() used different names.
> 
> This patch generalizes setup_pull_constants() to decide whether each
> uniform register should be a pull constant, push constant, or neither
> (because it's unused).  Then, it stores mappings from UNIFORM register
> numbers to params[] or pull_params[] indices in the push_constant_loc
> and pull_constant_loc arrays.  (We already did this for pull constants.)
> 
> Then, assign_curb_setup() just needs to consult the push_constant_loc
> array to get the real index into the params[] array.
> 
> This effectively folds all the remove_dead_constants() functionality
> into assign_constant_locations(), while being less irritable to work
> with.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 187 
> +++
>  src/mesa/drivers/dri/i965/brw_fs.h   |  13 +-
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   3 +-
>  3 files changed, 85 insertions(+), 118 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 3c8237a..5e01e78 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -885,8 +885,8 @@ fs_visitor::import_uniforms(fs_visitor *v)
> hash_table_call_foreach(v->variable_ht,
>  import_uniforms_callback,
>  variable_ht);
> -   this->params_remap = v->params_remap;
> -   this->nr_params_remap = v->nr_params_remap;
> +   this->push_constant_loc = v->push_constant_loc;
> +   this->uniforms = v->uniforms;
>  }
>  
>  /* Our support for uniforms is piggy-backed on the struct
> @@ -1397,11 +1397,8 @@ fs_visitor::assign_curb_setup()
>  {
> if (dispatch_width == 8) {
>c->prog_data.first_curbe_grf = c->nr_payload_regs;
> -  stage_prog_data->nr_params = uniforms;
> } else {
>c->prog_data.first_curbe_grf_16 = c->nr_payload_regs;
> -  /* Make sure we didn't try to sneak in an extra uniform */
> -  assert(uniforms == 0);
> }
>  
> c->prog_data.curb_read_length = ALIGN(stage_prog_data->nr_params, 8) / 8;
> @@ -1412,7 +1409,19 @@ fs_visitor::assign_curb_setup()
>  
>for (unsigned int i = 0; i < 3; i++) {
>if (inst->src[i].file == UNIFORM) {
> - int constant_nr = inst->src[i].reg + inst->src[i].reg_offset;
> + int uniform_nr = inst->src[i].reg + inst->src[i].reg_offset;
> +int constant_nr;
> +if (uniform_nr >= 0 && uniform_nr < (int) uniforms) {
> +   constant_nr = push_constant_loc[uniform_nr];
> +} else {
> +   /* Section 5.11 of the OpenGL 4.1 spec says:
> +* "Out-of-bounds reads return undefined values, which include
> +*  values from other variables of the active program or 
> zero."
> +* Just return the first push constant.
> +*/
> +   constant_nr = 0;
> +}
> +
>   struct brw_reg brw_reg = brw_vec1_grf(c->nr_payload_regs +
> constant_nr / 8,
> constant_nr % 8);
> @@ -1724,94 +1733,6 @@ fs_visitor::compact_virtual_grfs()
> }
>  }
>  
> -bool
> -fs_visitor::remove_dead_constants()
> -{
> -   if (dispatch_width == 8) {
> -  this->params_remap = ralloc_array(mem_ctx, int, uniforms);
> -  this->nr_params_remap = uniforms;
> -
> -  for (unsigned int i = 0; i < uniforms; i++)
> -  this->params_remap[i] = -1;
> -
> -  /* Find which params are still in use. */
> -  foreach_list(node, &this->instructions) {
> -  fs_inst *inst = (fs_inst *)node;
> -
> -  for (int i = 0; i < 3; i++) {
> - int constant_nr = inst->src[i].reg + inst->src[i].reg_offset;
> -
> - if (inst->src[i].file != UNIFORM)
> -continue;
> -
> - /* Section 5.11 of the OpenGL 4.3 spec says:
> -  *
> -  * "Out-of-bounds reads return undefined values, which include
> -  * values from other variables of the active program or zero."
> -  */
> - if (constant_nr < 0 || constant_nr >= (int)uniforms) {
> -constant_nr = 0;
> - }
> -
> - /* For now, set this to non-negative.  We'll give it the
> -  * actual new number in a moment, in order to keep the
> -  * register numbers nicely ordered.
> -  */
> - this->params_remap[constant_nr] = 0;
> -  }
> -  }
> -
> -  /

Re: [Mesa-dev] Wrong colors in 3D apps on big-endian systems

[Richard Sandiford]

Michel Dänzer  writes:
> On Die, 2014-03-11 at 11:08 +, Richard Sandiford wrote:
>> Michel Dänzer  writes:
>> > On Mon, 2014-03-10 at 10:11 +0100, Christian Zigotzky wrote:
>> >> 
>> >> The Fedora guys have solved the problem with the patch 
>> >> "mesa-9.2-llvmpipe-on-big-endian.patch". What do you think of this patch?
>> >
>> > It sounds like an older downstream attempt at fixing llvmpipe on big
>> > endian hosts, which was superseded by the upstream fixes, which were
>> > incidentally pushed by the 'Fedora guy' Adam Jackson. Adding him and
>> > Richard Sandiford (the author of the fixes) to CC, maybe they can help
>> > you fix up r600g.
>> 
>> I don't know the r600 code at all, sorry.
>
> Fair enough, but still I'm starting to regret that I didn't ask you guys
> to at least try working with others to get the hardware drivers fixed up
> for the llvmpipe big endian fixes before pushing them...
>
> Also, I was hoping there would be followup work extending the same
> approach for the remaining formats. Do you have any plans for that? The
> current half-finished situation is quite messy and confusing for people
> working on format related code.

I started down that route with the patch attached to:

   http://lists.freedesktop.org/archives/mesa-dev/2013-June/040594.html

It depended on those 8 patches being applied first, but although the
series got positive feedback, it didn't really go anywhere beyond that.
I suppose I should have pushed for someone to apply it.

I'm happy to retest and repost the series if someone would be willing
to put it in.  I can then go through the other patches I have locally
that were held up on those.  But (obviously) I'm not anything like a
frequent-enough contributor for commit access to make sense.

Thanks,
Richard

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

Re: [Mesa-dev] [PATCH 5/7] i965/fs: Don't renumber UNIFORM registers.

[Pohjolainen, Topi]

On Tue, Mar 11, 2014 at 11:48:54PM -0700, Kenneth Graunke wrote:
> Previously, remove_dead_constants() would renumber the UNIFORM registers
> to be sequential starting from zero, and the resulting register number
> would be used directly as an index into the params[] array.
> 
> This renumbering made it difficult to collect and save information about
> pull constant locations, since setup_pull_constants() and
> move_uniform_array_access_to_pull_constants() used different names.
> 
> This patch generalizes setup_pull_constants() to decide whether each
> uniform register should be a pull constant, push constant, or neither
> (because it's unused).  Then, it stores mappings from UNIFORM register
> numbers to params[] or pull_params[] indices in the push_constant_loc
> and pull_constant_loc arrays.  (We already did this for pull constants.)
> 
> Then, assign_curb_setup() just needs to consult the push_constant_loc
> array to get the real index into the params[] array.
> 
> This effectively folds all the remove_dead_constants() functionality
> into assign_constant_locations(), while being less irritable to work
> with.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 187 
> +++
>  src/mesa/drivers/dri/i965/brw_fs.h   |  13 +-
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   3 +-
>  3 files changed, 85 insertions(+), 118 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 3c8237a..5e01e78 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -885,8 +885,8 @@ fs_visitor::import_uniforms(fs_visitor *v)
> hash_table_call_foreach(v->variable_ht,
>  import_uniforms_callback,
>  variable_ht);
> -   this->params_remap = v->params_remap;
> -   this->nr_params_remap = v->nr_params_remap;
> +   this->push_constant_loc = v->push_constant_loc;
> +   this->uniforms = v->uniforms;
>  }
>  
>  /* Our support for uniforms is piggy-backed on the struct
> @@ -1397,11 +1397,8 @@ fs_visitor::assign_curb_setup()
>  {
> if (dispatch_width == 8) {
>c->prog_data.first_curbe_grf = c->nr_payload_regs;
> -  stage_prog_data->nr_params = uniforms;
> } else {
>c->prog_data.first_curbe_grf_16 = c->nr_payload_regs;
> -  /* Make sure we didn't try to sneak in an extra uniform */
> -  assert(uniforms == 0);
> }
>  
> c->prog_data.curb_read_length = ALIGN(stage_prog_data->nr_params, 8) / 8;
> @@ -1412,7 +1409,19 @@ fs_visitor::assign_curb_setup()
>  
>for (unsigned int i = 0; i < 3; i++) {
>if (inst->src[i].file == UNIFORM) {
> - int constant_nr = inst->src[i].reg + inst->src[i].reg_offset;
> + int uniform_nr = inst->src[i].reg + inst->src[i].reg_offset;
> +int constant_nr;
> +if (uniform_nr >= 0 && uniform_nr < (int) uniforms) {
> +   constant_nr = push_constant_loc[uniform_nr];
> +} else {
> +   /* Section 5.11 of the OpenGL 4.1 spec says:
> +* "Out-of-bounds reads return undefined values, which include
> +*  values from other variables of the active program or 
> zero."
> +* Just return the first push constant.
> +*/
> +   constant_nr = 0;
> +}
> +
>   struct brw_reg brw_reg = brw_vec1_grf(c->nr_payload_regs +
> constant_nr / 8,
> constant_nr % 8);
> @@ -1724,94 +1733,6 @@ fs_visitor::compact_virtual_grfs()
> }
>  }
>  
> -bool
> -fs_visitor::remove_dead_constants()
> -{
> -   if (dispatch_width == 8) {
> -  this->params_remap = ralloc_array(mem_ctx, int, uniforms);
> -  this->nr_params_remap = uniforms;
> -
> -  for (unsigned int i = 0; i < uniforms; i++)
> -  this->params_remap[i] = -1;
> -
> -  /* Find which params are still in use. */
> -  foreach_list(node, &this->instructions) {
> -  fs_inst *inst = (fs_inst *)node;
> -
> -  for (int i = 0; i < 3; i++) {
> - int constant_nr = inst->src[i].reg + inst->src[i].reg_offset;
> -
> - if (inst->src[i].file != UNIFORM)
> -continue;
> -
> - /* Section 5.11 of the OpenGL 4.3 spec says:
> -  *
> -  * "Out-of-bounds reads return undefined values, which include
> -  * values from other variables of the active program or zero."
> -  */
> - if (constant_nr < 0 || constant_nr >= (int)uniforms) {
> -constant_nr = 0;
> - }
> -
> - /* For now, set this to non-negative.  We'll give it the
> -  * actual new number in a moment, in order to keep the
> -  * register numbers nicely ordered.
> -  */
> - this->params_remap[constant_nr] = 0;
> -  }
> -  }
> -
> -  /

[Mesa-dev] [PATCH 1/6] mesa: Add GL_INTEL_performance_query definitions

[Petri Latvala]

Until Khronos adds the definitions to glext.h, add definitions for this
extension to gl.h.

Signed-off-by: Petri Latvala 
---
 include/GL/gl.h | 45 +
 1 file changed, 45 insertions(+)

diff --git a/include/GL/gl.h b/include/GL/gl.h
index 4e2932d..e2caf73 100644
--- a/include/GL/gl.h
+++ b/include/GL/gl.h
@@ -2104,6 +2104,51 @@ typedef void (APIENTRYP 
PFNGLEGLIMAGETARGETTEXTURE2DOESPROC) (GLenum target, GLe
 typedef void (APIENTRYP PFNGLEGLIMAGETARGETRENDERBUFFERSTORAGEOESPROC) (GLenum 
target, GLeglImageOES image);
 #endif
 
+#ifndef GL_INTEL_performance_query
+#define GL_INTEL_performance_query 1
+#define GL_PERFQUERY_SINGLE_CONTEXT_INTEL 0x
+#define GL_PERFQUERY_GLOBAL_CONTEXT_INTEL 0x0001
+#define GL_PERFQUERY_WAIT_INTEL   0x83FB
+#define GL_PERFQUERY_FLUSH_INTEL  0x83FA
+#define GL_PERFQUERY_DONOT_FLUSH_INTEL0x83F9
+#define GL_PERFQUERY_COUNTER_EVENT_INTEL  0x94F0
+#define GL_PERFQUERY_COUNTER_DURATION_NORM_INTEL 0x94F1
+#define GL_PERFQUERY_COUNTER_DURATION_RAW_INTEL 0x94F2
+#define GL_PERFQUERY_COUNTER_THROUGHPUT_INTEL 0x94F3
+#define GL_PERFQUERY_COUNTER_RAW_INTEL0x94F4
+#define GL_PERFQUERY_COUNTER_TIMESTAMP_INTEL 0x94F5
+#define GL_PERFQUERY_COUNTER_DATA_UINT32_INTEL 0x94F8
+#define GL_PERFQUERY_COUNTER_DATA_UINT64_INTEL 0x94F9
+#define GL_PERFQUERY_COUNTER_DATA_FLOAT_INTEL 0x94FA
+#define GL_PERFQUERY_COUNTER_DATA_DOUBLE_INTEL 0x94FB
+#define GL_PERFQUERY_COUNTER_DATA_BOOL32_INTEL 0x94FC
+#define GL_PERFQUERY_QUERY_NAME_LENGTH_MAX_INTEL 0x94FD
+#define GL_PERFQUERY_COUNTER_NAME_LENGTH_MAX_INTEL 0x94FE
+#define GL_PERFQUERY_COUNTER_DESC_LENGTH_MAX_INTEL 0x94FF
+#define GL_PERFQUERY_GPA_EXTENDED_COUNTERS_INTEL 0x9500
+typedef void (GLAPIENTRYP PFNGLBEGINPERFQUERYINTELPROC) (GLuint queryHandle);
+typedef void (GLAPIENTRYP PFNGLCREATEPERFQUERYINTELPROC) (GLuint queryId, 
GLuint *queryHandle);
+typedef void (GLAPIENTRYP PFNGLDELETEPERFQUERYINTELPROC) (GLuint queryHandle);
+typedef void (GLAPIENTRYP PFNGLENDPERFQUERYINTELPROC) (GLuint queryHandle);
+typedef void (GLAPIENTRYP PFNGLGETFIRSTPERFQUERYIDINTELPROC) (GLuint *queryId);
+typedef void (GLAPIENTRYP PFNGLGETNEXTPERFQUERYIDINTELPROC) (GLuint queryId, 
GLuint *nextQueryId);
+typedef void (GLAPIENTRYP PFNGLGETPERFCOUNTERINFOINTELPROC) (GLuint queryId, 
GLuint counterId, GLuint counterNameLength, GLchar *counterName, GLuint 
counterDescLength, GLchar *counterDesc, GLuint *counterOffset, GLuint 
*counterDataSize, GLuint *counterTypeEnum, GLuint *counterDataTypeEnum, 
GLuint64 *rawCounterMaxValue);
+typedef void (GLAPIENTRYP PFNGLGETPERFQUERYDATAINTELPROC) (GLuint queryHandle, 
GLuint flags, GLsizei dataSize, GLvoid *data, GLuint *bytesWritten);
+typedef void (GLAPIENTRYP PFNGLGETPERFQUERYIDBYNAMEINTELPROC) (GLchar 
*queryName, GLuint *queryId);
+typedef void (GLAPIENTRYP PFNGLGETPERFQUERYINFOINTELPROC) (GLuint queryId, 
GLuint queryNameLength, GLchar *queryName, GLuint *dataSize, GLuint 
*noCounters, GLuint *noInstances, GLuint *capsMask);
+#ifdef GL_GLEXT_PROTOTYPES
+GLAPI void GLAPIENTRY glBeginPerfQueryINTEL (GLuint queryHandle);
+GLAPI void GLAPIENTRY glCreatePerfQueryINTEL (GLuint queryId, GLuint 
*queryHandle);
+GLAPI void GLAPIENTRY glDeletePerfQueryINTEL (GLuint queryHandle);
+GLAPI void GLAPIENTRY glEndPerfQueryINTEL (GLuint queryHandle);
+GLAPI void GLAPIENTRY glGetFirstPerfQueryIdINTEL (GLuint *queryId);
+GLAPI void GLAPIENTRY glGetNextPerfQueryIdINTEL (GLuint queryId, GLuint 
*nextQueryId);
+GLAPI void GLAPIENTRY glGetPerfCounterInfoINTEL (GLuint queryId, GLuint 
counterId, GLuint counterNameLength, GLchar *counterName, GLuint 
counterDescLength, GLchar *counterDesc, GLuint *counterOffset, GLuint 
*counterDataSize, GLuint *counterTypeEnum, GLuint *counterDataTypeEnum, 
GLuint64 *rawCounterMaxValue);
+GLAPI void GLAPIENTRY glGetPerfQueryDataINTEL (GLuint queryHandle, GLuint 
flags, GLsizei dataSize, GLvoid *data, GLuint *bytesWritten);
+GLAPI void GLAPIENTRY glGetPerfQueryIdByNameINTEL (GLchar *queryName, GLuint 
*queryId);
+GLAPI void GLAPIENTRY glGetPerfQueryInfoINTEL (GLuint queryId, GLuint 
queryNameLength, GLchar *queryName, GLuint *dataSize, GLuint *noCounters, 
GLuint *noInstances, GLuint *capsMask);
+#endif
+#endif /* GL_INTEL_performance_query */
 
 /**
  ** NOTE!  If you add new functions to this file, or update
-- 
1.9.0

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

[Mesa-dev] [PATCH 6/6] i965: Enable INTEL_performance_query for Gen5+.

[Petri Latvala]

Signed-off-by: Petri Latvala 
---
 src/mesa/drivers/dri/i965/intel_extensions.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 5094c2b..78bf5b4 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -307,8 +307,10 @@ intelInitExtensions(struct gl_context *ctx)
   ctx->Extensions.ARB_stencil_texturing = true;
}
 
-   if (brw->gen == 5 || can_write_oacontrol(brw))
+   if (brw->gen == 5 || can_write_oacontrol(brw)) {
   ctx->Extensions.AMD_performance_monitor = true;
+  ctx->Extensions.INTEL_performance_query = true;
+   }
 
if (ctx->API == API_OPENGL_CORE)
   ctx->Extensions.ARB_base_instance = true;
-- 
1.9.0

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

[Mesa-dev] [PATCH 0/6] Implement INTEL_performance_query

[Petri Latvala]

This patch series implements the INTEL_performance_query extension.
With driver code for AMD_performance_monitor already in place,
implementing this extension was fairly straightforward.

Planned future improvements: Proper semantic types for counters and
data normalization (currently all counters are exposed as "RAW"
without per-second maximum). Pseudo-counters for things like "time
spent in shader compilation" or "forced recompilation event count".

Piglit tests for this extension have been sent to piglit mailing
list. All tests pass.

Petri Latvala (6):
  mesa: Add GL_INTEL_performance_query definitions
  Regenerate gl_mangle.h.
  mesa: Add INTEL_performance_query enums to tests/enum_strings.cpp
  mesa: Add core support for the GL_INTEL_performance_query extension.
  mesa: Implement INTEL_performance_query.
  i965: Enable INTEL_performance_query for Gen5+.

 include/GL/gl.h|  45 ++
 include/GL/gl_mangle.h | 371 +++-
 src/mapi/glapi/gen/INTEL_performance_query.xml |  93 
 src/mapi/glapi/gen/Makefile.am |   1 +
 src/mapi/glapi/gen/gl_API.xml  |   2 +
 src/mesa/drivers/dri/i965/intel_extensions.c   |   4 +-
 src/mesa/main/config.h |   8 +
 src/mesa/main/extensions.c |   1 +
 src/mesa/main/get.c|   1 +
 src/mesa/main/get_hash_params.py   |   6 +
 src/mesa/main/mtypes.h |   1 +
 src/mesa/main/performance_monitor.c| 585 +
 src/mesa/main/performance_monitor.h|  43 +-
 src/mesa/main/tests/dispatch_sanity.cpp|  12 +
 src/mesa/main/tests/enum_strings.cpp   |  18 +
 15 files changed, 1183 insertions(+), 8 deletions(-)
 create mode 100644 src/mapi/glapi/gen/INTEL_performance_query.xml

-- 
1.9.0

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

[Mesa-dev] [PATCH 4/6] mesa: Add core support for the GL_INTEL_performance_query extension.

[Petri Latvala]

Like AMD_performance_monitor, this extension provides an interface for
applications (and OpenGL-based tools) to access GPU performance
counters. Since the exact performance counters available vary between
vendors and hardware generations, the extension provides an API the
application can use to get the names, types, and minimum/maximum
values of all available counters.

Applications create performance queries based on available query
types, and begin/end measurement collection. Multiple queries can be
measuring simultaneously.

Signed-off-by: Petri Latvala 
---
 src/mapi/glapi/gen/INTEL_performance_query.xml |  93 +
 src/mapi/glapi/gen/Makefile.am |   1 +
 src/mapi/glapi/gen/gl_API.xml  |   2 +
 src/mesa/main/config.h |   8 ++
 src/mesa/main/extensions.c |   1 +
 src/mesa/main/get.c|   1 +
 src/mesa/main/get_hash_params.py   |   6 +
 src/mesa/main/mtypes.h |   1 +
 src/mesa/main/performance_monitor.c| 183 +
 src/mesa/main/performance_monitor.h|  43 +-
 src/mesa/main/tests/dispatch_sanity.cpp|  12 ++
 11 files changed, 350 insertions(+), 1 deletion(-)
 create mode 100644 src/mapi/glapi/gen/INTEL_performance_query.xml

diff --git a/src/mapi/glapi/gen/INTEL_performance_query.xml 
b/src/mapi/glapi/gen/INTEL_performance_query.xml
new file mode 100644
index 000..333856f
--- /dev/null
+++ b/src/mapi/glapi/gen/INTEL_performance_query.xml
@@ -0,0 +1,93 @@
+
+
+
+
+
+
+
+  
+
+  
+
+  
+
+
+  
+
+  
+
+
+  
+
+  
+
+
+
+
+
+
+
+  
+
+  
+
+
+
+
+
+
+
+
+
+
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+
+
+
+
+  
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+
+
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 7b3c118..1c5b61c 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -147,6 +147,7 @@ API_XML = \
EXT_texture_array.xml \
EXT_texture_integer.xml \
EXT_transform_feedback.xml \
+   INTEL_performance_query.xml \
NV_conditional_render.xml \
NV_primitive_restart.xml \
NV_texture_barrier.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index 9129d57..8fbf700 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -12840,6 +12840,8 @@
 
 
 
+http://www.w3.org/2001/XInclude"/>
+
 
 
 
diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h
index 30da5d4..c96502a 100644
--- a/src/mesa/main/config.h
+++ b/src/mesa/main/config.h
@@ -281,6 +281,14 @@
 #define MAX_VERTEX_STREAMS  4
 /*@}*/
 
+/** For GL_INTEL_performance_query */
+/*@{*/
+#define MAX_PERFQUERY_QUERY_NAME_LENGTH 256
+#define MAX_PERFQUERY_COUNTER_NAME_LENGTH   256
+#define MAX_PERFQUERY_COUNTER_DESC_LENGTH   1024
+#define PERFQUERY_HAVE_GPA_EXTENDED_COUNTERS 0
+/*@}*/
+
 /*
  * Color channel component order
  * 
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index a72284c..e8f125d 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -330,6 +330,7 @@ static const struct extension extension_table[] = {
{ "GL_IBM_rasterpos_clip",  o(dummy_true),  
GLL,1996 },
{ "GL_IBM_texture_mirrored_repeat", o(dummy_true),  
GLL,1998 },
{ "GL_INGR_blend_func_separate",o(EXT_blend_func_separate), 
GLL,1999 },
+   { "GL_INTEL_performance_query", o(INTEL_performance_query), 
GL | ES2,   2013 },
{ "GL_MESA_pack_invert",o(MESA_pack_invert),
GL, 2002 },
{ "GL_MESA_texture_signed_rgba",o(EXT_texture_snorm),   
GL, 2009 },
{ "GL_MESA_window_pos", o(dummy_true),  
GLL,2000 },
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index b190851..ba58f02 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -394,6 +394,7 @@ EXTRA_EXT(ARB_viewport_array);
 EXTRA_EXT(ARB_compute_shader);
 EXTRA_EXT(ARB_gpu_shader5);
 EXTRA_EXT2(ARB_transform_feedback3, ARB_gpu_shader5);
+EXTRA_EXT(INTEL_performance_query);
 
 static const int
 extra_ARB_color_buffer_float_or_glcore[] = {
diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py
index 674d003..4c0838e 100644
--- a/src/mesa/main/get_hash_params.py
+++ b/src/mesa/main/get_hash_params.py
@@ -311,6 +311,12 @@ descriptor=[
 # GL_ARB_get_program_binary / GL_OES_get_program_binary
   [ "NUM

[Mesa-dev] [PATCH 3/6] mesa: Add INTEL_performance_query enums to tests/enum_strings.cpp

[Petri Latvala]

Signed-off-by: Petri Latvala 
---
 src/mesa/main/tests/enum_strings.cpp | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/mesa/main/tests/enum_strings.cpp 
b/src/mesa/main/tests/enum_strings.cpp
index 3795700..d16eb36 100644
--- a/src/mesa/main/tests/enum_strings.cpp
+++ b/src/mesa/main/tests/enum_strings.cpp
@@ -807,6 +807,9 @@ const struct enum_info everything[] = {
{ 0x83F1, "GL_COMPRESSED_RGBA_S3TC_DXT1_EXT" },
{ 0x83F2, "GL_COMPRESSED_RGBA_S3TC_DXT3_ANGLE" },
{ 0x83F3, "GL_COMPRESSED_RGBA_S3TC_DXT5_ANGLE" },
+   { 0x83F9, "GL_PERFQUERY_DONOT_FLUSH_INTEL" },
+   { 0x83FA, "GL_PERFQUERY_FLUSH_INTEL" },
+   { 0x83FB, "GL_PERFQUERY_WAIT_INTEL" },
{ 0x844D, "GL_NEAREST_CLIPMAP_NEAREST_SGIX" },
{ 0x844E, "GL_NEAREST_CLIPMAP_LINEAR_SGIX" },
{ 0x844F, "GL_LINEAR_CLIPMAP_NEAREST_SGIX" },
@@ -1843,6 +1846,21 @@ const struct enum_info everything[] = {
{ 0x9271, "GL_COMPRESSED_SIGNED_R11_EAC" },
{ 0x9272, "GL_COMPRESSED_RG11_EAC" },
{ 0x9273, "GL_COMPRESSED_SIGNED_RG11_EAC" },
+   { 0x94F0, "GL_PERFQUERY_COUNTER_EVENT_INTEL" },
+   { 0x94F1, "GL_PERFQUERY_COUNTER_DURATION_NORM_INTEL" },
+   { 0x94F2, "GL_PERFQUERY_COUNTER_DURATION_RAW_INTEL" },
+   { 0x94F3, "GL_PERFQUERY_COUNTER_THROUGHPUT_INTEL" },
+   { 0x94F4, "GL_PERFQUERY_COUNTER_RAW_INTEL" },
+   { 0x94F5, "GL_PERFQUERY_COUNTER_TIMESTAMP_INTEL" },
+   { 0x94F8, "GL_PERFQUERY_COUNTER_DATA_UINT32_INTEL" },
+   { 0x94F9, "GL_PERFQUERY_COUNTER_DATA_UINT64_INTEL" },
+   { 0x94FA, "GL_PERFQUERY_COUNTER_DATA_FLOAT_INTEL" },
+   { 0x94FB, "GL_PERFQUERY_COUNTER_DATA_DOUBLE_INTEL" },
+   { 0x94FC, "GL_PERFQUERY_COUNTER_DATA_BOOL32_INTEL" },
+   { 0x94FD, "GL_PERFQUERY_QUERY_NAME_LENGTH_MAX_INTEL" },
+   { 0x94FE, "GL_PERFQUERY_COUNTER_NAME_LENGTH_MAX_INTEL" },
+   { 0x94FF, "GL_PERFQUERY_COUNTER_DESC_LENGTH_MAX_INTEL" },
+   { 0x9500, "GL_PERFQUERY_GPA_EXTENDED_COUNTERS_INTEL" },
{ 0x19262, "GL_RASTER_POSITION_UNCLIPPED_IBM" },
{ 0, NULL }
 };
-- 
1.9.0

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

[Mesa-dev] [PATCH 5/6] mesa: Implement INTEL_performance_query.

[Petri Latvala]

Using the existing driver hooks made for AMD_performance_monitor, implement
INTEL_performance_query functions.

Signed-off-by: Petri Latvala 
---
 src/mesa/main/performance_monitor.c | 476 +---
 1 file changed, 439 insertions(+), 37 deletions(-)

diff --git a/src/mesa/main/performance_monitor.c 
b/src/mesa/main/performance_monitor.c
index 183a895..bf58d45 100644
--- a/src/mesa/main/performance_monitor.c
+++ b/src/mesa/main/performance_monitor.c
@@ -137,6 +137,46 @@ get_counter(const struct gl_perf_monitor_group *group_obj, 
GLuint id)
return &group_obj->Counters[id];
 }
 
+/* For INTEL_performance_query, query id 0 is reserved to be invalid. We use
+ * index to Groups array + 1 as the query id. Same applies to counter id.
+ */
+static inline GLuint
+queryid_to_index(GLuint queryid)
+{
+   return queryid - 1;
+}
+
+static inline GLuint
+index_to_queryid(GLuint index)
+{
+   return index + 1;
+}
+
+static inline bool
+queryid_valid(const struct gl_context *ctx, GLuint queryid)
+{
+   return get_group(ctx, queryid_to_index(queryid)) != NULL;
+}
+
+static inline GLuint
+counterid_to_index(GLuint counterid)
+{
+   return counterid - 1;
+}
+
+static inline GLuint
+index_to_counterid(GLuint index)
+{
+   return index + 1;
+}
+
+static inline bool
+counterid_valid(const struct gl_perf_monitor_group *group_obj,
+GLuint counterid)
+{
+   return get_counter(group_obj, counterid_to_index(counterid)) != NULL;
+}
+
 /*/
 
 void GLAPIENTRY
@@ -645,19 +685,29 @@ _mesa_GetFirstPerfQueryIdINTEL(GLuint *queryId)
 {
GET_CURRENT_CONTEXT(ctx);
 
+   unsigned numGroups;
+
/* "If queryId pointer is equal to 0, INVALID_VALUE error is generated."
 */
if (!queryId) {
-  _mesa_error(ctx, GL_INVALID_VALUE, "glGetFirstPerfQueryIdINTEL(queryId 
== NULL)");
+  _mesa_error(ctx, GL_INVALID_VALUE,
+  "glGetFirstPerfQueryIdINTEL(queryId == NULL)");
   return;
}
 
+   numGroups = ctx->PerfMonitor.NumGroups;
+
/* "If the given hardware platform doesn't support any performance queries,
 * then the value of 0 is returned and INVALID_OPERATION error is raised."
 */
+   if (numGroups == 0) {
+  *queryId = 0;
+  _mesa_error(ctx, GL_INVALID_OPERATION,
+  "glGetFirstPerfQueryIdINTEL(no queries supported)");
+  return;
+   }
 
-   *queryId = 0;
-   _mesa_error(ctx, GL_INVALID_OPERATION, "glGetFirstPerfQueryIdINTEL(no 
queries supported)");
+   *queryId = index_to_queryid(0);
 }
 
 extern void GLAPIENTRY
@@ -667,22 +717,34 @@ _mesa_GetNextPerfQueryIdINTEL(GLuint queryId, GLuint 
*nextQueryId)
 
/* "If nextQueryId pointer is equal to 0, an INVALID_VALUE error is
 * generated."
-*
-* "Whenever error is generated, the value of 0 is returned."
 */
if (!nextQueryId) {
-  *nextQueryId = 0;
-  _mesa_error(ctx, GL_INVALID_VALUE, 
"glGetNextPerfQueryIdINTEL(nextQueryId == NULL)");
+  _mesa_error(ctx, GL_INVALID_VALUE,
+  "glGetNextPerfQueryIdINTEL(nextQueryId == NULL)");
   return;
}
 
/* "If the specified performance
 * query identifier is invalid then INVALID_VALUE error is generated."
-*
-* No queries are supported, so all queries are invalid.
 */
-   *nextQueryId = 0;
-   _mesa_error(ctx, GL_INVALID_VALUE, "glGetNextPerfQueryIdINTEL(invalid 
query)");
+   if (!queryid_valid(ctx, queryId)) {
+  /* "Whenever error is generated, the value of 0 is returned." */
+  *nextQueryId = 0;
+  _mesa_error(ctx, GL_INVALID_VALUE,
+  "glGetNextPerfQueryIdINTEL(invalid query)");
+  return;
+   }
+
+   ++queryId;
+
+   /* "If query identified by queryId is the last query available the value of
+* 0 is returned."
+*/
+   if (!queryid_valid(ctx, queryId)) {
+  *nextQueryId = 0;
+   } else {
+  *nextQueryId = queryId;
+   }
 }
 
 extern void GLAPIENTRY
@@ -690,13 +752,36 @@ _mesa_GetPerfQueryIdByNameINTEL(char *queryName, GLuint 
*queryId)
 {
GET_CURRENT_CONTEXT(ctx);
 
+   unsigned i;
+
+   /* "If queryName does not reference a valid query name, an INVALID_VALUE
+* error is generated."
+*/
+   if (!queryName) {
+  _mesa_error(ctx, GL_INVALID_VALUE,
+  "glGetPerfQueryIdByNameINTEL(queryName == NULL)");
+  return;
+   }
+
+   /* The specification does not state that this produces an error. */
+   if (!queryId) {
+  return;
+   }
+
+   for (i = 0; i < ctx->PerfMonitor.NumGroups; ++i) {
+  const struct gl_perf_monitor_group *group_obj = get_group(ctx, i);
+  if (strcmp(group_obj->Name, queryName) == 0) {
+ *queryId = index_to_queryid(i);
+ return;
+  }
+   }
+
/* "If queryName does not reference a valid query name, an INVALID_VALUE
 * error is generated."
-*
-* No queries are supported, so all query names are invalid.
 */
 
-   _mesa_error(ctx, GL_INVALID_V

Re: [Mesa-dev] [PATCH 6/6] i965: Enable INTEL_performance_query for Gen5+.

[Dragomir Ivanov]

What about other drivers supporting `AMD_performance_monitor`? Are there any?

On Wed, Mar 12, 2014 at 2:54 PM, Petri Latvala  wrote:
> Signed-off-by: Petri Latvala 
> ---
>  src/mesa/drivers/dri/i965/intel_extensions.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 5094c2b..78bf5b4 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -307,8 +307,10 @@ intelInitExtensions(struct gl_context *ctx)
>ctx->Extensions.ARB_stencil_texturing = true;
> }
>
> -   if (brw->gen == 5 || can_write_oacontrol(brw))
> +   if (brw->gen == 5 || can_write_oacontrol(brw)) {
>ctx->Extensions.AMD_performance_monitor = true;
> +  ctx->Extensions.INTEL_performance_query = true;
> +   }
>
> if (ctx->API == API_OPENGL_CORE)
>ctx->Extensions.ARB_base_instance = true;
> --
> 1.9.0
>
> ___
> 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/6] i965: Enable INTEL_performance_query for Gen5+.

[Petri Latvala]

On 03/12/2014 03:17 PM, Dragomir Ivanov wrote:

What about other drivers supporting `AMD_performance_monitor`? Are there any?


Other drivers don't support it.


--
Petri Latvala

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

Re: [Mesa-dev] static version of osmesa is missing llvm symbols, bug?

[Brian Paul]

On 03/11/2014 06:48 PM, Emil Velikov wrote:

On 12/03/14 00:08, Burlen Loring wrote:

yep, I'm using 10.1.0 and also noticed the same in 9.2.2.


In that case "it wasn't me" ;-)


in short static linking is essential for reasonable performance of
parallel applications on Cray systems. In this scenario the application
is duplicated across 100s/1000s of compute nodes which share a
remote-parallel file system (Lustre/GPFS). As a dynamically linked
application starts, shared library loading hit's the file system hard.
However, Cray suggests users run static linked executables. Their
application launcher bypasses the file system and copies statically
linked executable directly into a ram fs onto the set of compute nodes
that a given job is scheduled to run on. This avoids file system
contention that would be introduced by a parallel application's shared
library loading.

I maintain and support ParaView and interactive OpenGL based rendering
application on NERSC's Crays. Given that this is an interactive
application, having a reasonable startup time is important. Users have
complained and I've observed times when using shared libraries when app
startup takes more than 30 min! if we build fully static executable
startup time is reduced to seconds.

For me statically linked mesa is important, which is why I want to
report this. Without OSMesa we can't use these systems, and the new
llvmpipe OSMesa state tracker has been really awesome, allowing us to do
things we couldn't in the past without GPUs. I hope that you don't
discontinue static link support.


Had some additional patches that cleans up mesa's build a bit further
but I guess I'll bring back (classic and gallium) static osmesa first.
Especially considering the numbers you've quoted.


We have experienced some segfaults in llvmpipe OSMesa in the static
linked version (bt shows SEGV at
create_jit_texture_type.isra.1@draw_llvm.c:128). If I can reproduce this
off the Cray I will file a bug report. Thought I'd mention in case you
can confirm that there is a known issue with the static link.


AFAIK static linking with llvm is a bit of a hit and miss. For that
reason mesa 10.2 may be building against shared llvm. Then again I do
not work on that part of mesa, so take this with a pinch of salt.

Brian Paul is the only person that I've seen working/bug fixing osmesa
so I'm guessing that he'll take a look in due time. Meanwhile it may be
worth filing bug report(s) for these two boogers :-)


Yes, please file bugs with as much info as possible.  I don't think I 
can help with the static build issue (my autocon/make knowledge is 
pretty thin).  I'd be happy to look into the LLVM segfault though (when 
I have time).


-Brian

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

[Mesa-dev] Gallium u_gen_mipmap problem with queries

[Marek Olšák]

Hi everybody,

u_gen_mipmap doesn't disable queries, so all active queries are
incremented, even though mipmap generation shouldn't have any effect
on queries. Hardware drivers can disable queries, but this feature is
not exposed via the gallium interface. Generally, we want to disable
all queries except for the time_elapsed query for u_gen_mipmap. I
propose the following solutions to this problem:

1) Add pipe_context::pause_all_queries(pipe) and
resume_all_queries(pipe). It's self-explanatory.

2) Add pipe_context::pause_query(pipe, query) and resume_query(pipe,
query). This has the disadvantage of having to enumerate all active
queries in u_gen_mipmap.

3) Add pipe_rasterizer_state::disable_all_queries:1, or separate bits
disable_occlusion_queries:1 and disable_streamout_queries:1 (this
includes the primitives_generated query too).

4) Rewrite u_gen_mipmap to use pipe->blit.

Please comment.

Thanks.

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

Re: [Mesa-dev] [PATCH 0/6] Implement INTEL_performance_query

[Petri Latvala]

One concern I forgot to mention: Interoperability with 
AMD_performance_monitor.


With this patch series as is, after glBeginPerfQueryINTEL, there will be 
a value you can pass to glEndPerfMonitorAMD that won't produce 
GL_INVALID_VALUE as it should. Making sure context's monitor objects are 
accessible only through the extension that created them is something 
that needs to be done, but I'm not sure what would be the best way. 
Comments and hits welcome.



--
Petri Latvala

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

Re: [Mesa-dev] [PATCH] Release gl_debug_state when destroying context.

[Mike Stroyan]

Brian,

  Please push that.  I haven't gotten commit access for myself yet.


On Tue, Mar 11, 2014 at 5:42 PM, Brian Paul  wrote:

> On 03/11/2014 05:07 PM, Mike Stroyan wrote:
>
>> Commit 6e8d04a caused a leak by allocating ctx->Debug but never freeing
>> it.
>> Release the memory in _mesa_free_errors_data when destroying a context.
>> Use FREE to match CALLOC_STRUCT from _mesa_get_debug_state.
>> ---
>>   src/mesa/main/errors.c | 5 -
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
>> index 8ec6a8c..9151718 100644
>> --- a/src/mesa/main/errors.c
>> +++ b/src/mesa/main/errors.c
>> @@ -969,7 +969,7 @@ _mesa_init_errors(struct gl_context *ctx)
>>
>>   /**
>>* Loop through debug group stack tearing down states for
>> - * filtering debug messages.
>> + * filtering debug messages.  Then free debug output state.
>>*/
>>   void
>>   _mesa_free_errors_data(struct gl_context *ctx)
>> @@ -980,6 +980,9 @@ _mesa_free_errors_data(struct gl_context *ctx)
>> for (i = 0; i <= ctx->Debug->GroupStackDepth; i++) {
>>free_errors_data(ctx, i);
>> }
>> +  FREE(ctx->Debug);
>> +  /* set to NULL just in case it is used before context is
>> completely gone. */
>> +  ctx->Debug = NULL;
>>  }
>>   }
>>
>>
>>
> Reviewed-by: Brian Paul 
>
> Thanks, Mike!  Do you need me to push this for you?
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



-- 

 Mike Stroyan - Software Architect
 LunarG, Inc.  - The Graphics Experts
 Cell:  (970) 219-7905
 Email: m...@lunarg.com
 Website: http://www.lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [mesa-9.2][llvm-3.4] Requires some fixes from GIT master to build properly

[Sedat Dilek]

Hi,

I tried to build mesa-9.2 GIT branch with llvm-3.4 on Ubuntu/precise
AMD64 and needed the 2 following patches:

[PATCH 1/2] gallivm: Remove llvm::DisablePrettyStackTrace for LLVM >= 3.4.
[PATCH 2/2] gallivm: Remove NoFramePointerElimNonLeaf for LLVM >= 3.4.

Not sure if mesa-9.2.x is still maintained.
If yes, please cherry-pick them.
Thanks in advance.

Regards,
- Sedat -
From aa1d3db0c80b6c11fb9d1c1c5eacd97eb9c4de29 Mon Sep 17 00:00:00 2001
From: Vinson Lee 
Date: Sun, 3 Nov 2013 20:27:13 -0800
Subject: [PATCH 1/2] gallivm: Remove llvm::DisablePrettyStackTrace for LLVM >=
 3.4.

LLVM 3.4 r193971 removed llvm::DisablePrettyStackTrace and made the
pretty stack trace opt-in rather than opt-out.

The default value of DisablePrettyStackTrace has changed to true in LLVM
3.4 and newer.

Signed-off-by: Vinson Lee 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=60929
Reviewed-by: Tom Stellard 
Reviewed-by: Brian Paul 
---
 src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 1e5adb7..2f3020b 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -174,12 +174,14 @@ lp_set_target_options(void)
}
 #endif
 
+#if HAVE_LLVM < 0x0304
/*
 * By default LLVM adds a signal handler to output a pretty stack trace.
 * This signal handler is never removed, causing problems when unloading the
 * shared object where the gallium driver resides.
 */
llvm::DisablePrettyStackTrace = true;
+#endif
 
// If we have a native target, initialize it to ensure it is linked in and
// usable by the JIT.
-- 
1.9.0

From a8b18df4de204d452b1245417ac744b3d765159d Mon Sep 17 00:00:00 2001
From: Vinson Lee 
Date: Wed, 24 Jul 2013 23:28:27 -0700
Subject: [PATCH 2/2] gallivm: Remove NoFramePointerElimNonLeaf for LLVM >=
 3.4.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TargetOptions::NoFramePointerElimNonLeaf was removed in LLVM 3.4
r187093.

Signed-off-by: Vinson Lee 
Reviewed-by: José Fonseca 
---
 src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 2f3020b..5d3e22f 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -283,7 +283,9 @@ lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
 #endif
 
 #if defined(DEBUG) || defined(PROFILE)
+#if HAVE_LLVM < 0x0304
options.NoFramePointerElimNonLeaf = true;
+#endif
options.NoFramePointerElim = true;
 #endif
 
-- 
1.9.0

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

[Mesa-dev] Mesa 10.0.4

[Carl Worth]

Mesa 10.0.4 has been released. Mesa 10.0.4 is a bug fix release which
fixes bugs fixed since the 10.0.3 release, (see below for a list of
changes).

The tag in the git repository for Mesa 10.0.4 is 'mesa-10.0.4'.

Mesa 10.0.4 is available for download at
ftp://freedesktop.org/pub/mesa/10.0.4/

md5sums:
5a3c5b90776ec8a9fcd777c99e0607e2  MesaLib-10.0.4.tar.gz
8b148869d2620b0720c8a8d2b7eb3e38  MesaLib-10.0.4.tar.bz2
da2418d25bfbc273660af7e755fb367e  MesaLib-10.0.4.zip

I have verified building from the .tar.bz2 file by doing:

tar xjf MesaLib-10.0.4.tar.bz2
cd Mesa-10.0.4
./configure --enable-gallium-llvm --with-llvm-shared-libs
make -j6
make install

I have also verified that I pushed the tag.

-Carl

-- 
carl.d.wo...@intel.com

Changes from 10.0.3 to 10.0.4

Anuj Phogat (4):
  mesa: Generate correct error code in glDrawBuffers()
  mesa: Add GL_TEXTURE_CUBE_MAP_ARRAY to 
legal_get_tex_level_parameter_target()
  glsl: Fix condition to generate shader link error
  i965: Fix the region's pitch condition to use blitter

Brian Paul (8):
  r200: move driContextSetFlags(ctx) call after ctx var is initialized
  radeon: move driContextSetFlags(ctx) call after ctx var is initialized
  gallium/auxiliary/indices: replace free() with FREE()
  draw: fix incorrect color of flat-shaded clipped lines
  st/mesa: avoid sw fallback for getting/decompressing textures
  mesa: update assertion in detach_shader() for geom shaders
  mesa: do depth/stencil format conversion in glGetTexImage
  softpipe: use 64-bit arithmetic in softpipe_resource_layout()

Carl Worth (5):
  docs: Add md5sums for 10.0.3 release
  main: Avoid double-free of shader Label
  get-pick-list: Update to only find patches nominated for the 10.0 branch
  Update version to 10.0.4
  docs: Add release notes for 10.0.4

Chris Forbes (1):
  i965: Validate (and resolve) all the bound textures.

Christian König (1):
  radeon/uvd: fix feedback buffer handling v2

Daniel Kurtz (1):
  glsl: Add locking to builtin_builder singleton

Emil Velikov (3):
  dri/nouveau: Pass the API into _mesa_initialize_context
  nv50: correctly calculate the number of vertical blocks during transfer 
map
  dri/i9*5: correctly calculate the amount of system memory

Fredrik Höglund (3):
  mesa: Preserve the NewArrays state when copying a VAO
  glx: Fix the default values for GLXFBConfig attributes
  glx: Fix the GLXFBConfig attrib sort priorities

Hans (2):
  util: don't define isfinite(), isnan() for MSVC >= 1800
  mesa: don't define c99 math functions for MSVC >= 1800

Ian Romanick (6):
  meta: Release resources used by decompress_texture_image
  meta: Release resources used by _mesa_meta_DrawPixels
  meta: Fallback to software for GetTexImage of compressed 
GL_TEXTURE_CUBE_MAP_ARRAY
  meta: Consistenly use non-Apple VAO functions
  glcpp: Only warn for macro names containing __
  glsl: Only warn for macro names containing __

Ilia Mirkin (3):
  nv30: report 8 maximum inputs
  nouveau/video: make sure that firmware is present when checking caps
  nouveau: fix chipset checks for nv1a by using the oclass instead

Julien Cristau (1):
  glx/dri2: fix build failure on HURD

Kenneth Graunke (2):
  glsl: Don't lose precision qualifiers when encountering "centroid".
  i965: Create a hardware context before initializing state module.

Kusanagi Kouichi (1):
  targets/vdpau: Always use c++ to link

Marek Olšák (1):
  st/mesa: fix crash when a shader uses a TBO and it's not bound

Matt Turner (1):
  glsl: Initialize ubo_binding_mask flags to zero.

Paul Berry (2):
  glsl: Make condition_to_hir() callable from outside 
ast_iteration_statement.
  glsl: Fix continue statements in do-while loops.

Tom Stellard (1):
  r600g/compute: PIPE_CAP_COMPUTE should be false for pre-evergreen GPUs

Topi Pohjolainen (1):
  i965/blorp: do not use unnecessary hw-blending support



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

[Mesa-dev] PATCH: R600/SI: Instruction verifier improvements

[Tom Stellard]

Hi,

The attached patches add some more checks to the instruction verifier on SI
and also fix some false positives.  This should fix all the verifier errors
generated by the piglit tests.

As far as I can tell, there are no regressions with this series.  I had some 
trouble
making it through a full run on my Boanire system without hangs, so I would 
feel better
if someone else was able to test these.

-Tom
>From a31bd001e5b2705c32edce4950bb403c1dc1f010 Mon Sep 17 00:00:00 2001
From: Tom Stellard 
Date: Tue, 11 Mar 2014 10:11:55 -0400
Subject: [PATCH 1/3] R600/SI: Add generic checks to
 SIInstrInfo::verifyInstruction()

Added checks for number of operands and operand register classes.
---
 lib/Target/R600/SIInstrInfo.cpp | 41 +
 1 file changed, 41 insertions(+)

diff --git a/lib/Target/R600/SIInstrInfo.cpp b/lib/Target/R600/SIInstrInfo.cpp
index a239fb9..1f0a1a8 100644
--- a/lib/Target/R600/SIInstrInfo.cpp
+++ b/lib/Target/R600/SIInstrInfo.cpp
@@ -382,6 +382,47 @@ bool SIInstrInfo::verifyInstruction(const MachineInstr *MI,
   int Src1Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src1);
   int Src2Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src2);
 
+  // Make sure the number of operands is correct.
+  const MCInstrDesc &Desc = get(Opcode);
+  if (!Desc.isVariadic() &&
+  Desc.getNumOperands() != MI->getNumExplicitOperands()) {
+ ErrInfo = "Instruction has wrong number of operands.";
+ return false;
+  }
+
+  // Make sure the register classes are correct
+  for (unsigned i = 0, e = Desc.getNumOperands(); i != e; ++i) {
+switch (Desc.OpInfo[i].OperandType) {
+case MCOI::OPERAND_REGISTER:
+  break;
+case MCOI::OPERAND_IMMEDIATE:
+  if (!MI->getOperand(i).isImm() && !MI->getOperand(i).isFPImm()) {
+ErrInfo = "Expected immediate, but got non-immediate";
+return false;
+  }
+  // Fall-through
+default:
+  continue;
+}
+
+if (!MI->getOperand(i).isReg())
+  continue;
+
+int RegClass = Desc.OpInfo[i].RegClass;
+if (RegClass != -1) {
+  unsigned Reg = MI->getOperand(i).getReg();
+  if (TargetRegisterInfo::isVirtualRegister(Reg))
+continue;
+
+  const TargetRegisterClass *RC = RI.getRegClass(RegClass);
+  if (!RC->contains(Reg)) {
+ErrInfo = "Operand has incorrect register class.";
+return false;
+  }
+}
+  }
+
+
   // Verify VOP*
   if (isVOP1(Opcode) || isVOP2(Opcode) || isVOP3(Opcode) || isVOPC(Opcode)) {
 unsigned ConstantBusCount = 0;
-- 
1.8.1.5

>From 8abee76f5376a7bf5ef5f210493b58b18810902d Mon Sep 17 00:00:00 2001
From: Tom Stellard 
Date: Tue, 11 Mar 2014 17:28:57 -0400
Subject: [PATCH 2/3] R600/SI: Use correct dest register class for
 V_READFIRSTLANE_B32

This instructions writes to an 32-bit SGPR.  This change required adding
the 32-bit VCC_LO and VCC_HI registers, because the full VCC register
is 64 bits.

This fixes verifier errors on several of the indirect addressing piglit
tests.
---
 lib/Target/R600/AMDGPUAsmPrinter.cpp   |  3 ++-
 lib/Target/R600/SIInstructions.td  | 13 -
 lib/Target/R600/SILowerControlFlow.cpp |  5 +++--
 lib/Target/R600/SIRegisterInfo.td  | 13 +++--
 test/CodeGen/R600/private-memory.ll|  4 ++--
 5 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/lib/Target/R600/AMDGPUAsmPrinter.cpp b/lib/Target/R600/AMDGPUAsmPrinter.cpp
index ccc3d13..b166c45 100644
--- a/lib/Target/R600/AMDGPUAsmPrinter.cpp
+++ b/lib/Target/R600/AMDGPUAsmPrinter.cpp
@@ -210,7 +210,8 @@ void AMDGPUAsmPrinter::findNumUsedRegistersSI(MachineFunction &MF,
   continue;
 }
 unsigned reg = MO.getReg();
-if (reg == AMDGPU::VCC) {
+if (reg == AMDGPU::VCC || reg == AMDGPU::VCC_LO ||
+	reg == AMDGPU::VCC_HI) {
   VCCUsed = true;
   continue;
 }
diff --git a/lib/Target/R600/SIInstructions.td b/lib/Target/R600/SIInstructions.td
index 03e954f..9a18f7b 100644
--- a/lib/Target/R600/SIInstructions.td
+++ b/lib/Target/R600/SIInstructions.td
@@ -631,7 +631,18 @@ let neverHasSideEffects = 1, isMoveImm = 1 in {
 defm V_MOV_B32 : VOP1_32 <0x0001, "V_MOV_B32", []>;
 } // End neverHasSideEffects = 1, isMoveImm = 1
 
-defm V_READFIRSTLANE_B32 : VOP1_32 <0x0002, "V_READFIRSTLANE_B32", []>;
+let Uses = [EXEC] in {
+
+def V_READFIRSTLANE_B32 : VOP1 <
+  0x0002,
+  (outs SReg_32:$vdst),
+  (ins VReg_32:$src0),
+  "V_READFIRSTLANE_B32 $vdst, $src0",
+  []
+>;
+
+}
+
 defm V_CVT_I32_F64 : VOP1_32_64 <0x0003, "V_CVT_I32_F64",
   [(set i32:$dst, (fp_to_sint f64:$src0))]
 >;
diff --git a/lib/Target/R600/SILowerControlFlow.cpp b/lib/Target/R600/SILowerControlFlow.cpp
index 5ec4930..182f28b 100644
--- a/lib/Target/R600/SILowerControlFlow.cpp
+++ b/lib/Target/R600/SILowerControlFlow.cpp
@@ -345,12 +345,13 @@ void SILowerControlFlowPass::LoadM0(MachineInstr &MI, MachineInstr *MovRel) {
   .addReg(AMDGPU::EXEC);
 
   // 

[Mesa-dev] [PATCH 2/4] nouveau: typecast the prime_fd handle when calling nouveau_bo_set_prime

[Emil Velikov]

Core drm defines that the handle is of type int, while all drivers
treat it as uint internally. Typecast the value to silence gcc
warning messages and be consistent amongst all drivers.

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nouveau_screen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c 
b/src/gallium/drivers/nouveau/nouveau_screen.c
index f742a94..19892b1 100644
--- a/src/gallium/drivers/nouveau/nouveau_screen.c
+++ b/src/gallium/drivers/nouveau/nouveau_screen.c
@@ -123,7 +123,7 @@ nouveau_screen_bo_get_handle(struct pipe_screen *pscreen,
whandle->handle = bo->handle;
return TRUE;
} else if (whandle->type == DRM_API_HANDLE_TYPE_FD) {
-   return nouveau_bo_set_prime(bo, &whandle->handle) == 0;
+   return nouveau_bo_set_prime(bo, (int *)&whandle->handle) == 0;
} else {
return FALSE;
}
-- 
1.9.0

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

[Mesa-dev] [PATCH 3/4] nouveau: honor fread return value in the nouveau_compiler

[Emil Velikov]

There is little point of continuing if fread returns zero, as it
indicates that either the file is empty or cannot be read from.
Bail out if fread returns zero after closing the file.

Cc: Ilia Mirkin 
Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nouveau_compiler.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c 
b/src/gallium/drivers/nouveau/nouveau_compiler.c
index 5f1e35a..ac22035 100644
--- a/src/gallium/drivers/nouveau/nouveau_compiler.c
+++ b/src/gallium/drivers/nouveau/nouveau_compiler.c
@@ -173,9 +173,9 @@ main(int argc, char *argv[])
   return 1;
}
 
-   fread(text, 1, sizeof(text), f);
-   if (ferror(f)) {
+   if (!fread(text, 1, sizeof(text), f) || ferror(f)) {
   _debug_printf("Error reading file '%s'\n", filename);
+  fclose(f);
   return 1;
}
fclose(f);
-- 
1.9.0

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

[Mesa-dev] [PATCH 1/4] nv50: add missing brackets when handling the samplers array

[Emil Velikov]

Commit 3805a864b1d(nv50: assert before trying to out-of-bounds access
samplers) introduced a series of asserts as a precausion of a previous
illegal memory access.

Although it failed to encapsulate loop within nv50_sampler_state_delete
effectively failing to clear the sampler state, apart from exadurating
the illegal memory access issue.

Fixes gcc warning "array subscript is above array bounds" and
"Nesting level does not match indentation" and "Out-of-bounds read"
defects reported by Coverity.

Cc: "10.1" 
Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_state.c
index 862636b..647c01f 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
@@ -558,11 +558,12 @@ nv50_sampler_state_delete(struct pipe_context *pipe, void 
*hwcso)
 {
unsigned s, i;
 
-   for (s = 0; s < 3; ++s)
+   for (s = 0; s < 3; ++s) {
   assert(nv50_context(pipe)->num_samplers[s] <= PIPE_MAX_SAMPLERS);
   for (i = 0; i < nv50_context(pipe)->num_samplers[s]; ++i)
  if (nv50_context(pipe)->samplers[s][i] == hwcso)
 nv50_context(pipe)->samplers[s][i] = NULL;
+   }
 
nv50_screen_tsc_free(nv50_context(pipe)->screen, nv50_tsc_entry(hwcso));
 
-- 
1.9.0

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

[Mesa-dev] [PATCH 4/4] nvc0: minor cleanups in stream output handling

[Emil Velikov]

Constify the offsets parameter to silence gcc warning 'assignment
from incompatible pointer type' due to function prototype miss-match.

Use a boolean changed as a shorthand for target != current_target.

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_state.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
index 88dbaa1..05027a1 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
@@ -1031,7 +1031,7 @@ static void
 nvc0_set_transform_feedback_targets(struct pipe_context *pipe,
 unsigned num_targets,
 struct pipe_stream_output_target **targets,
-unsigned *offsets)
+const unsigned *offsets)
 {
struct nvc0_context *nvc0 = nvc0_context(pipe);
unsigned i;
@@ -1040,12 +1040,13 @@ nvc0_set_transform_feedback_targets(struct pipe_context 
*pipe,
assert(num_targets <= 4);
 
for (i = 0; i < num_targets; ++i) {
-  boolean append = (offsets[i] == ((unsigned)-1));
-  if (nvc0->tfbbuf[i] == targets[i] && append)
+  const boolean changed = nvc0->tfbbuf[i] != targets[i];
+  const boolean append = (offsets[i] == ((unsigned)-1));
+  if (!changed && append)
  continue;
   nvc0->tfbbuf_dirty |= 1 << i;
 
-  if (nvc0->tfbbuf[i] && nvc0->tfbbuf[i] != targets[i])
+  if (nvc0->tfbbuf[i] && changed)
  nvc0_so_target_save_offset(pipe, nvc0->tfbbuf[i], i, &serialize);
 
   if (targets[i] && !append)
-- 
1.9.0

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

Re: [Mesa-dev] [PATCH 8/7] i965: Accurately bail on SIMD16 compiles.

[Ian Romanick]

On 03/12/2014 02:51 AM, Ilia Mirkin wrote:
> On Wed, Mar 12, 2014 at 4:28 AM, Kenneth Graunke  
> wrote:
>> Ideally, we'd like to never even attempt the SIMD16 compile if we could
>> know ahead of time that it won't succeed---it's purely a waste of time.
>> This is especially important for state-based recompiles, which happen at
>> draw time.
>>
>> The fragment shader compiler has a number of checks like:
>>
>>if (dispatch_width == 16)
>>   fail("...some reason...");
>>
>> This patch introduces a new no16() function which replaces the above
>> pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
>> Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
>> issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
>> SIMD16 mode, no16() calls fail(), for safety's sake.)
>>
>> The great part is that this is not a heuristic---if the flag is set, we
>> know with 100% certainty that the SIMD16 compile would fail.  (It might
>> fail anyway if we run out of registers, but it's always worth trying.)
>>
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 69 
>> +++-
>>  src/mesa/drivers/dri/i965/brw_fs.h   |  4 ++
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +-
>>  3 files changed, 83 insertions(+), 34 deletions(-)
>>
>> I forgot to send this one out...it applies on top of the previous 7 patches.
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 62848be..9ad80c5 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum 
>> shader_time_shader_type type,
>>  }
>>
>>  void
>> -fs_visitor::fail(const char *format, ...)
>> +fs_visitor::vfail(const char *format, va_list va)
>>  {
>> -   va_list va;
>> char *msg;
>>
>> if (failed)
>> @@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
>>
>> failed = true;
>>
>> -   va_start(va, format);
>> msg = ralloc_vasprintf(mem_ctx, format, va);
>> -   va_end(va);
>> msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
>>
>> this->fail_msg = msg;
>> @@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
>> }
>>  }
>>
>> +void
>> +fs_visitor::fail(const char *format, ...)
>> +{
>> +   va_list va;
>> +
>> +   va_start(va, format);
>> +   vfail(format, va);
>> +   va_end(va);
>> +}
>> +
>> +/**
>> + * Mark this program as impossible to compile in SIMD16 mode.
>> + *
>> + * During the SIMD8 compile (which happens first), we can detect and flag
>> + * things that are unsupported in SIMD16 mode, so the compiler can skip
>> + * the SIMD16 compile altogether.
>> + *
>> + * During a SIMD16 compile (if one happens anyway), this just calls fail().
>> + */
>> +void
>> +fs_visitor::no16(const char *format, ...)
>> +{
>> +   va_list va;
>> +
>> +   va_start(va, format);
>> +
>> +   if (dispatch_width == 16) {
>> +  vfail(format, va);
> 
> I think there's a va_end() missing in this path. Not sure what the
> end-effect of that is, but I'm pretty sure that the recommendation is
> to always end the list before returning.

I don't know about that.  On some implementations, va_start and va_end
are macros that introduce a new block.  You can imagine an
implementation like:

typedef intptr_t va_list;

#define va_start(va, last) \
do { \
void *__va_temp; \
va = ((intptr_t)(void *)&last) + sizeof(last)

#define va_arg(va, type) \
(__va_temp = (void *) va, va += sizeof(type), *(type *) __va_temp)

#define va_end(va) \
} while (0)

This exact implementation doesn't work because it doesn't align the
pointers, but the general idea is valid.

I know that some implementations of varargs.h  used to work that way.
ANSI C may require that stdarg.h be more civilized.

>> +  return;
>> +   }
>> +
>> +   simd16_unsupported = true;
>> +
>> +   if (INTEL_DEBUG & DEBUG_PERF) {
>> +  if (no16_msg)
>> + ralloc_vasprintf_append(&no16_msg, format, va);
>> +  else
>> + no16_msg = ralloc_vasprintf(mem_ctx, format, va);
>> +   }
>> +
>> +   va_end(va);
>> +}
>> +
> ___
> 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/6] mesa: Add GL_INTEL_performance_query definitions

[Ian Romanick]

On 03/12/2014 05:54 AM, Petri Latvala wrote:
> Until Khronos adds the definitions to glext.h, add definitions for this
> extension to gl.h.

I don't know how we missed it, but the glext.h at
http://www.opengl.org/registry/api/GL/glext.h does have the definitions.
 We should just import that instead.

> Signed-off-by: Petri Latvala 
> ---
>  include/GL/gl.h | 45 +
>  1 file changed, 45 insertions(+)
> 
> diff --git a/include/GL/gl.h b/include/GL/gl.h
> index 4e2932d..e2caf73 100644
> --- a/include/GL/gl.h
> +++ b/include/GL/gl.h
> @@ -2104,6 +2104,51 @@ typedef void (APIENTRYP 
> PFNGLEGLIMAGETARGETTEXTURE2DOESPROC) (GLenum target, GLe
>  typedef void (APIENTRYP PFNGLEGLIMAGETARGETRENDERBUFFERSTORAGEOESPROC) 
> (GLenum target, GLeglImageOES image);
>  #endif
>  
> +#ifndef GL_INTEL_performance_query
> +#define GL_INTEL_performance_query 1
> +#define GL_PERFQUERY_SINGLE_CONTEXT_INTEL 0x
> +#define GL_PERFQUERY_GLOBAL_CONTEXT_INTEL 0x0001
> +#define GL_PERFQUERY_WAIT_INTEL   0x83FB
> +#define GL_PERFQUERY_FLUSH_INTEL  0x83FA
> +#define GL_PERFQUERY_DONOT_FLUSH_INTEL0x83F9
> +#define GL_PERFQUERY_COUNTER_EVENT_INTEL  0x94F0
> +#define GL_PERFQUERY_COUNTER_DURATION_NORM_INTEL 0x94F1
> +#define GL_PERFQUERY_COUNTER_DURATION_RAW_INTEL 0x94F2
> +#define GL_PERFQUERY_COUNTER_THROUGHPUT_INTEL 0x94F3
> +#define GL_PERFQUERY_COUNTER_RAW_INTEL0x94F4
> +#define GL_PERFQUERY_COUNTER_TIMESTAMP_INTEL 0x94F5
> +#define GL_PERFQUERY_COUNTER_DATA_UINT32_INTEL 0x94F8
> +#define GL_PERFQUERY_COUNTER_DATA_UINT64_INTEL 0x94F9
> +#define GL_PERFQUERY_COUNTER_DATA_FLOAT_INTEL 0x94FA
> +#define GL_PERFQUERY_COUNTER_DATA_DOUBLE_INTEL 0x94FB
> +#define GL_PERFQUERY_COUNTER_DATA_BOOL32_INTEL 0x94FC
> +#define GL_PERFQUERY_QUERY_NAME_LENGTH_MAX_INTEL 0x94FD
> +#define GL_PERFQUERY_COUNTER_NAME_LENGTH_MAX_INTEL 0x94FE
> +#define GL_PERFQUERY_COUNTER_DESC_LENGTH_MAX_INTEL 0x94FF
> +#define GL_PERFQUERY_GPA_EXTENDED_COUNTERS_INTEL 0x9500
> +typedef void (GLAPIENTRYP PFNGLBEGINPERFQUERYINTELPROC) (GLuint queryHandle);
> +typedef void (GLAPIENTRYP PFNGLCREATEPERFQUERYINTELPROC) (GLuint queryId, 
> GLuint *queryHandle);
> +typedef void (GLAPIENTRYP PFNGLDELETEPERFQUERYINTELPROC) (GLuint 
> queryHandle);
> +typedef void (GLAPIENTRYP PFNGLENDPERFQUERYINTELPROC) (GLuint queryHandle);
> +typedef void (GLAPIENTRYP PFNGLGETFIRSTPERFQUERYIDINTELPROC) (GLuint 
> *queryId);
> +typedef void (GLAPIENTRYP PFNGLGETNEXTPERFQUERYIDINTELPROC) (GLuint queryId, 
> GLuint *nextQueryId);
> +typedef void (GLAPIENTRYP PFNGLGETPERFCOUNTERINFOINTELPROC) (GLuint queryId, 
> GLuint counterId, GLuint counterNameLength, GLchar *counterName, GLuint 
> counterDescLength, GLchar *counterDesc, GLuint *counterOffset, GLuint 
> *counterDataSize, GLuint *counterTypeEnum, GLuint *counterDataTypeEnum, 
> GLuint64 *rawCounterMaxValue);
> +typedef void (GLAPIENTRYP PFNGLGETPERFQUERYDATAINTELPROC) (GLuint 
> queryHandle, GLuint flags, GLsizei dataSize, GLvoid *data, GLuint 
> *bytesWritten);
> +typedef void (GLAPIENTRYP PFNGLGETPERFQUERYIDBYNAMEINTELPROC) (GLchar 
> *queryName, GLuint *queryId);
> +typedef void (GLAPIENTRYP PFNGLGETPERFQUERYINFOINTELPROC) (GLuint queryId, 
> GLuint queryNameLength, GLchar *queryName, GLuint *dataSize, GLuint 
> *noCounters, GLuint *noInstances, GLuint *capsMask);
> +#ifdef GL_GLEXT_PROTOTYPES
> +GLAPI void GLAPIENTRY glBeginPerfQueryINTEL (GLuint queryHandle);
> +GLAPI void GLAPIENTRY glCreatePerfQueryINTEL (GLuint queryId, GLuint 
> *queryHandle);
> +GLAPI void GLAPIENTRY glDeletePerfQueryINTEL (GLuint queryHandle);
> +GLAPI void GLAPIENTRY glEndPerfQueryINTEL (GLuint queryHandle);
> +GLAPI void GLAPIENTRY glGetFirstPerfQueryIdINTEL (GLuint *queryId);
> +GLAPI void GLAPIENTRY glGetNextPerfQueryIdINTEL (GLuint queryId, GLuint 
> *nextQueryId);
> +GLAPI void GLAPIENTRY glGetPerfCounterInfoINTEL (GLuint queryId, GLuint 
> counterId, GLuint counterNameLength, GLchar *counterName, GLuint 
> counterDescLength, GLchar *counterDesc, GLuint *counterOffset, GLuint 
> *counterDataSize, GLuint *counterTypeEnum, GLuint *counterDataTypeEnum, 
> GLuint64 *rawCounterMaxValue);
> +GLAPI void GLAPIENTRY glGetPerfQueryDataINTEL (GLuint queryHandle, GLuint 
> flags, GLsizei dataSize, GLvoid *data, GLuint *bytesWritten);
> +GLAPI void GLAPIENTRY glGetPerfQueryIdByNameINTEL (GLchar *queryName, GLuint 
> *queryId);
> +GLAPI void GLAPIENTRY glGetPerfQueryInfoINTEL (GLuint queryId, GLuint 
> queryNameLength, GLchar *queryName, GLuint *dataSize, GLuint *noCounters, 
> GLuint *noInstances, GLuint *capsMask);
> +#endif
> +#endif /* GL_INTEL_performance_query */
>  
>  /**
>   ** NOTE!  If you add new functions to this file, or update
> 

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

Re: [Mesa-dev] [PATCH 8/7] i965: Accurately bail on SIMD16 compiles.

[Matt Turner]

On Wed, Mar 12, 2014 at 10:36 AM, Ian Romanick  wrote:
> On 03/12/2014 02:51 AM, Ilia Mirkin wrote:
>> On Wed, Mar 12, 2014 at 4:28 AM, Kenneth Graunke  
>> wrote:
>>> Ideally, we'd like to never even attempt the SIMD16 compile if we could
>>> know ahead of time that it won't succeed---it's purely a waste of time.
>>> This is especially important for state-based recompiles, which happen at
>>> draw time.
>>>
>>> The fragment shader compiler has a number of checks like:
>>>
>>>if (dispatch_width == 16)
>>>   fail("...some reason...");
>>>
>>> This patch introduces a new no16() function which replaces the above
>>> pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
>>> Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
>>> issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
>>> SIMD16 mode, no16() calls fail(), for safety's sake.)
>>>
>>> The great part is that this is not a heuristic---if the flag is set, we
>>> know with 100% certainty that the SIMD16 compile would fail.  (It might
>>> fail anyway if we run out of registers, but it's always worth trying.)
>>>
>>> Signed-off-by: Kenneth Graunke 
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 69 
>>> +++-
>>>  src/mesa/drivers/dri/i965/brw_fs.h   |  4 ++
>>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +-
>>>  3 files changed, 83 insertions(+), 34 deletions(-)
>>>
>>> I forgot to send this one out...it applies on top of the previous 7 patches.
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> index 62848be..9ad80c5 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> @@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum 
>>> shader_time_shader_type type,
>>>  }
>>>
>>>  void
>>> -fs_visitor::fail(const char *format, ...)
>>> +fs_visitor::vfail(const char *format, va_list va)
>>>  {
>>> -   va_list va;
>>> char *msg;
>>>
>>> if (failed)
>>> @@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
>>>
>>> failed = true;
>>>
>>> -   va_start(va, format);
>>> msg = ralloc_vasprintf(mem_ctx, format, va);
>>> -   va_end(va);
>>> msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
>>>
>>> this->fail_msg = msg;
>>> @@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
>>> }
>>>  }
>>>
>>> +void
>>> +fs_visitor::fail(const char *format, ...)
>>> +{
>>> +   va_list va;
>>> +
>>> +   va_start(va, format);
>>> +   vfail(format, va);
>>> +   va_end(va);
>>> +}
>>> +
>>> +/**
>>> + * Mark this program as impossible to compile in SIMD16 mode.
>>> + *
>>> + * During the SIMD8 compile (which happens first), we can detect and flag
>>> + * things that are unsupported in SIMD16 mode, so the compiler can skip
>>> + * the SIMD16 compile altogether.
>>> + *
>>> + * During a SIMD16 compile (if one happens anyway), this just calls fail().
>>> + */
>>> +void
>>> +fs_visitor::no16(const char *format, ...)
>>> +{
>>> +   va_list va;
>>> +
>>> +   va_start(va, format);
>>> +
>>> +   if (dispatch_width == 16) {
>>> +  vfail(format, va);
>>
>> I think there's a va_end() missing in this path. Not sure what the
>> end-effect of that is, but I'm pretty sure that the recommendation is
>> to always end the list before returning.
>
> I don't know about that.  On some implementations, va_start and va_end
> are macros that introduce a new block.  You can imagine an
> implementation like:
>
> typedef intptr_t va_list;
>
> #define va_start(va, last) \
> do { \
> void *__va_temp; \
> va = ((intptr_t)(void *)&last) + sizeof(last)
>
> #define va_arg(va, type) \
> (__va_temp = (void *) va, va += sizeof(type), *(type *) __va_temp)
>
> #define va_end(va) \
> } while (0)
>
> This exact implementation doesn't work because it doesn't align the
> pointers, but the general idea is valid.
>
> I know that some implementations of varargs.h  used to work that way.
> ANSI C may require that stdarg.h be more civilized.

My va_start man page says "On some systems, va_end contains a closing
'}' matching a '{' in va_start" in the NOTES section about the old
varargs.h macros, so I don't think that's the case anymore.

In fact, gcc defines them as:

#define va_start(v,l)   __builtin_va_start(v,l)
#define va_end(v)   __builtin_va_end(v)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/7] i965/fs: Don't renumber UNIFORM registers.

[Ian Romanick]

On 03/11/2014 11:48 PM, Kenneth Graunke wrote:
> Previously, remove_dead_constants() would renumber the UNIFORM registers
> to be sequential starting from zero, and the resulting register number
> would be used directly as an index into the params[] array.
> 
> This renumbering made it difficult to collect and save information about
> pull constant locations, since setup_pull_constants() and
> move_uniform_array_access_to_pull_constants() used different names.
> 
> This patch generalizes setup_pull_constants() to decide whether each
> uniform register should be a pull constant, push constant, or neither
> (because it's unused).  Then, it stores mappings from UNIFORM register
> numbers to params[] or pull_params[] indices in the push_constant_loc
> and pull_constant_loc arrays.  (We already did this for pull constants.)
> 
> Then, assign_curb_setup() just needs to consult the push_constant_loc
> array to get the real index into the params[] array.
> 
> This effectively folds all the remove_dead_constants() functionality
> into assign_constant_locations(), while being less irritable to work
> with.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 187 
> +++
>  src/mesa/drivers/dri/i965/brw_fs.h   |  13 +-
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   3 +-
>  3 files changed, 85 insertions(+), 118 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 3c8237a..5e01e78 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1412,7 +1409,19 @@ fs_visitor::assign_curb_setup()
>  
>for (unsigned int i = 0; i < 3; i++) {
>if (inst->src[i].file == UNIFORM) {
> - int constant_nr = inst->src[i].reg + inst->src[i].reg_offset;
> + int uniform_nr = inst->src[i].reg + inst->src[i].reg_offset;

Should be safe to fix the tabs here. :)

> +int constant_nr;
> +if (uniform_nr >= 0 && uniform_nr < (int) uniforms) {
> +   constant_nr = push_constant_loc[uniform_nr];
> +} else {
> +   /* Section 5.11 of the OpenGL 4.1 spec says:
> +* "Out-of-bounds reads return undefined values, which include
> +*  values from other variables of the active program or 
> zero."
> +* Just return the first push constant.
> +*/
> +   constant_nr = 0;
> +}
> +
>   struct brw_reg brw_reg = brw_vec1_grf(c->nr_payload_regs +
> constant_nr / 8,
> constant_nr % 8);

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

Re: [Mesa-dev] i965: Pull parameters in SIMD16 mode.

[Ian Romanick]

Patches 1 through 4 and 8 are

Reviewed-by: Ian Romanick 

I don't know the code well enough to give any evaluation of patch 5, and
all the others really depend on 5.

On 03/11/2014 11:48 PM, Kenneth Graunke wrote:
> Hello,
> 
> This series adds support for pull constants in SIMD16 mode.  By the end
> of the series, shader-db gains 78 extra SIMD16 programs, and the net
> code change is -39 lines.  In short:
> 
> better living through deleting code.
> 
> Most of the work in this series is refactoring the code to change how
> information about locations is tracked and how it flows between functions.
> Overall, I think it's much nicer at the end (though I'm clearly biased).
> 
> This is available in the 'pull16-v12' branch of ~kwg/mesa.  I tested it
> for Piglit regressions and shader-db code quality regressions at every
> single commit---getting a patch series that didn't regress at any point
> was kind of tricky.  Lots of obviously correct code would break
> horribly...hence the 12 branch revisions. :)
> 
> --Ken
> 
> ___
> 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/4] nouveau: honor fread return value in the nouveau_compiler

[Ilia Mirkin]

On Wed, Mar 12, 2014 at 1:24 PM, Emil Velikov  wrote:
> There is little point of continuing if fread returns zero, as it
> indicates that either the file is empty or cannot be read from.
> Bail out if fread returns zero after closing the file.
>
> Cc: Ilia Mirkin 
> Signed-off-by: Emil Velikov 

Reviewed-by: Ilia Mirkin 

The fclose() you added serves no purpose (program is exiting anyways),
but it also doesn't hurt.

> ---
>  src/gallium/drivers/nouveau/nouveau_compiler.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c 
> b/src/gallium/drivers/nouveau/nouveau_compiler.c
> index 5f1e35a..ac22035 100644
> --- a/src/gallium/drivers/nouveau/nouveau_compiler.c
> +++ b/src/gallium/drivers/nouveau/nouveau_compiler.c
> @@ -173,9 +173,9 @@ main(int argc, char *argv[])
>return 1;
> }
>
> -   fread(text, 1, sizeof(text), f);
> -   if (ferror(f)) {
> +   if (!fread(text, 1, sizeof(text), f) || ferror(f)) {
>_debug_printf("Error reading file '%s'\n", filename);
> +  fclose(f);
>return 1;
> }
> fclose(f);
> --
> 1.9.0
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Don't enable reset notification support on Gen4-5.

[Ian Romanick]

On 03/12/2014 01:43 AM, Kenneth Graunke wrote:
> arekm reported that using Chrome with GPU acceleration enabled on GM45
> triggered the hw_ctx != NULL assertion in brw_get_graphics_reset_status.
> 
> We definitely do not want to advertise reset notification support on
> Gen4-5 systems, since it needs hardware contexts, and we never even
> request a hardware context on those systems.
> 
> Cc: Ian Romanick 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/intel_screen.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> This may not be the best solution; if someone would like to send patches for
> a better one, I'm more than happy to go with something else.  Checking for
> hardware contexts is a bit awkward - we need to decide whether to advertise
> support here in the screen, but we don't create hardware contexts until
> context creation time, which is a fair bit later.  So, I'm not sure how to
> do better than the generation check, for now.

I'm confused.  I thought the ioctl was was supposed to fail with EINVAL
in that case.  Mika, what's your suggestion?

> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index 464cebf..12006be 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1342,13 +1342,18 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>  * no error.  If the ioctl is not supported, it always generate EINVAL.
>  * Use this to determine whether to advertise the __DRI2_ROBUSTNESS
>  * extension to the loader.
> +*
> +* Don't even try on pre-Gen6, since we don't attempt to use contexts 
> there.
>  */
> -   struct drm_i915_reset_stats stats;
> -   memset(&stats, 0, sizeof(stats));
> +   if (intelScreen->devinfo->gen >= 6) {
> +  struct drm_i915_reset_stats stats;
> +  memset(&stats, 0, sizeof(stats));
>  
> -   const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS, &stats);
> +  const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS, 
> &stats);
>  
> -   intelScreen->has_context_reset_notification = (ret != -1 || errno != 
> EINVAL);
> +  intelScreen->has_context_reset_notification =
> + (ret != -1 || errno != EINVAL);
> +   }
>  
> psp->extensions = !intelScreen->has_context_reset_notification
>? intelScreenExtensions : intelRobustScreenExtensions;
> 

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

Re: [Mesa-dev] [PATCH 1/4] nv50: add missing brackets when handling the samplers array

[Ilia Mirkin]

On Wed, Mar 12, 2014 at 1:24 PM, Emil Velikov  wrote:
> Commit 3805a864b1d(nv50: assert before trying to out-of-bounds access
> samplers) introduced a series of asserts as a precausion of a previous
> illegal memory access.
>
> Although it failed to encapsulate loop within nv50_sampler_state_delete
> effectively failing to clear the sampler state, apart from exadurating
> the illegal memory access issue.
>
> Fixes gcc warning "array subscript is above array bounds" and
> "Nesting level does not match indentation" and "Out-of-bounds read"
> defects reported by Coverity.
>
> Cc: "10.1" 
> Signed-off-by: Emil Velikov 

Reviewed-by: Ilia Mirkin 

> ---
>  src/gallium/drivers/nouveau/nv50/nv50_state.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_state.c
> index 862636b..647c01f 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
> @@ -558,11 +558,12 @@ nv50_sampler_state_delete(struct pipe_context *pipe, 
> void *hwcso)
>  {
> unsigned s, i;
>
> -   for (s = 0; s < 3; ++s)
> +   for (s = 0; s < 3; ++s) {
>assert(nv50_context(pipe)->num_samplers[s] <= PIPE_MAX_SAMPLERS);
>for (i = 0; i < nv50_context(pipe)->num_samplers[s]; ++i)
>   if (nv50_context(pipe)->samplers[s][i] == hwcso)
>  nv50_context(pipe)->samplers[s][i] = NULL;
> +   }
>
> nv50_screen_tsc_free(nv50_context(pipe)->screen, nv50_tsc_entry(hwcso));
>
> --
> 1.9.0
>
> ___
> 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] Gallium u_gen_mipmap problem with queries

[Roland Scheidegger]

Am 12.03.2014 15:00, schrieb Marek Olšák:
> Hi everybody,
> 
> u_gen_mipmap doesn't disable queries, so all active queries are
> incremented, even though mipmap generation shouldn't have any effect
> on queries. Hardware drivers can disable queries, but this feature is
> not exposed via the gallium interface. Generally, we want to disable
> all queries except for the time_elapsed query for u_gen_mipmap. I
> propose the following solutions to this problem:
> 
> 1) Add pipe_context::pause_all_queries(pipe) and
> resume_all_queries(pipe). It's self-explanatory.
> 
> 2) Add pipe_context::pause_query(pipe, query) and resume_query(pipe,
> query). This has the disadvantage of having to enumerate all active
> queries in u_gen_mipmap.
> 
> 3) Add pipe_rasterizer_state::disable_all_queries:1, or separate bits
> disable_occlusion_queries:1 and disable_streamout_queries:1 (this
> includes the primitives_generated query too).
> 
> 4) Rewrite u_gen_mipmap to use pipe->blit.
> 
> Please comment.
> 
> Thanks.
> 
> Marek

No idea what's best here. Though if you go for pause/resume query (or
all queries) wouldn't it be better to just have one function and pass in
some SUSPEND_QUERIES/RESUME_QUERIES bit?
Other than that, I guess if hw really can suspend just all queries
gathering with a single bit then an interface to disable all of them
at once would be preferable.

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

Re: [Mesa-dev] [PATCH 4/4] nvc0: minor cleanups in stream output handling

[Ilia Mirkin]

On Wed, Mar 12, 2014 at 1:24 PM, Emil Velikov  wrote:
> Constify the offsets parameter to silence gcc warning 'assignment
> from incompatible pointer type' due to function prototype miss-match.
>
> Use a boolean changed as a shorthand for target != current_target.
>
> Signed-off-by: Emil Velikov 

Reviewed-by: Ilia Mirkin 

> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_state.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> index 88dbaa1..05027a1 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> @@ -1031,7 +1031,7 @@ static void
>  nvc0_set_transform_feedback_targets(struct pipe_context *pipe,
>  unsigned num_targets,
>  struct pipe_stream_output_target 
> **targets,
> -unsigned *offsets)
> +const unsigned *offsets)
>  {
> struct nvc0_context *nvc0 = nvc0_context(pipe);
> unsigned i;
> @@ -1040,12 +1040,13 @@ nvc0_set_transform_feedback_targets(struct 
> pipe_context *pipe,
> assert(num_targets <= 4);
>
> for (i = 0; i < num_targets; ++i) {
> -  boolean append = (offsets[i] == ((unsigned)-1));
> -  if (nvc0->tfbbuf[i] == targets[i] && append)
> +  const boolean changed = nvc0->tfbbuf[i] != targets[i];
> +  const boolean append = (offsets[i] == ((unsigned)-1));
> +  if (!changed && append)
>   continue;
>nvc0->tfbbuf_dirty |= 1 << i;
>
> -  if (nvc0->tfbbuf[i] && nvc0->tfbbuf[i] != targets[i])
> +  if (nvc0->tfbbuf[i] && changed)
>   nvc0_so_target_save_offset(pipe, nvc0->tfbbuf[i], i, &serialize);
>
>if (targets[i] && !append)
> --
> 1.9.0
>
> ___
> 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 8/7] i965: Accurately bail on SIMD16 compiles.

[Kenneth Graunke]

On 03/12/2014 02:51 AM, Ilia Mirkin wrote:
> On Wed, Mar 12, 2014 at 4:28 AM, Kenneth Graunke  
> wrote:
>> Ideally, we'd like to never even attempt the SIMD16 compile if we could
>> know ahead of time that it won't succeed---it's purely a waste of time.
>> This is especially important for state-based recompiles, which happen at
>> draw time.
>>
>> The fragment shader compiler has a number of checks like:
>>
>>if (dispatch_width == 16)
>>   fail("...some reason...");
>>
>> This patch introduces a new no16() function which replaces the above
>> pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
>> Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
>> issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
>> SIMD16 mode, no16() calls fail(), for safety's sake.)
>>
>> The great part is that this is not a heuristic---if the flag is set, we
>> know with 100% certainty that the SIMD16 compile would fail.  (It might
>> fail anyway if we run out of registers, but it's always worth trying.)
>>
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 69 
>> +++-
>>  src/mesa/drivers/dri/i965/brw_fs.h   |  4 ++
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +-
>>  3 files changed, 83 insertions(+), 34 deletions(-)
>>
>> I forgot to send this one out...it applies on top of the previous 7 patches.
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 62848be..9ad80c5 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum 
>> shader_time_shader_type type,
>>  }
>>
>>  void
>> -fs_visitor::fail(const char *format, ...)
>> +fs_visitor::vfail(const char *format, va_list va)
>>  {
>> -   va_list va;
>> char *msg;
>>
>> if (failed)
>> @@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
>>
>> failed = true;
>>
>> -   va_start(va, format);
>> msg = ralloc_vasprintf(mem_ctx, format, va);
>> -   va_end(va);
>> msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
>>
>> this->fail_msg = msg;
>> @@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
>> }
>>  }
>>
>> +void
>> +fs_visitor::fail(const char *format, ...)
>> +{
>> +   va_list va;
>> +
>> +   va_start(va, format);
>> +   vfail(format, va);
>> +   va_end(va);
>> +}
>> +
>> +/**
>> + * Mark this program as impossible to compile in SIMD16 mode.
>> + *
>> + * During the SIMD8 compile (which happens first), we can detect and flag
>> + * things that are unsupported in SIMD16 mode, so the compiler can skip
>> + * the SIMD16 compile altogether.
>> + *
>> + * During a SIMD16 compile (if one happens anyway), this just calls fail().
>> + */
>> +void
>> +fs_visitor::no16(const char *format, ...)
>> +{
>> +   va_list va;
>> +
>> +   va_start(va, format);
>> +
>> +   if (dispatch_width == 16) {
>> +  vfail(format, va);
> 
> I think there's a va_end() missing in this path. Not sure what the
> end-effect of that is, but I'm pretty sure that the recommendation is
> to always end the list before returning.

Good catch, thanks!

For v2, I've changed the code to:

void
fs_visitor::no16(const char *format, ...)
{
   va_list va;

   va_start(va, format);

   if (dispatch_width == 16) {
  vfail(format, va);
   } else {
  simd16_unsupported = true;

  if (INTEL_DEBUG & DEBUG_PERF) {
 if (no16_msg)
ralloc_vasprintf_append(&no16_msg, format, va);
 else
no16_msg = ralloc_vasprintf(mem_ctx, format, va);
  }
   }

   va_end(va);
}

which should also address Ian's concern.  (I'm not sure whether it's
necessary, but it's easy enough to do, so why not play it safe?)

--Ken



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

[Mesa-dev] [PATCH] radeonsi/compute: Fix memory leak

[Aaron Watry]

Free shader buffer object for all kernels when deleting compute state.

Signed-off-by: Aaron Watry 
---
 src/gallium/drivers/radeonsi/si_compute.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_compute.c 
b/src/gallium/drivers/radeonsi/si_compute.c
index e16feab..c0637f6 100644
--- a/src/gallium/drivers/radeonsi/si_compute.c
+++ b/src/gallium/drivers/radeonsi/si_compute.c
@@ -301,6 +301,12 @@ static void si_delete_compute_state(struct pipe_context 
*ctx, void* state){
}
 
if (program->kernels) {
+   for (int i = 0; i < program->num_kernels; i++){
+   if (program->kernels[i].bo){
+   si_pipe_shader_destroy(ctx, 
&program->kernels[i]);
+   }
+   }
+   
FREE(program->kernels);
}
 
-- 
1.8.3.2

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

Re: [Mesa-dev] [PATCH 12/12] mesa/sso: Implement ValidateProgramPipeline

[Ian Romanick]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 03/05/2014 01:00 PM, Eric Anholt wrote:
> Ian Romanick  writes:
> 
>> From: Gregory Hainaut 
>> 
>> Implementation note: I don't use context for ralloc (don't know
>> how).
>> 
>> The check on PROGRAM_SEPARABLE flags is also done when the
>> pipeline isn't bound.  It doesn't make any sense in a DSA style
>> API.
>> 
>> Maybe we could replace _mesa_validate_program by 
>> _mesa_validate_program_pipeline.  For example we could recreate a
>> dummy pipeline object.  However the new function checks also the 
>> TEXTURE_IMAGE_UNIT number not sure of the impact.
>> 
>> V2: Fix memory leak with ralloc_strdup Formatting improvement
>> 
>> V3 (idr): * Actually fix the leak of the InfoLog. :) * Directly
>> generate logs in to gl_pipeline_object::InfoLog via 
>> ralloc_asprintf isntead of using a temporary buffer. * Split out
>> from previous uber patch. * Change spec references to include
>> section numbers, etc. * Fix a bug in checking that a different
>> program isn't active in a stage between two stages that have the
>> same program.  Specifically,
>> 
>> if (pipe->CurrentVertexProgram->Name ==
>> pipe->CurrentGeometryProgram->Name && 
>> pipe->CurrentGeometryProgram->Name !=
>> pipe->CurrentVertexProgram->Name)
>> 
>> should have been
>> 
>> if (pipe->CurrentVertexProgram->Name ==
>> pipe->CurrentFragmentProgram->Name && 
>> pipe->CurrentGeometryProgram->Name !=
>> pipe->CurrentVertexProgram->Name)
>> 
>> v4 (idr): Rework to use CurrentProgram array in loops.
>> 
>> Reviewed-by: Ian Romanick 
> 
>> diff --git a/src/mesa/main/pipelineobj.c
>> b/src/mesa/main/pipelineobj.c index 3db97c0..d371e88 100644 ---
>> a/src/mesa/main/pipelineobj.c +++ b/src/mesa/main/pipelineobj.c
> 
>> +   /* Section 2.11.11 (Shader Execution), subheading
>> "Validation," of the +* OpenGL 4.1 spec says: +* +*
>> "[INVALID_OPERATION] is generated by any command that transfers +
>> * vertices to the GL if: +* +* ... +* +
>> * - Any two active samplers in the current program object
>> are of +*   different types, but refer to the same
>> texture image unit. +* +* - The number of active
>> samplers in the program exceeds the +*   maximum
>> number of texture image units allowed." +*/ +   if
>> (!_mesa_sampler_uniforms_pipeline_are_valid(pipe)) +  goto
>> err; + +   pipe->Validated = GL_TRUE; +   return GL_TRUE;
> 
> What ensures that the sampler validate will be redone when sampler 
> uniforms change?

_mesa_valid_to_render calls _mesa_validate_program_pipeline.  If the
application calls glValidateProgramPipeline (getting an error),
changes the uniform, then calls glDrawArrays, the program will get
revalidated.

We should track some state (like sampler uniforms) to avoid some of
the draw-time validation cost.

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.14 (GNU/Linux)

iEYEARECAAYFAlMgpyAACgkQX1gOwKyEAw/RfQCglFHX/2r7/8dDFplszXgQuh/D
HFEAn3cvjnO8W/+nxeCAa2sBLMNSes6Q
=Biw1
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 8/7] i965: Accurately bail on SIMD16 compiles.

[Ian Romanick]

On 03/12/2014 11:30 AM, Kenneth Graunke wrote:
> On 03/12/2014 02:51 AM, Ilia Mirkin wrote:
>> On Wed, Mar 12, 2014 at 4:28 AM, Kenneth Graunke  
>> wrote:
>>> Ideally, we'd like to never even attempt the SIMD16 compile if we could
>>> know ahead of time that it won't succeed---it's purely a waste of time.
>>> This is especially important for state-based recompiles, which happen at
>>> draw time.
>>>
>>> The fragment shader compiler has a number of checks like:
>>>
>>>if (dispatch_width == 16)
>>>   fail("...some reason...");
>>>
>>> This patch introduces a new no16() function which replaces the above
>>> pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
>>> Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
>>> issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
>>> SIMD16 mode, no16() calls fail(), for safety's sake.)
>>>
>>> The great part is that this is not a heuristic---if the flag is set, we
>>> know with 100% certainty that the SIMD16 compile would fail.  (It might
>>> fail anyway if we run out of registers, but it's always worth trying.)
>>>
>>> Signed-off-by: Kenneth Graunke 
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 69 
>>> +++-
>>>  src/mesa/drivers/dri/i965/brw_fs.h   |  4 ++
>>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +-
>>>  3 files changed, 83 insertions(+), 34 deletions(-)
>>>
>>> I forgot to send this one out...it applies on top of the previous 7 patches.
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> index 62848be..9ad80c5 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> @@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum 
>>> shader_time_shader_type type,
>>>  }
>>>
>>>  void
>>> -fs_visitor::fail(const char *format, ...)
>>> +fs_visitor::vfail(const char *format, va_list va)
>>>  {
>>> -   va_list va;
>>> char *msg;
>>>
>>> if (failed)
>>> @@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
>>>
>>> failed = true;
>>>
>>> -   va_start(va, format);
>>> msg = ralloc_vasprintf(mem_ctx, format, va);
>>> -   va_end(va);
>>> msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
>>>
>>> this->fail_msg = msg;
>>> @@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
>>> }
>>>  }
>>>
>>> +void
>>> +fs_visitor::fail(const char *format, ...)
>>> +{
>>> +   va_list va;
>>> +
>>> +   va_start(va, format);
>>> +   vfail(format, va);
>>> +   va_end(va);
>>> +}
>>> +
>>> +/**
>>> + * Mark this program as impossible to compile in SIMD16 mode.
>>> + *
>>> + * During the SIMD8 compile (which happens first), we can detect and flag
>>> + * things that are unsupported in SIMD16 mode, so the compiler can skip
>>> + * the SIMD16 compile altogether.
>>> + *
>>> + * During a SIMD16 compile (if one happens anyway), this just calls fail().
>>> + */
>>> +void
>>> +fs_visitor::no16(const char *format, ...)
>>> +{
>>> +   va_list va;
>>> +
>>> +   va_start(va, format);
>>> +
>>> +   if (dispatch_width == 16) {
>>> +  vfail(format, va);
>>
>> I think there's a va_end() missing in this path. Not sure what the
>> end-effect of that is, but I'm pretty sure that the recommendation is
>> to always end the list before returning.
> 
> Good catch, thanks!
> 
> For v2, I've changed the code to:
> 
> void
> fs_visitor::no16(const char *format, ...)
> {
>va_list va;
> 
>va_start(va, format);
> 
>if (dispatch_width == 16) {
>   vfail(format, va);
>} else {
>   simd16_unsupported = true;
> 
>   if (INTEL_DEBUG & DEBUG_PERF) {
>  if (no16_msg)
> ralloc_vasprintf_append(&no16_msg, format, va);
>  else
> no16_msg = ralloc_vasprintf(mem_ctx, format, va);
>   }
>}
> 
>va_end(va);
> }
> 
> which should also address Ian's concern.  (I'm not sure whether it's
> necessary, but it's easy enough to do, so why not play it safe?)

I can get behind that. :)

> --Ken




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

Re: [Mesa-dev] [PATCH 03/12] mesa/sso: replace Shader binding point with _Shader

[Ian Romanick]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 03/05/2014 12:23 PM, Eric Anholt wrote:
> Ian Romanick  writes:
> 
>> From: Gregory Hainaut 
>> 
>> To avoid NULL pointer check a default pipeline object is
>> installed in _Shader when no program is current
>> 
>> The spec say that UseProgram/UseShaderProgramEXT/ActiveProgramEXT
>> got an higher priority over the pipeline object. When default
>> program is uninstall, the pipeline is used if any was bound.
>> 
>> Note: A careful rename need to be done now...
>> 
>> V2: formating improvement
>> 
>> V3 (idr): * Build fix.  The original patch added calls to
>> _mesa_use_shader_program with 4 parameters, but the fourth
>> parameter isn't added to that function until a much later patch.
>> Just drop that parameter for now. * Trivial reformatting. *
>> Updated comment of gl_context::_Shader
>> 
>> Reviewed-by: Ian Romanick  --- 
>> src/mesa/main/mtypes.h  | 15  
>> src/mesa/main/pipelineobj.c |  8  src/mesa/main/shaderapi.c
>> | 91 +++-- 3 files
>> changed, 111 insertions(+), 3 deletions(-)
>> 
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h 
>> index d05649c..8a03afd 100644 --- a/src/mesa/main/mtypes.h +++
>> b/src/mesa/main/mtypes.h @@ -2824,6 +2824,9 @@ struct
>> gl_pipeline_shader_state /** Currently bound pipeline object. See
>> _mesa_BindProgramPipeline() */ struct gl_pipeline_object
>> *Current;
>> 
>> +   /* Default Object to ensure that _Shader is never NULL */ +
>> struct gl_pipeline_object *Default; + /** Pipeline objects */ 
>> struct _mesa_HashTable *Objects; }; @@ -4131,6 +4134,18 @@ struct
>> gl_context
>> 
>> struct gl_pipeline_shader_state Pipeline; /**< GLSL pipeline
>> shader object state */ struct gl_pipeline_object Shader; /**<
>> GLSL shader object state */ + +   /** +* Current active
>> shader pipeline state +* +* Almost all internal users
>> want ::_Shader instead of ::Shader.  The +* exceptions are
>> bits of legacy GLSL API that do not know about separate +*
>> shader objects. +* +* Points to ::Shader,
>> ::Pipeline.Current, or ::Pipeline.Default. +*/ +   struct
>> gl_pipeline_object *_Shader; + struct gl_shader_compiler_options
>> ShaderCompilerOptions[MESA_SHADER_STAGES];
>> 
>> struct gl_query_state Query;  /**< occlusion, timer queries */ 
>> diff --git a/src/mesa/main/pipelineobj.c
>> b/src/mesa/main/pipelineobj.c index 27012df..849c781 100644 ---
>> a/src/mesa/main/pipelineobj.c +++ b/src/mesa/main/pipelineobj.c 
>> @@ -94,6 +94,10 @@ _mesa_init_pipeline(struct gl_context *ctx) 
>> ctx->Pipeline.Objects = _mesa_NewHashTable();
>> 
>> ctx->Pipeline.Current = NULL; + +   /* Install a default Pipeline
>> */ +   ctx->Pipeline.Default = _mesa_new_pipeline_object(ctx,
>> 0); +   _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
>> ctx->Pipeline.Default); }
>> 
>> 
>> @@ -117,6 +121,10 @@ _mesa_free_pipeline_data(struct gl_context
>> *ctx) { _mesa_HashDeleteAll(ctx->Pipeline.Objects,
>> delete_pipelineobj_cb, ctx); 
>> _mesa_DeleteHashTable(ctx->Pipeline.Objects); + +
>> _mesa_reference_pipeline_object(ctx, &ctx->_Shader, NULL); +
>> _mesa_delete_pipeline_object(ctx, ctx->Pipeline.Default); + }
>> 
>> /** diff --git a/src/mesa/main/shaderapi.c
>> b/src/mesa/main/shaderapi.c index 5060cbb..71b2ef5 100644 ---
>> a/src/mesa/main/shaderapi.c +++ b/src/mesa/main/shaderapi.c @@
>> -44,6 +44,7 @@ #include "main/hash.h" #include
>> "main/hash_table.h" #include "main/mtypes.h" +#include
>> "main/pipelineobj.h" #include "main/shaderapi.h" #include
>> "main/shaderobj.h" #include "main/transformfeedback.h" @@ -144,6
>> +145,8 @@ _mesa_free_shader_state(struct gl_context *ctx) 
>> _mesa_reference_shader_program(ctx, &ctx->Shader.ActiveProgram,
>> NULL);
>> 
>> /* Extended for ARB_separate_shader_objects */ +
>> _mesa_reference_pipeline_object(ctx, &ctx->_Shader, NULL); + 
>> assert(ctx->Shader.RefCount == 1); 
>> mtx_destroy(&ctx->Shader.Mutex); } @@ -1541,7 +1544,29 @@
>> _mesa_UseProgram(GLhandleARB program) shProg = NULL; }
>> 
>> -   _mesa_use_program(ctx, shProg); +   /* +* The executable
>> code for an individual shader stage is taken from the +*
>> current program for that stage.  If there is a current program
>> object +* for any shader stage or for uniform updates
>> established by UseProgram, +* UseShaderProgramEXT, or
>> ActiveProgramEXT, the current program for that +* stage (if
>> any) is considered current.  Otherwise, if there is a bound +
>> * program pipeline object ... +*/
> 
> This is a spec citation, and it would be nice to see it formatted
> like one in the 3 places it's quoted in the file.
> 
>> +   if (program) { +  /* Attach shader state to the binding
>> point */ +  _mesa_reference_pipeline_object(ctx,
>> &ctx->_Shader, &ctx->Shader); +  /* Update the program */ +
>> _mesa_use_program(ctx, shProg); +   } else { +  /* Must be
>> done first: detach the progam */ +  _mesa_use_progra

Re: [Mesa-dev] [PATCH 07/12] mesa/sso: Implement _mesa_UseProgramStages

[Ian Romanick]

On 03/05/2014 12:35 PM, Eric Anholt wrote:
> Ian Romanick  writes:
> 
>> From: Gregory Hainaut 
>>
>> Now arb_separate_shader_object-GetProgramPipelineiv should pass.
>>
>> V3 (idr):
>> * Change spec references to core OpenGL versions instead of issues in
>>   the extension spec.
>> * Split out from previous uber patch.
>>
>> v4 (idr): Use _mesa_has_geometry_shaders in _mesa_UseProgramStages to
>> detect availability of geometry shaders.
>>
>> Reviewed-by: Ian Romanick 
>> ---
>>  src/mesa/main/pipelineobj.c | 115 
>> 
>>  1 file changed, 115 insertions(+)
>>
>> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
>> index 849c781..7149578 100644
>> --- a/src/mesa/main/pipelineobj.c
>> +++ b/src/mesa/main/pipelineobj.c
> 
>> +   GLbitfield any_valid_stages = GL_VERTEX_SHADER_BIT | 
>> GL_FRAGMENT_SHADER_BIT;
>> +   if (_mesa_has_geometry_shaders(ctx))
>> +  any_valid_stages |= GL_GEOMETRY_SHADER_BIT;
>> +
>> +   if (stages != GL_ALL_SHADER_BITS && (stages  & ~any_valid_stages) != 0) {
> 
> Weird double space before &.
> 
>> +  _mesa_error(ctx, GL_INVALID_VALUE, "glUseProgramStages(Stages)");
>> +  return;
>> +   }
> 
>> +   if (program) {
>> +  /* Section 2.11.1 (Shader Objects) of the OpenGL 3.1 spec (and 
>> probably
>> +   * earlier) says:
>> +   *
>> +   * "Commands that accept shader or program object names will
>> +   * generate the error INVALID_VALUE if the provided name is not 
>> the
>> +   * name of either a shader or program object and INVALID_OPERATION
>> +   * if the provided name identifies an object that is not the
>> +   * expected type."
>> +   */
>> +  struct gl_shader *sh = _mesa_lookup_shader(ctx, program);
>> +  if (sh != NULL) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> +   "glUseProgramStages(progam is a shader object)");
>> + return;
>> +  }
> 
> This block could get dropped into the shProg == NULL case below, right?
> The code confused me as is.

The first block is checking whether the name is a shader, and the other
is checking that the name is a program.  The neat thing is the spec says
using a shader where a program is expected gives one error, but using a
name that is neither give a different error.

You've never seen code like this elsewhere in Mesa because it's usually
handled by _mesa_lookup_shader_program_err.  I'll change this code to
use that function instead.

>> +
>> +  shProg = _mesa_lookup_shader_program(ctx, program);
>> +  if (shProg == NULL) {
>> + _mesa_error(ctx, GL_INVALID_VALUE,
>> +   "glUseProgramStages(progam is not a program object)");
>> + return;
>> +  }
> 
> 
>> +   /* Section 2.11.4 (Program Pipeline Objects) of the OpenGL 4.1 spec
>> +* says:
>> +*
>> +* "If UseProgramStages is called with program set to zero or with a
>> +* program object that contains no executable code for the given
>> +* stages, it is as if the pipeline object has no programmable stage
>> +* configured for the indicated shader stages."
>> +*/
>> +   if ((stages & GL_VERTEX_SHADER_BIT) != 0)
>> +  _mesa_use_shader_program(ctx, GL_VERTEX_SHADER, shProg, pipe);
>> +
>> +   if ((stages & GL_FRAGMENT_SHADER_BIT) != 0)
>> +  _mesa_use_shader_program(ctx, GL_FRAGMENT_SHADER, shProg, pipe);
>> +
>> +   if ((stages & GL_GEOMETRY_SHADER_BIT) != 0)
>> +  _mesa_use_shader_program(ctx, GL_GEOMETRY_SHADER, shProg, pipe);
>>  }
> 
> The spec cite here doesn't seem to be explaining the code it's next to,
> to me.

That is fair... and at least partially my fault.  Greg's original
comment above this code was:

   /*
*  7.  What happens if you have a program object current for a shader stage,
*but the program object doesn't contain an executable for that stage?

*RESOLVED:  This is not an error; instead it is as though there were no
*program bound to that stage.  We have two different notions for
*programs bound to shader stages.  A program is "current" for a stage
*if it bound to that stage in the active program pipeline object.  A
*program is "active" for a stage if it is current and it has an
*executable for this stage.  In this case, the program would be current
*but not active.

*When no program is active for a stage, the stage will be replaced with
*fixed functionality logic (compatibility profile vertex and fragment),
*disabled (tessellation control and evaluation, geometry), or have
*undefined results (core profile vertex and fragment).
*/

Since the issues aren't captured in the core spec, I changed it to the
nearest thing I could find in the core spec.  Comparing the code with
the old comment and the new comment, I'm not very happy with it either.

How about:

   /* Enable individual stages from the program as requested b

Re: [Mesa-dev] [PATCH 5/6] mesa: Implement INTEL_performance_query.

[Ian Romanick]

On 03/12/2014 05:54 AM, Petri Latvala wrote:
> Using the existing driver hooks made for AMD_performance_monitor, implement
> INTEL_performance_query functions.
> 
> Signed-off-by: Petri Latvala 
> ---
>  src/mesa/main/performance_monitor.c | 476 
> +---
>  1 file changed, 439 insertions(+), 37 deletions(-)
> 
> diff --git a/src/mesa/main/performance_monitor.c 
> b/src/mesa/main/performance_monitor.c
> index 183a895..bf58d45 100644
> --- a/src/mesa/main/performance_monitor.c
> +++ b/src/mesa/main/performance_monitor.c
> @@ -137,6 +137,46 @@ get_counter(const struct gl_perf_monitor_group 
> *group_obj, GLuint id)
> return &group_obj->Counters[id];
>  }
>  
> +/* For INTEL_performance_query, query id 0 is reserved to be invalid. We use
> + * index to Groups array + 1 as the query id. Same applies to counter id.
> + */
> +static inline GLuint
> +queryid_to_index(GLuint queryid)
> +{
> +   return queryid - 1;
> +}
> +
> +static inline GLuint
> +index_to_queryid(GLuint index)
> +{
> +   return index + 1;
> +}
> +
> +static inline bool
> +queryid_valid(const struct gl_context *ctx, GLuint queryid)
> +{
> +   return get_group(ctx, queryid_to_index(queryid)) != NULL;
> +}
> +
> +static inline GLuint
> +counterid_to_index(GLuint counterid)
> +{
> +   return counterid - 1;
> +}
> +
> +static inline GLuint
> +index_to_counterid(GLuint index)
> +{
> +   return index + 1;
> +}
> +
> +static inline bool
> +counterid_valid(const struct gl_perf_monitor_group *group_obj,
> +GLuint counterid)
> +{
> +   return get_counter(group_obj, counterid_to_index(counterid)) != NULL;
> +}
> +
>  
> /*/
>  
>  void GLAPIENTRY
> @@ -645,19 +685,29 @@ _mesa_GetFirstPerfQueryIdINTEL(GLuint *queryId)
>  {
> GET_CURRENT_CONTEXT(ctx);
>  
> +   unsigned numGroups;
> +
> /* "If queryId pointer is equal to 0, INVALID_VALUE error is generated."
>  */
> if (!queryId) {
> -  _mesa_error(ctx, GL_INVALID_VALUE, "glGetFirstPerfQueryIdINTEL(queryId 
> == NULL)");
> +  _mesa_error(ctx, GL_INVALID_VALUE,
> +  "glGetFirstPerfQueryIdINTEL(queryId == NULL)");

The whitespace-only changes should go in the previous patch.  No reason
to add a line in one patch, then change the spacing in the next.

>return;
> }
>  
> +   numGroups = ctx->PerfMonitor.NumGroups;
> +
> /* "If the given hardware platform doesn't support any performance 
> queries,
>  * then the value of 0 is returned and INVALID_OPERATION error is raised."
>  */
> +   if (numGroups == 0) {
> +  *queryId = 0;
> +  _mesa_error(ctx, GL_INVALID_OPERATION,
> +  "glGetFirstPerfQueryIdINTEL(no queries supported)");
> +  return;
> +   }
>  
> -   *queryId = 0;
> -   _mesa_error(ctx, GL_INVALID_OPERATION, "glGetFirstPerfQueryIdINTEL(no 
> queries supported)");
> +   *queryId = index_to_queryid(0);
>  }
>  
>  extern void GLAPIENTRY
> @@ -667,22 +717,34 @@ _mesa_GetNextPerfQueryIdINTEL(GLuint queryId, GLuint 
> *nextQueryId)
>  
> /* "If nextQueryId pointer is equal to 0, an INVALID_VALUE error is
>  * generated."
> -*
> -* "Whenever error is generated, the value of 0 is returned."
>  */
> if (!nextQueryId) {
> -  *nextQueryId = 0;
> -  _mesa_error(ctx, GL_INVALID_VALUE, 
> "glGetNextPerfQueryIdINTEL(nextQueryId == NULL)");
> +  _mesa_error(ctx, GL_INVALID_VALUE,
> +  "glGetNextPerfQueryIdINTEL(nextQueryId == NULL)");
>return;
> }
>  
> /* "If the specified performance
>  * query identifier is invalid then INVALID_VALUE error is generated."
> -*
> -* No queries are supported, so all queries are invalid.
>  */
> -   *nextQueryId = 0;
> -   _mesa_error(ctx, GL_INVALID_VALUE, "glGetNextPerfQueryIdINTEL(invalid 
> query)");
> +   if (!queryid_valid(ctx, queryId)) {
> +  /* "Whenever error is generated, the value of 0 is returned." */
> +  *nextQueryId = 0;
> +  _mesa_error(ctx, GL_INVALID_VALUE,
> +  "glGetNextPerfQueryIdINTEL(invalid query)");
> +  return;
> +   }
> +
> +   ++queryId;
> +
> +   /* "If query identified by queryId is the last query available the value 
> of
> +* 0 is returned."
> +*/
> +   if (!queryid_valid(ctx, queryId)) {
> +  *nextQueryId = 0;
> +   } else {
> +  *nextQueryId = queryId;
> +   }
>  }
>  
>  extern void GLAPIENTRY
> @@ -690,13 +752,36 @@ _mesa_GetPerfQueryIdByNameINTEL(char *queryName, GLuint 
> *queryId)
>  {
> GET_CURRENT_CONTEXT(ctx);
>  
> +   unsigned i;
> +
> +   /* "If queryName does not reference a valid query name, an INVALID_VALUE
> +* error is generated."
> +*/
> +   if (!queryName) {
> +  _mesa_error(ctx, GL_INVALID_VALUE,
> +  "glGetPerfQueryIdByNameINTEL(queryName == NULL)");
> +  return;
> +   }
> +
> +   /* The specification does not state that this produces an error. */
> +   if 

Re: [Mesa-dev] [PATCH 1/6] glsl: Optimize pow(x, 2) into x * x.

[Ian Romanick]

On 03/12/2014 01:29 AM, Erik Faye-Lund wrote:
> On Wed, Mar 12, 2014 at 1:32 AM, Eric Anholt  wrote:
>> Erik Faye-Lund  writes:
>>
>>> On Wed, Mar 12, 2014 at 12:00 AM, Eric Anholt  wrote:
 Erik Faye-Lund  writes:

> On Tue, Mar 11, 2014 at 7:27 PM, Eric Anholt  wrote:
>> Erik Faye-Lund  writes:
>>
>>> On Tue, Mar 11, 2014 at 2:50 PM, Erik Faye-Lund  
>>> wrote:
 On Mon, Mar 10, 2014 at 11:54 PM, Matt Turner  
 wrote:
> Cuts two instructions out of SynMark's Gl32VSInstancing benchmark.
> ---
>  src/glsl/opt_algebraic.cpp | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
> index 5c49a78..8494bd9 100644
> --- a/src/glsl/opt_algebraic.cpp
> +++ b/src/glsl/opt_algebraic.cpp
> @@ -528,6 +528,14 @@ 
> ir_algebraic_visitor::handle_expression(ir_expression *ir)
>if (is_vec_two(op_const[0]))
>   return expr(ir_unop_exp2, ir->operands[1]);
>
> +  if (is_vec_two(op_const[1])) {
> + ir_variable *x = new(ir) ir_variable(ir->operands[1]->type, 
> "x",
> +  ir_var_temporary);
> + base_ir->insert_before(x);
> + base_ir->insert_before(assign(x, ir->operands[0]));
> + return mul(x, x);
> +  }
> +

 Is this safe? Since many GPUs implement pow(x, y) as exp2(log2(x) *
 y), this will give different results for if y comes from a uniform vs
 if it's a constant, no?
>>
>> Yes, but that wouldn't be covered by the "invariant" keyword.
>>
>>> To be a bit more clear: I don't think this is valid for expressions
>>> writing to variables marked as invariant (or expressions taking part
>>> in the calculations that leads up to invariant variable writes).
>>>
>>> I can't find anything allowing variance like this in the invariance
>>> section of the GLSL 3.30 specifications. In particular, the list
>>> following "To guarantee invariance of a particular output variable
>>> across two programs, the following must also be true" doesn't seem to
>>> require the values to be passed from the same source, only that the
>>> same values are passed. And in this case, the value 2.0 is usually
>>> exactly representable no matter what path it took there.
>>>
>>> Perhaps I'm being a bit too pedantic here, though.
>>
>> This file would do the same thing on the same expression tree in two
>> different programs, so "invariant" is fine (we've probably got other
>> problems with invariant, though).  The keyword you're probably thinking
>> of is "precise", which isn't in GLSL we implement yet.
>
> Are you saying that this only rewrites "x = pow(y, 2.0)" and not
> "const float e = 2.0; x = pow(y, e);"? If so, my point is moot,
> indeed. But if that's *not* the case, then I think we're in trouble
> still.

 The second would also get rewritten, because other passes will move the
 2.0 into the pow.

 I thought I understood your objection, but now I don't.  I think you'll
 have to lay out the pair of shaders involving the invariant keyword that
 you think that would be broken by this pass.
>>>
>>> My understanding is that
>>> ---8<---
>>> invariant varying float v;
>>> attribute float a;
>>> const float e = 2.0;
>>> void main()
>>> {
>>> v = pow(a, e);
>>> }
>>> ---8<---
>>> and
>>> ---8<---
>>> invariant varying float v;
>>> attribute float a;
>>> uniform float e;
>>> void main()
>>> {
>>> v = pow(a, e);
>>> }
>>> ---8<---
>>> ...should produce the exact same result, as long as the latter is
>>> passed 2.0 as the uniform e.
>>>
>>> Because v is marked as invariant, the expressions writing to v are the
>>> same, and the values passed in are the same.
>>>
>>> If we rewrite the first one to do "a * a", we get a different result
>>> on implementations that do "exp2(log2(a) * 2.0)" for the latter, due
>>> to floating-point normalization in the intermediate steps.
>>
>> I don't think that's what the spec authors intended from the keyword.  I
>> think what they intended was that if you had uniform float e in both
>> cases, but different code for setting *other* lvalues, that you'd still
>> get the same result in v.
> 
> I think that *might* be correct, but it doesn't seem to be what's
> actually defined. Invariance comes from ESSL, and FWIW, I was one of
> the spec authors in this case. But I don't remember the details down
> to this level, and the spec doesn't seem to clarify either.
> 
> However, since constant expressions are given some slack wrt how it's
> evaluated, I'm inclined to think that you're right about the spirit of
> the spec.
> 
> AFAIR, we introduced "invariant" to get rid of ftransform (since we'd
> alr

[Mesa-dev] [PATCH 01/13] glx: remove unused __glXClientInfo()

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/glx/glxcmds.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index 4b17d7c..7984715 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -1378,19 +1378,6 @@ glXQueryServerString(Display * dpy, int screen, int name)
return *str;
 }
 
-void
-__glXClientInfo(Display * dpy, int opcode)
-{
-   char *ext_str = __glXGetClientGLExtensionString();
-   int size = strlen(ext_str) + 1;
-
-   xcb_connection_t *c = XGetXCBConnection(dpy);
-   xcb_glx_client_info(c,
-   GLX_MAJOR_VERSION, GLX_MINOR_VERSION, size, ext_str);
-
-   free(ext_str);
-}
-
 
 /*
 ** EXT_import_context
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 03/13] mesa: Prevent negative indexing on noise2, noise3 and noise4

[Juha-Pekka Heikkila]

% operator could return negative value which would cause
indexing before perm table. Change %256 to &0xff

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/program/prog_noise.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/mesa/program/prog_noise.c b/src/mesa/program/prog_noise.c
index c258c5e..ac920c2 100644
--- a/src/mesa/program/prog_noise.c
+++ b/src/mesa/program/prog_noise.c
@@ -257,12 +257,12 @@ _mesa_noise2(GLfloat x, GLfloat y)
float y0 = y - Y0;
 
float x1, y1, x2, y2;
-   int ii, jj;
+   unsigned int ii, jj;
float t0, t1, t2;
 
/* For the 2D case, the simplex shape is an equilateral triangle. */
/* Determine which simplex we are in. */
-   int i1, j1;  /* Offsets for second (middle) corner of 
simplex in (i,j) coords */
+   unsigned int i1, j1; /* Offsets for second (middle) corner of 
simplex in (i,j) coords */
if (x0 > y0) {
   i1 = 1;
   j1 = 0;
@@ -282,8 +282,8 @@ _mesa_noise2(GLfloat x, GLfloat y)
y2 = y0 - 1.0f + 2.0f * G2;
 
/* Wrap the integer indices at 256, to avoid indexing perm[] out of bounds 
*/
-   ii = i % 256;
-   jj = j % 256;
+   ii = i & 0xff;
+   jj = j & 0xff;
 
/* Calculate the contribution from the three corners */
t0 = 0.5f - x0 * x0 - y0 * y0;
@@ -344,13 +344,13 @@ _mesa_noise3(GLfloat x, GLfloat y, GLfloat z)
float z0 = z - Z0;
 
float x1, y1, z1, x2, y2, z2, x3, y3, z3;
-   int ii, jj, kk;
+   unsigned int ii, jj, kk;
float t0, t1, t2, t3;
 
/* For the 3D case, the simplex shape is a slightly irregular tetrahedron. 
*/
/* Determine which simplex we are in. */
-   int i1, j1, k1;  /* Offsets for second corner of simplex in 
(i,j,k) coords */
-   int i2, j2, k2;  /* Offsets for third corner of simplex in 
(i,j,k) coords */
+   unsigned int i1, j1, k1; /* Offsets for second corner of simplex in 
(i,j,k) coords */
+   unsigned int i2, j2, k2; /* Offsets for third corner of simplex in 
(i,j,k) coords */
 
 /* This code would benefit from a backport from the GLSL version! */
if (x0 >= y0) {
@@ -423,9 +423,9 @@ _mesa_noise3(GLfloat x, GLfloat y, GLfloat z)
z3 = z0 - 1.0f + 3.0f * G3;
 
/* Wrap the integer indices at 256 to avoid indexing perm[] out of bounds */
-   ii = i % 256;
-   jj = j % 256;
-   kk = k % 256;
+   ii = i & 0xff;
+   jj = j & 0xff;
+   kk = k & 0xff;
 
/* Calculate the contribution from the four corners */
t0 = 0.6f - x0 * x0 - y0 * y0 - z0 * z0;
@@ -522,12 +522,12 @@ _mesa_noise4(GLfloat x, GLfloat y, GLfloat z, GLfloat w)
int c6 = (z0 > w0) ? 1 : 0;
int c = c1 + c2 + c3 + c4 + c5 + c6;
 
-   int i1, j1, k1, l1;  /* The integer offsets for the second simplex corner */
-   int i2, j2, k2, l2;  /* The integer offsets for the third simplex corner */
-   int i3, j3, k3, l3;  /* The integer offsets for the fourth simplex corner */
+   unsigned int i1, j1, k1, l1;  /* The integer offsets for the second simplex 
corner */
+   unsigned int i2, j2, k2, l2;  /* The integer offsets for the third simplex 
corner */
+   unsigned int i3, j3, k3, l3;  /* The integer offsets for the fourth simplex 
corner */
 
float x1, y1, z1, w1, x2, y2, z2, w2, x3, y3, z3, w3, x4, y4, z4, w4;
-   int ii, jj, kk, ll;
+   unsigned int ii, jj, kk, ll;
float t0, t1, t2, t3, t4;
 
/*
@@ -573,10 +573,10 @@ _mesa_noise4(GLfloat x, GLfloat y, GLfloat z, GLfloat w)
w4 = w0 - 1.0f + 4.0f * G4;
 
/* Wrap the integer indices at 256, to avoid indexing perm[] out of bounds 
*/
-   ii = i % 256;
-   jj = j % 256;
-   kk = k % 256;
-   ll = l % 256;
+   ii = i & 0xff;
+   jj = j & 0xff;
+   kk = k & 0xff;
+   ll = l & 0xff;
 
/* Calculate the contribution from the five corners */
t0 = 0.6f - x0 * x0 - y0 * y0 - z0 * z0 - w0 * w0;
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 04/13] mesa: Add missing null check in _mesa_parse_arb_program()

[Juha-Pekka Heikkila]

Add missing null check in program_parse.tab.c through
program_parse.y

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/program/program_parse.y | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/mesa/program/program_parse.y b/src/mesa/program/program_parse.y
index 6dde69d..1664740 100644
--- a/src/mesa/program/program_parse.y
+++ b/src/mesa/program/program_parse.y
@@ -2749,6 +2749,11 @@ _mesa_parse_arb_program(struct gl_context *ctx, GLenum 
target, const GLubyte *st
 */
state->prog->Instructions =
   _mesa_alloc_instructions(state->prog->NumInstructions + 1);
+
+   if (state->prog->Instructions == NULL) {
+  goto error;
+   }
+
inst = state->inst_head;
for (i = 0; i < state->prog->NumInstructions; i++) {
   struct asm_instruction *const temp = inst->next;
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 10/13] glsl: add missing null check in tfeedback_decl::init()

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/glsl/link_varyings.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index c925c00..226f012 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -290,7 +290,8 @@ tfeedback_decl::init(struct gl_context *ctx, const void 
*mem_ctx,
 * is converted from a float[8] to a vec4[2].
 */
if (ctx->ShaderCompilerOptions[MESA_SHADER_VERTEX].LowerClipDistance &&
-   strcmp(this->var_name, "gl_ClipDistance") == 0) {
+   this->var_name != NULL &&
+   strcmp(this->var_name, "gl_ClipDistance") == 0) {
   this->is_clip_distance_mesa = true;
}
 }
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 08/13] mesa: In emit_texenv() type mismatch was forced with typecast

[Juha-Pekka Heikkila]

Type mismatch caused random memory to be copied when casted
memory area was smaller than expected type.

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/main/ff_fragment_shader.cpp | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/mesa/main/ff_fragment_shader.cpp 
b/src/mesa/main/ff_fragment_shader.cpp
index cad67aa..3e00424 100644
--- a/src/mesa/main/ff_fragment_shader.cpp
+++ b/src/mesa/main/ff_fragment_shader.cpp
@@ -888,14 +888,15 @@ emit_texenv(texenv_fragment_program *p, GLuint unit)
 shift = new(p->mem_ctx) ir_constant((float)(1 << rgb_shift));
   }
   else {
-float const_data[4] = {
-   float(1 << rgb_shift),
-   float(1 << rgb_shift),
-   float(1 << rgb_shift),
-   float(1 << alpha_shift)
-};
-shift = new(p->mem_ctx) ir_constant(glsl_type::vec4_type,
-(ir_constant_data *)const_data);
+ ir_constant_data const_data = { .f = {
+ float(1 << rgb_shift),
+ float(1 << rgb_shift),
+ float(1 << rgb_shift),
+ float(1 << alpha_shift)
+ } };
+
+ shift = new(p->mem_ctx) ir_constant(glsl_type::vec4_type,
+ &const_data);
   }
 
   return saturate(mul(deref, shift));
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 13/13] mesa: add null checks in symbol_table.c

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/program/symbol_table.c | 33 ++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/mesa/program/symbol_table.c b/src/mesa/program/symbol_table.c
index 9462978..9c3556a 100644
--- a/src/mesa/program/symbol_table.c
+++ b/src/mesa/program/symbol_table.c
@@ -172,9 +172,14 @@ _mesa_symbol_table_push_scope(struct _mesa_symbol_table 
*table)
 {
 struct scope_level *const scope = calloc(1, sizeof(*scope));
 
-scope->next = table->current_scope;
-table->current_scope = scope;
-table->depth++;
+if (scope) {
+   scope->next = table->current_scope;
+   table->current_scope = scope;
+   table->depth++;
+}
+else {
+   _mesa_error_no_memory(__FUNCTION__);
+}
 }
 
 
@@ -254,6 +259,12 @@ _mesa_symbol_table_add_symbol(struct _mesa_symbol_table 
*table,
 
 if (hdr == NULL) {
hdr = calloc(1, sizeof(*hdr));
+
+   if (hdr == NULL) {
+  _mesa_error_no_memory(__FUNCTION__);
+  return -1;
+   }
+
hdr->name = strdup(name);
 
hash_table_insert(table->ht, hdr, hdr->name);
@@ -276,6 +287,12 @@ _mesa_symbol_table_add_symbol(struct _mesa_symbol_table 
*table,
return -1;
 
 sym = calloc(1, sizeof(*sym));
+
+if (sym == NULL) {
+   _mesa_error_no_memory(__FUNCTION__);
+   return -1;
+}
+
 sym->next_with_same_name = hdr->symbols;
 sym->next_with_same_scope = table->current_scope->symbols;
 sym->hdr = hdr;
@@ -311,6 +328,11 @@ _mesa_symbol_table_add_global_symbol(struct 
_mesa_symbol_table *table,
 
 if (hdr == NULL) {
 hdr = calloc(1, sizeof(*hdr));
+if (hdr == NULL) {
+   _mesa_error_no_memory(__FUNCTION__);
+   return -1;
+}
+
 hdr->name = strdup(name);
 
 hash_table_insert(table->ht, hdr, hdr->name);
@@ -340,6 +362,11 @@ _mesa_symbol_table_add_global_symbol(struct 
_mesa_symbol_table *table,
 }
 
 sym = calloc(1, sizeof(*sym));
+if (sym == NULL) {
+   _mesa_error_no_memory(__FUNCTION__);
+   return -1;
+}
+
 sym->next_with_same_scope = top_scope->symbols;
 sym->hdr = hdr;
 sym->name_space = name_space;
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 07/13] mesa: add extra null checks in vbo_rebase_prims()

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/vbo/vbo_rebase.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/mesa/vbo/vbo_rebase.c b/src/mesa/vbo/vbo_rebase.c
index f3fe5f7..7aa8d08 100644
--- a/src/mesa/vbo/vbo_rebase.c
+++ b/src/mesa/vbo/vbo_rebase.c
@@ -60,10 +60,15 @@ static void *rebase_##TYPE( const void *ptr,
\
 {  \
const TYPE *in = (TYPE *)ptr;   \
TYPE *tmp_indices = malloc(count * sizeof(TYPE));   \
-   GLuint i;   \
+   if (tmp_indices != NULL) {  \
+  GLuint i;\
\
-   for (i = 0; i < count; i++) \
-  tmp_indices[i] = in[i] - min_index;  \
+  for (i = 0; i < count; i++)  \
+ tmp_indices[i] = in[i] - min_index;   \
+   }   \
+   else {  \
+  _mesa_error_no_memory(__FUNCTION__); \
+   }   \
\
return (void *)tmp_indices; \
 }
@@ -148,6 +153,11 @@ void vbo_rebase_prims( struct gl_context *ctx,
*/
   tmp_prims = malloc(sizeof(*prim) * nr_prims);
 
+  if (tmp_prims == NULL) {
+ _mesa_error_no_memory(__FUNCTION__);
+ return;
+  }
+
   for (i = 0; i < nr_prims; i++) {
 tmp_prims[i] = prim[i];
 tmp_prims[i].basevertex -= min_index;
@@ -186,6 +196,10 @@ void vbo_rebase_prims( struct gl_context *ctx,
   if (map_ib) 
 ctx->Driver.UnmapBuffer(ctx, ib->obj, MAP_INTERNAL);
 
+  if (tmp_indices == NULL) {
+ return;
+  }
+
   tmp_ib.obj = ctx->Shared->NullBufferObj;
   tmp_ib.ptr = tmp_indices;
   tmp_ib.count = ib->count;
@@ -198,6 +212,11 @@ void vbo_rebase_prims( struct gl_context *ctx,
*/
   tmp_prims = malloc(sizeof(*prim) * nr_prims);
 
+  if (tmp_prims == NULL) {
+ _mesa_error_no_memory(__FUNCTION__);
+ return;
+  }
+
   for (i = 0; i < nr_prims; i++) {
 /* If this fails, it could indicate an application error:
  */
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 00/13] Klocwork patches

[Juha-Pekka Heikkila]

Most of these were on mesa-dev before, thank you again Brian and Ian for
commenting these earlier. I hope I did not miss any comment.

These depend on Ian's patch "mesa: Add _mesa_error_no_memory for logging 
out-of-memory messages" http://patchwork.freedesktop.org/patch/21355/

/Juha-Pekka

Juha-Pekka Heikkila (13):
  glx: remove unused __glXClientInfo()
  glx: add extra null check in getFBConfigs
  mesa: Prevent negative indexing on noise2, noise3 and noise4
  mesa: Add missing null check in _mesa_parse_arb_program()
  mesa: remove redundant running of check_symbol_table()
  mesa: add missing null checks in _tnl_register_fastpath()
  mesa: add extra null checks in vbo_rebase_prims()
  mesa: In emit_texenv() type mismatch was forced with typecast
  mesa: Add missing null checks into prog_hash_table.c
  glsl: add missing null check in tfeedback_decl::init()
  mesa: add missing null check in _mesa_NewHashTable()
  i965: check malloc return value in intel_resolve_map_set()
  mesa: add null checks in symbol_table.c

 src/glsl/link_varyings.cpp|  3 ++-
 src/glx/glxcmds.c | 13 -
 src/glx/glxext.c  |  4 +++
 src/mesa/drivers/dri/i965/intel_resolve_map.c |  7 +
 src/mesa/main/ff_fragment_shader.cpp  | 17 ++--
 src/mesa/main/hash.c  |  5 
 src/mesa/program/prog_hash_table.c| 23 +++-
 src/mesa/program/prog_noise.c | 36 -
 src/mesa/program/program_parse.y  |  5 
 src/mesa/program/symbol_table.c   | 39 +++
 src/mesa/tnl/t_vertex.c   | 14 --
 src/mesa/vbo/vbo_rebase.c | 25 ++---
 12 files changed, 135 insertions(+), 56 deletions(-)

-- 
1.8.1.2

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

[Mesa-dev] [PATCH 09/13] mesa: Add missing null checks into prog_hash_table.c

[Juha-Pekka Heikkila]

Check calloc return values in hash_table_insert() and
hash_table_replace()

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/program/prog_hash_table.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/mesa/program/prog_hash_table.c 
b/src/mesa/program/prog_hash_table.c
index f45ed46..617bdaa 100644
--- a/src/mesa/program/prog_hash_table.c
+++ b/src/mesa/program/prog_hash_table.c
@@ -143,10 +143,15 @@ hash_table_insert(struct hash_table *ht, void *data, 
const void *key)
 
 node = calloc(1, sizeof(*node));
 
-node->data = data;
-node->key = key;
+if (node != NULL) {
+   node->data = data;
+   node->key = key;
 
-insert_at_head(& ht->buckets[bucket], & node->link);
+   insert_at_head(& ht->buckets[bucket], & node->link);
+}
+else {
+   _mesa_error_no_memory(__FUNCTION__);
+}
 }
 
 bool
@@ -168,10 +173,16 @@ hash_table_replace(struct hash_table *ht, void *data, 
const void *key)
 
 hn = calloc(1, sizeof(*hn));
 
-hn->data = data;
-hn->key = key;
+if (hn != NULL) {
+   hn->data = data;
+   hn->key = key;
+
+   insert_at_head(& ht->buckets[bucket], & hn->link);
+}
+else {
+   _mesa_error_no_memory(__FUNCTION__);
+}
 
-insert_at_head(& ht->buckets[bucket], & hn->link);
 return false;
 }
 
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 11/13] mesa: add missing null check in _mesa_NewHashTable()

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/main/hash.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/mesa/main/hash.c b/src/mesa/main/hash.c
index 4c92005..592ef4d 100644
--- a/src/mesa/main/hash.c
+++ b/src/mesa/main/hash.c
@@ -115,6 +115,11 @@ _mesa_NewHashTable(void)
 
if (table) {
   table->ht = _mesa_hash_table_create(NULL, uint_key_compare);
+  if (table->ht == NULL) {
+ FREE(table);
+ return NULL;
+  }
+
   _mesa_hash_table_set_deleted_key(table->ht, uint_key(DELETED_KEY_VALUE));
   mtx_init(&table->Mutex, mtx_plain);
   mtx_init(&table->WalkMutex, mtx_plain);
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 12/13] i965: check malloc return value in intel_resolve_map_set()

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/drivers/dri/i965/intel_resolve_map.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_resolve_map.c 
b/src/mesa/drivers/dri/i965/intel_resolve_map.c
index 04b5c94..a338c5e 100644
--- a/src/mesa/drivers/dri/i965/intel_resolve_map.c
+++ b/src/mesa/drivers/dri/i965/intel_resolve_map.c
@@ -22,6 +22,7 @@
  */
 
 #include "intel_resolve_map.h"
+#include "main/imports.h"
 
 #include 
 #include 
@@ -51,6 +52,12 @@ intel_resolve_map_set(struct intel_resolve_map *head,
}
 
*tail = malloc(sizeof(**tail));
+
+   if (*tail == NULL) {
+  _mesa_error_no_memory(__FUNCTION__);
+  return;
+   }
+
(*tail)->prev = prev;
(*tail)->next = NULL;
(*tail)->level = level;
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 02/13] glx: add extra null check in getFBConfigs

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/glx/glxext.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/glx/glxext.c b/src/glx/glxext.c
index 4a195bd..0838cd1 100644
--- a/src/glx/glxext.c
+++ b/src/glx/glxext.c
@@ -677,6 +677,10 @@ static GLboolean
psc->serverGLXexts =
   __glXQueryServerString(dpy, priv->majorOpcode, screen, GLX_EXTENSIONS);
 
+   if (psc->serverGLXexts == NULL) {
+  return GL_FALSE;
+   }
+
LockDisplay(dpy);
 
psc->configs = NULL;
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 05/13] mesa: remove redundant running of check_symbol_table()

[Juha-Pekka Heikkila]

Nested for loops running through tables against which they
finally do an assert were ran also with optimized builds.

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/program/symbol_table.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/mesa/program/symbol_table.c b/src/mesa/program/symbol_table.c
index 2f41322..9462978 100644
--- a/src/mesa/program/symbol_table.c
+++ b/src/mesa/program/symbol_table.c
@@ -115,7 +115,7 @@ struct _mesa_symbol_table {
 static void
 check_symbol_table(struct _mesa_symbol_table *table)
 {
-#if 1
+#if !defined(NDEBUG)
 struct scope_level *scope;
 
 for (scope = table->current_scope; scope != NULL; scope = scope->next) {
@@ -134,7 +134,9 @@ check_symbol_table(struct _mesa_symbol_table *table)
 }
 }
 }
-#endif
+#else
+(void) table;
+#endif /* !defined(NDEBUG) */
 }
 
 void
-- 
1.8.1.2

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

[Mesa-dev] [PATCH 06/13] mesa: add missing null checks in _tnl_register_fastpath()

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/mesa/tnl/t_vertex.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/mesa/tnl/t_vertex.c b/src/mesa/tnl/t_vertex.c
index b3deac0..5cdf743 100644
--- a/src/mesa/tnl/t_vertex.c
+++ b/src/mesa/tnl/t_vertex.c
@@ -83,12 +83,22 @@ void _tnl_register_fastpath( struct tnl_clipspace *vtx,
struct tnl_clipspace_fastpath *fastpath = 
CALLOC_STRUCT(tnl_clipspace_fastpath);
GLuint i;
 
+   if (fastpath == NULL) {
+  _mesa_error_no_memory(__FUNCTION__);
+  return;
+   }
+
fastpath->vertex_size = vtx->vertex_size;
fastpath->attr_count = vtx->attr_count;
fastpath->match_strides = match_strides;
fastpath->func = vtx->emit;
-   fastpath->attr =
-  malloc(vtx->attr_count * sizeof(fastpath->attr[0]));
+   fastpath->attr = malloc(vtx->attr_count * sizeof(fastpath->attr[0]));
+
+   if (fastpath->attr == NULL) {
+  FREE(fastpath);
+  _mesa_error_no_memory(__FUNCTION__);
+  return;
+   }
 
for (i = 0; i < vtx->attr_count; i++) {
   fastpath->attr[i].format = vtx->attr[i].format;
-- 
1.8.1.2

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

Re: [Mesa-dev] [PATCH 05/13] mesa: remove redundant running of check_symbol_table()

[Matt Turner]

On Wed, Mar 12, 2014 at 2:11 PM, Juha-Pekka Heikkila
 wrote:
> Nested for loops running through tables against which they
> finally do an assert were ran also with optimized builds.
>
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/mesa/program/symbol_table.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/program/symbol_table.c b/src/mesa/program/symbol_table.c
> index 2f41322..9462978 100644
> --- a/src/mesa/program/symbol_table.c
> +++ b/src/mesa/program/symbol_table.c
> @@ -115,7 +115,7 @@ struct _mesa_symbol_table {
>  static void
>  check_symbol_table(struct _mesa_symbol_table *table)
>  {
> -#if 1
> +#if !defined(NDEBUG)

Probably use #ifndef?

>  struct scope_level *scope;
>
>  for (scope = table->current_scope; scope != NULL; scope = scope->next) {
> @@ -134,7 +134,9 @@ check_symbol_table(struct _mesa_symbol_table *table)
>  }
>  }
>  }
> -#endif
> +#else
> +(void) table;
> +#endif /* !defined(NDEBUG) */
>  }
>
>  void
> --
> 1.8.1.2

I would think the compiler would be able to recognize this function
does nothing in optimized builds and throw it out, since the assert()
is a no-op.

I guess it doesn't hurt to do this though.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/7] EGL_MESA_configless_context

[Kristian Høgsberg]

On Fri, Mar 7, 2014 at 10:05 AM, Neil Roberts  wrote:
> Here is a series of patches to add an extension which makes it
> possible to create an EGL context without specifying a config. A
> context created in this way can be bound with any surface using the
> same EGLDisplay rather than being restricted to those using the same
> config. The main use case is that it would then be possible for a
> Wayland compositor to drive two displays which may have different
> bit-depths without having to wastefully create two contexts.
>
> Mesa seems to mostly cope with this already so the patches are mostly
> just to add the extension and remove the restriction.
>
> There is also a patch for Piglit to try and test the extension. I
> tried running the branch though the 'all tests' test suite and apart
> from this new test now passing there was only one difference which is
> that the GLX_OML_sync_control/swapbuffersmsc-divisor-zero test failed.
> However if I run piglit-run.py again and set a regexp so it only runs
> that one test then it passes so I'm wondering if it might just be an
> intermittent failure.
>
> The main thorny issue with the extension is how to handle the initial
> value of glDrawBuffer when a configless context is used. If a config
> is used then we can just say the default is GL_BACK if the config is
> double-buffered and GL_FRONT otherwise. I have taken the approach that
> this decision is made the first time the context is bound rather than
> when it is first created. There should be no way to query the value of
> glDrawBuffer until the context is first bound so it shouldn't cause
> any harm that Mesa changes the value at that point. I think this is
> worth doing for the convenience to the application which would
> otherwise have to remember to explicitly set it to GL_BACK in the
> common case that double-buffering is used.
>
> I've also included a couple of patches to take advantage of the
> extension in Weston.

Thanks Neil, series applied to mesa and weston.

Kristian

> Regards,
> - Neil
> ___
> 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 07/13] mesa: add extra null checks in vbo_rebase_prims()

[Matt Turner]

On Wed, Mar 12, 2014 at 2:11 PM, Juha-Pekka Heikkila
 wrote:
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/mesa/vbo/vbo_rebase.c | 25 ++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_rebase.c b/src/mesa/vbo/vbo_rebase.c
> index f3fe5f7..7aa8d08 100644
> --- a/src/mesa/vbo/vbo_rebase.c
> +++ b/src/mesa/vbo/vbo_rebase.c
> @@ -60,10 +60,15 @@ static void *rebase_##TYPE( const void *ptr,  
>   \
>  {  \
> const TYPE *in = (TYPE *)ptr;   \
> TYPE *tmp_indices = malloc(count * sizeof(TYPE));   \
> -   GLuint i;   \
> +   if (tmp_indices != NULL) {  \
> +  GLuint i;\
> \
> -   for (i = 0; i < count; i++) \
> -  tmp_indices[i] = in[i] - min_index;  \
> +  for (i = 0; i < count; i++)  \
> + tmp_indices[i] = in[i] - min_index;   \
> +   }   \
> +   else {  \
> +  _mesa_error_no_memory(__FUNCTION__); \
> +   }   \

I'd probably check malloc's return value and return NULL if it failed
and avoid indenting the for loop.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/13] mesa: In emit_texenv() type mismatch was forced with typecast

[Matt Turner]

On Wed, Mar 12, 2014 at 2:11 PM, Juha-Pekka Heikkila
 wrote:
> Type mismatch caused random memory to be copied when casted
> memory area was smaller than expected type.
>
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/mesa/main/ff_fragment_shader.cpp | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/ff_fragment_shader.cpp 
> b/src/mesa/main/ff_fragment_shader.cpp
> index cad67aa..3e00424 100644
> --- a/src/mesa/main/ff_fragment_shader.cpp
> +++ b/src/mesa/main/ff_fragment_shader.cpp
> @@ -888,14 +888,15 @@ emit_texenv(texenv_fragment_program *p, GLuint unit)
>  shift = new(p->mem_ctx) ir_constant((float)(1 << rgb_shift));
>}
>else {
> -float const_data[4] = {
> -   float(1 << rgb_shift),
> -   float(1 << rgb_shift),
> -   float(1 << rgb_shift),
> -   float(1 << alpha_shift)
> -};
> -shift = new(p->mem_ctx) ir_constant(glsl_type::vec4_type,
> -(ir_constant_data *)const_data);
> + ir_constant_data const_data = { .f = {
> + float(1 << rgb_shift),
> + float(1 << rgb_shift),
> + float(1 << rgb_shift),
> + float(1 << alpha_shift)
> + } };

Huh. I'm confused. I thought C++ didn't allow designated initializers.

gcc accepts it here, and only warns with -pedantic:

warning: ISO C++ does not allow C99 designated initializers [-Wpedantic]

Maybe it's just that MSVC doesn't support them? Google is telling me
that the only way to initialize a union in C++ is by initializing its
first field.

In the case that we can actually use designated initializers, Let's
indent this like so:

 ir_constant_data const_data = {
.f = {
   float(1 << rgb_shift),
   float(1 << rgb_shift),
   float(1 << rgb_shift),
   float(1 << alpha_shift)
}
 };

(and hope gmail doesn't mess that up.)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 75797] EGL application crashes with BadDrawable at SwapBuffers

[bugzilla-daemon]

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

Chad Versace  changed:

   What|Removed |Added

 CC||chad.vers...@linux.intel.co
   ||m

-- 
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 09/13] mesa: Add missing null checks into prog_hash_table.c

[Matt Turner]

On Wed, Mar 12, 2014 at 2:11 PM, Juha-Pekka Heikkila
 wrote:
> Check calloc return values in hash_table_insert() and
> hash_table_replace()
>
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/mesa/program/prog_hash_table.c | 23 +--
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/program/prog_hash_table.c 
> b/src/mesa/program/prog_hash_table.c
> index f45ed46..617bdaa 100644
> --- a/src/mesa/program/prog_hash_table.c
> +++ b/src/mesa/program/prog_hash_table.c
> @@ -143,10 +143,15 @@ hash_table_insert(struct hash_table *ht, void *data, 
> const void *key)
>
>  node = calloc(1, sizeof(*node));
>
> -node->data = data;
> -node->key = key;
> +if (node != NULL) {
> +   node->data = data;
> +   node->key = key;
>
> -insert_at_head(& ht->buckets[bucket], & node->link);
> +   insert_at_head(& ht->buckets[bucket], & node->link);
> +}
> +else {
> +   _mesa_error_no_memory(__FUNCTION__);
> +}
>  }

Let's just check calloc's return value, and if it's NULL call
_mesa_error_no_memory and return. That way we don't have to touch the
other code in the function. Same comment for the other hunk.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 74563] Surfaceless contexts are not properly released by DRI drivers

[bugzilla-daemon]

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

Chad Versace  changed:

   What|Removed |Added

 CC||chad.vers...@linux.intel.co
   ||m

-- 
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 10/13] glsl: add missing null check in tfeedback_decl::init()

[Matt Turner]

On Wed, Mar 12, 2014 at 2:11 PM, Juha-Pekka Heikkila
 wrote:
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/glsl/link_varyings.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index c925c00..226f012 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -290,7 +290,8 @@ tfeedback_decl::init(struct gl_context *ctx, const void 
> *mem_ctx,
>  * is converted from a float[8] to a vec4[2].
>  */
> if (ctx->ShaderCompilerOptions[MESA_SHADER_VERTEX].LowerClipDistance &&
> -   strcmp(this->var_name, "gl_ClipDistance") == 0) {
> +   this->var_name != NULL &&
> +   strcmp(this->var_name, "gl_ClipDistance") == 0) {
>this->is_clip_distance_mesa = true;
> }
>  }
> --
> 1.8.1.2

Huh. I don't know what the right thing to do if ralloc_strndup fails here.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/13] Klocwork patches

[Matt Turner]

1, 3-7, and 9 are

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

Re: [Mesa-dev] static version of osmesa is missing llvm symbols, bug?

[Burlen Loring]

Guys, Thanks for the feedback and for continued support of static 
linking. I will file the bug reports.


Burlen

On 03/12/2014 06:30 AM, Brian Paul wrote:

On 03/11/2014 06:48 PM, Emil Velikov wrote:

On 12/03/14 00:08, Burlen Loring wrote:

yep, I'm using 10.1.0 and also noticed the same in 9.2.2.


In that case "it wasn't me" ;-)


in short static linking is essential for reasonable performance of
parallel applications on Cray systems. In this scenario the application
is duplicated across 100s/1000s of compute nodes which share a
remote-parallel file system (Lustre/GPFS). As a dynamically linked
application starts, shared library loading hit's the file system hard.
However, Cray suggests users run static linked executables. Their
application launcher bypasses the file system and copies statically
linked executable directly into a ram fs onto the set of compute nodes
that a given job is scheduled to run on. This avoids file system
contention that would be introduced by a parallel application's shared
library loading.

I maintain and support ParaView and interactive OpenGL based rendering
application on NERSC's Crays. Given that this is an interactive
application, having a reasonable startup time is important. Users have
complained and I've observed times when using shared libraries when app
startup takes more than 30 min! if we build fully static executable
startup time is reduced to seconds.

For me statically linked mesa is important, which is why I want to
report this. Without OSMesa we can't use these systems, and the new
llvmpipe OSMesa state tracker has been really awesome, allowing us 
to do

things we couldn't in the past without GPUs. I hope that you don't
discontinue static link support.


Had some additional patches that cleans up mesa's build a bit further
but I guess I'll bring back (classic and gallium) static osmesa first.
Especially considering the numbers you've quoted.


We have experienced some segfaults in llvmpipe OSMesa in the static
linked version (bt shows SEGV at
create_jit_texture_type.isra.1@draw_llvm.c:128). If I can reproduce 
this

off the Cray I will file a bug report. Thought I'd mention in case you
can confirm that there is a known issue with the static link.


AFAIK static linking with llvm is a bit of a hit and miss. For that
reason mesa 10.2 may be building against shared llvm. Then again I do
not work on that part of mesa, so take this with a pinch of salt.

Brian Paul is the only person that I've seen working/bug fixing osmesa
so I'm guessing that he'll take a look in due time. Meanwhile it may be
worth filing bug report(s) for these two boogers :-)


Yes, please file bugs with as much info as possible.  I don't think I 
can help with the static build issue (my autocon/make knowledge is 
pretty thin).  I'd be happy to look into the LLVM segfault though 
(when I have time).


-Brian

___
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 6/8] glsl: Remove ir_dereference::constant_referenced

[Ian Romanick]

From: Ian Romanick 

All of the functionality is implemented in a private function in the one
file where it is used.

Signed-off-by: Ian Romanick 
---
 src/glsl/ir.h   | 36 
 src/glsl/ir_constant_expression.cpp | 25 -
 2 files changed, 61 deletions(-)

diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index ed3f086..8fa3b9e 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -1914,15 +1914,6 @@ public:
 * Get the variable that is ultimately referenced by an r-value
 */
virtual ir_variable *variable_referenced() const = 0;
-
-   /**
-* Get the constant that is ultimately referenced by an r-value,
-* in a constant expression evaluation context.
-*
-* The offset is used when the reference is to a specific column of
-* a matrix.
-*/
-  virtual void constant_referenced(struct hash_table *variable_context, 
ir_constant *&store, int &offset) const = 0;
 };
 
 
@@ -1950,15 +1941,6 @@ public:
   return this->var;
}
 
-   /**
-* Get the constant that is ultimately referenced by an r-value,
-* in a constant expression evaluation context.
-*
-* The offset is used when the reference is to a specific column of
-* a matrix.
-*/
-   virtual void constant_referenced(struct hash_table *variable_context, 
ir_constant *&store, int &offset) const;
-
virtual ir_variable *whole_variable_referenced()
{
   /* ir_dereference_variable objects always dereference the entire
@@ -2010,15 +1992,6 @@ public:
   return this->array->variable_referenced();
}
 
-   /**
-* Get the constant that is ultimately referenced by an r-value,
-* in a constant expression evaluation context.
-*
-* The offset is used when the reference is to a specific column of
-* a matrix.
-*/
-   virtual void constant_referenced(struct hash_table *variable_context, 
ir_constant *&store, int &offset) const;
-
virtual void accept(ir_visitor *v)
{
   v->visit(this);
@@ -2058,15 +2031,6 @@ public:
   return this->record->variable_referenced();
}
 
-   /**
-* Get the constant that is ultimately referenced by an r-value,
-* in a constant expression evaluation context.
-*
-* The offset is used when the reference is to a specific column of
-* a matrix.
-*/
-   virtual void constant_referenced(struct hash_table *variable_context, 
ir_constant *&store, int &offset) const;
-
virtual void accept(ir_visitor *v)
{
   v->visit(this);
diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index 53db618..ea9e84a 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -387,14 +387,11 @@ unpack_half_1x16(uint16_t u)
 }
 
 /**
- * \name Functions to get the constant referenced by an r-value
- *
  * Get the constant that is ultimately referenced by an r-value, in a constant
  * expression evaluation context.
  *
  * The offset is used when the reference is to a specific column of a matrix.
  */
-/*@{*/
 static bool
 constant_referenced(const ir_dereference *deref,
 struct hash_table *variable_context,
@@ -489,28 +486,6 @@ constant_referenced(const ir_dereference *deref,
return store != NULL;
 }
 
-void
-ir_dereference_variable::constant_referenced(struct hash_table 
*variable_context,
-ir_constant *&store, int &offset) 
const
-{
-   ::constant_referenced(this, variable_context, store, offset);
-}
-
-void
-ir_dereference_array::constant_referenced(struct hash_table *variable_context,
- ir_constant *&store, int &offset) 
const
-{
-   ::constant_referenced(this, variable_context, store, offset);
-}
-
-void
-ir_dereference_record::constant_referenced(struct hash_table *variable_context,
-  ir_constant *&store, int &offset) 
const
-{
-   ::constant_referenced(this, variable_context, store, offset);
-}
-/*@}*/
-
 
 ir_constant *
 ir_rvalue::constant_expression_value(struct hash_table *variable_context)
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 0/8]

[Ian Romanick]

The goal of this series is to get constant_referenced out of the public
interface of the ir_dereference class hierarchy.  It has always seemed
odd to me that an interface that's specific to ir_constant_expression is
generally exposed.

In addition, having it live in that class hierarchy makes some other IR
work I'm doing (that may yet prove to be fruitless) more difficult.

 src/glsl/ir.h   |  36 
 src/glsl/ir_clone.cpp   |  10 ++--
 src/glsl/ir_constant_expression.cpp | 205 
+++--
 3 files changed, 110 insertions(+), 141 deletions(-)

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

[Mesa-dev] [PATCH 3/8] glsl: Fold implementation of ir_dereference_variable::constant_referenced into wrapper

[Ian Romanick]

From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index 4149a0e..c013dfd 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -400,6 +400,12 @@ constant_referenced(const ir_dereference *deref,
 struct hash_table *variable_context,
 ir_constant *&store, int &offset)
 {
+   store = NULL;
+   offset = 0;
+
+   if (variable_context == NULL)
+  return false;
+
switch (deref->ir_type) {
case ir_type_dereference_array:
   ((ir_dereference_array *) deref)->constant_referenced(variable_context,
@@ -411,15 +417,16 @@ constant_referenced(const ir_dereference *deref,
  store, offset);
   break;
 
-   case ir_type_dereference_variable:
-  ((ir_dereference_variable *) 
deref)->constant_referenced(variable_context,
-   store, offset);
+   case ir_type_dereference_variable: {
+  const ir_dereference_variable *const dv =
+ (const ir_dereference_variable *) deref;
+
+  store = (ir_constant *) hash_table_find(variable_context, dv->var);
   break;
+   }
 
default:
   assert(!"Should not get here.");
-  store = NULL;
-  offset = 0;
   break;
}
 
@@ -430,13 +437,7 @@ void
 ir_dereference_variable::constant_referenced(struct hash_table 
*variable_context,
 ir_constant *&store, int &offset) 
const
 {
-   if (variable_context) {
-  store = (ir_constant *)hash_table_find(variable_context, var);
-  offset = 0;
-   } else {
-  store = NULL;
-  offset = 0;
-   }
+   ::constant_referenced(this, variable_context, store, offset);
 }
 
 void
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 7/8] glsl: Minor clean ups in constant_referenced

[Ian Romanick]

From: Ian Romanick 

These could probably be squashed into one of the previous commits.

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index ea9e84a..e4f8a58 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -408,41 +408,36 @@ constant_referenced(const ir_dereference *deref,
   const ir_dereference_array *const da =
  (const ir_dereference_array *) deref;
 
-  ir_constant *index_c =
+  ir_constant *const index_c =
  da->array_index->constant_expression_value(variable_context);
 
   if (!index_c || !index_c->type->is_scalar() || 
!index_c->type->is_integer())
  break;
 
-  int index = index_c->type->base_type == GLSL_TYPE_INT ?
+  const int index = index_c->type->base_type == GLSL_TYPE_INT ?
  index_c->get_int_component(0) :
  index_c->get_uint_component(0);
 
   ir_constant *substore;
   int suboffset;
 
-  const ir_dereference *deref = da->array->as_dereference();
+  const ir_dereference *const deref = da->array->as_dereference();
   if (!deref)
  break;
 
   if (!constant_referenced(deref, variable_context, substore, suboffset))
  break;
 
-  const glsl_type *vt = da->array->type;
+  const glsl_type *const vt = da->array->type;
   if (vt->is_array()) {
  store = substore->get_array_element(index);
  offset = 0;
- break;
-  }
-  if (vt->is_matrix()) {
+  } else if (vt->is_matrix()) {
  store = substore;
  offset = index * vt->vector_elements;
- break;
-  }
-  if (vt->is_vector()) {
+  } else if (vt->is_vector()) {
  store = substore;
  offset = suboffset + index;
- break;
   }
 
   break;
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 4/8] glsl: Fold implementation of ir_dereference_record::constant_referenced into wrapper

[Ian Romanick]

From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 38 -
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index c013dfd..336ce17 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -412,10 +412,27 @@ constant_referenced(const ir_dereference *deref,
 store, offset);
   break;
 
-   case ir_type_dereference_record:
-  ((ir_dereference_record *) deref)->constant_referenced(variable_context,
- store, offset);
+   case ir_type_dereference_record: {
+  const ir_dereference_record *const dr =
+ (const ir_dereference_record *) deref;
+
+  const ir_dereference *const deref = dr->record->as_dereference();
+  if (!deref)
+ break;
+
+  ir_constant *substore;
+  int suboffset;
+
+  if (!constant_referenced(deref, variable_context, substore, suboffset))
+ break;
+
+  /* Since we're dropping it on the floor...
+   */
+  assert(suboffset == 0);
+
+  store = substore->get_record_field(dr->field);
   break;
+   }
 
case ir_type_dereference_variable: {
   const ir_dereference_variable *const dv =
@@ -493,20 +510,7 @@ void
 ir_dereference_record::constant_referenced(struct hash_table *variable_context,
   ir_constant *&store, int &offset) 
const
 {
-   ir_constant *substore;
-   int suboffset;
-   const ir_dereference *deref = record->as_dereference();
-   if (!deref) {
-  store = 0;
-  offset = 0;
-  return;
-   }
-
-   if (!::constant_referenced(deref, variable_context, substore, suboffset))
-  return;
-
-   store = substore->get_record_field(field);
-   offset = 0;
+   ::constant_referenced(this, variable_context, store, offset);
 }
 /*@}*/
 
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 5/8] glsl: Fold implementation of ir_dereference_array::constant_referenced into wrapper

[Ian Romanick]

From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 89 ++---
 1 file changed, 43 insertions(+), 46 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index 336ce17..53db618 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -407,10 +407,49 @@ constant_referenced(const ir_dereference *deref,
   return false;
 
switch (deref->ir_type) {
-   case ir_type_dereference_array:
-  ((ir_dereference_array *) deref)->constant_referenced(variable_context,
-store, offset);
+   case ir_type_dereference_array: {
+  const ir_dereference_array *const da =
+ (const ir_dereference_array *) deref;
+
+  ir_constant *index_c =
+ da->array_index->constant_expression_value(variable_context);
+
+  if (!index_c || !index_c->type->is_scalar() || 
!index_c->type->is_integer())
+ break;
+
+  int index = index_c->type->base_type == GLSL_TYPE_INT ?
+ index_c->get_int_component(0) :
+ index_c->get_uint_component(0);
+
+  ir_constant *substore;
+  int suboffset;
+
+  const ir_dereference *deref = da->array->as_dereference();
+  if (!deref)
+ break;
+
+  if (!constant_referenced(deref, variable_context, substore, suboffset))
+ break;
+
+  const glsl_type *vt = da->array->type;
+  if (vt->is_array()) {
+ store = substore->get_array_element(index);
+ offset = 0;
+ break;
+  }
+  if (vt->is_matrix()) {
+ store = substore;
+ offset = index * vt->vector_elements;
+ break;
+  }
+  if (vt->is_vector()) {
+ store = substore;
+ offset = suboffset + index;
+ break;
+  }
+
   break;
+   }
 
case ir_type_dereference_record: {
   const ir_dereference_record *const dr =
@@ -461,49 +500,7 @@ void
 ir_dereference_array::constant_referenced(struct hash_table *variable_context,
  ir_constant *&store, int &offset) 
const
 {
-   ir_constant *index_c = 
array_index->constant_expression_value(variable_context);
-
-   if (!index_c || !index_c->type->is_scalar() || 
!index_c->type->is_integer()) {
-  store = 0;
-  offset = 0;
-  return;
-   }
-
-   int index = index_c->type->base_type == GLSL_TYPE_INT ?
-  index_c->get_int_component(0) :
-  index_c->get_uint_component(0);
-
-   ir_constant *substore;
-   int suboffset;
-   const ir_dereference *deref = array->as_dereference();
-   if (!deref) {
-  store = 0;
-  offset = 0;
-  return;
-   }
-
-   if (!::constant_referenced(deref, variable_context, substore, suboffset))
-  return;
-
-   const glsl_type *vt = array->type;
-   if (vt->is_array()) {
-  store = substore->get_array_element(index);
-  offset = 0;
-  return;
-   }
-   if (vt->is_matrix()) {
-  store = substore;
-  offset = index * vt->vector_elements;
-  return;
-   }
-   if (vt->is_vector()) {
-  store = substore;
-  offset = suboffset + index;
-  return;
-   }
-
-   store = 0;
-   offset = 0;
+   ::constant_referenced(this, variable_context, store, offset);
 }
 
 void
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 2/8] glsl: Add wrapper function that calls ir_dereference::constant_referenced

[Ian Romanick]

From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 52 +
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index a31e579..4149a0e 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -395,6 +395,37 @@ unpack_half_1x16(uint16_t u)
  * The offset is used when the reference is to a specific column of a matrix.
  */
 /*@{*/
+static bool
+constant_referenced(const ir_dereference *deref,
+struct hash_table *variable_context,
+ir_constant *&store, int &offset)
+{
+   switch (deref->ir_type) {
+   case ir_type_dereference_array:
+  ((ir_dereference_array *) deref)->constant_referenced(variable_context,
+store, offset);
+  break;
+
+   case ir_type_dereference_record:
+  ((ir_dereference_record *) deref)->constant_referenced(variable_context,
+ store, offset);
+  break;
+
+   case ir_type_dereference_variable:
+  ((ir_dereference_variable *) 
deref)->constant_referenced(variable_context,
+   store, offset);
+  break;
+
+   default:
+  assert(!"Should not get here.");
+  store = NULL;
+  offset = 0;
+  break;
+   }
+
+   return store != NULL;
+}
+
 void
 ir_dereference_variable::constant_referenced(struct hash_table 
*variable_context,
 ir_constant *&store, int &offset) 
const
@@ -433,13 +464,8 @@ ir_dereference_array::constant_referenced(struct 
hash_table *variable_context,
   return;
}
 
-   deref->constant_referenced(variable_context, substore, suboffset);
-
-   if (!substore) {
-  store = 0;
-  offset = 0;
+   if (!::constant_referenced(deref, variable_context, substore, suboffset))
   return;
-   }
 
const glsl_type *vt = array->type;
if (vt->is_array()) {
@@ -475,13 +501,8 @@ ir_dereference_record::constant_referenced(struct 
hash_table *variable_context,
   return;
}
 
-   deref->constant_referenced(variable_context, substore, suboffset);
-
-   if (!substore) {
-  store = 0;
-  offset = 0;
+   if (!::constant_referenced(deref, variable_context, substore, suboffset))
   return;
-   }
 
store = substore->get_record_field(field);
offset = 0;
@@ -1814,9 +1835,8 @@ bool 
ir_function_signature::constant_expression_evaluate_expression_list(const s
 
 ir_constant *store = NULL;
 int offset = 0;
-asg->lhs->constant_referenced(variable_context, store, offset);
 
-if (!store)
+if (!constant_referenced(asg->lhs, variable_context, store, offset))
return false;
 
 ir_constant *value = 
asg->rhs->constant_expression_value(variable_context);
@@ -1847,9 +1867,9 @@ bool 
ir_function_signature::constant_expression_evaluate_expression_list(const s
 
 ir_constant *store = NULL;
 int offset = 0;
-call->return_deref->constant_referenced(variable_context, store, 
offset);
 
-if (!store)
+if (!constant_referenced(call->return_deref, variable_context,
+  store, offset))
return false;
 
 ir_constant *value = call->constant_expression_value(variable_context);
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 8/8] glsl: Clean up "unused parameter" warnings

[Ian Romanick]

From: Ian Romanick 

../../src/glsl/ir_constant_expression.cpp:486:1: warning: unused parameter 
'variable_context' [-Wunused-parameter]
../../src/glsl/ir_constant_expression.cpp:1633:1: warning: unused parameter 
'variable_context' [-Wunused-parameter]
../../src/glsl/ir_constant_expression.cpp:1752:1: warning: unused parameter 
'variable_context' [-Wunused-parameter]
../../src/glsl/ir_constant_expression.cpp:1761:1: warning: unused parameter 
'variable_context' [-Wunused-parameter]
../../src/glsl/ir_constant_expression.cpp:1769:1: warning: unused parameter 
'variable_context' [-Wunused-parameter]

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index e4f8a58..8afe8f7 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -483,7 +483,7 @@ constant_referenced(const ir_dereference *deref,
 
 
 ir_constant *
-ir_rvalue::constant_expression_value(struct hash_table *variable_context)
+ir_rvalue::constant_expression_value(struct hash_table *)
 {
assert(this->type->is_error());
return NULL;
@@ -1630,7 +1630,7 @@ ir_expression::constant_expression_value(struct 
hash_table *variable_context)
 
 
 ir_constant *
-ir_texture::constant_expression_value(struct hash_table *variable_context)
+ir_texture::constant_expression_value(struct hash_table *)
 {
/* texture lookups aren't constant expressions */
return NULL;
@@ -1749,7 +1749,7 @@ ir_dereference_array::constant_expression_value(struct 
hash_table *variable_cont
 
 
 ir_constant *
-ir_dereference_record::constant_expression_value(struct hash_table 
*variable_context)
+ir_dereference_record::constant_expression_value(struct hash_table *)
 {
ir_constant *v = this->record->constant_expression_value();
 
@@ -1758,7 +1758,7 @@ ir_dereference_record::constant_expression_value(struct 
hash_table *variable_con
 
 
 ir_constant *
-ir_assignment::constant_expression_value(struct hash_table *variable_context)
+ir_assignment::constant_expression_value(struct hash_table *)
 {
/* FINISHME: Handle CEs involving assignment (return RHS) */
return NULL;
@@ -1766,7 +1766,7 @@ ir_assignment::constant_expression_value(struct 
hash_table *variable_context)
 
 
 ir_constant *
-ir_constant::constant_expression_value(struct hash_table *variable_context)
+ir_constant::constant_expression_value(struct hash_table *)
 {
return this;
 }
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 1/8] glsl: Group all of the constant_referenced functions together

[Ian Romanick]

From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_clone.cpp   |  10 +-
 src/glsl/ir_constant_expression.cpp | 195 +++-
 2 files changed, 109 insertions(+), 96 deletions(-)

diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
index 311c992..167b80c 100644
--- a/src/glsl/ir_clone.cpp
+++ b/src/glsl/ir_clone.cpp
@@ -265,10 +265,12 @@ ir_assignment::clone(void *mem_ctx, struct hash_table 
*ht) const
if (this->condition)
   new_condition = this->condition->clone(mem_ctx, ht);
 
-   return new(mem_ctx) ir_assignment(this->lhs->clone(mem_ctx, ht),
-this->rhs->clone(mem_ctx, ht),
-new_condition,
-this->write_mask);
+   ir_assignment *cloned =
+  new(mem_ctx) ir_assignment(this->lhs->clone(mem_ctx, ht),
+ this->rhs->clone(mem_ctx, ht),
+ new_condition);
+   cloned->write_mask = this->write_mask;
+   return cloned;
 }
 
 ir_function *
diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index 7fa5a09..a31e579 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -386,6 +386,109 @@ unpack_half_1x16(uint16_t u)
return _mesa_half_to_float(u);
 }
 
+/**
+ * \name Functions to get the constant referenced by an r-value
+ *
+ * Get the constant that is ultimately referenced by an r-value, in a constant
+ * expression evaluation context.
+ *
+ * The offset is used when the reference is to a specific column of a matrix.
+ */
+/*@{*/
+void
+ir_dereference_variable::constant_referenced(struct hash_table 
*variable_context,
+ir_constant *&store, int &offset) 
const
+{
+   if (variable_context) {
+  store = (ir_constant *)hash_table_find(variable_context, var);
+  offset = 0;
+   } else {
+  store = NULL;
+  offset = 0;
+   }
+}
+
+void
+ir_dereference_array::constant_referenced(struct hash_table *variable_context,
+ ir_constant *&store, int &offset) 
const
+{
+   ir_constant *index_c = 
array_index->constant_expression_value(variable_context);
+
+   if (!index_c || !index_c->type->is_scalar() || 
!index_c->type->is_integer()) {
+  store = 0;
+  offset = 0;
+  return;
+   }
+
+   int index = index_c->type->base_type == GLSL_TYPE_INT ?
+  index_c->get_int_component(0) :
+  index_c->get_uint_component(0);
+
+   ir_constant *substore;
+   int suboffset;
+   const ir_dereference *deref = array->as_dereference();
+   if (!deref) {
+  store = 0;
+  offset = 0;
+  return;
+   }
+
+   deref->constant_referenced(variable_context, substore, suboffset);
+
+   if (!substore) {
+  store = 0;
+  offset = 0;
+  return;
+   }
+
+   const glsl_type *vt = array->type;
+   if (vt->is_array()) {
+  store = substore->get_array_element(index);
+  offset = 0;
+  return;
+   }
+   if (vt->is_matrix()) {
+  store = substore;
+  offset = index * vt->vector_elements;
+  return;
+   }
+   if (vt->is_vector()) {
+  store = substore;
+  offset = suboffset + index;
+  return;
+   }
+
+   store = 0;
+   offset = 0;
+}
+
+void
+ir_dereference_record::constant_referenced(struct hash_table *variable_context,
+  ir_constant *&store, int &offset) 
const
+{
+   ir_constant *substore;
+   int suboffset;
+   const ir_dereference *deref = record->as_dereference();
+   if (!deref) {
+  store = 0;
+  offset = 0;
+  return;
+   }
+
+   deref->constant_referenced(variable_context, substore, suboffset);
+
+   if (!substore) {
+  store = 0;
+  offset = 0;
+  return;
+   }
+
+   store = substore->get_record_field(field);
+   offset = 0;
+}
+/*@}*/
+
+
 ir_constant *
 ir_rvalue::constant_expression_value(struct hash_table *variable_context)
 {
@@ -1570,19 +1673,6 @@ ir_swizzle::constant_expression_value(struct hash_table 
*variable_context)
 }
 
 
-void
-ir_dereference_variable::constant_referenced(struct hash_table 
*variable_context,
-ir_constant *&store, int &offset) 
const
-{
-   if (variable_context) {
-  store = (ir_constant *)hash_table_find(variable_context, var);
-  offset = 0;
-   } else {
-  store = NULL;
-  offset = 0;
-   }
-}
-
 ir_constant *
 ir_dereference_variable::constant_expression_value(struct hash_table 
*variable_context)
 {
@@ -1610,60 +1700,6 @@ 
ir_dereference_variable::constant_expression_value(struct hash_table *variable_c
 }
 
 
-void
-ir_dereference_array::constant_referenced(struct hash_table *variable_context,
- ir_constant *&store, int &offset) 
const
-{
-   ir_constant *index_c = 
array_index->constant_expression_value(variable_context);
-
-   if (!index_c || !index_c->type->is_sca

[Mesa-dev] [PATCH] glsl: Fold implementation of ir_dereference_record::constant_referenced into wrapper

[Ian Romanick]

From: Ian Romanick 

v2: Don't shadow the variable 'deref' from a higher scope.

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 39 +
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index c013dfd..2e60363 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -412,10 +412,28 @@ constant_referenced(const ir_dereference *deref,
 store, offset);
   break;
 
-   case ir_type_dereference_record:
-  ((ir_dereference_record *) deref)->constant_referenced(variable_context,
- store, offset);
+   case ir_type_dereference_record: {
+  const ir_dereference_record *const dr =
+ (const ir_dereference_record *) deref;
+
+  const ir_dereference *const rec_deref = dr->record->as_dereference();
+  if (!rec_deref)
+ break;
+
+  ir_constant *substore;
+  int suboffset;
+
+  if (!constant_referenced(rec_deref, variable_context, substore,
+   suboffset))
+ break;
+
+  /* Since we're dropping it on the floor...
+   */
+  assert(suboffset == 0);
+
+  store = substore->get_record_field(dr->field);
   break;
+   }
 
case ir_type_dereference_variable: {
   const ir_dereference_variable *const dv =
@@ -493,20 +511,7 @@ void
 ir_dereference_record::constant_referenced(struct hash_table *variable_context,
   ir_constant *&store, int &offset) 
const
 {
-   ir_constant *substore;
-   int suboffset;
-   const ir_dereference *deref = record->as_dereference();
-   if (!deref) {
-  store = 0;
-  offset = 0;
-  return;
-   }
-
-   if (!::constant_referenced(deref, variable_context, substore, suboffset))
-  return;
-
-   store = substore->get_record_field(field);
-   offset = 0;
+   ::constant_referenced(this, variable_context, store, offset);
 }
 /*@}*/
 
-- 
1.8.1.4

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

Re: [Mesa-dev] [PATCH 1/8] glsl: Group all of the constant_referenced functions together

[Connor Abbott]

On Wed, Mar 12, 2014 at 6:49 PM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> Signed-off-by: Ian Romanick 
> ---
>  src/glsl/ir_clone.cpp   |  10 +-
>  src/glsl/ir_constant_expression.cpp | 195 
> +++-
>  2 files changed, 109 insertions(+), 96 deletions(-)
>
> diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
> index 311c992..167b80c 100644
> --- a/src/glsl/ir_clone.cpp
> +++ b/src/glsl/ir_clone.cpp
> @@ -265,10 +265,12 @@ ir_assignment::clone(void *mem_ctx, struct hash_table 
> *ht) const
> if (this->condition)
>new_condition = this->condition->clone(mem_ctx, ht);
>
> -   return new(mem_ctx) ir_assignment(this->lhs->clone(mem_ctx, ht),
> -this->rhs->clone(mem_ctx, ht),
> -new_condition,
> -this->write_mask);
> +   ir_assignment *cloned =
> +  new(mem_ctx) ir_assignment(this->lhs->clone(mem_ctx, ht),
> + this->rhs->clone(mem_ctx, ht),
> + new_condition);
> +   cloned->write_mask = this->write_mask;
> +   return cloned;

Looks like this is leftover from something else...

>  }
>
>  ir_function *
> diff --git a/src/glsl/ir_constant_expression.cpp 
> b/src/glsl/ir_constant_expression.cpp
> index 7fa5a09..a31e579 100644
> --- a/src/glsl/ir_constant_expression.cpp
> +++ b/src/glsl/ir_constant_expression.cpp
> @@ -386,6 +386,109 @@ unpack_half_1x16(uint16_t u)
> return _mesa_half_to_float(u);
>  }
>
> +/**
> + * \name Functions to get the constant referenced by an r-value
> + *
> + * Get the constant that is ultimately referenced by an r-value, in a 
> constant
> + * expression evaluation context.
> + *
> + * The offset is used when the reference is to a specific column of a matrix.
> + */
> +/*@{*/
> +void
> +ir_dereference_variable::constant_referenced(struct hash_table 
> *variable_context,
> +ir_constant *&store, int 
> &offset) const
> +{
> +   if (variable_context) {
> +  store = (ir_constant *)hash_table_find(variable_context, var);
> +  offset = 0;
> +   } else {
> +  store = NULL;
> +  offset = 0;
> +   }
> +}
> +
> +void
> +ir_dereference_array::constant_referenced(struct hash_table 
> *variable_context,
> + ir_constant *&store, int &offset) 
> const
> +{
> +   ir_constant *index_c = 
> array_index->constant_expression_value(variable_context);
> +
> +   if (!index_c || !index_c->type->is_scalar() || 
> !index_c->type->is_integer()) {
> +  store = 0;
> +  offset = 0;
> +  return;
> +   }
> +
> +   int index = index_c->type->base_type == GLSL_TYPE_INT ?
> +  index_c->get_int_component(0) :
> +  index_c->get_uint_component(0);
> +
> +   ir_constant *substore;
> +   int suboffset;
> +   const ir_dereference *deref = array->as_dereference();
> +   if (!deref) {
> +  store = 0;
> +  offset = 0;
> +  return;
> +   }
> +
> +   deref->constant_referenced(variable_context, substore, suboffset);
> +
> +   if (!substore) {
> +  store = 0;
> +  offset = 0;
> +  return;
> +   }
> +
> +   const glsl_type *vt = array->type;
> +   if (vt->is_array()) {
> +  store = substore->get_array_element(index);
> +  offset = 0;
> +  return;
> +   }
> +   if (vt->is_matrix()) {
> +  store = substore;
> +  offset = index * vt->vector_elements;
> +  return;
> +   }
> +   if (vt->is_vector()) {
> +  store = substore;
> +  offset = suboffset + index;
> +  return;
> +   }
> +
> +   store = 0;
> +   offset = 0;
> +}
> +
> +void
> +ir_dereference_record::constant_referenced(struct hash_table 
> *variable_context,
> +  ir_constant *&store, int &offset) 
> const
> +{
> +   ir_constant *substore;
> +   int suboffset;
> +   const ir_dereference *deref = record->as_dereference();
> +   if (!deref) {
> +  store = 0;
> +  offset = 0;
> +  return;
> +   }
> +
> +   deref->constant_referenced(variable_context, substore, suboffset);
> +
> +   if (!substore) {
> +  store = 0;
> +  offset = 0;
> +  return;
> +   }
> +
> +   store = substore->get_record_field(field);
> +   offset = 0;
> +}
> +/*@}*/
> +
> +
>  ir_constant *
>  ir_rvalue::constant_expression_value(struct hash_table *variable_context)
>  {
> @@ -1570,19 +1673,6 @@ ir_swizzle::constant_expression_value(struct 
> hash_table *variable_context)
>  }
>
>
> -void
> -ir_dereference_variable::constant_referenced(struct hash_table 
> *variable_context,
> -ir_constant *&store, int 
> &offset) const
> -{
> -   if (variable_context) {
> -  store = (ir_constant *)hash_table_find(variable_context, var);
> -  offset = 0;
> -   } else {
> -  store = NULL;
> -  offset = 0;
> -   }
> -}
> -
>  ir_constant *
>  ir_dereference_variable::constant_expression_value(st

[Mesa-dev] [PATCH v2] glsl: Fold implementation of ir_dereference_array::constant_referenced into wrapper

[Ian Romanick]

From: Ian Romanick 

v2: Don't shadow the variable 'deref' from a higher scope.

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 90 ++---
 1 file changed, 44 insertions(+), 46 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index 2e60363..f7174ad 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -407,10 +407,50 @@ constant_referenced(const ir_dereference *deref,
   return false;
 
switch (deref->ir_type) {
-   case ir_type_dereference_array:
-  ((ir_dereference_array *) deref)->constant_referenced(variable_context,
-store, offset);
+   case ir_type_dereference_array: {
+  const ir_dereference_array *const da =
+ (const ir_dereference_array *) deref;
+
+  ir_constant *index_c =
+ da->array_index->constant_expression_value(variable_context);
+
+  if (!index_c || !index_c->type->is_scalar() || 
!index_c->type->is_integer())
+ break;
+
+  int index = index_c->type->base_type == GLSL_TYPE_INT ?
+ index_c->get_int_component(0) :
+ index_c->get_uint_component(0);
+
+  ir_constant *substore;
+  int suboffset;
+
+  const ir_dereference *array_deref = da->array->as_dereference();
+  if (!array_deref)
+ break;
+
+  if (!constant_referenced(array_deref, variable_context, substore,
+   suboffset))
+ break;
+
+  const glsl_type *vt = da->array->type;
+  if (vt->is_array()) {
+ store = substore->get_array_element(index);
+ offset = 0;
+ break;
+  }
+  if (vt->is_matrix()) {
+ store = substore;
+ offset = index * vt->vector_elements;
+ break;
+  }
+  if (vt->is_vector()) {
+ store = substore;
+ offset = suboffset + index;
+ break;
+  }
+
   break;
+   }
 
case ir_type_dereference_record: {
   const ir_dereference_record *const dr =
@@ -462,49 +502,7 @@ void
 ir_dereference_array::constant_referenced(struct hash_table *variable_context,
  ir_constant *&store, int &offset) 
const
 {
-   ir_constant *index_c = 
array_index->constant_expression_value(variable_context);
-
-   if (!index_c || !index_c->type->is_scalar() || 
!index_c->type->is_integer()) {
-  store = 0;
-  offset = 0;
-  return;
-   }
-
-   int index = index_c->type->base_type == GLSL_TYPE_INT ?
-  index_c->get_int_component(0) :
-  index_c->get_uint_component(0);
-
-   ir_constant *substore;
-   int suboffset;
-   const ir_dereference *deref = array->as_dereference();
-   if (!deref) {
-  store = 0;
-  offset = 0;
-  return;
-   }
-
-   if (!::constant_referenced(deref, variable_context, substore, suboffset))
-  return;
-
-   const glsl_type *vt = array->type;
-   if (vt->is_array()) {
-  store = substore->get_array_element(index);
-  offset = 0;
-  return;
-   }
-   if (vt->is_matrix()) {
-  store = substore;
-  offset = index * vt->vector_elements;
-  return;
-   }
-   if (vt->is_vector()) {
-  store = substore;
-  offset = suboffset + index;
-  return;
-   }
-
-   store = 0;
-   offset = 0;
+   ::constant_referenced(this, variable_context, store, offset);
 }
 
 void
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 5.5/8] glsl: Just ignore non-dereference IR in constant_referenced

[Ian Romanick]

From: Ian Romanick 

This simplifies some of the call sites.

Signed-off-by: Ian Romanick 
---
 src/glsl/ir_constant_expression.cpp | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/glsl/ir_constant_expression.cpp 
b/src/glsl/ir_constant_expression.cpp
index f7174ad..7ffbde0 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -396,7 +396,7 @@ unpack_half_1x16(uint16_t u)
  */
 /*@{*/
 static bool
-constant_referenced(const ir_dereference *deref,
+constant_referenced(const ir_rvalue *rv,
 struct hash_table *variable_context,
 ir_constant *&store, int &offset)
 {
@@ -406,10 +406,10 @@ constant_referenced(const ir_dereference *deref,
if (variable_context == NULL)
   return false;
 
-   switch (deref->ir_type) {
+   switch (rv->ir_type) {
case ir_type_dereference_array: {
   const ir_dereference_array *const da =
- (const ir_dereference_array *) deref;
+ (const ir_dereference_array *) rv;
 
   ir_constant *index_c =
  da->array_index->constant_expression_value(variable_context);
@@ -424,11 +424,7 @@ constant_referenced(const ir_dereference *deref,
   ir_constant *substore;
   int suboffset;
 
-  const ir_dereference *array_deref = da->array->as_dereference();
-  if (!array_deref)
- break;
-
-  if (!constant_referenced(array_deref, variable_context, substore,
+  if (!constant_referenced(da->array, variable_context, substore,
suboffset))
  break;
 
@@ -454,16 +450,12 @@ constant_referenced(const ir_dereference *deref,
 
case ir_type_dereference_record: {
   const ir_dereference_record *const dr =
- (const ir_dereference_record *) deref;
-
-  const ir_dereference *const rec_deref = dr->record->as_dereference();
-  if (!rec_deref)
- break;
+ (const ir_dereference_record *) rv;
 
   ir_constant *substore;
   int suboffset;
 
-  if (!constant_referenced(rec_deref, variable_context, substore,
+  if (!constant_referenced(dr->record, variable_context, substore,
suboffset))
  break;
 
@@ -477,14 +469,13 @@ constant_referenced(const ir_dereference *deref,
 
case ir_type_dereference_variable: {
   const ir_dereference_variable *const dv =
- (const ir_dereference_variable *) deref;
+ (const ir_dereference_variable *) rv;
 
   store = (ir_constant *) hash_table_find(variable_context, dv->var);
   break;
}
 
default:
-  assert(!"Should not get here.");
   break;
}
 
-- 
1.8.1.4

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

Re: [Mesa-dev] [PATCH 1/8] glsl: Group all of the constant_referenced functions together

[Ian Romanick]

On 03/12/2014 04:11 PM, Connor Abbott wrote:
> On Wed, Mar 12, 2014 at 6:49 PM, Ian Romanick  wrote:
>> From: Ian Romanick 
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>  src/glsl/ir_clone.cpp   |  10 +-
>>  src/glsl/ir_constant_expression.cpp | 195 
>> +++-
>>  2 files changed, 109 insertions(+), 96 deletions(-)
>>
>> diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
>> index 311c992..167b80c 100644
>> --- a/src/glsl/ir_clone.cpp
>> +++ b/src/glsl/ir_clone.cpp
>> @@ -265,10 +265,12 @@ ir_assignment::clone(void *mem_ctx, struct hash_table 
>> *ht) const
>> if (this->condition)
>>new_condition = this->condition->clone(mem_ctx, ht);
>>
>> -   return new(mem_ctx) ir_assignment(this->lhs->clone(mem_ctx, ht),
>> -this->rhs->clone(mem_ctx, ht),
>> -new_condition,
>> -this->write_mask);
>> +   ir_assignment *cloned =
>> +  new(mem_ctx) ir_assignment(this->lhs->clone(mem_ctx, ht),
>> + this->rhs->clone(mem_ctx, ht),
>> + new_condition);
>> +   cloned->write_mask = this->write_mask;
>> +   return cloned;
> 
> Looks like this is leftover from something else...

Aye, that it is.  Good catch.  I'll remove it.

>>  }
>>
>>  ir_function *
>> diff --git a/src/glsl/ir_constant_expression.cpp 
>> b/src/glsl/ir_constant_expression.cpp
>> index 7fa5a09..a31e579 100644
>> --- a/src/glsl/ir_constant_expression.cpp
>> +++ b/src/glsl/ir_constant_expression.cpp
>> @@ -386,6 +386,109 @@ unpack_half_1x16(uint16_t u)
>> return _mesa_half_to_float(u);
>>  }
>>
>> +/**
>> + * \name Functions to get the constant referenced by an r-value
>> + *
>> + * Get the constant that is ultimately referenced by an r-value, in a 
>> constant
>> + * expression evaluation context.
>> + *
>> + * The offset is used when the reference is to a specific column of a 
>> matrix.
>> + */
>> +/*@{*/
>> +void
>> +ir_dereference_variable::constant_referenced(struct hash_table 
>> *variable_context,
>> +ir_constant *&store, int 
>> &offset) const
>> +{
>> +   if (variable_context) {
>> +  store = (ir_constant *)hash_table_find(variable_context, var);
>> +  offset = 0;
>> +   } else {
>> +  store = NULL;
>> +  offset = 0;
>> +   }
>> +}
>> +
>> +void
>> +ir_dereference_array::constant_referenced(struct hash_table 
>> *variable_context,
>> + ir_constant *&store, int &offset) 
>> const
>> +{
>> +   ir_constant *index_c = 
>> array_index->constant_expression_value(variable_context);
>> +
>> +   if (!index_c || !index_c->type->is_scalar() || 
>> !index_c->type->is_integer()) {
>> +  store = 0;
>> +  offset = 0;
>> +  return;
>> +   }
>> +
>> +   int index = index_c->type->base_type == GLSL_TYPE_INT ?
>> +  index_c->get_int_component(0) :
>> +  index_c->get_uint_component(0);
>> +
>> +   ir_constant *substore;
>> +   int suboffset;
>> +   const ir_dereference *deref = array->as_dereference();
>> +   if (!deref) {
>> +  store = 0;
>> +  offset = 0;
>> +  return;
>> +   }
>> +
>> +   deref->constant_referenced(variable_context, substore, suboffset);
>> +
>> +   if (!substore) {
>> +  store = 0;
>> +  offset = 0;
>> +  return;
>> +   }
>> +
>> +   const glsl_type *vt = array->type;
>> +   if (vt->is_array()) {
>> +  store = substore->get_array_element(index);
>> +  offset = 0;
>> +  return;
>> +   }
>> +   if (vt->is_matrix()) {
>> +  store = substore;
>> +  offset = index * vt->vector_elements;
>> +  return;
>> +   }
>> +   if (vt->is_vector()) {
>> +  store = substore;
>> +  offset = suboffset + index;
>> +  return;
>> +   }
>> +
>> +   store = 0;
>> +   offset = 0;
>> +}
>> +
>> +void
>> +ir_dereference_record::constant_referenced(struct hash_table 
>> *variable_context,
>> +  ir_constant *&store, int &offset) 
>> const
>> +{
>> +   ir_constant *substore;
>> +   int suboffset;
>> +   const ir_dereference *deref = record->as_dereference();
>> +   if (!deref) {
>> +  store = 0;
>> +  offset = 0;
>> +  return;
>> +   }
>> +
>> +   deref->constant_referenced(variable_context, substore, suboffset);
>> +
>> +   if (!substore) {
>> +  store = 0;
>> +  offset = 0;
>> +  return;
>> +   }
>> +
>> +   store = substore->get_record_field(field);
>> +   offset = 0;
>> +}
>> +/*@}*/
>> +
>> +
>>  ir_constant *
>>  ir_rvalue::constant_expression_value(struct hash_table *variable_context)
>>  {
>> @@ -1570,19 +1673,6 @@ ir_swizzle::constant_expression_value(struct 
>> hash_table *variable_context)
>>  }
>>
>>
>> -void
>> -ir_dereference_variable::constant_referenced(struct hash_table 
>> *variable_context,
>> -ir_constant *&store, int 
>> &offset) const
>> -{
>> 

Re: [Mesa-dev] [PATCH 2/8] glsl: Add wrapper function that calls ir_dereference::constant_referenced

[Connor Abbott]

On Wed, Mar 12, 2014 at 6:49 PM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> Signed-off-by: Ian Romanick 
> ---
>  src/glsl/ir_constant_expression.cpp | 52 
> +
>  1 file changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/src/glsl/ir_constant_expression.cpp 
> b/src/glsl/ir_constant_expression.cpp
> index a31e579..4149a0e 100644
> --- a/src/glsl/ir_constant_expression.cpp
> +++ b/src/glsl/ir_constant_expression.cpp
> @@ -395,6 +395,37 @@ unpack_half_1x16(uint16_t u)
>   * The offset is used when the reference is to a specific column of a matrix.
>   */
>  /*@{*/
> +static bool
> +constant_referenced(const ir_dereference *deref,
> +struct hash_table *variable_context,
> +ir_constant *&store, int &offset)
> +{
> +   switch (deref->ir_type) {
> +   case ir_type_dereference_array:
> +  ((ir_dereference_array *) deref)->constant_referenced(variable_context,
> +store, offset);
> +  break;
> +
> +   case ir_type_dereference_record:
> +  ((ir_dereference_record *) 
> deref)->constant_referenced(variable_context,
> + store, offset);
> +  break;
> +
> +   case ir_type_dereference_variable:
> +  ((ir_dereference_variable *) 
> deref)->constant_referenced(variable_context,
> +   store, 
> offset);
> +  break;
> +
> +   default:
> +  assert(!"Should not get here.");
> +  store = NULL;
> +  offset = 0;
> +  break;
> +   }
> +
> +   return store != NULL;
> +}
> +
>  void
>  ir_dereference_variable::constant_referenced(struct hash_table 
> *variable_context,
>  ir_constant *&store, int 
> &offset) const
> @@ -433,13 +464,8 @@ ir_dereference_array::constant_referenced(struct 
> hash_table *variable_context,
>return;
> }
>
> -   deref->constant_referenced(variable_context, substore, suboffset);
> -
> -   if (!substore) {
> -  store = 0;
> -  offset = 0;
> +   if (!::constant_referenced(deref, variable_context, substore, suboffset))
>return;
> -   }

Question: why do you have the :: here...

>
> const glsl_type *vt = array->type;
> if (vt->is_array()) {
> @@ -475,13 +501,8 @@ ir_dereference_record::constant_referenced(struct 
> hash_table *variable_context,
>return;
> }
>
> -   deref->constant_referenced(variable_context, substore, suboffset);
> -
> -   if (!substore) {
> -  store = 0;
> -  offset = 0;
> +   if (!::constant_referenced(deref, variable_context, substore, suboffset))
>return;
> -   }

and here...

>
> store = substore->get_record_field(field);
> offset = 0;
> @@ -1814,9 +1835,8 @@ bool 
> ir_function_signature::constant_expression_evaluate_expression_list(const s
>
>  ir_constant *store = NULL;
>  int offset = 0;
> -asg->lhs->constant_referenced(variable_context, store, offset);
>
> -if (!store)
> +if (!constant_referenced(asg->lhs, variable_context, store, offset))
> return false;

but not here...

>
>  ir_constant *value = 
> asg->rhs->constant_expression_value(variable_context);
> @@ -1847,9 +1867,9 @@ bool 
> ir_function_signature::constant_expression_evaluate_expression_list(const s
>
>  ir_constant *store = NULL;
>  int offset = 0;
> -call->return_deref->constant_referenced(variable_context, store, 
> offset);
>
> -if (!store)
> +if (!constant_referenced(call->return_deref, variable_context,
> +  store, offset))

or here? Compiler issues?

> return false;
>
>  ir_constant *value = 
> call->constant_expression_value(variable_context);
> --
> 1.8.1.4
>
> ___
> 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] mesa: Fix glGetVertexAttribi(GL_VERTEX_ATTRIB_ARRAY_SIZE)

[Anuj Phogat]

mesa currently returns 4 when GL_VERTEX_ATTRIB_ARRAY_SIZE is queried
for a vertex array initially set up with size=GL_BGRA. This patch
makes changes to return size=GL_BGRA as required by the spec.

Fixes Khronos OpenGL CTS test: vertex_array_bgra_basic.test

Signed-off-by: Anuj Phogat 
Cc: 
---
 src/mesa/main/arrayobj.c | 1 +
 src/mesa/main/mtypes.h   | 1 +
 src/mesa/main/varray.c   | 5 -
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index efb9930..2b234f3 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -200,6 +200,7 @@ init_array(struct gl_context *ctx,
array->Enabled = GL_FALSE;
array->Normalized = GL_FALSE;
array->Integer = GL_FALSE;
+   array->SizeBGRA = GL_FALSE;
array->_ElementSize = size * _mesa_sizeof_type(type);
array->VertexBinding = index;
 
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 2bad4ca..52874a7 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1545,6 +1545,7 @@ struct gl_vertex_attrib_array
GLboolean Enabled;   /**< Whether the array is enabled */
GLboolean Normalized;/**< Fixed-point values are normalized when 
converted to floats */
GLboolean Integer;   /**< Fixed-point values are not converted to 
floats */
+   GLboolean SizeBGRA;  /**< True if Size is GL_BGRA */
GLuint _ElementSize; /**< Size of each element in bytes */
GLuint VertexBinding;/**< Vertex buffer binding */
 };
diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index b4b6fa9..3bc361e 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -207,6 +207,7 @@ update_array_format(struct gl_context *ctx,
GLbitfield typeBit;
GLuint elementSize;
GLenum format = GL_RGBA;
+   GLboolean size_bgra = GL_FALSE;
 
if (_mesa_is_gles(ctx)) {
   legalTypesMask &= ~(FIXED_GL_BIT | DOUBLE_BIT | 
UNSIGNED_INT_10F_11F_11F_REV_BIT);
@@ -291,6 +292,7 @@ update_array_format(struct gl_context *ctx,
   }
 
   format = GL_BGRA;
+  size_bgra = GL_TRUE;
   size = 4;
}
else if (size < sizeMin || size > sizeMax || size > 4) {
@@ -335,6 +337,7 @@ update_array_format(struct gl_context *ctx,
array->Format = format;
array->Normalized = normalized;
array->Integer = integer;
+   array->SizeBGRA = size_bgra;
array->RelativeOffset = relativeOffset;
array->_ElementSize = elementSize;
 
@@ -738,7 +741,7 @@ get_vertex_array_attrib(struct gl_context *ctx, GLuint 
index, GLenum pname,
case GL_VERTEX_ATTRIB_ARRAY_ENABLED_ARB:
   return array->Enabled;
case GL_VERTEX_ATTRIB_ARRAY_SIZE_ARB:
-  return array->Size;
+  return (array->SizeBGRA ? GL_BGRA : array->Size);
case GL_VERTEX_ATTRIB_ARRAY_STRIDE_ARB:
   return array->Stride;
case GL_VERTEX_ATTRIB_ARRAY_TYPE_ARB:
-- 
1.8.3.1

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

[Mesa-dev] Z24S8 textures on nv04/nv10

[Ilia Mirkin]

Hi Francisco,

I'm looking at some piglit failures on nv10... full run at

http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html

The specific one is:

http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html

Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And
indeed, that case isn't handled. Looking through the #defines, I see a
NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And
even in the nv20 case, it's not actually used in the driver.

Does that mean that EXT_packed_depth_stencil should never have been
enabled on nouveau? I do see that it's a valid renderbuffer format
though. [It is now always enabled, since commit a92b9e60, but at the
time of the commit, it was also enabled for everything.

Any advice?

Thanks,

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

Re: [Mesa-dev] Gallium u_gen_mipmap problem with queries

[Marek Olšák]

I don't think there is a disable bit for all queries in our hardware.

We disable queries by "ending" them and resuming is like starting a
new one. We have to do that for context switches and blitting and
resource_copy_region anyway. Conditional rendering can be predicated
by an array of query results, so it's a natural thing to do.

To me, I think the 4th option is the best one, because it doesn't need
changes to the gallium interface.

Marek

On Wed, Mar 12, 2014 at 7:12 PM, Roland Scheidegger  wrote:
> Am 12.03.2014 15:00, schrieb Marek Olšák:
>> Hi everybody,
>>
>> u_gen_mipmap doesn't disable queries, so all active queries are
>> incremented, even though mipmap generation shouldn't have any effect
>> on queries. Hardware drivers can disable queries, but this feature is
>> not exposed via the gallium interface. Generally, we want to disable
>> all queries except for the time_elapsed query for u_gen_mipmap. I
>> propose the following solutions to this problem:
>>
>> 1) Add pipe_context::pause_all_queries(pipe) and
>> resume_all_queries(pipe). It's self-explanatory.
>>
>> 2) Add pipe_context::pause_query(pipe, query) and resume_query(pipe,
>> query). This has the disadvantage of having to enumerate all active
>> queries in u_gen_mipmap.
>>
>> 3) Add pipe_rasterizer_state::disable_all_queries:1, or separate bits
>> disable_occlusion_queries:1 and disable_streamout_queries:1 (this
>> includes the primitives_generated query too).
>>
>> 4) Rewrite u_gen_mipmap to use pipe->blit.
>>
>> Please comment.
>>
>> Thanks.
>>
>> Marek
>
> No idea what's best here. Though if you go for pause/resume query (or
> all queries) wouldn't it be better to just have one function and pass in
> some SUSPEND_QUERIES/RESUME_QUERIES bit?
> Other than that, I guess if hw really can suspend just all queries
> gathering with a single bit then an interface to disable all of them
> at once would be preferable.
>
> Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH V2] mesa: Fix error code generation in glReadPixels()

[Anuj Phogat]

Section 4.3.1, page 220, of OpenGL 3.3 specification explains
the error conditions for glreadPixels():

   "If the format is DEPTH_STENCIL, then values are taken from
both the depth buffer and the stencil buffer. If there is
no depth buffer or if there is no stencil buffer, then the
error INVALID_OPERATION occurs. If the type parameter is
not UNSIGNED_INT_24_8 or FLOAT_32_UNSIGNED_INT_24_8_REV,
then the error INVALID_ENUM occurs."

Fixes failing Khronos CTS test packed_depth_stencil_error.test

V2: Avoid code duplication

Cc: 
Signed-off-by: Anuj Phogat 
---
 src/mesa/main/glformats.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index 77cf263..32c4f42 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -1238,6 +1238,22 @@ GLenum
 _mesa_error_check_format_and_type(const struct gl_context *ctx,
   GLenum format, GLenum type)
 {
+   /* From OpenGL 3.3 spec, page 220:
+*"If the format is DEPTH_STENCIL, then values are taken from
+*both the depth buffer and the stencil buffer. If there is no
+*depth buffer or if there is no stencil buffer, then the error
+*INVALID_OPERATION occurs. If the type parameter is not
+*UNSIGNED_INT_24_8 or FLOAT_32_UNSIGNED_INT_24_8_REV, then the
+*error INVALID_ENUM occurs."
+*
+*OpenGL ES still generates GL_INVALID_OPERATION because glReadPixels
+*cannot be used to read depth or stencil in that API.
+*/
+   if (_mesa_is_desktop_gl(ctx) && format == GL_DEPTH_STENCIL
+   && type != GL_UNSIGNED_INT_24_8
+   && type != GL_FLOAT_32_UNSIGNED_INT_24_8_REV)
+  return GL_INVALID_ENUM;
+
/* special type-based checks (see glReadPixels, glDrawPixels error lists) */
switch (type) {
case GL_BITMAP:
-- 
1.8.3.1

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

[Mesa-dev] [PATCH V2] mesa: Fix glGetVertexAttribi(GL_VERTEX_ATTRIB_ARRAY_SIZE)

[Anuj Phogat]

mesa currently returns 4 when GL_VERTEX_ATTRIB_ARRAY_SIZE is queried
for a vertex array initially set up with size=GL_BGRA. This patch
makes changes to return size=GL_BGRA as required by the spec.

Fixes Khronos OpenGL CTS test: vertex_array_bgra_basic.test

V2: Use array->Format instead of adding a new variable

Signed-off-by: Anuj Phogat 
Cc: 
---
 src/mesa/main/varray.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index b4b6fa9..479d872 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -738,7 +738,7 @@ get_vertex_array_attrib(struct gl_context *ctx, GLuint 
index, GLenum pname,
case GL_VERTEX_ATTRIB_ARRAY_ENABLED_ARB:
   return array->Enabled;
case GL_VERTEX_ATTRIB_ARRAY_SIZE_ARB:
-  return array->Size;
+  return (array->Format == GL_BGRA) ? GL_BGRA : array->Size;
case GL_VERTEX_ATTRIB_ARRAY_STRIDE_ARB:
   return array->Stride;
case GL_VERTEX_ATTRIB_ARRAY_TYPE_ARB:
-- 
1.8.3.1

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

Re: [Mesa-dev] Wrong colors in 3D apps on big-endian systems

[Michel Dänzer]

On Mit, 2014-03-12 at 11:26 +, Richard Sandiford wrote:
> Michel Dänzer  writes:
> > On Die, 2014-03-11 at 11:08 +, Richard Sandiford wrote:
> >> Michel Dänzer  writes:
> >> > On Mon, 2014-03-10 at 10:11 +0100, Christian Zigotzky wrote:
> >> >> 
> >> >> The Fedora guys have solved the problem with the patch 
> >> >> "mesa-9.2-llvmpipe-on-big-endian.patch". What do you think of this 
> >> >> patch?
> >> >
> >> > It sounds like an older downstream attempt at fixing llvmpipe on big
> >> > endian hosts, which was superseded by the upstream fixes, which were
> >> > incidentally pushed by the 'Fedora guy' Adam Jackson. Adding him and
> >> > Richard Sandiford (the author of the fixes) to CC, maybe they can help
> >> > you fix up r600g.
> >> 
> >> I don't know the r600 code at all, sorry.
> >
> > Fair enough, but still I'm starting to regret that I didn't ask you guys
> > to at least try working with others to get the hardware drivers fixed up
> > for the llvmpipe big endian fixes before pushing them...
> >
> > Also, I was hoping there would be followup work extending the same
> > approach for the remaining formats. Do you have any plans for that? The
> > current half-finished situation is quite messy and confusing for people
> > working on format related code.
> 
> I started down that route with the patch attached to:
> 
>http://lists.freedesktop.org/archives/mesa-dev/2013-June/040594.html
> 
> It depended on those 8 patches being applied first, but although the
> series got positive feedback, it didn't really go anywhere beyond that.
> I suppose I should have pushed for someone to apply it.
> 
> I'm happy to retest and repost the series if someone would be willing
> to put it in.

Yes, please work with José on getting this in.


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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

Re: [Mesa-dev] PATCH: R600/SI: Instruction verifier improvements

[Michel Dänzer]

On Mit, 2014-03-12 at 09:49 -0700, Tom Stellard wrote:
> 
> The attached patches add some more checks to the instruction verifier on SI
> and also fix some false positives.  This should fix all the verifier errors
> generated by the piglit tests.

I can confirm your patches fix the false positive errors I'm seeing with
glamor.


> As far as I can tell, there are no regressions with this series.  I had some 
> trouble
> making it through a full run on my Boanire system without hangs, so I would 
> feel better
> if someone else was able to test these.

No regressions in piglit gpu.py and quick_cl.py on my Kaveri. But I
don't know about changes in verifier errors for those tests, and I'm not
sure which indirect addressing tests you're referring to in patch 2.

Tested-by: Michel Dänzer 


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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

Re: [Mesa-dev] [RFC PATCH 3/4] mesa: Prefer non-swizzled formats when prefer_no_swizzle set

[Chris Forbes]

Actually, after poking around a bit more, I think this is correct...

As of Brian's commit 657436da7ee, the packed formats are described as
being laid out from the LSB. This is consistent with this patch, with
the pack/unpack code, and the hardware documentation:

mesa: update packed format layout comments

Update the comments for the packed formats to accurately reflect the
layout of the bits in the pixel.  For example, for the packed format
MESA_FORMAT_R8G8B8A8, R is in the least significant position while A
is in the most-significant position of the 32-bit word.

Unfortunately the example higher up of how a type-P format is laid out
is still backwards, which adds to the confusion.

-- Chris

On Wed, Mar 12, 2014 at 12:36 PM, Chris Forbes  wrote:
> Yeah, you're right -- that looks bogus. There's been piles of
> confusion recently in formats.h about which end things are laid out
> from; the truth though is obviously in the pack/unpack code and the
> hardware format documentation.
>
> On Wed, Mar 12, 2014 at 9:12 AM, Francisco Jerez  
> wrote:
>> Chris Forbes  writes:
>>
>>> If prefer_no_swizzle is set, try:
>>> - The exact matching format
>>> - Formats with the required components in the correct order, plus a junk
>>>   component
>>> - finally, swizzled formats (BGRA etc)
>>>
>>> Signed-off-by: Chris Forbes 
>>> ---
>>>  src/mesa/main/texformat.c | 35 +++
>>>  1 file changed, 31 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
>>> index 33725dc..7cb42bc 100644
>>> --- a/src/mesa/main/texformat.c
>>> +++ b/src/mesa/main/texformat.c
>>> @@ -79,11 +79,13 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum 
>>> target,
>>>} else if (type == GL_UNSIGNED_INT_2_10_10_10_REV) {
>>>   RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
>>>}
>>> -  RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_UNORM);
>>> -  RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>> -  break;
>>> +  /* fallthrough */
>>>
>>> case GL_RGBA8:
>>> +  if (prefer_no_swizzle) {
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_UNORM);
>>> + break;
>>> +  }
>>
>> Are you sure this is correct on little endian machines?  According to
>> the docs, MESA_FORMAT_R8G8B8A8_UNORM would end up with the R component
>> in the most significant byte (highest array index) and the A component
>> in the least significant byte (lowest array index), which seems like the
>> opposite of what GL_RGBA8 is specified to be by
>> ARB_shader_image_load_store.
>>
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_UNORM);
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>>break;
>>> @@ -100,6 +102,9 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum 
>>> target,
>>>
>>> /* deep RGBA formats */
>>> case GL_RGB10_A2:
>>> +  if (prefer_no_swizzle) {
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_R10G10B10A2_UNORM);
>>> +  }
>>
>> This looks correct for any endianness.
>>
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>>break;
>>> @@ -119,6 +124,11 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum 
>>> target,
>>>}
>>>/* fallthrough */
>>> case GL_RGB8:
>>> +  if (prefer_no_swizzle) {
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_RGB_UNORM8);
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8X8_UNORM);
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_UNORM);
>>
>> The last two seem wrong for LE too.
>>
>>> +  }
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_BGR_UNORM8);
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8X8_UNORM);
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>> @@ -454,10 +464,19 @@ _mesa_choose_tex_format(struct gl_context *ctx, 
>>> GLenum target,
>>>break;
>>> case GL_RGB_SNORM:
>>> case GL_RGB8_SNORM:
>>> +  if (prefer_no_swizzle) {
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8X8_SNORM);
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>> +  }
>>
>> Same here.
>>
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_X8B8G8R8_SNORM);
>>> -  /* FALLTHROUGH */
>>> +  RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_SNORM);
>>> +  RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>> +  break;
>>> case GL_RGBA_SNORM:
>>> case GL_RGBA8_SNORM:
>>> +  if (prefer_no_swizzle) {
>>> + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>> +  }
>>
>> And here.
>>
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_SNORM);
>>>RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>>break;
>>> @@ -525,11 +544,19 @@ _mesa_choose_tex_format(struct gl_context *ctx, 
>>> GLenum target,
>>>
>>> case GL_SRGB_EXT:
>>> case GL_SRGB8_EXT:
>>> +  if (prefer_no_swizzle) {
>>> + /* there is no MESA_FORMAT_RGB_SRGB8 */
>>> + RETURN_

[Mesa-dev] removal of _glthread_GetID() breaks libglapi.so's ABI

[Chia-I Wu]

Hi Brian,

_glthread_GetID() was removed by this commit

commit 02cb04c68ffbdaffaf7513ddc951584cac29f209
Author: Brian Paul 
Date:   Tue Mar 4 15:24:16 2014 -0700

mesa: remove remaining uses of _glthread_GetID()

It turns out the function, declared in glapi.h, is a part of
libglapi.so ABI.  I am now getting this error when building GL
applications

/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../x86_64-linux-gnu/libGL.so:
undefined reference to `_glthread_GetID'

(gcc finds distro-provided libGL.so, which finds my build of
libglapi.so because of LD_LIBRARY_PATH).

I wonder if we should give the symbol available.  It could simply return zero...

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

Re: [Mesa-dev] [PATCH 5/7] i965/fs: Don't renumber UNIFORM registers.

[Kenneth Graunke]

On 03/12/2014 05:30 AM, Pohjolainen, Topi wrote:
> On Tue, Mar 11, 2014 at 11:48:54PM -0700, Kenneth Graunke wrote:
[snip]
>> +   stage_prog_data->nr_params = num_push_constants;
>> +
>> +   /* Up until now, the param[] array has been indexed by reg + reg_offset
>> +* of UNIFORM registers.  Condense it to only contain the uniforms we
>> +* chose to upload as push constants.
>> +*/
>> +   for (unsigned int i = 0; i < uniforms; i++) {
>> +  int remapped = push_constant_loc[i];
>> +
>> +  if (remapped == -1)
>> + continue;
>> +
> 
> I suppose it is obvious, but I had to read again the assigning loop above to 
> be
> certain, and hence there is no danger of writing before reading in
> "stage_prog_data->param[]":
> 
>  assert(remapped <= i);

Right...I agree, the assert is definitely nice to have.  (The old code I
moved didn't have one.)  I've added it for v2.

--Ken



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

Re: [Mesa-dev] [PATCH 5/7] i965/fs: Don't renumber UNIFORM registers.

[Kenneth Graunke]

On 03/12/2014 03:14 AM, Pohjolainen, Topi wrote:
> On Tue, Mar 11, 2014 at 11:48:54PM -0700, Kenneth Graunke wrote:
>> Previously, remove_dead_constants() would renumber the UNIFORM registers
>> to be sequential starting from zero, and the resulting register number
>> would be used directly as an index into the params[] array.
>>
>> This renumbering made it difficult to collect and save information about
>> pull constant locations, since setup_pull_constants() and
>> move_uniform_array_access_to_pull_constants() used different names.
>>
>> This patch generalizes setup_pull_constants() to decide whether each
>> uniform register should be a pull constant, push constant, or neither
>> (because it's unused).  Then, it stores mappings from UNIFORM register
>> numbers to params[] or pull_params[] indices in the push_constant_loc
>> and pull_constant_loc arrays.  (We already did this for pull constants.)
>>
>> Then, assign_curb_setup() just needs to consult the push_constant_loc
>> array to get the real index into the params[] array.
>>
>> This effectively folds all the remove_dead_constants() functionality
>> into assign_constant_locations(), while being less irritable to work
>> with.
>>
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 187 
>> +++
>>  src/mesa/drivers/dri/i965/brw_fs.h   |  13 +-
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   3 +-
>>  3 files changed, 85 insertions(+), 118 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 3c8237a..5e01e78 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -885,8 +885,8 @@ fs_visitor::import_uniforms(fs_visitor *v)
>> hash_table_call_foreach(v->variable_ht,
>> import_uniforms_callback,
>> variable_ht);
>> -   this->params_remap = v->params_remap;
>> -   this->nr_params_remap = v->nr_params_remap;
>> +   this->push_constant_loc = v->push_constant_loc;
>> +   this->uniforms = v->uniforms;
>>  }
>>  
>>  /* Our support for uniforms is piggy-backed on the struct
>> @@ -1397,11 +1397,8 @@ fs_visitor::assign_curb_setup()
>>  {
>> if (dispatch_width == 8) {
>>c->prog_data.first_curbe_grf = c->nr_payload_regs;
>> -  stage_prog_data->nr_params = uniforms;
>> } else {
>>c->prog_data.first_curbe_grf_16 = c->nr_payload_regs;
>> -  /* Make sure we didn't try to sneak in an extra uniform */
>> -  assert(uniforms == 0);
>> }
>>  
>> c->prog_data.curb_read_length = ALIGN(stage_prog_data->nr_params, 8) / 8;
>> @@ -1412,7 +1409,19 @@ fs_visitor::assign_curb_setup()
>>  
>>for (unsigned int i = 0; i < 3; i++) {
>>   if (inst->src[i].file == UNIFORM) {
>> -int constant_nr = inst->src[i].reg + inst->src[i].reg_offset;
>> +int uniform_nr = inst->src[i].reg + inst->src[i].reg_offset;
>> +int constant_nr;
>> +if (uniform_nr >= 0 && uniform_nr < (int) uniforms) {
>> +   constant_nr = push_constant_loc[uniform_nr];
>> +} else {
>> +   /* Section 5.11 of the OpenGL 4.1 spec says:
>> +* "Out-of-bounds reads return undefined values, which 
>> include
>> +*  values from other variables of the active program or 
>> zero."
>> +* Just return the first push constant.
>> +*/
>> +   constant_nr = 0;
>> +}
>> +
>>  struct brw_reg brw_reg = brw_vec1_grf(c->nr_payload_regs +
>>constant_nr / 8,
>>constant_nr % 8);
>> @@ -1724,94 +1733,6 @@ fs_visitor::compact_virtual_grfs()
>> }
>>  }
>>  
>> -bool
>> -fs_visitor::remove_dead_constants()
>> -{
>> -   if (dispatch_width == 8) {
>> -  this->params_remap = ralloc_array(mem_ctx, int, uniforms);
>> -  this->nr_params_remap = uniforms;
>> -
>> -  for (unsigned int i = 0; i < uniforms; i++)
>> - this->params_remap[i] = -1;
>> -
>> -  /* Find which params are still in use. */
>> -  foreach_list(node, &this->instructions) {
>> - fs_inst *inst = (fs_inst *)node;
>> -
>> - for (int i = 0; i < 3; i++) {
>> -int constant_nr = inst->src[i].reg + inst->src[i].reg_offset;
>> -
>> -if (inst->src[i].file != UNIFORM)
>> -   continue;
>> -
>> -/* Section 5.11 of the OpenGL 4.3 spec says:
>> - *
>> - * "Out-of-bounds reads return undefined values, which include
>> - * values from other variables of the active program or zero."
>> - */
>> -if (constant_nr < 0 || constant_nr >= (int)uniforms) {
>> -   constant_nr = 0;
>> -}
>> -
>> -/* For now, set this to non-negative.  We'll give it the
>> - * actual new number in a moment, in order to keep the
>> -