[Mesa-dev] [PATCH 2/3] i965/fs: Make emit_if_gen6 never fall back to emit_bool_to_cond_code.

2014-09-04 Thread Kenneth Graunke 
Matt and I believe that Sandybridge actually uses 0x for a
true comparison result, similar to Ivybridge.  This matches the
internal documentation, and empirical results, but contradicts the PRM.

So, the comment is inaccurate, and we can actually just handle these
directly without ever needing to fall through to the condition code
path.

Also, the vec4 backend has always done it this way, and has apparently
been working fine.  This patch makes the FS backend match the vec4
backend's behavior.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 8f2e1df..d7e3120 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -2378,14 +2378,24 @@ fs_visitor::emit_if_gen6(ir_if *ir)
 
   switch (expr-operation) {
   case ir_unop_logic_not:
+ emit(IF(op[0], fs_reg(0), BRW_CONDITIONAL_Z));
+ return;
+
   case ir_binop_logic_xor:
+ emit(IF(op[0], op[1], BRW_CONDITIONAL_NZ));
+ return;
+
   case ir_binop_logic_or:
+ temp = fs_reg(this, glsl_type::bool_type);
+ emit(OR(temp, op[0], op[1]));
+ emit(IF(temp, fs_reg(0), BRW_CONDITIONAL_NZ));
+ return;
+
   case ir_binop_logic_and:
- /* For operations on bool arguments, only the low bit of the bool is
-  * valid, and the others are undefined.  Fall back to the condition
-  * code path.
-  */
- break;
+ temp = fs_reg(this, glsl_type::bool_type);
+ emit(AND(temp, op[0], op[1]));
+ emit(IF(temp, fs_reg(0), BRW_CONDITIONAL_NZ));
+ return;
 
   case ir_unop_f2b:
 inst = emit(BRW_OPCODE_IF, reg_null_f, op[0], fs_reg(0));
@@ -2432,9 +2442,8 @@ fs_visitor::emit_if_gen6(ir_if *ir)
   }
}
 
-   emit_bool_to_cond_code(ir-condition);
-   fs_inst *inst = emit(BRW_OPCODE_IF);
-   inst-predicate = BRW_PREDICATE_NORMAL;
+   ir-condition-accept(this);
+   emit(IF(this-result, fs_reg(0), BRW_CONDITIONAL_NZ));
 }
 
 /**
-- 
2.1.0

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

[Mesa-dev] [PATCH 2/3] i965/fs: Make emit_if_gen6 never fall back to emit_bool_to_cond_code.

2014-09-04 Thread Kenneth Graunke 
Matt and I believe that Sandybridge actually uses 0x for a
true comparison result, similar to Ivybridge.  This matches the
internal documentation, and empirical results, but contradicts the PRM.

So, the comment is inaccurate, and we can actually just handle these
directly without ever needing to fall through to the condition code
path.

Also, the vec4 backend has always done it this way, and has apparently
been working fine.  This patch makes the FS backend match the vec4
backend's behavior.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

I'm not sure whether this is necessary for stable backporting or not.
It may be required by the next patch.  Probably shouldn't be, but I
haven't tried it.

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 8f2e1df..d7e3120 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -2378,14 +2378,24 @@ fs_visitor::emit_if_gen6(ir_if *ir)
 
   switch (expr-operation) {
   case ir_unop_logic_not:
+ emit(IF(op[0], fs_reg(0), BRW_CONDITIONAL_Z));
+ return;
+
   case ir_binop_logic_xor:
+ emit(IF(op[0], op[1], BRW_CONDITIONAL_NZ));
+ return;
+
   case ir_binop_logic_or:
+ temp = fs_reg(this, glsl_type::bool_type);
+ emit(OR(temp, op[0], op[1]));
+ emit(IF(temp, fs_reg(0), BRW_CONDITIONAL_NZ));
+ return;
+
   case ir_binop_logic_and:
- /* For operations on bool arguments, only the low bit of the bool is
-  * valid, and the others are undefined.  Fall back to the condition
-  * code path.
-  */
- break;
+ temp = fs_reg(this, glsl_type::bool_type);
+ emit(AND(temp, op[0], op[1]));
+ emit(IF(temp, fs_reg(0), BRW_CONDITIONAL_NZ));
+ return;
 
   case ir_unop_f2b:
 inst = emit(BRW_OPCODE_IF, reg_null_f, op[0], fs_reg(0));
@@ -2432,9 +2442,8 @@ fs_visitor::emit_if_gen6(ir_if *ir)
   }
}
 
-   emit_bool_to_cond_code(ir-condition);
-   fs_inst *inst = emit(BRW_OPCODE_IF);
-   inst-predicate = BRW_PREDICATE_NORMAL;
+   ir-condition-accept(this);
+   emit(IF(this-result, fs_reg(0), BRW_CONDITIONAL_NZ));
 }
 
 /**
-- 
2.1.0

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

Re: [Mesa-dev] [PATCH 2/3] i965/fs: Make emit_if_gen6 never fall back to emit_bool_to_cond_code.

2014-09-04 Thread Matt Turner 
On Thu, Sep 4, 2014 at 12:18 AM, Kenneth Graunke kenn...@whitecape.org wrote:
 Matt and I believe that Sandybridge actually uses 0x for a
 true comparison result, similar to Ivybridge.  This matches the
 internal documentation, and empirical results, but contradicts the PRM.

 So, the comment is inaccurate, and we can actually just handle these
 directly without ever needing to fall through to the condition code
 path.

 Also, the vec4 backend has always done it this way, and has apparently
 been working fine.  This patch makes the FS backend match the vec4
 backend's behavior.

 Signed-off-by: Kenneth Graunke kenn...@whitecape.org
 ---
  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 25 +
  1 file changed, 17 insertions(+), 8 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index 8f2e1df..d7e3120 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -2378,14 +2378,24 @@ fs_visitor::emit_if_gen6(ir_if *ir)

switch (expr-operation) {
case ir_unop_logic_not:
 + emit(IF(op[0], fs_reg(0), BRW_CONDITIONAL_Z));
 + return;
 +
case ir_binop_logic_xor:
 + emit(IF(op[0], op[1], BRW_CONDITIONAL_NZ));
 + return;
 +
case ir_binop_logic_or:
 + temp = fs_reg(this, glsl_type::bool_type);
 + emit(OR(temp, op[0], op[1]));
 + emit(IF(temp, fs_reg(0), BRW_CONDITIONAL_NZ));
 + return;
 +
case ir_binop_logic_and:
 - /* For operations on bool arguments, only the low bit of the bool is
 -  * valid, and the others are undefined.  Fall back to the condition
 -  * code path.
 -  */
 - break;
 + temp = fs_reg(this, glsl_type::bool_type);
 + emit(AND(temp, op[0], op[1]));

For this and the OR case, we might as well generate the flag and use a
predicated IF if we have to use two instructions. That would let the
SEL peephole not emit an extra CMP.

One of us can do this in a follow up. This patch is fine since it
makes the code like the vec4 code.

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