I spent some time looking at this and trying and I did not find anything that did not look reasonable to me in general. I have a question below, but in any case this is:
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> BTW, now I also see why you need patch 5, you want to use the ir_builder for things like add() directly. When I was reading that patch I was wondering why you would not just emit expressions with ir_binop_*, which I guess would have also been perfectly possible and would have made patch 5 unnecessary and also avoid the need for the if/else you have in print_without_declaration(const ir_expression *ir), but I guess that would've also made the generated code a bit less readable. Iago On Tue, 2016-10-25 at 17:59 -0700, Ian Romanick wrote: > From: Ian Romanick <ian.d.roman...@intel.com> > (...) > +ir_visitor_status > +ir_builder_print_visitor::visit(ir_constant *ir) > +{ > + const unsigned my_index = next_ir_index++; > + > + _mesa_hash_table_insert(index_map, ir, (void *)(uintptr_t) > my_index); > + > + if (ir->type == glsl_type::uint_type || > + ir->type == glsl_type::int_type || > + ir->type == glsl_type::float_type || > + ir->type == glsl_type::bool_type) { > + print_with_indent("ir_constant *const r%04X = ", my_index); > + print_without_declaration(ir); > + print_without_indent(";\n"); > + return visit_continue; > + } > + > + ir_constant_data all_zero; > + memset(&all_zero, 0, sizeof(all_zero)); > + > + if (memcmp(&ir->value, &all_zero, sizeof(all_zero)) == 0) { > + print_with_indent("ir_constant *const r%04X = ", my_index); > + print_without_declaration(ir); > + print_without_indent(";\n"); > + } else { > + print_with_indent("ir_constant_data r%04X_data;\n", my_index); > + print_with_indent("memset(&r%04X_data, 0, > sizeof(ir_constant_data));\n", > + my_index); > + for (unsigned i = 0; i < 16; i++) { > + switch (ir->type->base_type) { > + case GLSL_TYPE_UINT: > + if (ir->value.u[i] != 0) > + print_without_indent("r%04X_data.u[%u] = %u;\n", > + my_index, i, ir->value.u[i]); > + break; > + case GLSL_TYPE_INT: > + if (ir->value.i[i] != 0) > + print_without_indent("r%04X_data.i[%u] = %i;\n", > + my_index, i, ir->value.i[i]); > + break; > + case GLSL_TYPE_FLOAT: > + if (ir->value.u[i] != 0) > + print_without_indent("r%04X_data.u[%u] = 0x%08x; /* > %f */\n", > + my_index, > + i, > + ir->value.u[i], > + ir->value.f[i]); > + break; > + case GLSL_TYPE_DOUBLE: { > + uint64_t v; > + > + STATIC_ASSERT(sizeof(double) == sizeof(uint64_t)); > + > + memcpy(&v, &ir->value.d[i], sizeof(v)); > + if (v != 0) > + /* FIXME: This won't actually work until > ARB_gpu_shader_int64 > + * support lands. > + */ > + print_without_indent("r%04X_data.u64[%u] = 0x%016" > PRIx64 "; /* %g */\n", > + my_index, i, v, ir->value.d[i]); Why not just copy this to data.d (instead of data.u64) as a double value directly? > + break; > + } > + case GLSL_TYPE_BOOL: > + if (ir->value.u[i] != 0) > + print_without_indent("r%04X_data.u[%u] = 1;\n", > my_index, i); > + break; > + default: > + unreachable("Invalid constant type"); > + } > + } > + > + print_with_indent("ir_constant *const r%04X = new(mem_ctx) > ir_constant(glsl_type::%s_type, &r%04X_data);\n", > + my_index, > + ir->type->name, > + my_index); > + } > + > + return visit_continue; > +} > + > +void > +ir_builder_print_visitor::print_without_declaration(const ir_swizzle > *ir) > +{ > + const struct hash_entry *const he = > + _mesa_hash_table_search(index_map, ir->val); > + > + if (ir->mask.num_components == 1) { > + static const char swiz[4] = { 'x', 'y', 'z', 'w' }; > + > + if (is_simple_operand(ir->val)) { > + print_without_indent("swizzle_%c(", swiz[ir->mask.x]); > + print_without_declaration(ir->val); > + print_without_indent(")"); > + } else { > + print_without_indent("swizzle_%c(r%04X)", > + swiz[ir->mask.x], > + (unsigned)(uintptr_t) he->data); > + } > + } else { > + static const char swiz[4] = { 'X', 'Y', 'Z', 'W' }; > + > + print_without_indent("swizzle(r%04X, MAKE_SWIZZLE4(SWIZZLE_%c, > SWIZZLE_%c, SWIZZLE_%c, SWIZZLE_%c), %u)", > + (unsigned)(uintptr_t) he->data, > + swiz[ir->mask.x], > + swiz[ir->mask.y], > + swiz[ir->mask.z], > + swiz[ir->mask.w], > + ir->mask.num_components); > + } > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_leave(ir_swizzle *ir) > +{ > + const unsigned my_index = next_ir_index++; > + > + _mesa_hash_table_insert(index_map, ir, (void *)(uintptr_t) > my_index); > + > + print_with_indent("ir_swizzle *const r%04X = ", my_index); > + print_without_declaration(ir); > + print_without_indent(";\n"); > + > + return visit_continue; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_enter(ir_assignment *ir) > +{ > + ir_expression *const rhs_expr = ir->rhs->as_expression(); > + > + if (!is_simple_operand(ir->rhs) && rhs_expr == NULL) > + return visit_continue; > + > + if (rhs_expr != NULL) { > + const unsigned num_op = rhs_expr->get_num_operands(); > + > + for (unsigned i = 0; i < num_op; i++) { > + if (is_simple_operand(rhs_expr->operands[i])) > + continue; > + > + rhs_expr->operands[i]->accept(this); > + } > + } > + > + ir_visitor_status s; > + > + this->in_assignee = true; > + s = ir->lhs->accept(this); > + this->in_assignee = false; > + if (s != visit_continue) > + return (s == visit_continue_with_parent) ? visit_continue : s; > + > + assert(ir->condition == NULL); > + > + const struct hash_entry *const he_lhs = > + _mesa_hash_table_search(index_map, ir->lhs); > + > + print_with_indent("body.emit(assign(r%04X, ", > + (unsigned)(uintptr_t) he_lhs->data); > + print_without_declaration(ir->rhs); > + print_without_indent(", 0x%02x));\n\n", ir->write_mask); > + > + return visit_continue_with_parent; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_leave(ir_assignment *ir) > +{ > + const struct hash_entry *const he_lhs = > + _mesa_hash_table_search(index_map, ir->lhs); > + > + const struct hash_entry *const he_rhs = > + _mesa_hash_table_search(index_map, ir->rhs); > + > + assert(ir->condition == NULL); > + > + print_with_indent("body.emit(assign(r%04X, r%04X, 0x%02x));\n\n", > + (unsigned)(uintptr_t) he_lhs->data, > + (unsigned)(uintptr_t) he_rhs->data, > + ir->write_mask); > + > + return visit_continue; > +} > + > +void > +ir_builder_print_visitor::print_without_declaration(const > ir_expression *ir) > +{ > + const unsigned num_op = ir->get_num_operands(); > + > + static const char *const arity[] = { > + "", "unop", "binop", "triop", "quadop" > + }; > + > + switch (ir->operation) { > + case ir_unop_neg: > + case ir_binop_add: > + case ir_binop_sub: > + 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: > + case ir_binop_lshift: > + case ir_binop_rshift: > + case ir_binop_bit_and: > + case ir_binop_bit_xor: > + case ir_binop_bit_or: > + case ir_binop_logic_and: > + case ir_binop_logic_xor: > + case ir_binop_logic_or: > + print_without_indent("%s(", > + ir_expression_operation_enum_strings[ir- > >operation]); > + break; > + default: > + print_without_indent("expr(ir_%s_%s, ", > + arity[num_op], > + ir_expression_operation_enum_strings[ir- > >operation]); > + break; > + } > + > + for (unsigned i = 0; i < num_op; i++) { > + if (is_simple_operand(ir->operands[i])) > + print_without_declaration(ir->operands[i]); > + else { > + const struct hash_entry *const he = > + _mesa_hash_table_search(index_map, ir->operands[i]); > + > + print_without_indent("r%04X", (unsigned)(uintptr_t) he- > >data); > + } > + > + if (i < num_op - 1) > + print_without_indent(", "); > + } > + > + print_without_indent(")"); > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_enter(ir_expression *ir) > +{ > + const unsigned num_op = ir->get_num_operands(); > + > + for (unsigned i = 0; i < num_op; i++) { > + if (is_simple_operand(ir->operands[i])) > + continue; > + > + ir->operands[i]->accept(this); > + } > + > + const unsigned my_index = next_ir_index++; > + > + _mesa_hash_table_insert(index_map, ir, (void *)(uintptr_t) > my_index); > + > + print_with_indent("ir_expression *const r%04X = ", my_index); > + print_without_declaration(ir); > + print_without_indent(";\n"); > + > + return visit_continue_with_parent; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_enter(ir_if *ir) > +{ > + const unsigned my_index = next_ir_index++; > + > + print_with_indent("/* IF CONDITION */\n"); > + > + ir_visitor_status s = ir->condition->accept(this); > + if (s != visit_continue) > + return (s == visit_continue_with_parent) ? visit_continue : s; > + > + const struct hash_entry *const he = > + _mesa_hash_table_search(index_map, ir->condition); > + > + print_with_indent("ir_if *f%04X = new(mem_ctx) > ir_if(operand(r%04X).val);\n", > + my_index, > + (unsigned)(uintptr_t) he->data); > + print_with_indent("exec_list *const f%04X_parent_instructions = > body.instructions;\n\n", > + my_index); > + > + indentation++; > + print_with_indent("/* THEN INSTRUCTIONS */\n"); > + print_with_indent("body.instructions = &f%04X- > >then_instructions;\n\n", > + my_index); > + > + if (s != visit_continue_with_parent) { > + s = visit_list_elements(this, &ir->then_instructions); > + if (s == visit_stop) > + return s; > + } > + > + print_without_indent("\n"); > + > + if (!ir->else_instructions.is_empty()) { > + print_with_indent("/* ELSE INSTRUCTIONS */\n"); > + print_with_indent("body.instructions = &f%04X- > >else_instructions;\n\n", > + my_index); > + > + if (s != visit_continue_with_parent) { > + s = visit_list_elements(this, &ir->else_instructions); > + if (s == visit_stop) > + return s; > + } > + > + print_without_indent("\n"); > + } > + > + indentation--; > + > + print_with_indent("body.instructions = > f%04X_parent_instructions;\n", > + my_index); > + print_with_indent("body.emit(f%04X);\n\n", > + my_index); > + print_with_indent("/* END IF */\n\n"); > + > + return visit_continue_with_parent; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_leave(ir_return *ir) > +{ > + const struct hash_entry *const he = > + _mesa_hash_table_search(index_map, ir->value); > + > + print_with_indent("body.emit(ret(r%04X));\n\n", > + (unsigned)(uintptr_t) he->data); > + > + return visit_continue; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_leave(ir_call *ir) > +{ > + const unsigned my_index = next_ir_index++; > + > + print_without_indent("\n"); > + print_with_indent("/* CALL %s */\n", ir->callee_name()); > + print_with_indent("exec_list r%04X_parameters;\n", my_index); > + > + foreach_in_list(ir_dereference_variable, param, &ir- > >actual_parameters) { > + const struct hash_entry *const he = > + _mesa_hash_table_search(index_map, param); > + > + print_with_indent("r%04X_parameters.push_tail(operand(r%04X).v > al);\n", > + my_index, > + (unsigned)(uintptr_t) he->data); > + } > + > + char return_deref_string[32]; > + if (ir->return_deref) { > + const struct hash_entry *const he = > + _mesa_hash_table_search(index_map, ir->return_deref); > + > + snprintf(return_deref_string, sizeof(return_deref_string), > + "operand(r%04X).val", > + (unsigned)(uintptr_t) he->data); > + } else { > + strcpy(return_deref_string, "NULL"); > + } > + > + print_with_indent("body.emit(new(mem_ctx) ir_call(shader- > >symbols->get_function(\"%s\"),\n", > + ir->callee_name()); > + print_with_indent(" %s, > &r%04X_parameters);\n\n", > + return_deref_string, > + my_index); > + return visit_continue; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_enter(ir_loop *ir) > +{ > + const unsigned my_index = next_ir_index++; > + > + _mesa_hash_table_insert(index_map, ir, (void *)(uintptr_t) > my_index); > + > + print_with_indent("/* LOOP BEGIN */\n"); > + print_with_indent("ir_loop *f%04X = new(mem_ctx) ir_loop();\n", > my_index); > + print_with_indent("exec_list *const f%04X_parent_instructions = > body.instructions;\n\n", > + my_index); > + > + indentation++; > + > + print_with_indent("body.instructions = &f%04X- > >body_instructions;\n\n", > + my_index); > + > + return visit_continue; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit_leave(ir_loop *ir) > +{ > + const struct hash_entry *const he = > + _mesa_hash_table_search(index_map, ir); > + > + indentation--; > + > + print_with_indent("/* LOOP END */\n\n"); > + print_with_indent("body.instructions = > f%04X_parent_instructions;\n", > + (unsigned)(uintptr_t) he->data); > + print_with_indent("body.emit(f%04X);\n\n", > + (unsigned)(uintptr_t) he->data); > + > + return visit_continue; > +} > + > +ir_visitor_status > +ir_builder_print_visitor::visit(ir_loop_jump *ir) > +{ > + print_with_indent("body.emit(new(mem_ctx) > ir_loop_jump(ir_loop_jump::jump_%s));\n\n", > + ir->is_break() ? "break" : "continue"); > + return visit_continue; > +} > diff --git a/src/compiler/glsl/ir_builder_print_visitor.h > b/src/compiler/glsl/ir_builder_print_visitor.h > new file mode 100644 > index 0000000..a2deab2 > --- /dev/null > +++ b/src/compiler/glsl/ir_builder_print_visitor.h > @@ -0,0 +1,32 @@ > +/* -*- c++ -*- */ > +/* > + * Copyright © 2016 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person > obtaining a > + * copy of this software and associated documentation files (the > "Software"), > + * to deal in the Software without restriction, including without > limitation > + * the rights to use, copy, modify, merge, publish, distribute, > sublicense, > + * and/or sell copies of the Software, and to permit persons to whom > the > + * Software is furnished to do so, subject to the following > conditions: > + * > + * The above copyright notice and this permission notice (including > the next > + * paragraph) shall be included in all copies or substantial > portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES > OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > OTHER > + * DEALINGS IN THE SOFTWARE. > + */ > + > +#pragma once > +#ifndef IR_BUILDER_PRINT_VISITOR_H > +#define IR_BUILDER_PRINT_VISITOR_H > + > +extern void > +_mesa_print_builder_for_ir(FILE *f, exec_list *instructions); > + > +#endif /* IR_BUILDER_PRINT_VISITOR_H */ _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev