<div dir="ltr">On 15 September 2013 00:10, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Add a subclass of ir_rvalue that represents an atomic operation on some ir_variable. Also define a new IR visitor method, and implement IR builder, printer and reader support for it. </blockquote><div> </div><div>I don't think this approach is going to be reliable. A lot of optimization passes assume that ir_rvalues are free of side effects, so they can freely duplicate, consolidate, dead-code-eliminate, or reorder them. It's not safe to do any of those things to atomic operations, because it may change the meaning of the program. </div><div>IMHO, the best way to represent atomic operations in the IR would be to use LLVM-style intrinsics (we've discussed this idea previously here: <a href="http://lists.freedesktop.org/archives/mesa-dev/2013-August/043768.html">http://lists.freedesktop.org/archives/mesa-dev/2013-August/043768.html</a>). The basic idea is that in the IR, they would be treated as ir_call nodes, but they wouldn't be subject to linking or inlining, so they would stay in the IR tree all the way to the back-end, which would translate them into their native back-end representation. The advantage of this approach is that all of our optimization passes already assume that function calls can have side effects, and know how to track function arguments and return values when computing things like variable lifetimes, so the optimization logic wouldn't have to change at all. </div><div>If we don't go the intrinsics route, an alternative would be to represent atomic operations as a subtype of ir_instruction. The disadvantage of that is that it means we'll have to teach all the optimization passes about atomic operations, which is a lot of tedious work that's easy to get wrong. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> --- src/glsl/ir.cpp | 2 +- src/glsl/ir.h | 42 ++++++++++++++++++++++++++ src/glsl/ir_builder.cpp | 7 +++++ src/glsl/ir_builder.h | 2 ++ src/glsl/ir_clone.cpp | 11 +++++++ src/glsl/ir_constant_expression.cpp | 7 +++++ src/glsl/ir_hierarchical_visitor.cpp | 16 ++++++++++ src/glsl/ir_hierarchical_visitor.h | 2 ++ src/glsl/ir_hv_accept.cpp | 14 +++++++++ src/glsl/ir_print_visitor.cpp | 20 ++++++++++++ src/glsl/ir_print_visitor.h | 1 + src/glsl/ir_reader.cpp | 38 +++++++++++++++++++++++ src/glsl/ir_rvalue_visitor.cpp | 18 +++++++++++ src/glsl/ir_rvalue_visitor.h | 3 ++ src/glsl/ir_visitor.h | 2 ++ src/mesa/drivers/dri/i965/brw_fs.h | 1 + src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 5 +++ src/mesa/drivers/dri/i965/brw_vec4.h | 1 + src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 +++ src/mesa/program/ir_to_mesa.cpp | 7 +++++ src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 7 +++++ 21 files changed, 210 insertions(+), 1 deletion(-) diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp index 1b17999..83bcda2 100644 --- a/src/glsl/ir.cpp +++ b/src/glsl/ir.cpp @@ -1565,7 +1565,7 @@ ir_swizzle::variable_referenced() const ir_variable::ir_variable(const struct glsl_type *type, const char *name, ir_variable_mode mode) : max_array_access(0), read_only(false), centroid(false), invariant(false), - mode(mode), interpolation(INTERP_QUALIFIER_NONE) + mode(mode), interpolation(INTERP_QUALIFIER_NONE), atomic() { this->ir_type = ir_type_variable; this->type = type; diff --git a/src/glsl/ir.h b/src/glsl/ir.h index 2637b40..c4b4677 100644 --- a/src/glsl/ir.h +++ b/src/glsl/ir.h @@ -83,6 +83,7 @@ enum ir_node_type { ir_type_texture, ir_type_emit_vertex, ir_type_end_primitive, + ir_type_atomic, ir_type_max /**< maximum ir_type enum number, for validation */ }; @@ -547,6 +548,14 @@ public: int binding; /** + * Location an atomic counter is stored at. + */ + struct { + int buffer_index; + int offset; + } atomic; + + /** * Built-in state that backs this uniform * * Once set at variable creation, \c state_slots must remain invariant. @@ -2085,6 +2094,39 @@ public: virtual ir_visitor_status accept(ir_hierarchical_visitor *); }; +enum ir_atomic_opcode { + ir_atomic_read, + ir_atomic_inc, + ir_atomic_dec +}; + +class ir_atomic : public ir_rvalue { +public: + ir_atomic(enum ir_atomic_opcode op, ir_dereference *location = NULL) + : op(op), location(location) + { + this->type = glsl_type::get_instance(GLSL_TYPE_UINT, 1, 1); + this->ir_type = ir_type_atomic; + } + + virtual ir_atomic *clone(void *mem_ctx, struct hash_table *) const; + + virtual ir_constant *constant_expression_value(struct hash_table *variable_context = NULL); + + virtual void accept(ir_visitor *v) + { + v->visit(this); + } + + virtual ir_visitor_status accept(ir_hierarchical_visitor *); + + /** Kind of atomic instruction. */ + enum ir_atomic_opcode op; + + /** Variable this atomic instruction operates on. */ + ir_dereference *location; +}; + /** * Apply a visitor to each IR node in a list */ diff --git a/src/glsl/ir_builder.cpp b/src/glsl/ir_builder.cpp index 98b4322..35f075a 100644 --- a/src/glsl/ir_builder.cpp +++ b/src/glsl/ir_builder.cpp @@ -535,4 +535,11 @@ if_tree(operand condition, return result; } +ir_atomic * +atomic(ir_atomic_opcode op, deref counter) +{ + void *mem_ctx = ralloc_parent(counter.val); + return new(mem_ctx) ir_atomic(op, counter.val); +} + } /* namespace ir_builder */ diff --git a/src/glsl/ir_builder.h b/src/glsl/ir_builder.h index 6a5f771..4a214fa 100644 --- a/src/glsl/ir_builder.h +++ b/src/glsl/ir_builder.h @@ -210,4 +210,6 @@ ir_if *if_tree(operand condition, ir_instruction *then_branch, ir_instruction *else_branch); +ir_atomic *atomic(ir_atomic_opcode op, deref counter); + } /* namespace ir_builder */ diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp index b70b7db..9475809 100644 --- a/src/glsl/ir_clone.cpp +++ b/src/glsl/ir_clone.cpp @@ -51,6 +51,8 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht) const var->location = this->location; var->index = this->index; var->binding = this->binding; + var->atomic.buffer_index = this->atomic.buffer_index; + var->atomic.offset = this->atomic.offset; var->warn_extension = this->warn_extension; var->origin_upper_left = this->origin_upper_left; var->pixel_center_integer = this->pixel_center_integer; @@ -396,6 +398,15 @@ ir_constant::clone(void *mem_ctx, struct hash_table *ht) const return NULL; } +ir_atomic * +ir_atomic::clone(void *mem_ctx, struct hash_table *ht) const +{ + ir_atomic *atom = new(mem_ctx) ir_atomic(op); + + atom->location = location->clone(mem_ctx, ht); + + return atom; +} class fixup_ir_call_visitor : public ir_hierarchical_visitor { public: diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp index a67470b..faed0fe 100644 --- a/src/glsl/ir_constant_expression.cpp +++ b/src/glsl/ir_constant_expression.cpp @@ -1728,6 +1728,13 @@ ir_call::constant_expression_value(struct hash_table *variable_context) } +ir_constant * +ir_atomic::constant_expression_value(struct hash_table *variable_context) +{ + return NULL; +} + + bool ir_function_signature::constant_expression_evaluate_expression_list(const struct exec_list &body, struct hash_table *variable_context, ir_constant **result) diff --git a/src/glsl/ir_hierarchical_visitor.cpp b/src/glsl/ir_hierarchical_visitor.cpp index 2e606dd..49af569 100644 --- a/src/glsl/ir_hierarchical_visitor.cpp +++ b/src/glsl/ir_hierarchical_visitor.cpp @@ -303,6 +303,22 @@ ir_hierarchical_visitor::visit_leave(ir_if *ir) return visit_continue; } +ir_visitor_status +ir_hierarchical_visitor::visit_enter(ir_atomic *ir) +{ + if (this->callback != NULL) + this->callback(ir, this->data); + + return visit_continue; +} + +ir_visitor_status +ir_hierarchical_visitor::visit_leave(ir_atomic *ir) +{ + (void) ir; + return visit_continue; +} + void ir_hierarchical_visitor::run(exec_list *instructions) { diff --git a/src/glsl/ir_hierarchical_visitor.h b/src/glsl/ir_hierarchical_visitor.h index 647d2e0..ba515f8 100644 --- a/src/glsl/ir_hierarchical_visitor.h +++ b/src/glsl/ir_hierarchical_visitor.h @@ -137,6 +137,8 @@ public: virtual ir_visitor_status visit_leave(class ir_discard *); virtual ir_visitor_status visit_enter(class ir_if *); virtual ir_visitor_status visit_leave(class ir_if *); + virtual ir_visitor_status visit_enter(class ir_atomic *); + virtual ir_visitor_status visit_leave(class ir_atomic *); /*@}*/ diff --git a/src/glsl/ir_hv_accept.cpp b/src/glsl/ir_hv_accept.cpp index 76a607d..5c9c010 100644 --- a/src/glsl/ir_hv_accept.cpp +++ b/src/glsl/ir_hv_accept.cpp @@ -428,3 +428,17 @@ ir_end_primitive::accept(ir_hierarchical_visitor *v) { return v->visit(this); } + +ir_visitor_status +ir_atomic::accept(ir_hierarchical_visitor *v) +{ + ir_visitor_status s = v->visit_enter(this); + if (s != visit_continue) + return (s == visit_continue_with_parent) ? visit_continue : s; + + s = location->accept(v); + if (s != visit_continue) + return (s == visit_continue_with_parent) ? visit_continue : s; + + return (s == visit_stop) ? s : v->visit_leave(this); +} diff --git a/src/glsl/ir_print_visitor.cpp b/src/glsl/ir_print_visitor.cpp index b518310..fc1dbab 100644 --- a/src/glsl/ir_print_visitor.cpp +++ b/src/glsl/ir_print_visitor.cpp @@ -561,3 +561,23 @@ ir_print_visitor::visit(ir_end_primitive *ir) { printf("(end-primitive)"); } + +void ir_print_visitor::visit(ir_atomic *ir) +{ + printf("(atomic "); + + switch (ir->op) { + case ir_atomic_read: + printf("read "); + break; + case ir_atomic_inc: + printf("inc "); + break; + case ir_atomic_dec: + printf("dec "); + break; + } + + ir->location->accept(this); + printf(")"); +} diff --git a/src/glsl/ir_print_visitor.h b/src/glsl/ir_print_visitor.h index 865376f..e10c480 100644 --- a/src/glsl/ir_print_visitor.h +++ b/src/glsl/ir_print_visitor.h @@ -71,6 +71,7 @@ public: virtual void visit(ir_loop_jump *); virtual void visit(ir_emit_vertex *); virtual void visit(ir_end_primitive *); + virtual void visit(ir_atomic *); /*@}*/ private: diff --git a/src/glsl/ir_reader.cpp b/src/glsl/ir_reader.cpp index ec35b68..91a2847 100644 --- a/src/glsl/ir_reader.cpp +++ b/src/glsl/ir_reader.cpp @@ -61,6 +61,7 @@ private: ir_texture *read_texture(s_expression *); ir_emit_vertex *read_emit_vertex(s_expression *); ir_end_primitive *read_end_primitive(s_expression *); + ir_atomic *read_atomic(s_expression *); ir_dereference *read_dereference(s_expression *); ir_dereference_variable *read_var_ref(s_expression *); @@ -548,6 +549,8 @@ ir_reader::read_rvalue(s_expression *expr) rvalue = read_expression(list); } else if (strcmp(tag->value(), "constant") == 0) { rvalue = read_constant(list); + } else if (strcmp(tag->value(), "atomic") == 0) { + rvalue = read_atomic(list); } else { rvalue = read_texture(list); if (rvalue == NULL && !state->error) @@ -1105,3 +1108,38 @@ ir_reader::read_end_primitive(s_expression *expr) ir_read_error(NULL, "when reading end-primitive"); return NULL; } + +ir_atomic * +ir_reader::read_atomic(s_expression *expr) +{ + s_symbol *s_tag = NULL; + s_expression *s_location = NULL; + ir_atomic_opcode op; + + s_pattern pat[] = + { "atomic", s_tag, s_location }; + + if (!MATCH(expr, pat)) { + ir_read_error(NULL, "when reading atomic"); + return NULL; + } + + if (!strcmp("read", s_tag->value())) { + op = ir_atomic_read; + } else if (!strcmp("inc", s_tag->value())) { + op = ir_atomic_inc; + } else if (!strcmp("dec", s_tag->value())) { + op = ir_atomic_dec; + } else { + ir_read_error(NULL, "unexpected atomic op %s", s_tag->value()); + return NULL; + } + + ir_dereference *location = read_dereference(s_location); + if (!location) { + ir_read_error(NULL, "when reading location in (atomic ...)"); + return NULL; + } + + return new(mem_ctx) ir_atomic(op, location); +} diff --git a/src/glsl/ir_rvalue_visitor.cpp b/src/glsl/ir_rvalue_visitor.cpp index 8eb1c62..f918959 100644 --- a/src/glsl/ir_rvalue_visitor.cpp +++ b/src/glsl/ir_rvalue_visitor.cpp @@ -145,6 +145,12 @@ ir_rvalue_base_visitor::rvalue_visit(ir_if *ir) return visit_continue; } +ir_visitor_status +ir_rvalue_base_visitor::rvalue_visit(ir_atomic *ir) +{ + return visit_continue; +} + ir_visitor_status ir_rvalue_visitor::visit_leave(ir_expression *ir) @@ -201,6 +207,12 @@ ir_rvalue_visitor::visit_leave(ir_if *ir) } ir_visitor_status +ir_rvalue_visitor::visit_leave(ir_atomic *ir) +{ + return rvalue_visit(ir); +} + +ir_visitor_status ir_rvalue_enter_visitor::visit_enter(ir_expression *ir) { return rvalue_visit(ir); @@ -253,3 +265,9 @@ ir_rvalue_enter_visitor::visit_enter(ir_if *ir) { return rvalue_visit(ir); } + +ir_visitor_status +ir_rvalue_enter_visitor::visit_enter(ir_atomic *ir) +{ + return rvalue_visit(ir); +} diff --git a/src/glsl/ir_rvalue_visitor.h b/src/glsl/ir_rvalue_visitor.h index 2179fa5..31b3d78 100644 --- a/src/glsl/ir_rvalue_visitor.h +++ b/src/glsl/ir_rvalue_visitor.h @@ -41,6 +41,7 @@ public: ir_visitor_status rvalue_visit(ir_return *); ir_visitor_status rvalue_visit(ir_swizzle *); ir_visitor_status rvalue_visit(ir_texture *); + ir_visitor_status rvalue_visit(ir_atomic *); virtual void handle_rvalue(ir_rvalue **rvalue) = 0; }; @@ -57,6 +58,7 @@ public: virtual ir_visitor_status visit_leave(ir_return *); virtual ir_visitor_status visit_leave(ir_swizzle *); virtual ir_visitor_status visit_leave(ir_texture *); + virtual ir_visitor_status visit_leave(ir_atomic *); }; class ir_rvalue_enter_visitor : public ir_rvalue_base_visitor { @@ -71,4 +73,5 @@ public: virtual ir_visitor_status visit_enter(ir_return *); virtual ir_visitor_status visit_enter(ir_swizzle *); virtual ir_visitor_status visit_enter(ir_texture *); + virtual ir_visitor_status visit_enter(ir_atomic *); }; diff --git a/src/glsl/ir_visitor.h b/src/glsl/ir_visitor.h index 40f96ff..5cb21a2 100644 --- a/src/glsl/ir_visitor.h +++ b/src/glsl/ir_visitor.h @@ -65,6 +65,7 @@ public: virtual void visit(class ir_loop_jump *) = 0; virtual void visit(class ir_emit_vertex *) = 0; virtual void visit(class ir_end_primitive *) = 0; + virtual void visit(class ir_atomic *) = 0; /*@}*/ }; @@ -85,6 +86,7 @@ public: virtual void visit(class ir_call *) {} virtual void visit(class ir_emit_vertex *) {} virtual void visit(class ir_end_primitive *) {} + virtual void visit(class ir_atomic *) {} }; #endif /* __cplusplus */ diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h index 7cd9fd8..e78267e 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.h +++ b/src/mesa/drivers/dri/i965/brw_fs.h @@ -225,6 +225,7 @@ public: void visit(ir_function_signature *ir); void visit(ir_emit_vertex *); void visit(ir_end_primitive *); + void visit(ir_atomic *); void swizzle_result(ir_texture *ir, fs_reg orig_val, int sampler); diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index 0345329..aaadb1d 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -2110,6 +2110,11 @@ fs_visitor::visit(ir_end_primitive *) assert(!"not reached"); } +void +fs_visitor::visit(ir_atomic *ir) +{ +} + fs_inst * fs_visitor::emit(fs_inst inst) { diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h index c177d05..13c9166 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.h +++ b/src/mesa/drivers/dri/i965/brw_vec4.h @@ -328,6 +328,7 @@ public: virtual void visit(ir_if *); virtual void visit(ir_emit_vertex *); virtual void visit(ir_end_primitive *); + virtual void visit(ir_atomic *); /*@}*/ src_reg result; diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp index 9770f13..a13ce1b 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -2462,6 +2462,11 @@ vec4_visitor::visit(ir_end_primitive *) } void +vec4_visitor::visit(ir_atomic *ir) +{ +} + +void vec4_visitor::emit_ndc_computation() { /* Get the position */ diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp index 44f6d08..5c77267 100644 --- a/src/mesa/program/ir_to_mesa.cpp +++ b/src/mesa/program/ir_to_mesa.cpp @@ -262,6 +262,7 @@ public: virtual void visit(ir_if *); virtual void visit(ir_emit_vertex *); virtual void visit(ir_end_primitive *); + virtual void visit(ir_atomic *); /*@}*/ src_reg result; @@ -2264,6 +2265,12 @@ ir_to_mesa_visitor::visit(ir_end_primitive *ir) assert(!"Geometry shaders not supported."); } +void +ir_to_mesa_visitor::visit(ir_atomic *ir) +{ + assert(!"Atomics not supported."); +} + ir_to_mesa_visitor::ir_to_mesa_visitor() { result.file = PROGRAM_UNDEFINED; diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 67a8ed6..822e0d7 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -366,6 +366,7 @@ public: virtual void visit(ir_if *); virtual void visit(ir_emit_vertex *); virtual void visit(ir_end_primitive *); + virtual void visit(ir_atomic *); /*@}*/ st_src_reg result; @@ -3019,6 +3020,12 @@ glsl_to_tgsi_visitor::visit(ir_end_primitive *ir) assert(!"Geometry shaders not supported."); } +void +glsl_to_tgsi_visitor::visit(ir_atomic *ir) +{ + assert(!"Atomics not supported."); +} + glsl_to_tgsi_visitor::glsl_to_tgsi_visitor() { result.file = PROGRAM_UNDEFINED; -- 1.8.3.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>