[Mesa-dev] [PATCH 7/7] glsl: Remove ir_binop_greater and ir_binop_lequal expressions

Ian Romanick idr at freedesktop.org
Tue Oct 17 18:14:10 UTC 2017


From: Ian Romanick <ian.d.romanick at intel.com>

NIR does not have these instructions.  TGSI and Mesa IR both implement
them using < and >=, repsectively.  Removing them deletes a bunch of
code and means I don't have to add code to the SPIR-V generator for
them.

v2: Rebase on 2+ years of change... and fix a major bug added in the
rebase.

   text	   data	    bss	    dec	    hex	filename
8255291	 268856	 294072	8818219	 868e2b	32-bit i965_dri.so before
8254235	 268856	 294072	8817163	 868a0b	32-bit i965_dri.so after
7815339	 345592	 420592	8581523	 82f193	64-bit i965_dri.so before
7813995	 345560	 420592	8580147	 82ec33	64-bit i965_dri.so after

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
---
 src/compiler/glsl/ast_to_hir.cpp                   | 13 ++++++++--
 src/compiler/glsl/builtin_functions.cpp            | 23 ++++++++++++-----
 src/compiler/glsl/glsl_to_nir.cpp                  | 24 -----------------
 src/compiler/glsl/ir.cpp                           |  2 --
 src/compiler/glsl/ir_builder.cpp                   |  4 +--
 src/compiler/glsl/ir_builder_print_visitor.cpp     |  2 --
 src/compiler/glsl/ir_expression_operation.py       |  2 --
 src/compiler/glsl/ir_validate.cpp                  |  2 --
 src/compiler/glsl/loop_analysis.cpp                | 23 +++++++----------
 src/compiler/glsl/opt_algebraic.cpp                |  4 ---
 .../glsl/tests/lower_jumps/create_test_cases.py    |  2 +-
 src/intel/compiler/brw_shader.cpp                  |  4 ---
 src/mesa/program/ir_to_mesa.cpp                    | 30 ----------------------
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp         | 20 ---------------
 14 files changed, 39 insertions(+), 116 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 6090ee9..441404f 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -1337,8 +1337,8 @@ ast_expression::do_hir(exec_list *instructions,
       ir_binop_lshift,
       ir_binop_rshift,
       ir_binop_less,
-      ir_binop_greater,
-      ir_binop_lequal,
+      ir_binop_less,    /* This is correct.  See the ast_greater case below. */
+      ir_binop_gequal,  /* This is correct.  See the ast_lequal case below. */
       ir_binop_gequal,
       ir_binop_all_equal,
       ir_binop_any_nequal,
@@ -1487,6 +1487,15 @@ ast_expression::do_hir(exec_list *instructions,
       assert(type->is_error()
              || (type->is_boolean() && type->is_scalar()));
 
+      /* Like NIR, GLSL IR does not have opcodes for > or <=.  Instead, swap
+       * the arguments and use < or >=.
+       */
+      if (this->oper == ast_greater || this->oper == ast_lequal) {
+         ir_rvalue *const tmp = op[0];
+         op[0] = op[1];
+         op[1] = tmp;
+      }
+
       result = new(ctx) ir_expression(operations[this->oper], type,
                                       op[0], op[1]);
       error_emitted = type->is_error();
diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
index 9df9671..530cdc0 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -744,7 +744,8 @@ private:
                                 ir_expression_operation opcode,
                                 const glsl_type *return_type,
                                 const glsl_type *param0_type,
-                                const glsl_type *param1_type);
+                                const glsl_type *param1_type,
+                                bool swap_operands = false);
 
 #define B0(X) ir_function_signature *_##X();
 #define B1(X) ir_function_signature *_##X(const glsl_type *);
@@ -3634,12 +3635,18 @@ builtin_builder::binop(builtin_available_predicate avail,
                        ir_expression_operation opcode,
                        const glsl_type *return_type,
                        const glsl_type *param0_type,
-                       const glsl_type *param1_type)
+                       const glsl_type *param1_type,
+                       bool swap_operands)
 {
    ir_variable *x = in_var(param0_type, "x");
    ir_variable *y = in_var(param1_type, "y");
    MAKE_SIG(return_type, avail, 2, x, y);
-   body.emit(ret(expr(opcode, x, y)));
+
+   if (swap_operands)
+      body.emit(ret(expr(opcode, y, x)));
+   else
+      body.emit(ret(expr(opcode, x, y)));
+
    return sig;
 }
 
@@ -4972,16 +4979,18 @@ ir_function_signature *
 builtin_builder::_lessThanEqual(builtin_available_predicate avail,
                                 const glsl_type *type)
 {
-   return binop(avail, ir_binop_lequal,
-                glsl_type::bvec(type->vector_elements), type, type);
+   return binop(avail, ir_binop_gequal,
+                glsl_type::bvec(type->vector_elements), type, type,
+                true);
 }
 
 ir_function_signature *
 builtin_builder::_greaterThan(builtin_available_predicate avail,
                               const glsl_type *type)
 {
-   return binop(avail, ir_binop_greater,
-                glsl_type::bvec(type->vector_elements), type, type);
+   return binop(avail, ir_binop_less,
+                glsl_type::bvec(type->vector_elements), type, type,
+                true);
 }
 
 ir_function_signature *
diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp
index 5e9544f..2ddf237 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -1799,30 +1799,6 @@ nir_visitor::visit(ir_expression *ir)
          result = nir_slt(&b, srcs[0], srcs[1]);
       }
       break;
-   case ir_binop_greater:
-      if (supports_ints) {
-         if (type_is_float(types[0]))
-            result = nir_flt(&b, srcs[1], srcs[0]);
-         else if (type_is_signed(types[0]))
-            result = nir_ilt(&b, srcs[1], srcs[0]);
-         else
-            result = nir_ult(&b, srcs[1], srcs[0]);
-      } else {
-         result = nir_slt(&b, srcs[1], srcs[0]);
-      }
-      break;
-   case ir_binop_lequal:
-      if (supports_ints) {
-         if (type_is_float(types[0]))
-            result = nir_fge(&b, srcs[1], srcs[0]);
-         else if (type_is_signed(types[0]))
-            result = nir_ige(&b, srcs[1], srcs[0]);
-         else
-            result = nir_uge(&b, srcs[1], srcs[0]);
-      } else {
-         result = nir_slt(&b, srcs[1], srcs[0]);
-      }
-      break;
    case ir_binop_gequal:
       if (supports_ints) {
          if (type_is_float(types[0]))
diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 18bf1eb..2c61dd9 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -487,10 +487,8 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, ir_rvalue *op1)
 
    case ir_binop_equal:
    case ir_binop_nequal:
-   case ir_binop_lequal:
    case ir_binop_gequal:
    case ir_binop_less:
-   case ir_binop_greater:
       assert(op0->type == op1->type);
       this->type = glsl_type::get_instance(GLSL_TYPE_BOOL,
 					   op0->type->vector_elements, 1);
diff --git a/src/compiler/glsl/ir_builder.cpp b/src/compiler/glsl/ir_builder.cpp
index 8d61533..416d8c7 100644
--- a/src/compiler/glsl/ir_builder.cpp
+++ b/src/compiler/glsl/ir_builder.cpp
@@ -371,13 +371,13 @@ less(operand a, operand b)
 ir_expression*
 greater(operand a, operand b)
 {
-   return expr(ir_binop_greater, a, b);
+   return expr(ir_binop_less, b, a);
 }
 
 ir_expression*
 lequal(operand a, operand b)
 {
-   return expr(ir_binop_lequal, a, b);
+   return expr(ir_binop_gequal, b, a);
 }
 
 ir_expression*
diff --git a/src/compiler/glsl/ir_builder_print_visitor.cpp b/src/compiler/glsl/ir_builder_print_visitor.cpp
index 3e30c5d..dfe4bb2 100644
--- a/src/compiler/glsl/ir_builder_print_visitor.cpp
+++ b/src/compiler/glsl/ir_builder_print_visitor.cpp
@@ -551,8 +551,6 @@ ir_builder_print_visitor::print_without_declaration(const ir_expression *ir)
    case ir_binop_mul:
    case ir_binop_imul_high:
    case ir_binop_less:
-   case ir_binop_greater:
-   case ir_binop_lequal:
    case ir_binop_gequal:
    case ir_binop_equal:
    case ir_binop_nequal:
diff --git a/src/compiler/glsl/ir_expression_operation.py b/src/compiler/glsl/ir_expression_operation.py
index 52f2550..d854292 100644
--- a/src/compiler/glsl/ir_expression_operation.py
+++ b/src/compiler/glsl/ir_expression_operation.py
@@ -602,8 +602,6 @@ ir_expression_operation = [
    # Binary comparison operators which return a boolean vector.
    # The type of both operands must be equal.
    operation("less", 2, printable_name="<", source_types=numeric_types, dest_type=bool_type, c_expression="{src0} < {src1}"),
-   operation("greater", 2, printable_name=">", source_types=numeric_types, dest_type=bool_type, c_expression="{src0} > {src1}"),
-   operation("lequal", 2, printable_name="<=", source_types=numeric_types, dest_type=bool_type, c_expression="{src0} <= {src1}"),
    operation("gequal", 2, printable_name=">=", source_types=numeric_types, dest_type=bool_type, c_expression="{src0} >= {src1}"),
    operation("equal", 2, printable_name="==", source_types=all_types, dest_type=bool_type, c_expression="{src0} == {src1}"),
    operation("nequal", 2, printable_name="!=", source_types=all_types, dest_type=bool_type, c_expression="{src0} != {src1}"),
diff --git a/src/compiler/glsl/ir_validate.cpp b/src/compiler/glsl/ir_validate.cpp
index e368224..aa07f8a 100644
--- a/src/compiler/glsl/ir_validate.cpp
+++ b/src/compiler/glsl/ir_validate.cpp
@@ -647,8 +647,6 @@ ir_validate::visit_leave(ir_expression *ir)
       break;
 
    case ir_binop_less:
-   case ir_binop_greater:
-   case ir_binop_lequal:
    case ir_binop_gequal:
    case ir_binop_equal:
    case ir_binop_nequal:
diff --git a/src/compiler/glsl/loop_analysis.cpp b/src/compiler/glsl/loop_analysis.cpp
index 2979e09..0fb6e9f 100644
--- a/src/compiler/glsl/loop_analysis.cpp
+++ b/src/compiler/glsl/loop_analysis.cpp
@@ -87,7 +87,8 @@ find_initial_value(ir_loop *loop, ir_variable *var)
 
 static int
 calculate_iterations(ir_rvalue *from, ir_rvalue *to, ir_rvalue *increment,
-                     enum ir_expression_operation op, bool continue_from_then)
+                     enum ir_expression_operation op, bool continue_from_then,
+                     bool swap_compare_operands)
 {
    if (from == NULL || to == NULL || increment == NULL)
       return -1;
@@ -154,8 +155,9 @@ calculate_iterations(ir_rvalue *from, ir_rvalue *to, ir_rvalue *increment,
       ir_expression *const add =
          new(mem_ctx) ir_expression(ir_binop_add, mul->type, mul, from);
 
-      ir_expression *cmp =
-         new(mem_ctx) ir_expression(op, glsl_type::bool_type, add, to);
+      ir_expression *cmp = swap_compare_operands
+         ? new(mem_ctx) ir_expression(op, glsl_type::bool_type, to, add)
+         : new(mem_ctx) ir_expression(op, glsl_type::bool_type, add, to);
       if (continue_from_then)
          cmp = new(mem_ctx) ir_expression(ir_unop_logic_not, cmp);
 
@@ -582,8 +584,6 @@ loop_analysis::visit_leave(ir_loop *ir)
 
       switch (cond->operation) {
       case ir_binop_less:
-      case ir_binop_greater:
-      case ir_binop_lequal:
       case ir_binop_gequal: {
 	 /* The expressions that we care about will either be of the form
 	  * 'counter < limit' or 'limit < counter'.  Figure out which is
@@ -592,18 +592,12 @@ loop_analysis::visit_leave(ir_loop *ir)
 	 ir_rvalue *counter = cond->operands[0]->as_dereference_variable();
 	 ir_constant *limit = cond->operands[1]->as_constant();
 	 enum ir_expression_operation cmp = cond->operation;
+         bool swap_compare_operands = false;
 
 	 if (limit == NULL) {
 	    counter = cond->operands[1]->as_dereference_variable();
 	    limit = cond->operands[0]->as_constant();
-
-	    switch (cmp) {
-	    case ir_binop_less:    cmp = ir_binop_greater; break;
-	    case ir_binop_greater: cmp = ir_binop_less;    break;
-	    case ir_binop_lequal:  cmp = ir_binop_gequal;  break;
-	    case ir_binop_gequal:  cmp = ir_binop_lequal;  break;
-	    default: assert(!"Should not get here.");
-	    }
+            swap_compare_operands = true;
 	 }
 
 	 if ((counter == NULL) || (limit == NULL))
@@ -616,7 +610,8 @@ loop_analysis::visit_leave(ir_loop *ir)
          loop_variable *lv = ls->get(var);
          if (lv != NULL && lv->is_induction_var()) {
             t->iterations = calculate_iterations(init, limit, lv->increment,
-                                                 cmp, t->continue_from_then);
+                                                 cmp, t->continue_from_then,
+                                                 swap_compare_operands);
 
             if (incremented_before_terminator(ir, var, t->ir)) {
                t->iterations--;
diff --git a/src/compiler/glsl/opt_algebraic.cpp b/src/compiler/glsl/opt_algebraic.cpp
index 31d1f74..ce5f265 100644
--- a/src/compiler/glsl/opt_algebraic.cpp
+++ b/src/compiler/glsl/opt_algebraic.cpp
@@ -438,8 +438,6 @@ ir_algebraic_visitor::handle_expression(ir_expression *ir)
 
       switch (op_expr[0]->operation) {
       case ir_binop_less:    new_op = ir_binop_gequal;  break;
-      case ir_binop_greater: new_op = ir_binop_lequal;  break;
-      case ir_binop_lequal:  new_op = ir_binop_greater; break;
       case ir_binop_gequal:  new_op = ir_binop_less;    break;
       case ir_binop_equal:   new_op = ir_binop_nequal;  break;
       case ir_binop_nequal:  new_op = ir_binop_equal;   break;
@@ -697,8 +695,6 @@ ir_algebraic_visitor::handle_expression(ir_expression *ir)
       break;
 
    case ir_binop_less:
-   case ir_binop_lequal:
-   case ir_binop_greater:
    case ir_binop_gequal:
    case ir_binop_equal:
    case ir_binop_nequal:
diff --git a/src/compiler/glsl/tests/lower_jumps/create_test_cases.py b/src/compiler/glsl/tests/lower_jumps/create_test_cases.py
index 623487e..88a74a3 100644
--- a/src/compiler/glsl/tests/lower_jumps/create_test_cases.py
+++ b/src/compiler/glsl/tests/lower_jumps/create_test_cases.py
@@ -84,7 +84,7 @@ def const_bool(value):
 
 def gt_zero(var_name):
     """Create Construct the expression var_name > 0"""
-    return ['expression', 'bool', '>', ['var_ref', var_name], const_float(0)]
+    return ['expression', 'bool', '<', const_float(0), ['var_ref', var_name]]
 
 
 # The following functions can be used to build complex control flow
diff --git a/src/intel/compiler/brw_shader.cpp b/src/intel/compiler/brw_shader.cpp
index 53d0742..059580b 100644
--- a/src/intel/compiler/brw_shader.cpp
+++ b/src/intel/compiler/brw_shader.cpp
@@ -76,10 +76,6 @@ brw_conditional_for_comparison(unsigned int op)
    switch (op) {
    case ir_binop_less:
       return BRW_CONDITIONAL_L;
-   case ir_binop_greater:
-      return BRW_CONDITIONAL_G;
-   case ir_binop_lequal:
-      return BRW_CONDITIONAL_LE;
    case ir_binop_gequal:
       return BRW_CONDITIONAL_GE;
    case ir_binop_equal:
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index e4803aa..aa33063 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -1135,22 +1135,6 @@ ir_to_mesa_visitor::visit(ir_expression *ir)
    case ir_binop_less:
       emit(ir, OPCODE_SLT, result_dst, op[0], op[1]);
       break;
-   case ir_binop_greater:
-      /* Negating the operands (as opposed to switching the order of the
-       * operands) produces the correct result when both are +/-Inf.
-       */
-      op[0].negate = ~op[0].negate;
-      op[1].negate = ~op[1].negate;
-      emit(ir, OPCODE_SLT, result_dst, op[0], op[1]);
-      break;
-   case ir_binop_lequal:
-      /* Negating the operands (as opposed to switching the order of the
-       * operands) produces the correct result when both are +/-Inf.
-       */
-      op[0].negate = ~op[0].negate;
-      op[1].negate = ~op[1].negate;
-      emit(ir, OPCODE_SGE, result_dst, op[0], op[1]);
-      break;
    case ir_binop_gequal:
       emit(ir, OPCODE_SGE, result_dst, op[0], op[1]);
       break;
@@ -1758,10 +1742,6 @@ ir_to_mesa_visitor::process_move_condition(ir_rvalue *ir)
       /*      a is -  0  +            -  0  +
        * (a <  0)  T  F  F  ( a < 0)  T  F  F
        * (0 <  a)  F  F  T  (-a < 0)  F  F  T
-       * (a <= 0)  T  T  F  (-a < 0)  F  F  T  (swap order of other operands)
-       * (0 <= a)  F  T  T  ( a < 0)  T  F  F  (swap order of other operands)
-       * (a >  0)  F  F  T  (-a < 0)  F  F  T
-       * (0 >  a)  T  F  F  ( a < 0)  T  F  F
        * (a >= 0)  F  T  T  ( a < 0)  T  F  F  (swap order of other operands)
        * (0 >= a)  T  T  F  (-a < 0)  F  F  T  (swap order of other operands)
        *
@@ -1775,16 +1755,6 @@ ir_to_mesa_visitor::process_move_condition(ir_rvalue *ir)
 	    negate = zero_on_left;
 	    break;
 
-	 case ir_binop_greater:
-	    switch_order = false;
-	    negate = !zero_on_left;
-	    break;
-
-	 case ir_binop_lequal:
-	    switch_order = true;
-	    negate = !zero_on_left;
-	    break;
-
 	 case ir_binop_gequal:
 	    switch_order = true;
 	    negate = zero_on_left;
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index a45f004..292748e 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -1500,12 +1500,6 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* ir, st_src_reg *op)
    case ir_binop_less:
       emit_asm(ir, TGSI_OPCODE_SLT, result_dst, op[0], op[1]);
       break;
-   case ir_binop_greater:
-      emit_asm(ir, TGSI_OPCODE_SLT, result_dst, op[1], op[0]);
-      break;
-   case ir_binop_lequal:
-      emit_asm(ir, TGSI_OPCODE_SGE, result_dst, op[1], op[0]);
-      break;
    case ir_binop_gequal:
       emit_asm(ir, TGSI_OPCODE_SGE, result_dst, op[0], op[1]);
       break;
@@ -2808,10 +2802,6 @@ glsl_to_tgsi_visitor::process_move_condition(ir_rvalue *ir)
       /*      a is -  0  +            -  0  +
        * (a <  0)  T  F  F  ( a < 0)  T  F  F
        * (0 <  a)  F  F  T  (-a < 0)  F  F  T
-       * (a <= 0)  T  T  F  (-a < 0)  F  F  T  (swap order of other operands)
-       * (0 <= a)  F  T  T  ( a < 0)  T  F  F  (swap order of other operands)
-       * (a >  0)  F  F  T  (-a < 0)  F  F  T
-       * (0 >  a)  T  F  F  ( a < 0)  T  F  F
        * (a >= 0)  F  T  T  ( a < 0)  T  F  F  (swap order of other operands)
        * (0 >= a)  T  T  F  (-a < 0)  F  F  T  (swap order of other operands)
        *
@@ -2825,16 +2815,6 @@ glsl_to_tgsi_visitor::process_move_condition(ir_rvalue *ir)
             negate = zero_on_left;
             break;
 
-         case ir_binop_greater:
-            switch_order = false;
-            negate = !zero_on_left;
-            break;
-
-         case ir_binop_lequal:
-            switch_order = true;
-            negate = !zero_on_left;
-            break;
-
          case ir_binop_gequal:
             switch_order = true;
             negate = zero_on_left;
-- 
2.9.5



More information about the mesa-dev mailing list