Re: [Mesa-dev] [PATCH 2/2] nir/validate: Validate that only float ALU outputs are saturated

2015-02-03 Thread Erik Faye-Lund
On Tue, Feb 3, 2015 at 9:43 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 ---
  src/glsl/nir/nir_validate.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
 index 7c801b2..89dfdf8 100644
 --- a/src/glsl/nir/nir_validate.c
 +++ b/src/glsl/nir/nir_validate.c
 @@ -239,6 +239,14 @@ validate_alu_dest(nir_alu_dest *dest, validate_state 
 *state)
  * register/SSA value
  */
 assert(is_packed || !(dest-write_mask  ~((1  dest_size) - 1)));
 +
 +   /* validate that saturate is only ever used on instructions with
 +* destinations of type float
 +*/
 +   nir_alu_instr *alu = nir_instr_as_alu(state-instr);
 +   assert(nir_op_infos[alu-op].output_type == nir_type_float ||
 +  !dest-saturate);

I think this can end up generating a warning on builds with asserts
disabled due to alu being written but never read. Perhaps just do
nir_instr_as_alu(state-instr)-op directly in the expression? It's
a tad less readable, though :/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] nir/validate: Validate that only float ALU outputs are saturated

2015-02-03 Thread Connor Abbott
Reviewed-by: Connor Abbott cwabbo...@gmail.com

Funny, I thought we already did this... whoops.

On Tue, Feb 3, 2015 at 3:43 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 ---
  src/glsl/nir/nir_validate.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
 index 7c801b2..89dfdf8 100644
 --- a/src/glsl/nir/nir_validate.c
 +++ b/src/glsl/nir/nir_validate.c
 @@ -239,6 +239,14 @@ validate_alu_dest(nir_alu_dest *dest, validate_state 
 *state)
  * register/SSA value
  */
 assert(is_packed || !(dest-write_mask  ~((1  dest_size) - 1)));
 +
 +   /* validate that saturate is only ever used on instructions with
 +* destinations of type float
 +*/
 +   nir_alu_instr *alu = nir_instr_as_alu(state-instr);
 +   assert(nir_op_infos[alu-op].output_type == nir_type_float ||
 +  !dest-saturate);
 +
 validate_dest(dest-dest, state);
  }

 --
 2.2.2

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


Re: [Mesa-dev] [PATCH 2/2] nir/validate: Validate that only float ALU outputs are saturated

2015-02-03 Thread Jason Ekstrand
On Tue, Feb 3, 2015 at 1:08 PM, Erik Faye-Lund kusmab...@gmail.com wrote:

 On Tue, Feb 3, 2015 at 9:43 PM, Jason Ekstrand ja...@jlekstrand.net
 wrote:
  ---
   src/glsl/nir/nir_validate.c | 8 
   1 file changed, 8 insertions(+)
 
  diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
  index 7c801b2..89dfdf8 100644
  --- a/src/glsl/nir/nir_validate.c
  +++ b/src/glsl/nir/nir_validate.c
  @@ -239,6 +239,14 @@ validate_alu_dest(nir_alu_dest *dest,
 validate_state *state)
   * register/SSA value
   */
  assert(is_packed || !(dest-write_mask  ~((1  dest_size) - 1)));
  +
  +   /* validate that saturate is only ever used on instructions with
  +* destinations of type float
  +*/
  +   nir_alu_instr *alu = nir_instr_as_alu(state-instr);
  +   assert(nir_op_infos[alu-op].output_type == nir_type_float ||
  +  !dest-saturate);

 I think this can end up generating a warning on builds with asserts
 disabled due to alu being written but never read. Perhaps just do
 nir_instr_as_alu(state-instr)-op directly in the expression? It's
 a tad less readable, though :/


It doesn't even get compiled if we have no asserts.  There's a giant #ifdef
DEBUG surrounding the entire file.
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev