Re: [Mesa-dev] [PATCH v2 7/7] i965/nir: Use signed integer type for booleans

2015-03-21 Thread Jason Ekstrand 
On Fri, Mar 20, 2015 at 2:21 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Fri, Mar 20, 2015 at 1:56 PM, Matt Turner matts...@gmail.com wrote:
 On Fri, Mar 20, 2015 at 1:37 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Fri, Mar 20, 2015 at 1:12 PM, Matt Turner matts...@gmail.com wrote:
 On Fri, Mar 20, 2015 at 11:24 AM, Jason Ekstrand ja...@jlekstrand.net 
 wrote:
 -  emit(MOV(result, masked));
 +  emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked));

 This hunk isn't necessary. masked is vgrf(glsl_type::int_type).

 This hunk retypes result, not masked.

 Oh, yeah. But result is set at the top of the function like this:

fs_reg result = get_nir_dest(instr-dest.dest);
result.type = brw_type_for_nir_type(nir_op_infos[instr-op].output_type);

 Isn't that sufficient? We're only going to be resolving things that
 were bool-typed to begin with, which should mean that the other
 changes in this patch handled it.

I just looked at it and remembered what the problem was.  Right now,
iand, ior, and ixor and defined to take and produce unsigned types.
Why?  I don't know.  I guess they seemed more like unsigned operations
to Connor.  We could change that easily enough to make them signed,
but leaving this hunk in makes the FS backend do the right thing
regardless.
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 7/7] i965/nir: Use signed integer type for booleans

2015-03-20 Thread Matt Turner 
On Fri, Mar 20, 2015 at 1:37 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Fri, Mar 20, 2015 at 1:12 PM, Matt Turner matts...@gmail.com wrote:
 On Fri, Mar 20, 2015 at 11:24 AM, Jason Ekstrand ja...@jlekstrand.net 
 wrote:
 -  emit(MOV(result, masked));
 +  emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked));

 This hunk isn't necessary. masked is vgrf(glsl_type::int_type).

 This hunk retypes result, not masked.

Oh, yeah. But result is set at the top of the function like this:

   fs_reg result = get_nir_dest(instr-dest.dest);
   result.type = brw_type_for_nir_type(nir_op_infos[instr-op].output_type);

Isn't that sufficient? We're only going to be resolving things that
were bool-typed to begin with, which should mean that the other
changes in this patch handled it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 7/7] i965/nir: Use signed integer type for booleans

2015-03-20 Thread Jason Ekstrand 
FS instructions with NIR on i965:
total instructions in shared programs: 2663561 - 2619051 (-1.67%)
instructions in affected programs: 1612965 - 1568455 (-2.76%)
helped:5455
HURT:  12

FS instructions with NIR on g4x:
total instructions in shared programs: 2352633 - 2307908 (-1.90%)
instructions in affected programs: 1441842 - 1397117 (-3.10%)
helped:5463
HURT:  11

FS instructions with NIR on ilk:
total instructions in shared programs: 3997305 - 3934278 (-1.58%)
instructions in affected programs: 2189409 - 2126382 (-2.88%)
helped:8969
HURT:  22

FS instructions with NIR on hsw (snb and ivb were similar):
total instructions in shared programs: 4109389 - 4109242 (-0.00%)
instructions in affected programs: 109869 - 109722 (-0.13%)
helped:339
HURT:  190

No SIMD16 programs were gained or lost on any platform
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 +++---
 1 file changed, 3 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 5fa528c..f79143d 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -522,7 +522,7 @@ fs_visitor::nir_emit_if(nir_if *if_stmt)
/* first, put the condition into f0 */
fs_inst *inst = emit(MOV(reg_null_d,
 retype(get_nir_src(if_stmt-condition),
-   BRW_REGISTER_TYPE_UD)));
+   BRW_REGISTER_TYPE_D)));
inst-conditional_mod = BRW_CONDITIONAL_NZ;
 
emit(IF(BRW_PREDICATE_NORMAL));
@@ -598,9 +598,9 @@ static brw_reg_type
 brw_type_for_nir_type(nir_alu_type type)
 {
switch (type) {
-   case nir_type_bool:
case nir_type_unsigned:
   return BRW_REGISTER_TYPE_UD;
+   case nir_type_bool:
case nir_type_int:
   return BRW_REGISTER_TYPE_D;
case nir_type_float:
@@ -1280,7 +1280,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
   fs_reg masked = vgrf(glsl_type::int_type);
   emit(AND(masked, result, fs_reg(1)));
   masked.negate = true;
-  emit(MOV(result, masked));
+  emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked));
}
 }
 
-- 
2.3.3

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

Re: [Mesa-dev] [PATCH v2 7/7] i965/nir: Use signed integer type for booleans

2015-03-20 Thread Jason Ekstrand 
On Fri, Mar 20, 2015 at 1:12 PM, Matt Turner matts...@gmail.com wrote:
 On Fri, Mar 20, 2015 at 11:24 AM, Jason Ekstrand ja...@jlekstrand.net wrote:
 FS instructions with NIR on i965:
 total instructions in shared programs: 2663561 - 2619051 (-1.67%)
 instructions in affected programs: 1612965 - 1568455 (-2.76%)
 helped:5455
 HURT:  12

 FS instructions with NIR on g4x:
 total instructions in shared programs: 2352633 - 2307908 (-1.90%)
 instructions in affected programs: 1441842 - 1397117 (-3.10%)
 helped:5463
 HURT:  11

 FS instructions with NIR on ilk:
 total instructions in shared programs: 3997305 - 3934278 (-1.58%)
 instructions in affected programs: 2189409 - 2126382 (-2.88%)
 helped:8969
 HURT:  22

 FS instructions with NIR on hsw (snb and ivb were similar):
 total instructions in shared programs: 4109389 - 4109242 (-0.00%)
 instructions in affected programs: 109869 - 109722 (-0.13%)
 helped:339
 HURT:  190

 No SIMD16 programs were gained or lost on any platform
 ---
  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 +++---
  1 file changed, 3 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 5fa528c..f79143d 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 @@ -522,7 +522,7 @@ fs_visitor::nir_emit_if(nir_if *if_stmt)
 /* first, put the condition into f0 */
 fs_inst *inst = emit(MOV(reg_null_d,
  retype(get_nir_src(if_stmt-condition),
 -   BRW_REGISTER_TYPE_UD)));
 +   BRW_REGISTER_TYPE_D)));
 inst-conditional_mod = BRW_CONDITIONAL_NZ;

 emit(IF(BRW_PREDICATE_NORMAL));
 @@ -598,9 +598,9 @@ static brw_reg_type
  brw_type_for_nir_type(nir_alu_type type)
  {
 switch (type) {
 -   case nir_type_bool:
 case nir_type_unsigned:
return BRW_REGISTER_TYPE_UD;
 +   case nir_type_bool:
 case nir_type_int:
return BRW_REGISTER_TYPE_D;
 case nir_type_float:
 @@ -1280,7 +1280,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
fs_reg masked = vgrf(glsl_type::int_type);
emit(AND(masked, result, fs_reg(1)));
masked.negate = true;
 -  emit(MOV(result, masked));
 +  emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked));

 This hunk isn't necessary. masked is vgrf(glsl_type::int_type).

This hunk retypes result, not masked.
--Jason


 With that hunk dropped,

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

Re: [Mesa-dev] [PATCH v2 7/7] i965/nir: Use signed integer type for booleans

2015-03-20 Thread Matt Turner 
On Fri, Mar 20, 2015 at 11:24 AM, Jason Ekstrand ja...@jlekstrand.net wrote:
 FS instructions with NIR on i965:
 total instructions in shared programs: 2663561 - 2619051 (-1.67%)
 instructions in affected programs: 1612965 - 1568455 (-2.76%)
 helped:5455
 HURT:  12

 FS instructions with NIR on g4x:
 total instructions in shared programs: 2352633 - 2307908 (-1.90%)
 instructions in affected programs: 1441842 - 1397117 (-3.10%)
 helped:5463
 HURT:  11

 FS instructions with NIR on ilk:
 total instructions in shared programs: 3997305 - 3934278 (-1.58%)
 instructions in affected programs: 2189409 - 2126382 (-2.88%)
 helped:8969
 HURT:  22

 FS instructions with NIR on hsw (snb and ivb were similar):
 total instructions in shared programs: 4109389 - 4109242 (-0.00%)
 instructions in affected programs: 109869 - 109722 (-0.13%)
 helped:339
 HURT:  190

 No SIMD16 programs were gained or lost on any platform
 ---
  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 +++---
  1 file changed, 3 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 5fa528c..f79143d 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 @@ -522,7 +522,7 @@ fs_visitor::nir_emit_if(nir_if *if_stmt)
 /* first, put the condition into f0 */
 fs_inst *inst = emit(MOV(reg_null_d,
  retype(get_nir_src(if_stmt-condition),
 -   BRW_REGISTER_TYPE_UD)));
 +   BRW_REGISTER_TYPE_D)));
 inst-conditional_mod = BRW_CONDITIONAL_NZ;

 emit(IF(BRW_PREDICATE_NORMAL));
 @@ -598,9 +598,9 @@ static brw_reg_type
  brw_type_for_nir_type(nir_alu_type type)
  {
 switch (type) {
 -   case nir_type_bool:
 case nir_type_unsigned:
return BRW_REGISTER_TYPE_UD;
 +   case nir_type_bool:
 case nir_type_int:
return BRW_REGISTER_TYPE_D;
 case nir_type_float:
 @@ -1280,7 +1280,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
fs_reg masked = vgrf(glsl_type::int_type);
emit(AND(masked, result, fs_reg(1)));
masked.negate = true;
 -  emit(MOV(result, masked));
 +  emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked));

This hunk isn't necessary. masked is vgrf(glsl_type::int_type).

With that hunk dropped,

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

Re: [Mesa-dev] [PATCH v2 7/7] i965/nir: Use signed integer type for booleans

2015-03-20 Thread Jason Ekstrand 
On Fri, Mar 20, 2015 at 1:56 PM, Matt Turner matts...@gmail.com wrote:
 On Fri, Mar 20, 2015 at 1:37 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Fri, Mar 20, 2015 at 1:12 PM, Matt Turner matts...@gmail.com wrote:
 On Fri, Mar 20, 2015 at 11:24 AM, Jason Ekstrand ja...@jlekstrand.net 
 wrote:
 -  emit(MOV(result, masked));
 +  emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked));

 This hunk isn't necessary. masked is vgrf(glsl_type::int_type).

 This hunk retypes result, not masked.

 Oh, yeah. But result is set at the top of the function like this:

fs_reg result = get_nir_dest(instr-dest.dest);
result.type = brw_type_for_nir_type(nir_op_infos[instr-op].output_type);

 Isn't that sufficient? We're only going to be resolving things that
 were bool-typed to begin with, which should mean that the other
 changes in this patch handled it.

Right... Ok, I'll drop the hunk.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev