[Mesa-dev] [PATCH 7/7] glsl: Remove ir_binop_greater and ir_binop_lequal expressions
Nicolai Hähnle
nhaehnle at gmail.com
Tue Oct 17 20:00:42 UTC 2017
Good riddance!
Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
On 17.10.2017 20:14, Ian Romanick wrote:
> 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;
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list