[Mesa-dev] [PATCH 5/5] i965/vec4: Propagate swizzles correctly during copy propagation.

[Francisco Jerez]

This simplifies the code that iterates over the per-component values
found in the matching copy_entry struct and checks whether the
register regions that were copied to each component are similar enough
to be treated as a single (reswizzled) value which can be propagated
into the current instruction.

Aside from being scattered between opt_copy_propagation(),
try_copy_propagate(), and try_constant_propagate(), what I found
terribly confusing about the preexisting logic was that
opt_copy_propagation() tried to reorder the array of values according
to the swizzle of the instruction source, which meant one would have
had to invert the reordering applied at the top level in order to find
out which component to take from each value (we were just taking the
i-th component from the i-th value, which is not correct in general).
The saturate mask was also being swizzled incorrectly.

This consolidates the logic for matching multiple components of a
copy_entry into a single function which returns the result as a
regular src_reg on success, as if the copy had been performed with a
single MOV instruction copying all components of the src_reg into the
destination.

Fixes several ARB_vertex_program MOV test-cases from:
 https://cgit.freedesktop.org/~kwg/piglit/log/?h=arb_program
---
 .../drivers/dri/i965/brw_vec4_copy_propagation.cpp | 123 ++---
 1 file changed, 57 insertions(+), 66 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
index b4a150a..fc8b721 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -85,21 +85,65 @@ is_logic_op(enum opcode opcode)
opcode == BRW_OPCODE_NOT);
 }
 
+/**
+ * Get the origin of a copy as a single register if all components present in
+ * the given readmask originate from the same register and have compatible
+ * regions, otherwise return a BAD_FILE register.
+ */
+static src_reg
+get_copy_value(const copy_entry , unsigned readmask)
+{
+   unsigned swz[4] = {};
+   src_reg value;
+
+   for (unsigned i = 0; i < 4; i++) {
+  if (readmask & (1 << i)) {
+ if (entry.value[i]) {
+src_reg src = *entry.value[i];
+
+if (src.file == IMM) {
+   swz[i] = i;
+} else {
+   swz[i] = BRW_GET_SWZ(src.swizzle, i);
+   /* Overwrite the original swizzle so the src_reg::equals call
+* below doesn't care about it, the correct swizzle will be
+* calculated once the swizzles of all components are known.
+*/
+   src.swizzle = BRW_SWIZZLE_XYZW;
+}
+
+if (value.file == BAD_FILE)
+   value = src;
+
+else if (!value.equals(src))
+   return src_reg();
+
+ } else {
+return src_reg();
+ }
+  }
+   }
+
+   return swizzle(value, brw_compose_swizzle(
+ brw_swizzle_for_mask(readmask),
+ BRW_SWIZZLE4(swz[0], swz[1], swz[2], swz[3])));
+}
+
 static bool
 try_constant_propagate(const struct brw_device_info *devinfo,
vec4_instruction *inst,
-   int arg, struct copy_entry *entry)
+   int arg, const copy_entry *entry)
 {
/* For constant propagation, we only handle the same constant
 * across all 4 channels.  Some day, we should handle the 8-bit
 * float vector format, which would let us constant propagate
 * vectors better.
+* We could be more aggressive here -- some channels might not get used
+* based on the destination writemask.
 */
-   src_reg value = *entry->value[0];
-   for (int i = 1; i < 4; i++) {
-  if (!value.equals(*entry->value[i]))
-return false;
-   }
+   src_reg value = get_copy_value(
+  *entry,
+  brw_apply_inv_swizzle_to_mask(inst->src[arg].swizzle, WRITEMASK_XYZW));
 
if (value.file != IMM)
   return false;
@@ -238,38 +282,14 @@ try_constant_propagate(const struct brw_device_info 
*devinfo,
 static bool
 try_copy_propagate(const struct brw_device_info *devinfo,
vec4_instruction *inst, int arg,
-   struct copy_entry *entry, int attributes_per_reg)
+   const copy_entry *entry, int attributes_per_reg)
 {
/* Build up the value we are propagating as if it were the source of a
 * single MOV
 */
-   /* For constant propagation, we only handle the same constant
-* across all 4 channels.  Some day, we should handle the 8-bit
-* float vector format, which would let us constant propagate
-* vectors better.
-*/
-   src_reg value = *entry->value[0];
-   for (int i = 1; i < 4; i++) {
-  /* This is equals() except we don't care about the swizzle. */
-  if (value.file != entry->value[i]->file ||
-  value.nr != entry->value[i]->nr ||
- 

[Mesa-dev] [PATCH 4/5] i965: Don't try copy propagation if constant propagation succeeded.

[Francisco Jerez]

It cannot get any better.
---
 src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp   | 2 +-
 src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
index 9dbe13d..01fbde1 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
@@ -738,7 +738,7 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, 
bblock_t *block,
 if (try_constant_propagate(inst, entry))
progress = true;
 
-if (try_copy_propagate(inst, i, entry))
+else if (try_copy_propagate(inst, i, entry))
progress = true;
  }
   }
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
index 5c25164..b4a150a 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -454,7 +454,7 @@ vec4_visitor::opt_copy_propagation(bool do_constant_prop)
  if (do_constant_prop && try_constant_propagate(devinfo, inst, i, 
))
 progress = true;
 
- if (try_copy_propagate(devinfo, inst, i, , attributes_per_reg))
+ else if (try_copy_propagate(devinfo, inst, i, , 
attributes_per_reg))
progress = true;
   }
 
-- 
2.7.0

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

[Mesa-dev] [PATCH 1/5] i965: Pass symbolic swizzle to brw_swizzle() as a single argument.

[Francisco Jerez]

And replace brw_swizzle1() with brw_swizzle().  Seems slightly cleaner
and will allow reusing brw_swizzle() in the vec4 back-end more easily.
---
 src/mesa/drivers/dri/i965/brw_clip_unfilled.c |  6 --
 src/mesa/drivers/dri/i965/brw_clip_util.c | 13 +++--
 src/mesa/drivers/dri/i965/brw_eu_emit.c   |  2 +-
 src/mesa/drivers/dri/i965/brw_reg.h   | 15 ---
 4 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c 
b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
index 3c18858..d333d10 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
@@ -85,8 +85,10 @@ static void compute_tri_direction( struct brw_clip_compile 
*c )
/* Take their crossproduct:
 */
brw_set_default_access_mode(p, BRW_ALIGN_16);
-   brw_MUL(p, vec4(brw_null_reg()), brw_swizzle(e, 1,2,0,3),  
brw_swizzle(f,2,0,1,3));
-   brw_MAC(p, vec4(e),  negate(brw_swizzle(e, 2,0,1,3)), 
brw_swizzle(f,1,2,0,3));
+   brw_MUL(p, vec4(brw_null_reg()), brw_swizzle(e, BRW_SWIZZLE_YZXW),
+   brw_swizzle(f, BRW_SWIZZLE_ZXYW));
+   brw_MAC(p, vec4(e),  negate(brw_swizzle(e, BRW_SWIZZLE_ZXYW)),
+   brw_swizzle(f, BRW_SWIZZLE_YZXW));
brw_set_default_access_mode(p, BRW_ALIGN_1);
 
brw_MUL(p, c->reg.dir, c->reg.dir, vec4(e));
diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c 
b/src/mesa/drivers/dri/i965/brw_clip_util.c
index 7ef3305..3e6664e 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_util.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
@@ -98,7 +98,8 @@ void brw_clip_project_position(struct brw_clip_compile *c, 
struct brw_reg pos )
/* value.xyz *= value.rhw
 */
brw_set_default_access_mode(p, BRW_ALIGN_16);
-   brw_MUL(p, brw_writemask(pos, WRITEMASK_XYZ), pos, brw_swizzle1(pos, W));
+   brw_MUL(p, brw_writemask(pos, WRITEMASK_XYZ), pos,
+   brw_swizzle(pos, BRW_SWIZZLE_));
brw_set_default_access_mode(p, BRW_ALIGN_1);
 }
 
@@ -194,11 +195,11 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
   brw_set_default_access_mode(p, BRW_ALIGN_16);
   brw_MOV(p,
   brw_writemask(t_nopersp, WRITEMASK_ZW),
-  brw_swizzle(tmp, 0, 1, 0, 1));
+  brw_swizzle(tmp, BRW_SWIZZLE_XYXY));
 
   /* t_nopersp = vec4(v1.xy, dest.xy) - v0.xyxy */
   brw_ADD(p, t_nopersp, t_nopersp,
-  negate(brw_swizzle(v0_ndc_copy, 0, 1, 0, 1)));
+  negate(brw_swizzle(v0_ndc_copy, BRW_SWIZZLE_XYXY)));
 
   /* Add the absolute values of the X and Y deltas so that if
* the points aren't in the same place on the screen we get
@@ -212,8 +213,8 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
*/
   brw_ADD(p,
   brw_writemask(t_nopersp, WRITEMASK_XY),
-  brw_abs(brw_swizzle(t_nopersp, 0, 2, 0, 0)),
-  brw_abs(brw_swizzle(t_nopersp, 1, 3, 0, 0)));
+  brw_abs(brw_swizzle(t_nopersp, BRW_SWIZZLE_XZXZ)),
+  brw_abs(brw_swizzle(t_nopersp, BRW_SWIZZLE_YWYW)));
   brw_set_default_access_mode(p, BRW_ALIGN_1);
 
   /* If the points are in the same place, just substitute a
@@ -234,7 +235,7 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
   brw_MUL(p, vec1(t_nopersp), vec1(t_nopersp),
 vec1(suboffset(t_nopersp, 1)));
   brw_set_default_access_mode(p, BRW_ALIGN_16);
-  brw_MOV(p, t_nopersp, brw_swizzle(t_nopersp, 0, 0, 0, 0));
+  brw_MOV(p, t_nopersp, brw_swizzle(t_nopersp, BRW_SWIZZLE_));
   brw_set_default_access_mode(p, BRW_ALIGN_1);
 
   release_tmp(c, tmp);
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 35d8039..2e4d7be 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -3399,7 +3399,7 @@ brw_broadcast(struct brw_codegen *p,
   */
  inst = brw_MOV(p,
 brw_null_reg(),
-stride(brw_swizzle1(idx, 0), 0, 4, 1));
+stride(brw_swizzle(idx, BRW_SWIZZLE_), 0, 4, 1));
  brw_inst_set_pred_control(devinfo, inst, BRW_PREDICATE_NONE);
  brw_inst_set_cond_modifier(devinfo, inst, BRW_CONDITIONAL_NZ);
  brw_inst_set_flag_reg_nr(devinfo, inst, 1);
diff --git a/src/mesa/drivers/dri/i965/brw_reg.h 
b/src/mesa/drivers/dri/i965/brw_reg.h
index a2a4a40..a4bcfca 100644
--- a/src/mesa/drivers/dri/i965/brw_reg.h
+++ b/src/mesa/drivers/dri/i965/brw_reg.h
@@ -81,7 +81,9 @@ struct brw_device_info;
 #define BRW_SWIZZLE_  BRW_SWIZZLE4(2,2,2,2)
 #define BRW_SWIZZLE_  BRW_SWIZZLE4(3,3,3,3)
 #define BRW_SWIZZLE_XYXY  BRW_SWIZZLE4(0,1,0,1)
+#define BRW_SWIZZLE_XZXZ  BRW_SWIZZLE4(0,2,0,2)
 #define BRW_SWIZZLE_YZXW  BRW_SWIZZLE4(1,2,0,3)
+#define BRW_SWIZZLE_YWYW  BRW_SWIZZLE4(1,3,1,3)
 #define BRW_SWIZZLE_ZXYW  BRW_SWIZZLE4(2,0,1,3)
 #define 

[Mesa-dev] [PATCH 3/5] i965/vec4: Use swizzle() to swizzle immediates during constant propagation.

[Francisco Jerez]

swizzle() works for vector immediates other than VF.
---
 .../drivers/dri/i965/brw_vec4_copy_propagation.cpp| 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
index 6bd9928..5c25164 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -76,22 +76,6 @@ is_channel_updated(vec4_instruction *inst, src_reg 
*values[4], int ch)
inst->dst.writemask & (1 << BRW_GET_SWZ(src->swizzle, ch)));
 }
 
-static unsigned
-swizzle_vf_imm(unsigned vf4, unsigned swizzle)
-{
-   union {
-  unsigned vf4;
-  uint8_t vf[4];
-   } v = { vf4 }, ret;
-
-   ret.vf[0] = v.vf[BRW_GET_SWZ(swizzle, 0)];
-   ret.vf[1] = v.vf[BRW_GET_SWZ(swizzle, 1)];
-   ret.vf[2] = v.vf[BRW_GET_SWZ(swizzle, 2)];
-   ret.vf[3] = v.vf[BRW_GET_SWZ(swizzle, 3)];
-
-   return ret.vf4;
-}
-
 static bool
 is_logic_op(enum opcode opcode)
 {
@@ -144,8 +128,7 @@ try_constant_propagate(const struct brw_device_info 
*devinfo,
   }
}
 
-   if (value.type == BRW_REGISTER_TYPE_VF)
-  value.ud = swizzle_vf_imm(value.ud, inst->src[arg].swizzle);
+   value = swizzle(value, inst->src[arg].swizzle);
 
switch (inst->opcode) {
case BRW_OPCODE_MOV:
-- 
2.7.0

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

[Mesa-dev] [PATCH 2/5] i965: Add support for swizzling arbitrary immediates to (brw_)swizzle().

[Francisco Jerez]

Scalar immediates used to be handled correctly by swizzle() (as the
identity) but since commit 58fa9d47b536403c4e3ca5d6a2495691338388fd it
will corrupt the contents of the immediate.  Vector immediates were
never handled correctly, but we had ad-hoc code to swizzle VF
immediates in the vec4 copy propagation pass.  This takes care of
swizzling V and UV in addition.
---
 src/mesa/drivers/dri/i965/brw_eu.c  | 39 +
 src/mesa/drivers/dri/i965/brw_ir_vec4.h |  6 -
 src/mesa/drivers/dri/i965/brw_reg.h |  7 --
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu.c 
b/src/mesa/drivers/dri/i965/brw_eu.c
index 40ec87d..36bdc7c 100644
--- a/src/mesa/drivers/dri/i965/brw_eu.c
+++ b/src/mesa/drivers/dri/i965/brw_eu.c
@@ -110,6 +110,45 @@ brw_swap_cmod(uint32_t cmod)
}
 }
 
+/**
+ * Get the least significant bit offset of the i+1-th component of immediate
+ * type \p type.  For \p i equal to the two's complement of j, return the
+ * offset of the j-th component starting from the end of the vector.  For
+ * scalar register types return zero.
+ */
+static unsigned
+imm_shift(enum brw_reg_type type, unsigned i)
+{
+   if (type == BRW_REGISTER_TYPE_VF)
+  return 8 * (i & 3);
+   else if (type == BRW_REGISTER_TYPE_UV || type == BRW_REGISTER_TYPE_V)
+  return 4 * (i & 7);
+   else
+  return 0;
+}
+
+/**
+ * Swizzle an arbitrary immediate \p x of the given type according to the
+ * permutation specified as \p swz.
+ */
+uint32_t
+brw_swizzle_immediate(enum brw_reg_type type, uint32_t x, unsigned swz)
+{
+   if (imm_shift(type, 1)) {
+  const unsigned n = 32 / imm_shift(type, 1);
+  uint32_t y = 0;
+
+  for (unsigned i = 0; i < n; i++)
+ y |= x >> imm_shift(type, (i & ~3) + BRW_GET_SWZ(swz, i & 3))
+<< imm_shift(type, ~0u)
+>> imm_shift(type, ~0u - i);
+
+  return y;
+   } else {
+  return x;
+   }
+}
+
 void
 brw_set_default_exec_size(struct brw_codegen *p, unsigned value)
 {
diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h 
b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
index 660beca..2b6872e 100644
--- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
@@ -76,7 +76,11 @@ offset(src_reg reg, unsigned delta)
 static inline src_reg
 swizzle(src_reg reg, unsigned swizzle)
 {
-   reg.swizzle = brw_compose_swizzle(swizzle, reg.swizzle);
+   if (reg.file == IMM)
+  reg.ud = brw_swizzle_immediate(reg.type, reg.ud, swizzle);
+   else
+  reg.swizzle = brw_compose_swizzle(swizzle, reg.swizzle);
+
return reg;
 }
 
diff --git a/src/mesa/drivers/dri/i965/brw_reg.h 
b/src/mesa/drivers/dri/i965/brw_reg.h
index a4bcfca..74ff67f 100644
--- a/src/mesa/drivers/dri/i965/brw_reg.h
+++ b/src/mesa/drivers/dri/i965/brw_reg.h
@@ -223,6 +223,7 @@ enum PACKED brw_reg_type {
 unsigned brw_reg_type_to_hw_type(const struct brw_device_info *devinfo,
  enum brw_reg_type type, enum brw_reg_file 
file);
 const char *brw_reg_type_letters(unsigned brw_reg_type);
+uint32_t brw_swizzle_immediate(enum brw_reg_type type, uint32_t x, unsigned 
swz);
 
 #define REG_SIZE (8*4)
 
@@ -876,9 +877,11 @@ get_element_d(struct brw_reg reg, unsigned elt)
 static inline struct brw_reg
 brw_swizzle(struct brw_reg reg, unsigned swz)
 {
-   assert(reg.file != BRW_IMMEDIATE_VALUE);
+   if (reg.file == BRW_IMMEDIATE_VALUE)
+  reg.ud = brw_swizzle_immediate(reg.type, reg.ud, swz);
+   else
+  reg.swizzle = brw_compose_swizzle(swz, reg.swizzle);
 
-   reg.swizzle = brw_compose_swizzle(swz, reg.swizzle);
return reg;
 }
 
-- 
2.7.0

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

[Mesa-dev] [Bug 92850] Segfault loading War Thunder

[bugzilla-daemon]

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

Konstantin A. Lepikhov  changed:

   What|Removed |Added

 CC||lakos...@altlinux.org

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

Re: [Mesa-dev] [PATCH] anv: remove stray ; after if

[Jason Ekstrand]

On Fri, Feb 26, 2016 at 1:28 PM, Thomas H.P. Andersen 
wrote:

>
>
> On Fri, Feb 26, 2016 at 7:54 AM, Jason Ekstrand 
> wrote:
>
>>
>> On Feb 25, 2016 5:33 PM, "Matt Turner"  wrote:
>> >
>> > Indeed, that looks like a mistake.
>>
>> Yes, yes it is.  Good catch.
>>
>> Reviewed-by: Jason Ekstrand 
>>
>
> Thanks. Can I ask one of you to push it?
>


Pushed. Thanks!
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 26/28] glsl: lower tessellation varyings packed with component layout qualifier

[Timothy Arceri]

On Thu, 2016-02-25 at 18:32 -0800, Kenneth Graunke wrote:
> On Tuesday, December 29, 2015 4:00:26 PM PST Timothy Arceri wrote:
> > For tessellation shaders we cannot just copy everything to the
> > packed
> > varyings like we do in other stages as tessellation uses shared
> > memory for
> > varyings, therefore it is only safe to copy array elements that the
> > shader
> > actually uses.
> 
> Also, you can only copy the exact *components* written by the shader.
> For example, one nasty thing a valid TCS might do is:
> 
>    patch out ivec4 foo;
>    foo[gl_InvocationID] = gl_InvocationID;
> 
> which, given four threads, will write <0, 1, 2, 3> to the
> vector.  But
> if each thread writes the whole vec4 by accident, you may end up with
> garbage in 3/4 of the components.

I had trouble trying to implement the above example on the Nvidia blob,
I only ever seemed to get one component written.

Are you sure thats valid? The spec says:

"Tessellation control shaders will get undefined results if one
invocation reads a per-vertex or per-patch attribute written by another
invocation at any point during the same phase, or if two invocations
attempt to write different values to the same per-patch output in a
single phase."

I've also been under the wrong assumption that different invocations
could overwrite each others per-vertex attributes but looking at the
spec that is not that case.

"Each invocation can read the attributes of any vertex in the input or
output patches, but can only write per-vertex attributes for the
corresponding output patch vertex."

"Tessellation control shaders must use the special variable
gl_InvocationID as the vertex number index when writing to per-vertex
output variables."

So if we really don't have to handle your example then that makes thing
s a bit simpler and I can likely clean this up a bit.

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

Re: [Mesa-dev] [PATCH] anv: remove stray ; after if

[Thomas H.P. Andersen]

On Fri, Feb 26, 2016 at 7:54 AM, Jason Ekstrand 
wrote:

>
> On Feb 25, 2016 5:33 PM, "Matt Turner"  wrote:
> >
> > Indeed, that looks like a mistake.
>
> Yes, yes it is.  Good catch.
>
> Reviewed-by: Jason Ekstrand 
>

Thanks. Can I ask one of you to push it?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] glsl: raise warning when using uninitialized variables

[Ian Romanick]

On 02/26/2016 07:09 AM, Alejandro Piñeiro wrote:
> v2:
>  * Take into account out varyings too (Timothy Arceri)
>  * Fix style (Timothy Arceri)
>  * Use a new ast_expression variable, instead of an
>ast_expression::hir new parameter (Timothy Arceri)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94129
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index 49e4858..ac451df 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -1899,6 +1899,13 @@ ast_expression::do_hir(exec_list *instructions,
>if (var != NULL) {
>   var->data.used = true;
>   result = new(ctx) ir_dereference_variable(var);
> +
> + if ((var->data.mode == ir_var_auto || var->data.mode == 
> ir_var_shader_out)
> + && !this->is_lhs
> + && result->variable_referenced()->data.assigned != true) {
> +_mesa_glsl_warning(, state, "`%s' used uninitialized",
> +   this->primary_expression.identifier);

It seems like this might miss cases like

void main()
{
int i;
int a[2];

a[i] = 0;
}

> + }
>} else {
>   _mesa_glsl_error(& loc, state, "`%s' undeclared",
>this->primary_expression.identifier);
> 

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

Re: [Mesa-dev] [PATCH 1/2] glsl: add is_lhs bool on ast_expression

[Ian Romanick]

On 02/26/2016 07:09 AM, Alejandro Piñeiro wrote:
> Useful to know if a expression is the recipient of an assignment
> or not, that would be used to (for example) raise warnings of
> "use of uninitialized variable" without getting a false positive
> when assigning first a variable.
> 
> By default the value is false, and it is assigned to true on
> the following cases:
>  * The lhs assignments subexpression
>  * At ast_array_index, on the array itself.
>  * While handling the method on an array, to avoid the warning
>calling array.length
>  * When computed the cached test expression at test_to_hir, to
>avoid a duplicate warning on the test expression of a switch.
> 
> set_is_lhs setter is added, because in some cases (like ast_field_selection)
> the value need to be propagated on the expression tree. To avoid doing the
> propatagion if not needed, it skips if no primary_expression.identifier is
> available.
> 
> v2: use a new bool on ast_expression, instead of a new parameter
> on ast_expression::hir (Timothy Arceri)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94129
> ---
>  src/compiler/glsl/ast.h|  5 +
>  src/compiler/glsl/ast_function.cpp |  3 +++
>  src/compiler/glsl/ast_to_hir.cpp   | 30 ++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
> index 9aa5bb9..d07b595 100644
> --- a/src/compiler/glsl/ast.h
> +++ b/src/compiler/glsl/ast.h
> @@ -263,6 +263,11 @@ public:
>  * This pointer may be \c NULL.
>  */
> const char *non_lvalue_description;
> +
> +   void set_is_lhs(bool new_value);
> +
> +private:
> +   bool is_lhs = false;

This is valid C++?  I thought you could only initialize static members
in this way, and everything else had to be initialized by the constructor.

>  };
>  
>  class ast_expression_bin : public ast_expression {
> diff --git a/src/compiler/glsl/ast_function.cpp 
> b/src/compiler/glsl/ast_function.cpp
> index 1a44020..49afccc 100644
> --- a/src/compiler/glsl/ast_function.cpp
> +++ b/src/compiler/glsl/ast_function.cpp
> @@ -1727,6 +1727,9 @@ ast_function_expression::handle_method(exec_list 
> *instructions,
> const char *method;
> method = field->primary_expression.identifier;
>  
> +   /* This would prevent to raise "unitialized variable" warnings when 
> calling
  uninitialized

> +* array.length. */

Here and elsewhere, the closing */ of a multiline comment goes on its
own line.

> +   field->subexpressions[0]->set_is_lhs(true);
> op = field->subexpressions[0]->hir(instructions, state);
> if (strcmp(method, "length") == 0) {
>if (!this->expressions.is_empty()) {
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index db5ec9a..49e4858 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -1248,6 +1248,23 @@ ast_expression::hir_no_rvalue(exec_list *instructions,
> do_hir(instructions, state, false);
>  }
>  
> +void
> +ast_expression::set_is_lhs(bool new_value)
> +{
> +   /* is_lhs is tracked only to print "variable used unitialized warnings", 
> if
uninitialized

> +* we lack a identifier we can just skip, so also skipping going through
> +* subexpressions (see below) */
> +   if (this->primary_expression.identifier == NULL)
> +  return;
> +
> +   this->is_lhs = new_value;
> +
> +   /* We need to go through the subexpressions tree to cover cases like
> +* ast_field_selection */
> +   if (this->subexpressions[0] != NULL)
> +  this->subexpressions[0]->set_is_lhs(new_value);
> +}
> +
>  ir_rvalue *
>  ast_expression::do_hir(exec_list *instructions,
> struct _mesa_glsl_parse_state *state,
> @@ -1323,6 +1340,7 @@ ast_expression::do_hir(exec_list *instructions,
>break;
>  
> case ast_assign: {
> +  this->subexpressions[0]->set_is_lhs(true);
>op[0] = this->subexpressions[0]->hir(instructions, state);
>op[1] = this->subexpressions[1]->hir(instructions, state);
>  
> @@ -1592,6 +1610,7 @@ ast_expression::do_hir(exec_list *instructions,
> case ast_div_assign:
> case ast_add_assign:
> case ast_sub_assign: {
> +  this->subexpressions[0]->set_is_lhs(true);
>op[0] = this->subexpressions[0]->hir(instructions, state);
>op[1] = this->subexpressions[1]->hir(instructions, state);
>  
> @@ -1618,6 +1637,7 @@ ast_expression::do_hir(exec_list *instructions,
> }
>  
> case ast_mod_assign: {
> +  this->subexpressions[0]->set_is_lhs(true);
>op[0] = this->subexpressions[0]->hir(instructions, state);
>op[1] = this->subexpressions[1]->hir(instructions, state);
>  
> @@ -1640,6 +1660,7 @@ ast_expression::do_hir(exec_list *instructions,
>  
> case ast_ls_assign:
> case ast_rs_assign: {
> +  

Re: [Mesa-dev] MesaGL <-> non-Mesa OpenCL interop interface

[Marek Olšák]

On Sat, Feb 6, 2016 at 6:53 PM, Jason Ekstrand  wrote:
> I'm adding Chad to the Cc.  At some point, it would be good to get Beignet
> playing well with mesa.  Personally, I have substantial reservations about
> getting the kernel involved in surface layout for anything more than 2-D
> non-mipmapped non-array surfaces.  Laying them out can be complicated, and
> committing to a stable API, or worse a kernel API seems a bit tricky.  But
> I'll let chad rant about that :-)

It looks like we might have to pass surface parameters via interop
functions after all in order to support texture view sharing. Oh well.

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

Re: [Mesa-dev] [PATCH 1/2] i965/fs: fix precision of f2b

[Francisco Jerez]

Iago Toral  writes:

> On Fri, 2016-02-26 at 16:28 +0100, Roland Scheidegger wrote:
>> Am 26.02.2016 um 11:25 schrieb Iago Toral:
>> > 
>> > On Thu, 2016-02-25 at 18:20 -0500, Ilia Mirkin wrote:
>> >> On Thu, Feb 25, 2016 at 6:16 PM, Francisco Jerez  
>> >> wrote:
>> >>> Ian Romanick  writes:
>> >>>
>>  On 02/25/2016 12:13 PM, Francisco Jerez wrote:
>> > Ian Romanick  writes:
>> >
>> >> On 02/25/2016 08:46 AM, Roland Scheidegger wrote:
>> >>> Am 25.02.2016 um 11:15 schrieb Iago Toral Quiroga:
>>  From the OpenGL 4.2 spec:
>> 
>>  "When a constructor is used to convert any integer or 
>>  floating-point type to a
>>   bool, 0 and 0.0 are converted to false, and non-zero values are 
>>  converted to
>>   true."
>> 
>>  Thus, even the smallest non-zero floating value should be 
>>  translated to true.
>>  This behavior has been verified also with proprietary NVIDIA 
>>  drivers.
>> 
>>  Currently, we implement this conversion as a cmp.nz operation with 
>>  floats,
>>  subject to floating-point precision limitations, and as a result, 
>>  relatively
>>  small non-zero floating point numbers return false instead of true.
>> 
>>  This patch fixes the problem by getting rid of the sign bit (to 
>>  cover the case
>>  of -0.0) and testing the result against 0u using an integer 
>>  comparison instead.
>>  ---
>>   src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 ---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>> 
>>  diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
>>  b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>  index db20c71..7d62d7e 100644
>>  --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>  +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>  @@ -913,9 +913,18 @@ fs_visitor::nir_emit_alu(const fs_builder 
>>  , nir_alu_instr *instr)
>> bld.MOV(result, negate(op[0]));
>> break;
>> 
>>  -   case nir_op_f2b:
>>  -  bld.CMP(result, op[0], brw_imm_f(0.0f), BRW_CONDITIONAL_NZ);
>>  -  break;
>>  +   case nir_op_f2b: {
>>  +  /* Because comparing to 0.0f is subject to precision 
>>  limitations, do the
>>  +   * comparison using integers (we need to get rid of the sign 
>>  bit for that)
>>  +   */
>>  +  if (devinfo->gen >= 8)
>>  + op[0] = resolve_source_modifiers(op[0]);
>>  +  op[0] = retype(op[0], BRW_REGISTER_TYPE_UD);
>>  +  bld.AND(op[0], op[0], brw_imm_ud(0x7FFFu));
>>  +  bld.CMP(result, op[0], brw_imm_ud(0u), BRW_CONDITIONAL_NZ);
>>  +   break;
>>  +   }
>>  +
>>  case nir_op_i2b:
>> bld.CMP(result, op[0], brw_imm_d(0), BRW_CONDITIONAL_NZ);
>> break;
>> 
>> >>>
>> >>> Does that fix anything? I don't really see a problem with the 
>> >>> existing
>> >>> logic. Yes any "non-zero value" should be converted to true. But 
>> >>> surely
>> >>> that definition cannot include denorms, which you are always allowed 
>> >>> to
>> >>> flush to zero.
>> >>> (Albeit I can't tell what the result would be with NaNs with the 
>> >>> float
>> >>> compare, nor what the result actually should be in this case since 
>> >>> glsl
>> >>> doesn't require NaNs neither.)
>> >>
>> >> Based on this and Jason's comments, I think we need a bunch of new 
>> >> tests.
>> >>
>> >>  - smallest positive normal number
>> >>  - abs of smallest positive normal number
>> >>  - neg of "   ""  "
>> >>  - largest positive subnormal number
>> >>  - abs of largest positive subnormal number
>> >>  - neg of""""
>> >>  - all of the above with negative numbers
>> >>  - NaN
>> >>  - abs of NaN
>> >>  - neg of  "
>> >>
>> >> Perhaps others? +/-Inf just for kicks?
>> >
>> > What's the point?  The result of most of the above (except possibly
>> > bool(NaN)) is undefined by the spec:
>> >
>> > "Any denormalized value input into a shader or potentially generated by
>> >  any operation in a shader can be flushed to 0. [...] NaNs are not
>> >  required to be generated. [...] Operations and built-in functions that
>> >  operate on a NaN are not required to return a NaN as the result."
>> 
>>  Except that apparently one major OpenGL vendor does something well
>>  defined that's different than what we do.
>> >>>
>> >>> I'm skeptical that nVidia would treat single-precision denorms
>> >>> 

[Mesa-dev] [PATCH vulkan] anv/pipeline: Try to create all pipelines

[Neil Roberts]

According to the Vulkan 1.0 spec section 9.4:

“When an application attempts to create many pipelines in a single
 command, it is possible that some subset may fail creation. In that
 case, the corresponding entries in the pPipelines output array will
 be filled with VK_NULL_HANDLE values. If any pipeline fails creation
 (for example, due to out of memory errors), the vkCreate*Pipelines
 commands will return an error code. The implementation will attempt
 to create all pipelines, and only return VK_NULL_HANDLE values for
 those that actually failed.”

Previously anv_Create{Graphics,Compute}Pipelines would destroy any
previous pipelines that it created. The problem with this is that if
the application is expecting the driver to behave like the spec then
it may try to free any pipelines that were successfully created by
iterating through the results array. If any of them succeeded then the
pointer will be a stale pointer and the application would probably
crash.
---
 src/intel/vulkan/anv_pipeline.c | 52 +++--
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
index 1173b4f..d3a01df 100644
--- a/src/intel/vulkan/anv_pipeline.c
+++ b/src/intel/vulkan/anv_pipeline.c
@@ -1231,24 +1231,28 @@ VkResult anv_CreateGraphicsPipelines(
 const VkAllocationCallbacks*pAllocator,
 VkPipeline* pPipelines)
 {
-   VkResult result = VK_SUCCESS;
+   VkResult this_result;
+   VkResult overall_result = VK_SUCCESS;
 
unsigned i = 0;
for (; i < count; i++) {
-  result = anv_graphics_pipeline_create(_device,
-pipelineCache,
-[i],
-NULL, pAllocator, [i]);
-  if (result != VK_SUCCESS) {
- for (unsigned j = 0; j < i; j++) {
-anv_DestroyPipeline(_device, pPipelines[j], pAllocator);
- }
-
- return result;
+  this_result = anv_graphics_pipeline_create(_device,
+ pipelineCache,
+ [i],
+ NULL,
+ pAllocator,
+ [i]);
+  if (this_result != VK_SUCCESS) {
+ /* According to the spec this should try to create all pipelines and
+  * if any fail then it will set the corresponding pipeline handle to
+  * NULL.
+  */
+ pPipelines[i] = VK_NULL_HANDLE;
+ overall_result = this_result;
   }
}
 
-   return VK_SUCCESS;
+   return overall_result;
 }
 
 static VkResult anv_compute_pipeline_create(
@@ -1287,21 +1291,23 @@ VkResult anv_CreateComputePipelines(
 const VkAllocationCallbacks*pAllocator,
 VkPipeline* pPipelines)
 {
-   VkResult result = VK_SUCCESS;
+   VkResult this_result;
+   VkResult overall_result = VK_SUCCESS;
 
unsigned i = 0;
for (; i < count; i++) {
-  result = anv_compute_pipeline_create(_device, pipelineCache,
-   [i],
-   pAllocator, [i]);
-  if (result != VK_SUCCESS) {
- for (unsigned j = 0; j < i; j++) {
-anv_DestroyPipeline(_device, pPipelines[j], pAllocator);
- }
-
- return result;
+  this_result = anv_compute_pipeline_create(_device, pipelineCache,
+[i],
+pAllocator, [i]);
+  if (this_result != VK_SUCCESS) {
+ /* According to the spec this should try to create all pipelines and
+  * if any fail then it will set the corresponding pipeline handle to
+  * NULL.
+  */
+ pPipelines[i] = VK_NULL_HANDLE;
+ overall_result = this_result;
   }
}
 
-   return VK_SUCCESS;
+   return overall_result;
 }
-- 
2.5.0

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

[Mesa-dev] [Bug 94295] [swrast] piglit shader_runner fast_color_clear/all-colors regression

[bugzilla-daemon]

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

--- Comment #3 from Vinson Lee  ---
(In reply to Plamena Manolova from comment #2)

> Could you verify whether this patch fixes this issue for you?

Test still segfaults with patch id=121982.

#0  find_empty_block (uniform=, prog=) at
glsl/link_uniforms.cpp:1054
1054   foreach_list_typed(struct empty_uniform_block, block, link,
(gdb) l
1049   const unsigned entries = MAX2(1, uniform->array_elements);
1050
1051   if (exec_list_is_empty(>EmptyUniformLocations))
1052  return -1;
1053
1054   foreach_list_typed(struct empty_uniform_block, block, link,
1055  >EmptyUniformLocations) {
1056  /* Found a block with enough slots to fit the uniform */
1057  if (block->slots == entries) {
1058 unsigned start = block->start;
(gdb) print block
$1 = (empty_uniform_block *) 0x0

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

Re: [Mesa-dev] [PATCH] i965: Set dest type to UW for several send messages

[Francisco Jerez]

Jordan Justen  writes:

> On 2016-02-22 18:52:13, Francisco Jerez wrote:
>> Jordan Justen  writes:
>> 
>> > Without this, on SIMD 16 the send instruction destination will appear
>> > to write more than one destination register, causing the simulator to
>> > report an error.
>> >
>> > Of course, the send instruction can actually write more than one
>> > destination register regardless of the type set for the destination,
>> > so this is a bit strange.
>> >
>> > Suggested-by: Kenneth Graunke 
>> > Signed-off-by: Jordan Justen 
>> > ---
>> >  src/mesa/drivers/dri/i965/brw_eu_emit.c| 4 +++-
>> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 +-
>> >  2 files changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
>> > b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> > index 35d8039..83b262b 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> > @@ -2601,6 +2601,7 @@ brw_send_indirect_surface_message(struct brw_codegen 
>> > *p,
>> >surface = addr;
>> > }
>> >  
>> > +   dst = retype(dst, BRW_REGISTER_TYPE_UW);
>> 
>> Any reason you didn't change this in brw_send_indirect_message()
>> instead?  That would also make sure that anything calling it directly
>> instead of going through brw_send_indirect_surface_message() would get
>> the destination type fixed up too.
>> 
>
> Yeah, that works too. r-b with that?
>
Yeah, feel free to put my R-b with that change.

> -Jordan
>
>> > insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
>> > brw_inst_set_mlen(devinfo, insn, message_len);
>> > brw_inst_set_rlen(devinfo, insn, response_len);
>> > @@ -3207,6 +3208,7 @@ brw_memory_fence(struct brw_codegen *p,
>> >  * message doesn't write anything back.
>> >  */
>> > insn = next_insn(p, BRW_OPCODE_SEND);
>> > +   dst = retype(dst, BRW_REGISTER_TYPE_UW);
>> > brw_set_dest(p, insn, dst);
>> > brw_set_src0(p, insn, dst);
>> > brw_set_memory_fence_message(p, insn, GEN7_SFID_DATAPORT_DATA_CACHE,
>> > @@ -3473,7 +3475,7 @@ brw_barrier(struct brw_codegen *p, struct brw_reg 
>> > src)
>> > assert(devinfo->gen >= 7);
>> >  
>> > inst = next_insn(p, BRW_OPCODE_SEND);
>> > -   brw_set_dest(p, inst, brw_null_reg());
>> > +   brw_set_dest(p, inst, retype(brw_null_reg(), BRW_REGISTER_TYPE_UW));
>> > brw_set_src0(p, inst, src);
>> > brw_set_src1(p, inst, brw_null_reg());
>> >  
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
>> > b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> > index ef58584..b58c938 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> > @@ -431,7 +431,7 @@ fs_generator::generate_cs_terminate(fs_inst *inst, 
>> > struct brw_reg payload)
>> >  
>> > insn = brw_next_insn(p, BRW_OPCODE_SEND);
>> >  
>> > -   brw_set_dest(p, insn, brw_null_reg());
>> > +   brw_set_dest(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_UW));
>> > brw_set_src0(p, insn, payload);
>> > brw_set_src1(p, insn, brw_imm_d(0));
>> >  
>> > -- 
>> > 2.7.0
>> >
>> > ___
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH] i965: Set dest type to UW for several send messages

[Jordan Justen]

On 2016-02-22 18:52:13, Francisco Jerez wrote:
> Jordan Justen  writes:
> 
> > Without this, on SIMD 16 the send instruction destination will appear
> > to write more than one destination register, causing the simulator to
> > report an error.
> >
> > Of course, the send instruction can actually write more than one
> > destination register regardless of the type set for the destination,
> > so this is a bit strange.
> >
> > Suggested-by: Kenneth Graunke 
> > Signed-off-by: Jordan Justen 
> > ---
> >  src/mesa/drivers/dri/i965/brw_eu_emit.c| 4 +++-
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 +-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> > b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > index 35d8039..83b262b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > @@ -2601,6 +2601,7 @@ brw_send_indirect_surface_message(struct brw_codegen 
> > *p,
> >surface = addr;
> > }
> >  
> > +   dst = retype(dst, BRW_REGISTER_TYPE_UW);
> 
> Any reason you didn't change this in brw_send_indirect_message()
> instead?  That would also make sure that anything calling it directly
> instead of going through brw_send_indirect_surface_message() would get
> the destination type fixed up too.
> 

Yeah, that works too. r-b with that?

-Jordan

> > insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
> > brw_inst_set_mlen(devinfo, insn, message_len);
> > brw_inst_set_rlen(devinfo, insn, response_len);
> > @@ -3207,6 +3208,7 @@ brw_memory_fence(struct brw_codegen *p,
> >  * message doesn't write anything back.
> >  */
> > insn = next_insn(p, BRW_OPCODE_SEND);
> > +   dst = retype(dst, BRW_REGISTER_TYPE_UW);
> > brw_set_dest(p, insn, dst);
> > brw_set_src0(p, insn, dst);
> > brw_set_memory_fence_message(p, insn, GEN7_SFID_DATAPORT_DATA_CACHE,
> > @@ -3473,7 +3475,7 @@ brw_barrier(struct brw_codegen *p, struct brw_reg src)
> > assert(devinfo->gen >= 7);
> >  
> > inst = next_insn(p, BRW_OPCODE_SEND);
> > -   brw_set_dest(p, inst, brw_null_reg());
> > +   brw_set_dest(p, inst, retype(brw_null_reg(), BRW_REGISTER_TYPE_UW));
> > brw_set_src0(p, inst, src);
> > brw_set_src1(p, inst, brw_null_reg());
> >  
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
> > b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > index ef58584..b58c938 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > @@ -431,7 +431,7 @@ fs_generator::generate_cs_terminate(fs_inst *inst, 
> > struct brw_reg payload)
> >  
> > insn = brw_next_insn(p, BRW_OPCODE_SEND);
> >  
> > -   brw_set_dest(p, insn, brw_null_reg());
> > +   brw_set_dest(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_UW));
> > brw_set_src0(p, insn, payload);
> > brw_set_src1(p, insn, brw_imm_d(0));
> >  
> > -- 
> > 2.7.0
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/5] i965/cfg: Eliminate an empty then-branch of an if/else/endif

[Matt Turner]

On Fri, Feb 26, 2016 at 10:58 AM, Ian Romanick  wrote:
> Ok.  Also... is there a reason 'else in else/endif' starts with .
> instead of a -?

Oops. Just a mistake :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/5] i965/cfg: Eliminate an empty then-branch of an if/else/endif

[Ian Romanick]

On 02/26/2016 09:32 AM, Matt Turner wrote:
> On Thu, Feb 25, 2016 at 2:40 PM, Ian Romanick  wrote:
>> From: Ian Romanick 
>>
>> On BDW,
>>
>> total instructions in shared programs: 8448571 -> 8448367 (-0.00%)
>> instructions in affected programs: 21000 -> 20796 (-0.97%)
>> helped: 116
>> HURT: 0
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>  src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp | 18 ++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp 
>> b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> index 7aa72b1..149596f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> @@ -34,6 +34,7 @@
>>   *   - if/endif
>>   *   . else in else/endif
>>   *   - if/else/endif
>> + *   - then in if/else/endif
> 
> This list has been " in " so this
> line should be "else in if/else"

Ok.  Also... is there a reason 'else in else/endif' starts with .
instead of a -?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 91556] Struct / union sizes being calculated incorrectly

[bugzilla-daemon]

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

--- Comment #8 from Pavan Yalamanchili  ---
@serge won't this break the original intent of the padding size ? For example
float3 would be broken in this case.

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

Re: [Mesa-dev] [PATCH 3/5] i965/cfg: Eliminate an empty then-branch of an if/else/endif

[Matt Turner]

On Thu, Feb 25, 2016 at 2:40 PM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> On BDW,
>
> total instructions in shared programs: 8448571 -> 8448367 (-0.00%)
> instructions in affected programs: 21000 -> 20796 (-0.97%)
> helped: 116
> HURT: 0
>
> Signed-off-by: Ian Romanick 
> ---
>  src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp 
> b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> index 7aa72b1..149596f 100644
> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> @@ -34,6 +34,7 @@
>   *   - if/endif
>   *   . else in else/endif
>   *   - if/else/endif
> + *   - then in if/else/endif

This list has been " in " so this
line should be "else in if/else"
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/5] i965/cfg: Eliminate an empty then-branch of an if/else/endif

[Matt Turner]

On Thu, Feb 25, 2016 at 5:56 PM, Francisco Jerez  wrote:
> Ian Romanick  writes:
>
>> From: Ian Romanick 
>>
>> On BDW,
>>
>> total instructions in shared programs: 8448571 -> 8448367 (-0.00%)
>> instructions in affected programs: 21000 -> 20796 (-0.97%)
>> helped: 116
>> HURT: 0
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>  src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp | 18 ++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp 
>> b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> index 7aa72b1..149596f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> @@ -34,6 +34,7 @@
>>   *   - if/endif
>>   *   . else in else/endif
>>   *   - if/else/endif
>> + *   - then in if/else/endif
>>   */
>>  bool
>>  dead_control_flow_eliminate(backend_shader *s)
>> @@ -114,6 +115,23 @@ dead_control_flow_eliminate(backend_shader *s)
>>
>>  progress = true;
>>   }
>> +  } else if (inst->opcode == BRW_OPCODE_ELSE &&
>> + prev_inst->opcode == BRW_OPCODE_IF) {
>> + bblock_t *const else_block = block;
>> + bblock_t *const if_block = prev_block;
>> + backend_instruction *const if_inst = prev_inst;
>> + backend_instruction *const else_inst = inst;
>> +
>> + /* Since the else-branch is becoming the new then-branch, the
>> +  * condition has to be inverted.
>> +  */
>> + if_inst->predicate_inverse = !if_inst->predicate_inverse;
>> + else_inst->remove(else_block);
>> +
>> + if (if_block->can_combine_with(else_block))
>> +if_block->combine_with(else_block);
>
> Ugh, IIRC backend_instruction::remove(block) will remove the block
> behind your back when it becomes empty (and it will here because ELSE
> can only be the only instruction left inside 'block' whenever you hit
> this path), so you're passing a pointer to a no-longer-existing block to
> (can_)combine_with().  I believe this will never let you combine the
> blocks anyway because the previous block ends with an IF instruction
> which you haven't removed, so this is effectively a no-op [assuming it
> doesn't crash ;)].  If you drop the two lines above you can put my

Exactly right.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] i965/fs: fix precision of f2b

[Connor Abbott]

On Fri, Feb 26, 2016 at 10:39 AM, Iago Toral  wrote:
> On Fri, 2016-02-26 at 16:28 +0100, Roland Scheidegger wrote:
>> Am 26.02.2016 um 11:25 schrieb Iago Toral:
>> >
>> > On Thu, 2016-02-25 at 18:20 -0500, Ilia Mirkin wrote:
>> >> On Thu, Feb 25, 2016 at 6:16 PM, Francisco Jerez  
>> >> wrote:
>> >>> Ian Romanick  writes:
>> >>>
>>  On 02/25/2016 12:13 PM, Francisco Jerez wrote:
>> > Ian Romanick  writes:
>> >
>> >> On 02/25/2016 08:46 AM, Roland Scheidegger wrote:
>> >>> Am 25.02.2016 um 11:15 schrieb Iago Toral Quiroga:
>>  From the OpenGL 4.2 spec:
>> 
>>  "When a constructor is used to convert any integer or 
>>  floating-point type to a
>>   bool, 0 and 0.0 are converted to false, and non-zero values are 
>>  converted to
>>   true."
>> 
>>  Thus, even the smallest non-zero floating value should be 
>>  translated to true.
>>  This behavior has been verified also with proprietary NVIDIA 
>>  drivers.
>> 
>>  Currently, we implement this conversion as a cmp.nz operation with 
>>  floats,
>>  subject to floating-point precision limitations, and as a result, 
>>  relatively
>>  small non-zero floating point numbers return false instead of true.
>> 
>>  This patch fixes the problem by getting rid of the sign bit (to 
>>  cover the case
>>  of -0.0) and testing the result against 0u using an integer 
>>  comparison instead.
>>  ---
>>   src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 ---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>> 
>>  diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
>>  b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>  index db20c71..7d62d7e 100644
>>  --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>  +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>  @@ -913,9 +913,18 @@ fs_visitor::nir_emit_alu(const fs_builder 
>>  , nir_alu_instr *instr)
>> bld.MOV(result, negate(op[0]));
>> break;
>> 
>>  -   case nir_op_f2b:
>>  -  bld.CMP(result, op[0], brw_imm_f(0.0f), BRW_CONDITIONAL_NZ);
>>  -  break;
>>  +   case nir_op_f2b: {
>>  +  /* Because comparing to 0.0f is subject to precision 
>>  limitations, do the
>>  +   * comparison using integers (we need to get rid of the sign 
>>  bit for that)
>>  +   */
>>  +  if (devinfo->gen >= 8)
>>  + op[0] = resolve_source_modifiers(op[0]);
>>  +  op[0] = retype(op[0], BRW_REGISTER_TYPE_UD);
>>  +  bld.AND(op[0], op[0], brw_imm_ud(0x7FFFu));
>>  +  bld.CMP(result, op[0], brw_imm_ud(0u), BRW_CONDITIONAL_NZ);
>>  +   break;
>>  +   }
>>  +
>>  case nir_op_i2b:
>> bld.CMP(result, op[0], brw_imm_d(0), BRW_CONDITIONAL_NZ);
>> break;
>> 
>> >>>
>> >>> Does that fix anything? I don't really see a problem with the 
>> >>> existing
>> >>> logic. Yes any "non-zero value" should be converted to true. But 
>> >>> surely
>> >>> that definition cannot include denorms, which you are always allowed 
>> >>> to
>> >>> flush to zero.
>> >>> (Albeit I can't tell what the result would be with NaNs with the 
>> >>> float
>> >>> compare, nor what the result actually should be in this case since 
>> >>> glsl
>> >>> doesn't require NaNs neither.)
>> >>
>> >> Based on this and Jason's comments, I think we need a bunch of new 
>> >> tests.
>> >>
>> >>  - smallest positive normal number
>> >>  - abs of smallest positive normal number
>> >>  - neg of "   ""  "
>> >>  - largest positive subnormal number
>> >>  - abs of largest positive subnormal number
>> >>  - neg of""""
>> >>  - all of the above with negative numbers
>> >>  - NaN
>> >>  - abs of NaN
>> >>  - neg of  "
>> >>
>> >> Perhaps others? +/-Inf just for kicks?
>> >
>> > What's the point?  The result of most of the above (except possibly
>> > bool(NaN)) is undefined by the spec:
>> >
>> > "Any denormalized value input into a shader or potentially generated by
>> >  any operation in a shader can be flushed to 0. [...] NaNs are not
>> >  required to be generated. [...] Operations and built-in functions that
>> >  operate on a NaN are not required to return a NaN as the result."
>> 
>>  Except that apparently one major OpenGL vendor does something well
>>  defined that's different than what we do.
>> >>>
>> >>> I'm skeptical that nVidia would treat 

[Mesa-dev] [PATCH v2 2/3] gallium/r600: Don't let h/w do endian swap for colorformat

[Oded Gabbay]

Since the rework on gallium pipe formats, there is no more need to do
endian swap of the colorformat in the h/w, because the conversion between
mesa format and gallium (pipe) format takes endianess into account (see
the big #if in p_format.h).

v2: return ENDIAN_NONE only for four 8-bits components
(V_0280A0_COLOR_8_8_8_8)

Signed-off-by: Oded Gabbay 
Cc: "11.1 11.2" 
---
 src/gallium/drivers/r600/r600_state_common.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/gallium/drivers/r600/r600_state_common.c 
b/src/gallium/drivers/r600/r600_state_common.c
index c3346f2..b231d1e 100644
--- a/src/gallium/drivers/r600/r600_state_common.c
+++ b/src/gallium/drivers/r600/r600_state_common.c
@@ -2721,6 +2721,13 @@ uint32_t r600_colorformat_endian_swap(uint32_t 
colorformat)
 
/* 32-bit buffers. */
case V_0280A0_COLOR_8_8_8_8:
+   /*
+* No need to do endian swaps on four 8-bits components,
+* as mesa<-->pipe formats conversion take into account
+* the endianess
+*/
+   return ENDIAN_NONE;
+
case V_0280A0_COLOR_2_10_10_10:
case V_0280A0_COLOR_8_24:
case V_0280A0_COLOR_24_8:
-- 
2.5.0

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

[Mesa-dev] [PATCH v2 3/3] gallium/radeon: disable evergreen_do_fast_color_clear for BE

[Oded Gabbay]

This function is currently broken for BE. I assume it's because of
util_pack_color(). Until I fix this path, I prefer to disable it so users
would be able to see correct colors on their desktop and applications.

Together with the two following patches:
- gallium/r600: Don't let h/w do endian swap for colorformat
- gallium/radeon: remove separate BE path in r600_translate_colorswap

it fixes BZ#72877 and BZ#92039

Signed-off-by: Oded Gabbay 
Cc: "11.1 11.2" 
Reviewed-by: Marek Olšák 
---
 src/gallium/drivers/radeon/r600_texture.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/gallium/drivers/radeon/r600_texture.c 
b/src/gallium/drivers/radeon/r600_texture.c
index 454d0f1..0b31d0a 100644
--- a/src/gallium/drivers/radeon/r600_texture.c
+++ b/src/gallium/drivers/radeon/r600_texture.c
@@ -1408,6 +1408,11 @@ void evergreen_do_fast_color_clear(struct 
r600_common_context *rctx,
 {
int i;
 
+   /* This function is broken in BE, so just disable this path for now */
+#ifdef PIPE_ARCH_BIG_ENDIAN
+   return;
+#endif
+
if (rctx->render_cond)
return;
 
-- 
2.5.0

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

[Mesa-dev] [PATCH v2 1/3] gallium/radeon: remove separate BE path in r600_translate_colorswap

[Oded Gabbay]

After further testing, it appears there is no need for
separate BE path in r600_translate_colorswap()

The only fix remaining is the change of the last if statement, in the 4
channels case. Originally, it contained an invalid swizzle configuration
that never got hit, in LE or BE. So the fix is relevant for both systems.

This patch adds an additional 120 available visuals for LE and BE,
as seen in glxinfo

v2:
Tested for regressions by running piglit gpu.py with CAICOS (r600g) on
x86-64 machine. No regressions found.

Signed-off-by: Oded Gabbay 
Cc: "11.1 11.2" 
Reviewed-by: Marek Olšák 
---
 src/gallium/drivers/radeon/r600_texture.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/src/gallium/drivers/radeon/r600_texture.c 
b/src/gallium/drivers/radeon/r600_texture.c
index 9a3ccb5..454d0f1 100644
--- a/src/gallium/drivers/radeon/r600_texture.c
+++ b/src/gallium/drivers/radeon/r600_texture.c
@@ -1293,25 +1293,14 @@ unsigned r600_translate_colorswap(enum pipe_format 
format)
break;
case 4:
/* check the middle channels, the 1st and 4th channel can be 
NONE */
-#ifdef PIPE_ARCH_LITTLE_ENDIAN
if (HAS_SWIZZLE(1,Y) && HAS_SWIZZLE(2,Z))
return V_0280A0_SWAP_STD; /* XYZW */
else if (HAS_SWIZZLE(1,Z) && HAS_SWIZZLE(2,Y))
return V_0280A0_SWAP_STD_REV; /* WZYX */
else if (HAS_SWIZZLE(1,Y) && HAS_SWIZZLE(2,X))
return V_0280A0_SWAP_ALT; /* ZYXW */
-   else if (HAS_SWIZZLE(1,X) && HAS_SWIZZLE(2,Y))
-   return V_0280A0_SWAP_ALT_REV; /* WXYZ */
-#else
-   if (HAS_SWIZZLE(1,W) && HAS_SWIZZLE(2,X))
-   return V_0280A0_SWAP_STD_REV; /* ZWXY */
-   else if (HAS_SWIZZLE(1,X) && HAS_SWIZZLE(2,W))
-   return V_0280A0_SWAP_STD; /* YXWZ */
-   else if (HAS_SWIZZLE(1,W) && HAS_SWIZZLE(2,Z))
-   return V_0280A0_SWAP_ALT_REV; /* XWZY */
else if (HAS_SWIZZLE(1,Z) && HAS_SWIZZLE(2,W))
-   return V_0280A0_SWAP_ALT; /* YZWX */
-#endif
+   return V_0280A0_SWAP_ALT_REV; /* YZWX */
break;
}
return ~0U;
-- 
2.5.0

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

Re: [Mesa-dev] [PATCH] mesa: expose GL_EXT_texture_sRGB_decode on GLES 3.0+

[Samuel Iglesias Gonsálvez]

On 2016-02-26 15:30, Ilia Mirkin wrote:

On Fri, Feb 26, 2016 at 7:27 AM, Samuel Iglesias Gonsálvez
 wrote:
On Fri, Feb 26, 2016 at 01:01:28PM +0100, Samuel Iglesias Gonsálvez 
wrote:

Reviewed-by: Samuel Iglesias Gonsálvez 



I forgot one comment... See below.


On Sat, Feb 20, 2016 at 04:00:49PM -0500, Ilia Mirkin wrote:
> Could be exposed on earlier GLES versions if we supported EXT_sRGB, but
> we don't, for now.
>
> Signed-off-by: Ilia Mirkin 
> ---
>
> Passes all the relevant dEQP tests (although they only test state, not the
> actual decoding... but those bits are mostly ctx-agnostic paths).
>
> Note that this ext is part of the ANDROID_extension_pack_es31a
>
>  src/mesa/main/extensions_table.h | 2 +-
>  src/mesa/main/texparam.c | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/extensions_table.h 
b/src/mesa/main/extensions_table.h
> index e4ca2b6..74cb3d8 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -248,7 +248,7 @@ EXT(EXT_texture_object  , dummy_true
>  EXT(EXT_texture_rectangle   , NV_texture_rectangle   
, GLL,  x ,  x ,  x , 2004)
>  EXT(EXT_texture_rg  , ARB_texture_rg 
,  x ,  x ,  x , ES2, 2011)
>  EXT(EXT_texture_sRGB, EXT_texture_sRGB   
, GLL, GLC,  x ,  x , 2004)
> -EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode
, GLL, GLC,  x ,  x , 2006)
> +EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode
, GLL, GLC,  x ,  30, 2006)
>  EXT(EXT_texture_shared_exponent , EXT_texture_shared_exponent
, GLL, GLC,  x ,  x , 2004)
>  EXT(EXT_texture_snorm   , EXT_texture_snorm  
, GLL, GLC,  x ,  x , 2009)
>  EXT(EXT_texture_swizzle , EXT_texture_swizzle
, GLL, GLC,  x ,  x , 2008)
> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> index 20770a7..3b769f4 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -568,8 +568,7 @@ set_tex_parameteri(struct gl_context *ctx,
>goto invalid_pname;
>
> case GL_TEXTURE_SRGB_DECODE_EXT:
> -  if (_mesa_is_desktop_gl(ctx)
> -  && ctx->Extensions.EXT_texture_sRGB_decode) {
> +  if (ctx->Extensions.EXT_texture_sRGB_decode) {


Taking another look at it, this line can be changed to be:

   if(_mesa_has_EXT_texture_sRGB_decode(ctx))

to take advantage of this API.


I thought about that... the thing is that there are 7 other places
that check for ctx->Extensions.EXT_texture_sRGB_decode, but without
the _mesa_is_desktop_gl check. A separate change to flip all of them
over could make sense, but tbh, seems hardly worth it.



Yeah, I agree. For that reason I kept my R-b with or without this 
change.


Thanks,

Sam


With or without this change,

Reviewed-by: Samuel Iglesias Gonsálvez 


Thanks!




>   GLenum decode = params[0];
>
>   if (!target_allows_setting_sampler_parameters(texObj->Target))
> --
> 2.4.10
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev





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



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

Re: [Mesa-dev] [PATCH] mesa: add GL_OES_gpu_shader5 and GL_EXT_gpu_shader5 support

[Samuel Iglesias Gonsálvez]

On 2016-02-26 15:32, Ilia Mirkin wrote:

On Fri, Feb 26, 2016 at 6:55 AM, Samuel Iglesias Gonsálvez
 wrote:

On Fri, Feb 19, 2016 at 07:10:24PM -0500, Ilia Mirkin wrote:
The two extensions are identical, and are largely taking bits of 
already

existing desktop functionality. We continue to do a poor job of
supporting the 'precise' keyword, just like we do on desktop.

This passes the relevant dEQP tests that I could find.

Signed-off-by: Ilia Mirkin 
---
 docs/GL3.txt |  2 +-
 src/compiler/glsl/ast_array_index.cpp| 20 +--
 src/compiler/glsl/builtin_functions.cpp  | 99 
+++-

 src/compiler/glsl/glcpp/glcpp-parse.y|  4 ++
 src/compiler/glsl/glsl_lexer.ll  |  2 +-
 src/compiler/glsl/glsl_parser_extras.cpp |  2 +
 src/compiler/glsl/glsl_parser_extras.h   |  4 ++
 src/mesa/main/extensions_table.h |  2 +
 8 files changed, 88 insertions(+), 47 deletions(-)

diff --git a/docs/GL3.txt b/docs/GL3.txt
index 2e528d4..e7d40de 100644
--- a/docs/GL3.txt
+++ b/docs/GL3.txt
@@ -245,7 +245,7 @@ GLES3.2, GLSL ES 3.2
   GL_OES_draw_buffers_indexed  not started
   GL_OES_draw_elements_base_vertex DONE (all 
drivers)
   GL_OES_geometry_shader   started 
(Marta)
-  GL_OES_gpu_shader5   not started 
(based on parts of GL_ARB_gpu_shader5, which is done for some 
drivers)
+  GL_OES_gpu_shader5   DONE (all 
drivers that support GL_ARB_gpu_shader5)

   GL_OES_primitive_bounding boxnot started
   GL_OES_sample_shadingDONE (nvc0, 
r600, radeonsi)
   GL_OES_sample_variables  DONE (nvc0, 
r600, radeonsi)
diff --git a/src/compiler/glsl/ast_array_index.cpp 
b/src/compiler/glsl/ast_array_index.cpp

index f5baeb9..af5e89e 100644
--- a/src/compiler/glsl/ast_array_index.cpp
+++ b/src/compiler/glsl/ast_array_index.cpp
@@ -236,13 +236,22 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
 _mesa_glsl_error(, state, "unsized array index must 
be constant");

  }
   } else if (array->type->without_array()->is_interface()
- && (array->variable_referenced()->data.mode == 
ir_var_uniform ||
- array->variable_referenced()->data.mode == 
ir_var_shader_storage)
- && !state->is_version(400, 0) && 
!state->ARB_gpu_shader5_enable) {
+ && ((array->variable_referenced()->data.mode == 
ir_var_uniform

+  && !state->is_version(400, 320)
+  && !state->ARB_gpu_shader5_enable
+  && !state->EXT_gpu_shader5_enable
+  && !state->OES_gpu_shader5_enable) ||
+ (array->variable_referenced()->data.mode == 
ir_var_shader_storage

+  && !state->is_version(400, 0)
+  && !state->ARB_gpu_shader5_enable))) {
   /* Page 50 in section 4.3.9 of the OpenGL ES 3.10 spec says:
*
* "All indices used to index a uniform or shader storage 
block

* array must be constant integral expressions."
+  *
+  * But OES_gpu_shader5 (and ESSL 3.20) relax this to allow 
indexing

+  * on uniform blocks but not shader storage blocks.
+  *


Indention here.


This file is a sad mix of tabs and spaces. Should I use tabs instead
like the other lines do? Fix the other lines?



I prefer to fix the other lines of the comment.

Thanks!

Sam



Other than that,

Reviewed-by: Samuel Iglesias Gonsálvez 


Thanks!

  -ilia

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

Re: [Mesa-dev] [PATCH 1/2] i965/fs: fix precision of f2b

[Iago Toral]

On Fri, 2016-02-26 at 16:28 +0100, Roland Scheidegger wrote:
> Am 26.02.2016 um 11:25 schrieb Iago Toral:
> > 
> > On Thu, 2016-02-25 at 18:20 -0500, Ilia Mirkin wrote:
> >> On Thu, Feb 25, 2016 at 6:16 PM, Francisco Jerez  
> >> wrote:
> >>> Ian Romanick  writes:
> >>>
>  On 02/25/2016 12:13 PM, Francisco Jerez wrote:
> > Ian Romanick  writes:
> >
> >> On 02/25/2016 08:46 AM, Roland Scheidegger wrote:
> >>> Am 25.02.2016 um 11:15 schrieb Iago Toral Quiroga:
>  From the OpenGL 4.2 spec:
> 
>  "When a constructor is used to convert any integer or floating-point 
>  type to a
>   bool, 0 and 0.0 are converted to false, and non-zero values are 
>  converted to
>   true."
> 
>  Thus, even the smallest non-zero floating value should be translated 
>  to true.
>  This behavior has been verified also with proprietary NVIDIA drivers.
> 
>  Currently, we implement this conversion as a cmp.nz operation with 
>  floats,
>  subject to floating-point precision limitations, and as a result, 
>  relatively
>  small non-zero floating point numbers return false instead of true.
> 
>  This patch fixes the problem by getting rid of the sign bit (to 
>  cover the case
>  of -0.0) and testing the result against 0u using an integer 
>  comparison instead.
>  ---
>   src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 ---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
>  diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
>  b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>  index db20c71..7d62d7e 100644
>  --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>  +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>  @@ -913,9 +913,18 @@ fs_visitor::nir_emit_alu(const fs_builder , 
>  nir_alu_instr *instr)
> bld.MOV(result, negate(op[0]));
> break;
> 
>  -   case nir_op_f2b:
>  -  bld.CMP(result, op[0], brw_imm_f(0.0f), BRW_CONDITIONAL_NZ);
>  -  break;
>  +   case nir_op_f2b: {
>  +  /* Because comparing to 0.0f is subject to precision 
>  limitations, do the
>  +   * comparison using integers (we need to get rid of the sign 
>  bit for that)
>  +   */
>  +  if (devinfo->gen >= 8)
>  + op[0] = resolve_source_modifiers(op[0]);
>  +  op[0] = retype(op[0], BRW_REGISTER_TYPE_UD);
>  +  bld.AND(op[0], op[0], brw_imm_ud(0x7FFFu));
>  +  bld.CMP(result, op[0], brw_imm_ud(0u), BRW_CONDITIONAL_NZ);
>  +   break;
>  +   }
>  +
>  case nir_op_i2b:
> bld.CMP(result, op[0], brw_imm_d(0), BRW_CONDITIONAL_NZ);
> break;
> 
> >>>
> >>> Does that fix anything? I don't really see a problem with the existing
> >>> logic. Yes any "non-zero value" should be converted to true. But 
> >>> surely
> >>> that definition cannot include denorms, which you are always allowed 
> >>> to
> >>> flush to zero.
> >>> (Albeit I can't tell what the result would be with NaNs with the float
> >>> compare, nor what the result actually should be in this case since 
> >>> glsl
> >>> doesn't require NaNs neither.)
> >>
> >> Based on this and Jason's comments, I think we need a bunch of new 
> >> tests.
> >>
> >>  - smallest positive normal number
> >>  - abs of smallest positive normal number
> >>  - neg of "   ""  "
> >>  - largest positive subnormal number
> >>  - abs of largest positive subnormal number
> >>  - neg of""""
> >>  - all of the above with negative numbers
> >>  - NaN
> >>  - abs of NaN
> >>  - neg of  "
> >>
> >> Perhaps others? +/-Inf just for kicks?
> >
> > What's the point?  The result of most of the above (except possibly
> > bool(NaN)) is undefined by the spec:
> >
> > "Any denormalized value input into a shader or potentially generated by
> >  any operation in a shader can be flushed to 0. [...] NaNs are not
> >  required to be generated. [...] Operations and built-in functions that
> >  operate on a NaN are not required to return a NaN as the result."
> 
>  Except that apparently one major OpenGL vendor does something well
>  defined that's different than what we do.
> >>>
> >>> I'm skeptical that nVidia would treat single-precision denorms
> >>> inconsistently between datatype constructors and other floating-point
> >>> arithmetic, but assuming that's the case it would be an argument for
> >>> proposing the spec change to Khronos 

Re: [Mesa-dev] [PATCH 1/2] i965/fs: fix precision of f2b

[Roland Scheidegger]

Am 26.02.2016 um 11:25 schrieb Iago Toral:
> 
> On Thu, 2016-02-25 at 18:20 -0500, Ilia Mirkin wrote:
>> On Thu, Feb 25, 2016 at 6:16 PM, Francisco Jerez  
>> wrote:
>>> Ian Romanick  writes:
>>>
 On 02/25/2016 12:13 PM, Francisco Jerez wrote:
> Ian Romanick  writes:
>
>> On 02/25/2016 08:46 AM, Roland Scheidegger wrote:
>>> Am 25.02.2016 um 11:15 schrieb Iago Toral Quiroga:
 From the OpenGL 4.2 spec:

 "When a constructor is used to convert any integer or floating-point 
 type to a
  bool, 0 and 0.0 are converted to false, and non-zero values are 
 converted to
  true."

 Thus, even the smallest non-zero floating value should be translated 
 to true.
 This behavior has been verified also with proprietary NVIDIA drivers.

 Currently, we implement this conversion as a cmp.nz operation with 
 floats,
 subject to floating-point precision limitations, and as a result, 
 relatively
 small non-zero floating point numbers return false instead of true.

 This patch fixes the problem by getting rid of the sign bit (to cover 
 the case
 of -0.0) and testing the result against 0u using an integer comparison 
 instead.
 ---
  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 index db20c71..7d62d7e 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 @@ -913,9 +913,18 @@ fs_visitor::nir_emit_alu(const fs_builder , 
 nir_alu_instr *instr)
bld.MOV(result, negate(op[0]));
break;

 -   case nir_op_f2b:
 -  bld.CMP(result, op[0], brw_imm_f(0.0f), BRW_CONDITIONAL_NZ);
 -  break;
 +   case nir_op_f2b: {
 +  /* Because comparing to 0.0f is subject to precision 
 limitations, do the
 +   * comparison using integers (we need to get rid of the sign 
 bit for that)
 +   */
 +  if (devinfo->gen >= 8)
 + op[0] = resolve_source_modifiers(op[0]);
 +  op[0] = retype(op[0], BRW_REGISTER_TYPE_UD);
 +  bld.AND(op[0], op[0], brw_imm_ud(0x7FFFu));
 +  bld.CMP(result, op[0], brw_imm_ud(0u), BRW_CONDITIONAL_NZ);
 +   break;
 +   }
 +
 case nir_op_i2b:
bld.CMP(result, op[0], brw_imm_d(0), BRW_CONDITIONAL_NZ);
break;

>>>
>>> Does that fix anything? I don't really see a problem with the existing
>>> logic. Yes any "non-zero value" should be converted to true. But surely
>>> that definition cannot include denorms, which you are always allowed to
>>> flush to zero.
>>> (Albeit I can't tell what the result would be with NaNs with the float
>>> compare, nor what the result actually should be in this case since glsl
>>> doesn't require NaNs neither.)
>>
>> Based on this and Jason's comments, I think we need a bunch of new tests.
>>
>>  - smallest positive normal number
>>  - abs of smallest positive normal number
>>  - neg of "   ""  "
>>  - largest positive subnormal number
>>  - abs of largest positive subnormal number
>>  - neg of""""
>>  - all of the above with negative numbers
>>  - NaN
>>  - abs of NaN
>>  - neg of  "
>>
>> Perhaps others? +/-Inf just for kicks?
>
> What's the point?  The result of most of the above (except possibly
> bool(NaN)) is undefined by the spec:
>
> "Any denormalized value input into a shader or potentially generated by
>  any operation in a shader can be flushed to 0. [...] NaNs are not
>  required to be generated. [...] Operations and built-in functions that
>  operate on a NaN are not required to return a NaN as the result."

 Except that apparently one major OpenGL vendor does something well
 defined that's different than what we do.
>>>
>>> I'm skeptical that nVidia would treat single-precision denorms
>>> inconsistently between datatype constructors and other floating-point
>>> arithmetic, but assuming that's the case it would be an argument for
>>> proposing the spec change to Khronos rather than introducing a dubiously
>>> compliant change into the back-end.  I think I would argue against
>>> making such a change in the spec in any case, because even though it's
>>> implementation-defined whether denorms are flushed or not, the following
>>> is guaranteed by the spec AFAIUI:
>>>
>>> | if (bool(f))

[Mesa-dev] [PATCH 1/2] glsl: add is_lhs bool on ast_expression

[Alejandro Piñeiro]

Useful to know if a expression is the recipient of an assignment
or not, that would be used to (for example) raise warnings of
"use of uninitialized variable" without getting a false positive
when assigning first a variable.

By default the value is false, and it is assigned to true on
the following cases:
 * The lhs assignments subexpression
 * At ast_array_index, on the array itself.
 * While handling the method on an array, to avoid the warning
   calling array.length
 * When computed the cached test expression at test_to_hir, to
   avoid a duplicate warning on the test expression of a switch.

set_is_lhs setter is added, because in some cases (like ast_field_selection)
the value need to be propagated on the expression tree. To avoid doing the
propatagion if not needed, it skips if no primary_expression.identifier is
available.

v2: use a new bool on ast_expression, instead of a new parameter
on ast_expression::hir (Timothy Arceri)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94129
---
 src/compiler/glsl/ast.h|  5 +
 src/compiler/glsl/ast_function.cpp |  3 +++
 src/compiler/glsl/ast_to_hir.cpp   | 30 ++
 3 files changed, 38 insertions(+)

diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index 9aa5bb9..d07b595 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -263,6 +263,11 @@ public:
 * This pointer may be \c NULL.
 */
const char *non_lvalue_description;
+
+   void set_is_lhs(bool new_value);
+
+private:
+   bool is_lhs = false;
 };
 
 class ast_expression_bin : public ast_expression {
diff --git a/src/compiler/glsl/ast_function.cpp 
b/src/compiler/glsl/ast_function.cpp
index 1a44020..49afccc 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -1727,6 +1727,9 @@ ast_function_expression::handle_method(exec_list 
*instructions,
const char *method;
method = field->primary_expression.identifier;
 
+   /* This would prevent to raise "unitialized variable" warnings when calling
+* array.length. */
+   field->subexpressions[0]->set_is_lhs(true);
op = field->subexpressions[0]->hir(instructions, state);
if (strcmp(method, "length") == 0) {
   if (!this->expressions.is_empty()) {
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index db5ec9a..49e4858 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -1248,6 +1248,23 @@ ast_expression::hir_no_rvalue(exec_list *instructions,
do_hir(instructions, state, false);
 }
 
+void
+ast_expression::set_is_lhs(bool new_value)
+{
+   /* is_lhs is tracked only to print "variable used unitialized warnings", if
+* we lack a identifier we can just skip, so also skipping going through
+* subexpressions (see below) */
+   if (this->primary_expression.identifier == NULL)
+  return;
+
+   this->is_lhs = new_value;
+
+   /* We need to go through the subexpressions tree to cover cases like
+* ast_field_selection */
+   if (this->subexpressions[0] != NULL)
+  this->subexpressions[0]->set_is_lhs(new_value);
+}
+
 ir_rvalue *
 ast_expression::do_hir(exec_list *instructions,
struct _mesa_glsl_parse_state *state,
@@ -1323,6 +1340,7 @@ ast_expression::do_hir(exec_list *instructions,
   break;
 
case ast_assign: {
+  this->subexpressions[0]->set_is_lhs(true);
   op[0] = this->subexpressions[0]->hir(instructions, state);
   op[1] = this->subexpressions[1]->hir(instructions, state);
 
@@ -1592,6 +1610,7 @@ ast_expression::do_hir(exec_list *instructions,
case ast_div_assign:
case ast_add_assign:
case ast_sub_assign: {
+  this->subexpressions[0]->set_is_lhs(true);
   op[0] = this->subexpressions[0]->hir(instructions, state);
   op[1] = this->subexpressions[1]->hir(instructions, state);
 
@@ -1618,6 +1637,7 @@ ast_expression::do_hir(exec_list *instructions,
}
 
case ast_mod_assign: {
+  this->subexpressions[0]->set_is_lhs(true);
   op[0] = this->subexpressions[0]->hir(instructions, state);
   op[1] = this->subexpressions[1]->hir(instructions, state);
 
@@ -1640,6 +1660,7 @@ ast_expression::do_hir(exec_list *instructions,
 
case ast_ls_assign:
case ast_rs_assign: {
+  this->subexpressions[0]->set_is_lhs(true);
   op[0] = this->subexpressions[0]->hir(instructions, state);
   op[1] = this->subexpressions[1]->hir(instructions, state);
   type = shift_result_type(op[0]->type, op[1]->type, this->oper, state,
@@ -1658,6 +1679,7 @@ ast_expression::do_hir(exec_list *instructions,
case ast_and_assign:
case ast_xor_assign:
case ast_or_assign: {
+  this->subexpressions[0]->set_is_lhs(true);
   op[0] = this->subexpressions[0]->hir(instructions, state);
   op[1] = this->subexpressions[1]->hir(instructions, state);
   type = bit_logic_result_type(op[0], op[1], this->oper, state, );
@@ -1839,6 +1861,10 @@ 

[Mesa-dev] [Bug 94291] llvmpipe tests fail if built on skylake i7-6700k

[bugzilla-daemon]

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

--- Comment #1 from Roland Scheidegger  ---
Could you show the instruction where it crashed (and the disassembly)?

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

[Mesa-dev] [PATCH 2/2] glsl: raise warning when using uninitialized variables

[Alejandro Piñeiro]

v2:
 * Take into account out varyings too (Timothy Arceri)
 * Fix style (Timothy Arceri)
 * Use a new ast_expression variable, instead of an
   ast_expression::hir new parameter (Timothy Arceri)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94129
---
 src/compiler/glsl/ast_to_hir.cpp | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 49e4858..ac451df 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -1899,6 +1899,13 @@ ast_expression::do_hir(exec_list *instructions,
   if (var != NULL) {
  var->data.used = true;
  result = new(ctx) ir_dereference_variable(var);
+
+ if ((var->data.mode == ir_var_auto || var->data.mode == 
ir_var_shader_out)
+ && !this->is_lhs
+ && result->variable_referenced()->data.assigned != true) {
+_mesa_glsl_warning(, state, "`%s' used uninitialized",
+   this->primary_expression.identifier);
+ }
   } else {
  _mesa_glsl_error(& loc, state, "`%s' undeclared",
   this->primary_expression.identifier);
-- 
2.5.0

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

[Mesa-dev] [PATCH 0/2] glsl: add "use of uninitialized variable" warning

[Alejandro Piñeiro]

As I reference, this week I sent a RFC series for this feature:
https://lists.freedesktop.org/archives/mesa-dev/2016-February/108442.html

This new series uses the feedback provided by Timothy Arceri (thanks!), in
order to handle the doubts I had to consider the original series as an RFC,
and handle some extra cases that were not managed by the original series.

My only doubt is that probably the name of the variable "is_lhs" is probably
not the best one, as it is used also to mark ast_expressions that are not
the lhs of an assignment.

test/all piglit run was made. No regressions.

Alejandro Piñeiro (2):
  glsl: add is_lhs bool on ast_expression
  glsl: raise warning when using uninitialized variables

 src/compiler/glsl/ast.h|  5 +
 src/compiler/glsl/ast_function.cpp |  3 +++
 src/compiler/glsl/ast_to_hir.cpp   | 37 +
 3 files changed, 45 insertions(+)

-- 
2.5.0

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

Re: [Mesa-dev] [PATCH] mesa: add GL_OES_gpu_shader5 and GL_EXT_gpu_shader5 support

[Ilia Mirkin]

On Fri, Feb 26, 2016 at 6:55 AM, Samuel Iglesias Gonsálvez
 wrote:
> On Fri, Feb 19, 2016 at 07:10:24PM -0500, Ilia Mirkin wrote:
>> The two extensions are identical, and are largely taking bits of already
>> existing desktop functionality. We continue to do a poor job of
>> supporting the 'precise' keyword, just like we do on desktop.
>>
>> This passes the relevant dEQP tests that I could find.
>>
>> Signed-off-by: Ilia Mirkin 
>> ---
>>  docs/GL3.txt |  2 +-
>>  src/compiler/glsl/ast_array_index.cpp| 20 +--
>>  src/compiler/glsl/builtin_functions.cpp  | 99 
>> +++-
>>  src/compiler/glsl/glcpp/glcpp-parse.y|  4 ++
>>  src/compiler/glsl/glsl_lexer.ll  |  2 +-
>>  src/compiler/glsl/glsl_parser_extras.cpp |  2 +
>>  src/compiler/glsl/glsl_parser_extras.h   |  4 ++
>>  src/mesa/main/extensions_table.h |  2 +
>>  8 files changed, 88 insertions(+), 47 deletions(-)
>>
>> diff --git a/docs/GL3.txt b/docs/GL3.txt
>> index 2e528d4..e7d40de 100644
>> --- a/docs/GL3.txt
>> +++ b/docs/GL3.txt
>> @@ -245,7 +245,7 @@ GLES3.2, GLSL ES 3.2
>>GL_OES_draw_buffers_indexed  not started
>>GL_OES_draw_elements_base_vertex DONE (all drivers)
>>GL_OES_geometry_shader   started (Marta)
>> -  GL_OES_gpu_shader5   not started (based 
>> on parts of GL_ARB_gpu_shader5, which is done for some drivers)
>> +  GL_OES_gpu_shader5   DONE (all drivers 
>> that support GL_ARB_gpu_shader5)
>>GL_OES_primitive_bounding boxnot started
>>GL_OES_sample_shadingDONE (nvc0, r600, 
>> radeonsi)
>>GL_OES_sample_variables  DONE (nvc0, r600, 
>> radeonsi)
>> diff --git a/src/compiler/glsl/ast_array_index.cpp 
>> b/src/compiler/glsl/ast_array_index.cpp
>> index f5baeb9..af5e89e 100644
>> --- a/src/compiler/glsl/ast_array_index.cpp
>> +++ b/src/compiler/glsl/ast_array_index.cpp
>> @@ -236,13 +236,22 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>>  _mesa_glsl_error(, state, "unsized array index must be 
>> constant");
>>   }
>>} else if (array->type->without_array()->is_interface()
>> - && (array->variable_referenced()->data.mode == 
>> ir_var_uniform ||
>> - array->variable_referenced()->data.mode == 
>> ir_var_shader_storage)
>> - && !state->is_version(400, 0) && 
>> !state->ARB_gpu_shader5_enable) {
>> + && ((array->variable_referenced()->data.mode == 
>> ir_var_uniform
>> +  && !state->is_version(400, 320)
>> +  && !state->ARB_gpu_shader5_enable
>> +  && !state->EXT_gpu_shader5_enable
>> +  && !state->OES_gpu_shader5_enable) ||
>> + (array->variable_referenced()->data.mode == 
>> ir_var_shader_storage
>> +  && !state->is_version(400, 0)
>> +  && !state->ARB_gpu_shader5_enable))) {
>>/* Page 50 in section 4.3.9 of the OpenGL ES 3.10 spec says:
>> *
>> * "All indices used to index a uniform or shader storage block
>> * array must be constant integral expressions."
>> +  *
>> +  * But OES_gpu_shader5 (and ESSL 3.20) relax this to allow indexing
>> +  * on uniform blocks but not shader storage blocks.
>> +  *
>
> Indention here.

This file is a sad mix of tabs and spaces. Should I use tabs instead
like the other lines do? Fix the other lines?

>
> Other than that,
>
> Reviewed-by: Samuel Iglesias Gonsálvez 

Thanks!

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

Re: [Mesa-dev] [PATCH] mesa: expose GL_EXT_texture_sRGB_decode on GLES 3.0+

[Ilia Mirkin]

On Fri, Feb 26, 2016 at 7:27 AM, Samuel Iglesias Gonsálvez
 wrote:
> On Fri, Feb 26, 2016 at 01:01:28PM +0100, Samuel Iglesias Gonsálvez wrote:
>> Reviewed-by: Samuel Iglesias Gonsálvez 
>>
>
> I forgot one comment... See below.
>
>> On Sat, Feb 20, 2016 at 04:00:49PM -0500, Ilia Mirkin wrote:
>> > Could be exposed on earlier GLES versions if we supported EXT_sRGB, but
>> > we don't, for now.
>> >
>> > Signed-off-by: Ilia Mirkin 
>> > ---
>> >
>> > Passes all the relevant dEQP tests (although they only test state, not the
>> > actual decoding... but those bits are mostly ctx-agnostic paths).
>> >
>> > Note that this ext is part of the ANDROID_extension_pack_es31a
>> >
>> >  src/mesa/main/extensions_table.h | 2 +-
>> >  src/mesa/main/texparam.c | 3 +--
>> >  2 files changed, 2 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/src/mesa/main/extensions_table.h 
>> > b/src/mesa/main/extensions_table.h
>> > index e4ca2b6..74cb3d8 100644
>> > --- a/src/mesa/main/extensions_table.h
>> > +++ b/src/mesa/main/extensions_table.h
>> > @@ -248,7 +248,7 @@ EXT(EXT_texture_object  , 
>> > dummy_true
>> >  EXT(EXT_texture_rectangle   , NV_texture_rectangle
>> >, GLL,  x ,  x ,  x , 2004)
>> >  EXT(EXT_texture_rg  , ARB_texture_rg  
>> >,  x ,  x ,  x , ES2, 2011)
>> >  EXT(EXT_texture_sRGB, EXT_texture_sRGB
>> >, GLL, GLC,  x ,  x , 2004)
>> > -EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode 
>> >, GLL, GLC,  x ,  x , 2006)
>> > +EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode 
>> >, GLL, GLC,  x ,  30, 2006)
>> >  EXT(EXT_texture_shared_exponent , EXT_texture_shared_exponent 
>> >, GLL, GLC,  x ,  x , 2004)
>> >  EXT(EXT_texture_snorm   , EXT_texture_snorm   
>> >, GLL, GLC,  x ,  x , 2009)
>> >  EXT(EXT_texture_swizzle , EXT_texture_swizzle 
>> >, GLL, GLC,  x ,  x , 2008)
>> > diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
>> > index 20770a7..3b769f4 100644
>> > --- a/src/mesa/main/texparam.c
>> > +++ b/src/mesa/main/texparam.c
>> > @@ -568,8 +568,7 @@ set_tex_parameteri(struct gl_context *ctx,
>> >goto invalid_pname;
>> >
>> > case GL_TEXTURE_SRGB_DECODE_EXT:
>> > -  if (_mesa_is_desktop_gl(ctx)
>> > -  && ctx->Extensions.EXT_texture_sRGB_decode) {
>> > +  if (ctx->Extensions.EXT_texture_sRGB_decode) {
>
> Taking another look at it, this line can be changed to be:
>
>if(_mesa_has_EXT_texture_sRGB_decode(ctx))
>
> to take advantage of this API.

I thought about that... the thing is that there are 7 other places
that check for ctx->Extensions.EXT_texture_sRGB_decode, but without
the _mesa_is_desktop_gl check. A separate change to flip all of them
over could make sense, but tbh, seems hardly worth it.

> With or without this change,
>
> Reviewed-by: Samuel Iglesias Gonsálvez 

Thanks!

>
>> >   GLenum decode = params[0];
>> >
>> >   if (!target_allows_setting_sampler_parameters(texObj->Target))
>> > --
>> > 2.4.10
>> >
>> > ___
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] android: re-generate git_sha1.h if git HEAD updated

[Emil Velikov]

On 26 February 2016 at 07:09, Chih-Wei Huang  wrote:
> The git_sha1.h has to depend on the git HEAD
> otherwise it will never be updated.
>
> Signed-off-by: Chih-Wei Huang 
> ---
>  src/mesa/Android.gen.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/Android.gen.mk b/src/mesa/Android.gen.mk
> index a985f0a..e567102 100644
> --- a/src/mesa/Android.gen.mk
> +++ b/src/mesa/Android.gen.mk
> @@ -69,7 +69,7 @@ define es-gen
> $(hide) $(PRIVATE_SCRIPT) $(1) $(PRIVATE_XML) > $@
>  endef
>
> -$(intermediates)/main/git_sha1.h:
> +$(intermediates)/main/git_sha1.h: $(wildcard $(MESA_TOP)/.git/HEAD)

I'd suggest porting over the autotools approach (via a temporary file)
or even moving the hunk to Makefile.gen (or Makefile.common) and using
it in both builds.

Related: wondering about moving the includes/cflags - although with
the aosp build changes, things might be problematic. Do you foresee
(from Android POV) any issues with doing that ?

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

Re: [Mesa-dev] [PATCH 2/3] gallium/r600: Don't let h/w do endian swap for colorformat

[Emil Velikov]

On 26 February 2016 at 07:35, Oded Gabbay  wrote:
> On Fri, Feb 26, 2016 at 9:32 AM, Michel Dänzer  wrote:
>> On 26.02.2016 16:14, Oded Gabbay wrote:
>>> On Fri, Feb 26, 2016 at 5:01 AM, Michel Dänzer  wrote:

 [ Dropping mesa-stable list from Cc, since sending patches there by
 e-mail before they've landed on master is basically noise ]
>>>
>>> Problem is that I sometimes later forget to add stable :)
>>
>> Note that I'm only referring to sending patches to the mesa-stable list
>> by e-mail, which isn't necessary for them to be backported to stable
>> branches. The stable branch maintainer will pick patches for backporting
>> using the bin/get-pick-list.sh script.
>>
>> Adding the mesa-stable tag to the commit log is of course fine per se.
>>
> Yeah, I understand, but git send-email is configured to automatically
> adds the cc: tag. Maybe I should disable it...
>
Gents, please don't be afraid to "spam" mesa-stable with such things.
We all forget to land the odd patch and having a reference, allows me
to give you a gentle ping ;-)

The idea is to keep things as open as possible/flexible, so it suits
(almost) everyone's approach and at the same time, there's a smaller
chance of things falling through the cracks.

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

[Mesa-dev] [Bug 91556] Struct / union sizes being calculated incorrectly

[bugzilla-daemon]

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

--- Comment #7 from Serge Martin  ---
This simple change should fix the size calculation.
However the kernel still receive garbage, but we are working on it since
GROMACS have the same problem.

Also the serie found at
https://lists.freedesktop.org/archives/piglit/2016-February/019074.html
is tracking this error

diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index 4d11c24..74c9511 100644
--- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
+++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
@@ -547,8 +547,10 @@ namespace {
module::argument::sign_ext :
module::argument::zero_ext);

+const unsigned arg_store_alloc = TD.getTypeAllocSize(arg_type);
+
 args.push_back(
-   module::argument(module::argument::scalar, arg_api_size,
+   module::argument(module::argument::scalar, arg_store_alloc,
 target_size, target_align, ext_type));
  }
   }

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

Re: [Mesa-dev] [PATCH] mesa: expose GL_EXT_texture_sRGB_decode on GLES 3.0+

[Samuel Iglesias Gonsálvez]

On Fri, Feb 26, 2016 at 01:01:28PM +0100, Samuel Iglesias Gonsálvez wrote:
> Reviewed-by: Samuel Iglesias Gonsálvez 
> 

I forgot one comment... See below.

> On Sat, Feb 20, 2016 at 04:00:49PM -0500, Ilia Mirkin wrote:
> > Could be exposed on earlier GLES versions if we supported EXT_sRGB, but
> > we don't, for now.
> > 
> > Signed-off-by: Ilia Mirkin 
> > ---
> > 
> > Passes all the relevant dEQP tests (although they only test state, not the
> > actual decoding... but those bits are mostly ctx-agnostic paths).
> > 
> > Note that this ext is part of the ANDROID_extension_pack_es31a
> > 
> >  src/mesa/main/extensions_table.h | 2 +-
> >  src/mesa/main/texparam.c | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/mesa/main/extensions_table.h 
> > b/src/mesa/main/extensions_table.h
> > index e4ca2b6..74cb3d8 100644
> > --- a/src/mesa/main/extensions_table.h
> > +++ b/src/mesa/main/extensions_table.h
> > @@ -248,7 +248,7 @@ EXT(EXT_texture_object  , dummy_true
> >  EXT(EXT_texture_rectangle   , NV_texture_rectangle 
> >   , GLL,  x ,  x ,  x , 2004)
> >  EXT(EXT_texture_rg  , ARB_texture_rg   
> >   ,  x ,  x ,  x , ES2, 2011)
> >  EXT(EXT_texture_sRGB, EXT_texture_sRGB 
> >   , GLL, GLC,  x ,  x , 2004)
> > -EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode  
> >   , GLL, GLC,  x ,  x , 2006)
> > +EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode  
> >   , GLL, GLC,  x ,  30, 2006)
> >  EXT(EXT_texture_shared_exponent , EXT_texture_shared_exponent  
> >   , GLL, GLC,  x ,  x , 2004)
> >  EXT(EXT_texture_snorm   , EXT_texture_snorm
> >   , GLL, GLC,  x ,  x , 2009)
> >  EXT(EXT_texture_swizzle , EXT_texture_swizzle  
> >   , GLL, GLC,  x ,  x , 2008)
> > diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> > index 20770a7..3b769f4 100644
> > --- a/src/mesa/main/texparam.c
> > +++ b/src/mesa/main/texparam.c
> > @@ -568,8 +568,7 @@ set_tex_parameteri(struct gl_context *ctx,
> >goto invalid_pname;
> >  
> > case GL_TEXTURE_SRGB_DECODE_EXT:
> > -  if (_mesa_is_desktop_gl(ctx)
> > -  && ctx->Extensions.EXT_texture_sRGB_decode) {
> > +  if (ctx->Extensions.EXT_texture_sRGB_decode) {

Taking another look at it, this line can be changed to be:

   if(_mesa_has_EXT_texture_sRGB_decode(ctx))

to take advantage of this API. With or without this change,

Reviewed-by: Samuel Iglesias Gonsálvez 

> >   GLenum decode = params[0];
> >  
> >   if (!target_allows_setting_sampler_parameters(texObj->Target))
> > -- 
> > 2.4.10
> > 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev



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



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

Re: [Mesa-dev] [PATCH] mesa: expose GL_EXT_texture_sRGB_decode on GLES 3.0+

[Samuel Iglesias Gonsálvez]

Reviewed-by: Samuel Iglesias Gonsálvez 

On Sat, Feb 20, 2016 at 04:00:49PM -0500, Ilia Mirkin wrote:
> Could be exposed on earlier GLES versions if we supported EXT_sRGB, but
> we don't, for now.
> 
> Signed-off-by: Ilia Mirkin 
> ---
> 
> Passes all the relevant dEQP tests (although they only test state, not the
> actual decoding... but those bits are mostly ctx-agnostic paths).
> 
> Note that this ext is part of the ANDROID_extension_pack_es31a
> 
>  src/mesa/main/extensions_table.h | 2 +-
>  src/mesa/main/texparam.c | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/main/extensions_table.h 
> b/src/mesa/main/extensions_table.h
> index e4ca2b6..74cb3d8 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -248,7 +248,7 @@ EXT(EXT_texture_object  , dummy_true
>  EXT(EXT_texture_rectangle   , NV_texture_rectangle   
> , GLL,  x ,  x ,  x , 2004)
>  EXT(EXT_texture_rg  , ARB_texture_rg 
> ,  x ,  x ,  x , ES2, 2011)
>  EXT(EXT_texture_sRGB, EXT_texture_sRGB   
> , GLL, GLC,  x ,  x , 2004)
> -EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode
> , GLL, GLC,  x ,  x , 2006)
> +EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode
> , GLL, GLC,  x ,  30, 2006)
>  EXT(EXT_texture_shared_exponent , EXT_texture_shared_exponent
> , GLL, GLC,  x ,  x , 2004)
>  EXT(EXT_texture_snorm   , EXT_texture_snorm  
> , GLL, GLC,  x ,  x , 2009)
>  EXT(EXT_texture_swizzle , EXT_texture_swizzle
> , GLL, GLC,  x ,  x , 2008)
> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> index 20770a7..3b769f4 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -568,8 +568,7 @@ set_tex_parameteri(struct gl_context *ctx,
>goto invalid_pname;
>  
> case GL_TEXTURE_SRGB_DECODE_EXT:
> -  if (_mesa_is_desktop_gl(ctx)
> -  && ctx->Extensions.EXT_texture_sRGB_decode) {
> +  if (ctx->Extensions.EXT_texture_sRGB_decode) {
>   GLenum decode = params[0];
>  
>   if (!target_allows_setting_sampler_parameters(texObj->Target))
> -- 
> 2.4.10
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH] mesa: add GL_OES_gpu_shader5 and GL_EXT_gpu_shader5 support

[Samuel Iglesias Gonsálvez]

On Fri, Feb 19, 2016 at 07:10:24PM -0500, Ilia Mirkin wrote:
> The two extensions are identical, and are largely taking bits of already
> existing desktop functionality. We continue to do a poor job of
> supporting the 'precise' keyword, just like we do on desktop.
> 
> This passes the relevant dEQP tests that I could find.
> 
> Signed-off-by: Ilia Mirkin 
> ---
>  docs/GL3.txt |  2 +-
>  src/compiler/glsl/ast_array_index.cpp| 20 +--
>  src/compiler/glsl/builtin_functions.cpp  | 99 
> +++-
>  src/compiler/glsl/glcpp/glcpp-parse.y|  4 ++
>  src/compiler/glsl/glsl_lexer.ll  |  2 +-
>  src/compiler/glsl/glsl_parser_extras.cpp |  2 +
>  src/compiler/glsl/glsl_parser_extras.h   |  4 ++
>  src/mesa/main/extensions_table.h |  2 +
>  8 files changed, 88 insertions(+), 47 deletions(-)
> 
> diff --git a/docs/GL3.txt b/docs/GL3.txt
> index 2e528d4..e7d40de 100644
> --- a/docs/GL3.txt
> +++ b/docs/GL3.txt
> @@ -245,7 +245,7 @@ GLES3.2, GLSL ES 3.2
>GL_OES_draw_buffers_indexed  not started
>GL_OES_draw_elements_base_vertex DONE (all drivers)
>GL_OES_geometry_shader   started (Marta)
> -  GL_OES_gpu_shader5   not started (based on 
> parts of GL_ARB_gpu_shader5, which is done for some drivers)
> +  GL_OES_gpu_shader5   DONE (all drivers 
> that support GL_ARB_gpu_shader5)
>GL_OES_primitive_bounding boxnot started
>GL_OES_sample_shadingDONE (nvc0, r600, 
> radeonsi)
>GL_OES_sample_variables  DONE (nvc0, r600, 
> radeonsi)
> diff --git a/src/compiler/glsl/ast_array_index.cpp 
> b/src/compiler/glsl/ast_array_index.cpp
> index f5baeb9..af5e89e 100644
> --- a/src/compiler/glsl/ast_array_index.cpp
> +++ b/src/compiler/glsl/ast_array_index.cpp
> @@ -236,13 +236,22 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>  _mesa_glsl_error(, state, "unsized array index must be 
> constant");
>   }
>} else if (array->type->without_array()->is_interface()
> - && (array->variable_referenced()->data.mode == 
> ir_var_uniform ||
> - array->variable_referenced()->data.mode == 
> ir_var_shader_storage)
> - && !state->is_version(400, 0) && 
> !state->ARB_gpu_shader5_enable) {
> + && ((array->variable_referenced()->data.mode == 
> ir_var_uniform
> +  && !state->is_version(400, 320)
> +  && !state->ARB_gpu_shader5_enable
> +  && !state->EXT_gpu_shader5_enable
> +  && !state->OES_gpu_shader5_enable) ||
> + (array->variable_referenced()->data.mode == 
> ir_var_shader_storage
> +  && !state->is_version(400, 0)
> +  && !state->ARB_gpu_shader5_enable))) {
>/* Page 50 in section 4.3.9 of the OpenGL ES 3.10 spec says:
> *
> * "All indices used to index a uniform or shader storage block
> * array must be constant integral expressions."
> +  *
> +  * But OES_gpu_shader5 (and ESSL 3.20) relax this to allow indexing
> +  * on uniform blocks but not shader storage blocks.
> +  *

Indention here.

Other than that,

Reviewed-by: Samuel Iglesias Gonsálvez 

Sam

> */
>_mesa_glsl_error(, state, "%s block array index must be constant",
>array->variable_referenced()->data.mode
> @@ -279,7 +288,10 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
> * dynamically uniform expression is undefined.
> */
>if (array->type->without_array()->is_sampler()) {
> - if (!state->is_version(400, 0) && !state->ARB_gpu_shader5_enable) {
> + if (!state->is_version(400, 320) &&
> + !state->ARB_gpu_shader5_enable &&
> + !state->EXT_gpu_shader5_enable &&
> + !state->OES_gpu_shader5_enable) {
>  if (state->is_version(130, 300))
> _mesa_glsl_error(, state,
>  "sampler arrays indexed with non-constant "
> diff --git a/src/compiler/glsl/builtin_functions.cpp 
> b/src/compiler/glsl/builtin_functions.cpp
> index 6576650..b862da0 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -240,6 +240,21 @@ gpu_shader5(const _mesa_glsl_parse_state *state)
>  }
>  
>  static bool
> +gpu_shader5_es(const _mesa_glsl_parse_state *state)
> +{
> +   return state->is_version(400, 320) ||
> +  state->ARB_gpu_shader5_enable ||
> +  state->EXT_gpu_shader5_enable ||
> +  state->OES_gpu_shader5_enable;
> +}
> +
> +static bool
> +es31_not_gs5(const _mesa_glsl_parse_state *state)
> +{
> +   

Re: [Mesa-dev] [PATCH 3/3] gallium/radeon: disable evergreen_do_fast_color_clear for BE

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Thu, Feb 25, 2016 at 10:09 PM, Oded Gabbay  wrote:
> This function is currently broken for BE. I assume it's because of
> util_pack_color(). Until I fix this path, I prefer to disable it so users
> would be able to see correct colors on their desktop and applications.
>
> Together with the two following patches:
> - gallium/r600: Don't let h/w do endian swap for colorformat
> - gallium/radeon: remove separate BE path in r600_translate_colorswap
>
> it fixes BZ#72877 and BZ#92039
>
> Signed-off-by: Oded Gabbay 
> Cc: "11.1 11.2" 
> ---
>  src/gallium/drivers/radeon/r600_texture.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
> b/src/gallium/drivers/radeon/r600_texture.c
> index 454d0f1..0b31d0a 100644
> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -1408,6 +1408,11 @@ void evergreen_do_fast_color_clear(struct 
> r600_common_context *rctx,
>  {
> int i;
>
> +   /* This function is broken in BE, so just disable this path for now */
> +#ifdef PIPE_ARCH_BIG_ENDIAN
> +   return;
> +#endif
> +
> if (rctx->render_cond)
> return;
>
> --
> 2.5.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] gallium/radeon: remove separate BE path in r600_translate_colorswap

[Marek Olšák]

On Fri, Feb 26, 2016 at 12:38 PM, Oded Gabbay  wrote:
> On Fri, Feb 26, 2016 at 12:51 PM, Marek Olšák  wrote:
>> On Thu, Feb 25, 2016 at 10:09 PM, Oded Gabbay  wrote:
>>> After further testing, it appears there is no need for
>>> separate BE path in r600_translate_colorswap()
>>>
>>> The only fix remaining is the change of the last if statement, in the 4
>>> channels case. Originally, it contained an invalid swizzle configuration
>>> that never got hit, in LE or BE. So the fix is relevant for both systems.
>>>
>>> This patch adds an additional 120 available visuals for LE and BE,
>>> as seen in glxinfo
>>
>> Really? I don't see how this patch can add anything.
>>
>> Marek
>
> Look harder :)
> What I meant to say is that the patch I sent a couple of days ago
> fixed a certain swizzle that didn't exist anymore:
> else if (HAS_SWIZZLE(1,X) && HAS_SWIZZLE(2,Y))
>
> If you look at util/u_format_table.c (autogenerated file), you will
> see there isn't such as combination, when the number of channels is 4
> (there are such combinations with less than 4 channels).
>
> So existing 4-channel formats didn't get caught here, while this if
> looked for a non-existing swizzle configuration.
>
> I changed it to:
> else if (HAS_SWIZZLE(1,Z) && HAS_SWIZZLE(2,W))
> to capture those 4-channel formats that weren't caught
>
> And in the patch i just sent, I removed the #ifdef
> PIPE_ARCH_LITTLE_ENDIAN, so now this new check is also relevant for
> x86.
>
> Once you get additional formats recognized and supported, the number
> of visuals increases. It was 360 before this patch, and 480 after it,
> in x86.

Sounds good.

Reviewed-by: Marek Olšák 

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

Re: [Mesa-dev] [PATCH 1/3] gallium/radeon: remove separate BE path in r600_translate_colorswap

[Oded Gabbay]

On Fri, Feb 26, 2016 at 5:03 AM, Michel Dänzer  wrote:
> On 26.02.2016 06:09, Oded Gabbay wrote:
>> After further testing, it appears there is no need for
>> separate BE path in r600_translate_colorswap()
>>
>> The only fix remaining is the change of the last if statement, in the 4
>> channels case. Originally, it contained an invalid swizzle configuration
>> that never got hit, in LE or BE. So the fix is relevant for both systems.
>>
>> This patch adds an additional 120 available visuals for LE and BE,
>> as seen in glxinfo
>
> Did you test that this doesn't cause any regressions in piglit gpu.py on
> x86? (Ideally with both r600g and radeonsi)
>
>
> --
> Earthling Michel Dänzer   |   http://www.amd.com
> Libre software enthusiast | Mesa and X developer

I  tested it now with r600g (CAICOS in skylake computer) - there were
no regressions.
I didn't manage to run to completion with radeonsi (TAHITI) - without
my patches ! I didn't even try with my patches. It always got a kernel
crash somewhere during the ~8600 tests

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

Re: [Mesa-dev] [PATCH 1/3] gallium/radeon: remove separate BE path in r600_translate_colorswap

[Oded Gabbay]

On Fri, Feb 26, 2016 at 12:51 PM, Marek Olšák  wrote:
> On Thu, Feb 25, 2016 at 10:09 PM, Oded Gabbay  wrote:
>> After further testing, it appears there is no need for
>> separate BE path in r600_translate_colorswap()
>>
>> The only fix remaining is the change of the last if statement, in the 4
>> channels case. Originally, it contained an invalid swizzle configuration
>> that never got hit, in LE or BE. So the fix is relevant for both systems.
>>
>> This patch adds an additional 120 available visuals for LE and BE,
>> as seen in glxinfo
>
> Really? I don't see how this patch can add anything.
>
> Marek

Look harder :)
What I meant to say is that the patch I sent a couple of days ago
fixed a certain swizzle that didn't exist anymore:
else if (HAS_SWIZZLE(1,X) && HAS_SWIZZLE(2,Y))

If you look at util/u_format_table.c (autogenerated file), you will
see there isn't such as combination, when the number of channels is 4
(there are such combinations with less than 4 channels).

So existing 4-channel formats didn't get caught here, while this if
looked for a non-existing swizzle configuration.

I changed it to:
else if (HAS_SWIZZLE(1,Z) && HAS_SWIZZLE(2,W))
to capture those 4-channel formats that weren't caught

And in the patch i just sent, I removed the #ifdef
PIPE_ARCH_LITTLE_ENDIAN, so now this new check is also relevant for
x86.

Once you get additional formats recognized and supported, the number
of visuals increases. It was 360 before this patch, and 480 after it,
in x86.

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

[Mesa-dev] [Bug 94295] [swrast] piglit shader_runner fast_color_clear/all-colors regression

[bugzilla-daemon]

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

--- Comment #2 from Plamena Manolova  ---
(In reply to Vinson Lee from comment #0)
> mesa: d1509a5848dee57b933139ad2610e99ae09cb5ec (master 11.3.0-devel)
> 
> $ ./bin/shader_runner tests/fast_color_clear/all-colors.shader_test -auto
> Segmentation fault (core dumped)
> 
> 
> (gdb) bt
> #0  find_empty_block (prog=0xf2ae10, uniform=0xf2f030) at
> glsl/link_uniforms.cpp:1051
> #1  link_assign_uniform_locations (prog=prog@entry=0xf2ae10,
> boolean_true=1065353216,
> num_explicit_uniform_locs=num_explicit_uniform_locs@entry=4294967295, 
> max_uniform_locs=98304) at glsl/link_uniforms.cpp:1238
> #2  0x7fd99ef73db9 in link_shaders (ctx=ctx@entry=0x7fd9a4a99010,
> prog=prog@entry=0xf2ae10) at glsl/linker.cpp:4566
> #3  0x7fd99eecb3fb in _mesa_glsl_link_shader
> (ctx=ctx@entry=0x7fd9a4a99010, prog=prog@entry=0xf2ae10) at
> program/ir_to_mesa.cpp:3036
> #4  0x7fd99edd1b8a in link_program (ctx=0x7fd9a4a99010,
> program=) at main/shaderapi.c:1048
> #5  0x7fd9a45dafec in stub_glLinkProgram (program=3) at
> piglit/tests/util/piglit-dispatch-gen.c:32599
> #6  0x0040776a in link_and_use_shaders () at
> piglit/tests/shaders/shader_runner.c:1042
> #7  0x0040e02c in piglit_init (argc=2, argv=0x7ffd6815d008) at
> piglit/tests/shaders/shader_runner.c:3292
> #8  0x7fd9a464b7fb in run_test (gl_fw=0xd34c20, argc=2,
> argv=0x7ffd6815d008)
> at piglit/tests/util/piglit-framework-gl/piglit_winsys_framework.c:73
> #9  0x7fd9a462ff6a in piglit_gl_test_run (argc=2, argv=0x7ffd6815d008,
> config=0x7ffd6815cec0)
> at piglit/tests/util/piglit-framework-gl.c:199
> #10 0x00405b50 in main (argc=2, argv=0x7ffd6815d008) at
> piglit/tests/shaders/shader_runner.c:54
> (gdb) l
> 1046  find_empty_block(struct gl_shader_program *prog,
> 1047   struct gl_uniform_storage *uniform)
> 1048  {
> 1049 const unsigned entries = MAX2(1, uniform->array_elements);
> 1050  
> 1051 foreach_list_typed(struct empty_uniform_block, block, link,
> 1052>EmptyUniformLocations) {
> 1053/* Found a block with enough slots to fit the uniform */
> 1054if (block->slots == entries) {
> 1055   unsigned start = block->start;
> (gdb) print block
> $1 = (empty_uniform_block *) 0x0
> (gdb) print prog->EmptyUniformLocations
> $2 = {head = 0x0, tail = 0x0, tail_pred = 0x0}
> 
> 
> 65dfb3048e8291675ca33581aeff8921f7ea509d is the first bad commit
> commit 65dfb3048e8291675ca33581aeff8921f7ea509d
> Author: Plamena Manolova 
> Date:   Thu Feb 11 15:00:02 2016 +0200
> 
> compiler/glsl: Fix uniform location counting.
> 
> This patch moves the calculation of current uniforms to
> link_uniforms, which makes use of UniformRemapTable which
> stores all the reserved uniform locations.
> 
> Location assignment for implicit uniforms now tries to use
> any gaps left in the table after the location assignment
> for explicit uniforms. This gives us more space to store more
> uniforms.
> 
> Patch is based on earlier patch with following changes/additions:
> 
>1: Move the counting of explicit locations to
>   check_explicit_uniform_locations and then pass
>   the number to link_assign_uniform_locations.
>2: Count the number of empty slots in UniformRemapTable
>   and store them in a list_head.
>3: Try to find an empty slot for implicit locations from
>   the list, if that fails resize UniformRemapTable.
> 
> Fixes following CTS tests:
>ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max
>   
> ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max-array
> 
> Signed-off-by: Tapani Pälli 
> Signed-off-by: Plamena Manolova 
> Reviewed-by: Ilia Mirkin 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93696
> 
> :04 04 5848c556c369c2c798c1c1e036c70c740b56a97a
> 25915fac71a54954aafd0139a55045ba394969e6 Msrc
> bisect run success

Hi Vinson,
Could you verify whether this patch fixes this issue for you?

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

[Mesa-dev] [Bug 94295] [swrast] piglit shader_runner fast_color_clear/all-colors regression

[bugzilla-daemon]

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

--- Comment #1 from Plamena Manolova  ---
Created attachment 121982
  --> https://bugs.freedesktop.org/attachment.cgi?id=121982=edit
Proposed Patch

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

Re: [Mesa-dev] [PATCH 1/3] gallium/radeon: remove separate BE path in r600_translate_colorswap

[Marek Olšák]

On Thu, Feb 25, 2016 at 10:09 PM, Oded Gabbay  wrote:
> After further testing, it appears there is no need for
> separate BE path in r600_translate_colorswap()
>
> The only fix remaining is the change of the last if statement, in the 4
> channels case. Originally, it contained an invalid swizzle configuration
> that never got hit, in LE or BE. So the fix is relevant for both systems.
>
> This patch adds an additional 120 available visuals for LE and BE,
> as seen in glxinfo

Really? I don't see how this patch can add anything.

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

Re: [Mesa-dev] [PATCH 1/2] radeonsi: allow dumping shader disassemblies to a file

[Marek Olšák]

On Fri, Feb 26, 2016 at 4:27 AM, Michel Dänzer  wrote:
>
> What's the purpose of this change? Unless I'm missing something, only
> stderr is ever passed in.

The second patch uses it.

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

Re: [Mesa-dev] [PATCH 1/2] i965/fs: fix precision of f2b

[Iago Toral]

On Thu, 2016-02-25 at 18:20 -0500, Ilia Mirkin wrote:
> On Thu, Feb 25, 2016 at 6:16 PM, Francisco Jerez  
> wrote:
> > Ian Romanick  writes:
> >
> >> On 02/25/2016 12:13 PM, Francisco Jerez wrote:
> >>> Ian Romanick  writes:
> >>>
>  On 02/25/2016 08:46 AM, Roland Scheidegger wrote:
> > Am 25.02.2016 um 11:15 schrieb Iago Toral Quiroga:
> >> From the OpenGL 4.2 spec:
> >>
> >> "When a constructor is used to convert any integer or floating-point 
> >> type to a
> >>  bool, 0 and 0.0 are converted to false, and non-zero values are 
> >> converted to
> >>  true."
> >>
> >> Thus, even the smallest non-zero floating value should be translated 
> >> to true.
> >> This behavior has been verified also with proprietary NVIDIA drivers.
> >>
> >> Currently, we implement this conversion as a cmp.nz operation with 
> >> floats,
> >> subject to floating-point precision limitations, and as a result, 
> >> relatively
> >> small non-zero floating point numbers return false instead of true.
> >>
> >> This patch fixes the problem by getting rid of the sign bit (to cover 
> >> the case
> >> of -0.0) and testing the result against 0u using an integer comparison 
> >> instead.
> >> ---
> >>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 ---
> >>  1 file changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
> >> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> >> index db20c71..7d62d7e 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> >> @@ -913,9 +913,18 @@ fs_visitor::nir_emit_alu(const fs_builder , 
> >> nir_alu_instr *instr)
> >>bld.MOV(result, negate(op[0]));
> >>break;
> >>
> >> -   case nir_op_f2b:
> >> -  bld.CMP(result, op[0], brw_imm_f(0.0f), BRW_CONDITIONAL_NZ);
> >> -  break;
> >> +   case nir_op_f2b: {
> >> +  /* Because comparing to 0.0f is subject to precision 
> >> limitations, do the
> >> +   * comparison using integers (we need to get rid of the sign 
> >> bit for that)
> >> +   */
> >> +  if (devinfo->gen >= 8)
> >> + op[0] = resolve_source_modifiers(op[0]);
> >> +  op[0] = retype(op[0], BRW_REGISTER_TYPE_UD);
> >> +  bld.AND(op[0], op[0], brw_imm_ud(0x7FFFu));
> >> +  bld.CMP(result, op[0], brw_imm_ud(0u), BRW_CONDITIONAL_NZ);
> >> +   break;
> >> +   }
> >> +
> >> case nir_op_i2b:
> >>bld.CMP(result, op[0], brw_imm_d(0), BRW_CONDITIONAL_NZ);
> >>break;
> >>
> >
> > Does that fix anything? I don't really see a problem with the existing
> > logic. Yes any "non-zero value" should be converted to true. But surely
> > that definition cannot include denorms, which you are always allowed to
> > flush to zero.
> > (Albeit I can't tell what the result would be with NaNs with the float
> > compare, nor what the result actually should be in this case since glsl
> > doesn't require NaNs neither.)
> 
>  Based on this and Jason's comments, I think we need a bunch of new tests.
> 
>   - smallest positive normal number
>   - abs of smallest positive normal number
>   - neg of "   ""  "
>   - largest positive subnormal number
>   - abs of largest positive subnormal number
>   - neg of""""
>   - all of the above with negative numbers
>   - NaN
>   - abs of NaN
>   - neg of  "
> 
>  Perhaps others? +/-Inf just for kicks?
> >>>
> >>> What's the point?  The result of most of the above (except possibly
> >>> bool(NaN)) is undefined by the spec:
> >>>
> >>> "Any denormalized value input into a shader or potentially generated by
> >>>  any operation in a shader can be flushed to 0. [...] NaNs are not
> >>>  required to be generated. [...] Operations and built-in functions that
> >>>  operate on a NaN are not required to return a NaN as the result."
> >>
> >> Except that apparently one major OpenGL vendor does something well
> >> defined that's different than what we do.
> >
> > I'm skeptical that nVidia would treat single-precision denorms
> > inconsistently between datatype constructors and other floating-point
> > arithmetic, but assuming that's the case it would be an argument for
> > proposing the spec change to Khronos rather than introducing a dubiously
> > compliant change into the back-end.  I think I would argue against
> > making such a change in the spec in any case, because even though it's
> > implementation-defined whether denorms are flushed or not, the following
> > is guaranteed by the spec AFAIUI:
> >
> > | if (bool(f))
> > |random_arithmetic_on(f /* Guaranteed not 

Re: [Mesa-dev] ANNOUNCE: An open-source Vulkan driver for Intel hardware

[Olivier Galibert]

Ok, I can tell you that 3DSTATE_DEPTH_BUFFER and
3DSTATE_STENCIL_BUFFER seem perfectly correct (assuming the gem
address-patching-in works for the depth buffer address).  I'll see if
I can find a past version that works.

  OG.


On Wed, Feb 17, 2016 at 4:31 PM, Jason Ekstrand  wrote:
> On Tue, Feb 16, 2016 at 11:22 PM, Olivier Galibert 
> wrote:
>>
>> I'm actually interested about how one goes about debugging that kind
>> of problem, if you have pointers.  I would have an idea or two on how
>> to go about it if it was in userspace only, but once it crosses into
>> the kernel I'm not sure what strategies are best.
>
>
> This is almost certainly a userspace problem.  I mentioned before that  it's
> probably a depth/stencil problem.  I remember having similar problems a few
> months ago when I was reviving gen7.  I know that depth/stencil did work at
> some point.
>
> I would start by looking at is where we emit the 3DSTATE_DEPTH_BUFFER and
> 3DSTATE_STENCIL_BUFFER and trying to see if we're setting something up
> wrong.  Sometimes it's just a matter of looking at the documentation and
> comparing the values we're setting to the docs and seeing if the make sense.
> That's where I'd start.
>
> You could also try to go back a little ways (don't to past the update to
> 1.0) to see if you can find a point where depth/stencil worked and try and
> bisect to find where it broke.  That may also provide hints as to what's
> going wrong.
>
> Hope that helps,
> --Jason
>
>>
>>
>> Best,
>>
>>   OG.
>>
>>
>> On Wed, Feb 17, 2016 at 2:51 AM, Jason Ekstrand 
>> wrote:
>> > On Tue, Feb 16, 2016 at 1:21 PM, Olivier Galibert 
>> > wrote:
>> >>
>> >>   Hi,
>> >>
>> >> I'm getting gpu hangs with the lunarg examples (cube and tri) on my
>> >> Haswell (64 bits).  I attach /sys/class/drm/card0/error fwiw.  How
>> >> should I go about debugging that?
>> >
>> >
>> > It's a depth-stencil issue and we know about it.   The gen7 code needs
>> > some
>> > love.   I think Kristian and Jordan have been working on it.
>> > --Jason
>> >
>> >>
>> >>
>> >>   OG.
>> >>
>> >>
>> >> On Tue, Feb 16, 2016 at 4:19 PM, Jason Ekstrand 
>> >> wrote:
>> >> > The Intel mesa team is pleased to announce a brand-new open-source
>> >> > Vulkan
>> >> > driver for Intel hardware.  We've been working hard on this over the
>> >> > course
>> >> > of the past year or so and are excited to finally share it with the
>> >> > community.  We will work on up-streaming the driver in the next few
>> >> > weeks
>> >> > and hope to have it all in place in time for mesa 11.3 (mesa 12?).
>> >> > In
>> >> > the
>> >> > mean time, the driver can be found in the "vulkan" branch of the mesa
>> >> > git
>> >> > repo on freedesktop.org:
>> >> >
>> >> > https://cgit.freedesktop.org/mesa/mesa/log/?h=vulkan
>> >> >
>> >> > More information on building the driver and running a few simple apps
>> >> > can
>> >> > be found on the 01.org web site:
>> >> >
>> >> >
>> >> >
>> >> > https://01.org/linuxgraphics/blogs/jekstrand/2016/open-source-vulkan-drivers-intel-hardware
>> >> >
>> >> > We have talked to people at Red Hat and Cannonical and binaries
>> >> > should
>> >> > be
>> >> > available for Fedora and Ubuntu soon.  We will update the page on
>> >> > 01.org
>> >> > with links as soon as they are available.
>> >> >
>> >> > We have also created a small test suite called crucible which
>> >> > contains a
>> >> > few hundred tests (mostly for miptrees) that we created when bringing
>> >> > up
>> >> > the driver.  This isn't really intended to be the piglit of vulkan.
>> >> > With
>> >> > the CTS being publicly available, most cross-platform tests should go
>> >> > there.  We mostly made crucible so that we could write a few tests
>> >> > early
>> >> > on
>> >> > to get us going and for tests that were targetted specifically at our
>> >> > implementation.  None the less, they may prove useful to someone and
>> >> > we
>> >> > are
>> >> > happy to share them.  The crucible source code can be found at
>> >> >
>> >> > https://cgit.freedesktop.org/mesa/crucible/
>> >> >
>> >> > Frequently Asked Questions:
>> >> >
>> >> > What all hardware does it support?
>> >> >
>> >> >The driver currently supports Sky Lake all the way back to Ivy
>> >> > Bridge.
>> >> >The driver is Vulkan 1.0 conformant for 64-bit builds on Sky Lake,
>> >> >Broadwell, and Braswell.  We are still having a couple of 32-bit
>> >> > issues
>> >> >and support for Haswell, Ivy Bridge, and Bay Trail should be
>> >> > considered
>> >> >experimental.
>> >> >
>> >> > How much code is shared between the Vulkan and GL drivers?
>> >> >
>> >> >For shaders, we're using a SPIR-V to NIR pass which is new, and a
>> >> > few
>> >> >new NIR lowering passes for things that we previously depended on
>> >> > GLSL
>> >> >IR to handle.  Beyond that, we're using the same core NIR and the
>> >> > same
>> >> >

Re: [Mesa-dev] [PATCH 01/10] i965/nir: Do lower_io late for fragment shaders

[Iago Toral]

Series is:

Reviewed-by: Iago Toral Quiroga 

On Thu, 2016-02-25 at 11:01 -0800, Kenneth Graunke wrote:
> From: Jason Ekstrand 
> 
> The Vulkan driver wants to be able to delete fragment outputs that are
> beyond key.nr_color_regions; this is a lot easier if we lower outputs at
> specialization time rather than link time.
> 
> (Rationale added to commit message by Ken)
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 1 +
>  src/mesa/drivers/dri/i965/brw_nir.c  | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index b506040..6c9ba36 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -5594,6 +5594,7 @@ brw_compile_fs(const struct brw_compiler *compiler, 
> void *log_data,
> nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> shader = brw_nir_apply_sampler_key(shader, compiler->devinfo, >tex,
>true);
> +   shader = brw_nir_lower_io(shader, compiler->devinfo, true, false, NULL);
> shader = brw_postprocess_nir(shader, compiler->devinfo, true);
>  
> /* key->alpha_test_func means simulating alpha testing via discards,
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
> b/src/mesa/drivers/dri/i965/brw_nir.c
> index 41059b3..61acf38 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -627,7 +627,8 @@ brw_create_nir(struct brw_context *brw,
>  
> if (nir->stage != MESA_SHADER_VERTEX &&
> nir->stage != MESA_SHADER_TESS_CTRL &&
> -   nir->stage != MESA_SHADER_TESS_EVAL) {
> +   nir->stage != MESA_SHADER_TESS_EVAL &&
> +   nir->stage != MESA_SHADER_FRAGMENT) {
>nir = brw_nir_lower_io(nir, devinfo, is_scalar, false, NULL);
> }
>  


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

Re: [Mesa-dev] [PATCH 2/3] gallium/r600: Don't let h/w do endian swap for colorformat

[Oded Gabbay]

On Fri, Feb 26, 2016 at 9:35 AM, Oded Gabbay  wrote:
> On Fri, Feb 26, 2016 at 9:32 AM, Michel Dänzer  wrote:
>> On 26.02.2016 16:14, Oded Gabbay wrote:
>>> On Fri, Feb 26, 2016 at 5:01 AM, Michel Dänzer  wrote:

 [ Dropping mesa-stable list from Cc, since sending patches there by
 e-mail before they've landed on master is basically noise ]
>>>
>>> Problem is that I sometimes later forget to add stable :)
>>
>> Note that I'm only referring to sending patches to the mesa-stable list
>> by e-mail, which isn't necessary for them to be backported to stable
>> branches. The stable branch maintainer will pick patches for backporting
>> using the bin/get-pick-list.sh script.
>>
>> Adding the mesa-stable tag to the commit log is of course fine per se.
>>
> Yeah, I understand, but git send-email is configured to automatically
> adds the cc: tag. Maybe I should disable it...
>
>>
 On 26.02.2016 06:09, Oded Gabbay wrote:
> Since the rework on gallium pipe formats, there is no more need to do
> endian swap of the colorformat in the h/w, because the conversion between
> mesa format and gallium (pipe) format takes endianess into account (see
> the big #if in p_format.h).

 That may be true for (some?) formats with 4 components of 8 bits, but
 I'd be surprised if it was true for all formats handled by this
 function. Just as one example, consider formats with 32 bits per component.

>>>
>>> I first wanted to get these 3 patches out of the gate so people could
>>> have a working desktop in the most default form they are working (4
>>> components of 8 bits). I promise I will continu to work on this and
>>> will aspire to reach parity with LE, but I'm doing this on my free
>>> time so it could take some time.
>>>
>>> I will definitely want to check all formats.
>>
>> Then you can just add the return ENDIAN_NONE in the
>> V_0280A0_COLOR_8_8_8_8 case instead of at the beginning of the function.
>> That should address Matt's concern as well.
>>
> Hmm, maybe I should. I will check to see if this doesn't cause
> regressions from what I have arrived to and will update here.
>
> Oded
>
>>
>> --
>> Earthling Michel Dänzer   |   http://www.amd.com
>> Libre software enthusiast | Mesa and X developer

OK, that works as well, I'll send a new patch-set soon, once I finish
running piglit on x86.

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