[Mesa-dev] [PATCH 09/24] glsl: Add IR node for atomic operations.
Paul Berry
stereotype441 at gmail.com
Tue Sep 17 12:50:46 PDT 2013
On 15 September 2013 00:10, Francisco Jerez <currojerez at riseup.net> wrote:
> 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.
>
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.
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:
http://lists.freedesktop.org/archives/mesa-dev/2013-August/043768.html).
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.
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.
> ---
> 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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130917/1b02eb25/attachment-0001.html>
More information about the mesa-dev
mailing list