This is much simpler. This patch is Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
On 09/21/2016 10:20 PM, Kenneth Graunke wrote: > We want to check prior to optimization - otherwise we might fail to > detect cases where barrier() is in control flow which is always taken > (and therefore gets optimized away). > > We don't currently loop unroll if there are function calls inside; > otherwise we might have a problem detecting barrier() in loops that > get unrolled as well. > > Tapani's switch handling code adds a loop around switch statements, so > even with the mess of if ladders, we'll properly reject it. > > Enforcing these rules at compile time makes more sense more sense than > link time. Doing it at ast-to-hir time (rather than as an IR pass) > allows us to emit an error message with proper line numbers. > (Otherwise, I would have preferred the IR pass...) > > Fixes spec/arb_tessellation_shader/compiler/barrier-switch-always.tesc. > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/compiler/glsl/ast_function.cpp | 19 ++++++++ > src/compiler/glsl/linker.cpp | 99 > -------------------------------------- > 2 files changed, 19 insertions(+), 99 deletions(-) > > diff --git a/src/compiler/glsl/ast_function.cpp > b/src/compiler/glsl/ast_function.cpp > index ea3ac87..7e62ab7 100644 > --- a/src/compiler/glsl/ast_function.cpp > +++ b/src/compiler/glsl/ast_function.cpp > @@ -2143,6 +2143,25 @@ ast_function_expression::hir(exec_list *instructions, > /* an error has already been emitted */ > value = ir_rvalue::error_value(ctx); > } else { > + if (state->stage == MESA_SHADER_TESS_CTRL && > + sig->is_builtin() && strcmp(func_name, "barrier") == 0) { > + if (state->current_function == NULL || > + strcmp(state->current_function->function_name(), "main") != > 0) { > + _mesa_glsl_error(&loc, state, > + "barrier() may only be used in main()"); > + } > + > + if (state->found_return) { > + _mesa_glsl_error(&loc, state, > + "barrier() may not be used after return"); > + } > + > + if (instructions != &state->current_function->body) { > + _mesa_glsl_error(&loc, state, > + "barrier() may not be used in control flow"); > + } > + } > + > value = generate_call(instructions, sig, > &actual_parameters, sub_var, array_idx, > state); > if (!value) { > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp > index f3eece2..606d006 100644 > --- a/src/compiler/glsl/linker.cpp > +++ b/src/compiler/glsl/linker.cpp > @@ -260,97 +260,6 @@ public: > } > }; > > -class barrier_use_visitor : public ir_hierarchical_visitor { > -public: > - barrier_use_visitor(gl_shader_program *prog) > - : prog(prog), in_main(false), after_return(false), control_flow(0) > - { > - } > - > - virtual ~barrier_use_visitor() > - { > - /* empty */ > - } > - > - virtual ir_visitor_status visit_enter(ir_function *ir) > - { > - if (strcmp(ir->name, "main") == 0) > - in_main = true; > - > - return visit_continue; > - } > - > - virtual ir_visitor_status visit_leave(ir_function *) > - { > - in_main = false; > - after_return = false; > - return visit_continue; > - } > - > - virtual ir_visitor_status visit_leave(ir_return *) > - { > - after_return = true; > - return visit_continue; > - } > - > - virtual ir_visitor_status visit_enter(ir_if *) > - { > - ++control_flow; > - return visit_continue; > - } > - > - virtual ir_visitor_status visit_leave(ir_if *) > - { > - --control_flow; > - return visit_continue; > - } > - > - virtual ir_visitor_status visit_enter(ir_loop *) > - { > - ++control_flow; > - return visit_continue; > - } > - > - virtual ir_visitor_status visit_leave(ir_loop *) > - { > - --control_flow; > - return visit_continue; > - } > - > - /* FINISHME: `switch` is not expressed at the IR level -- it's already > - * been lowered to a mess of `if`s. We'll correctly disallow any use of > - * barrier() in a conditional path within the switch, but not in a path > - * which is always hit. > - */ > - > - virtual ir_visitor_status visit_enter(ir_call *ir) > - { > - if (ir->use_builtin && strcmp(ir->callee_name(), "barrier") == 0) { > - /* Use of barrier(); determine if it is legal: */ > - if (!in_main) { > - linker_error(prog, "Builtin barrier() may only be used in main"); > - return visit_stop; > - } > - > - if (after_return) { > - linker_error(prog, "Builtin barrier() may not be used after > return"); > - return visit_stop; > - } > - > - if (control_flow != 0) { > - linker_error(prog, "Builtin barrier() may not be used inside > control flow"); > - return visit_stop; > - } > - } > - return visit_continue; > - } > - > -private: > - gl_shader_program *prog; > - bool in_main, after_return; > - int control_flow; > -}; > - > /** > * Visitor that determines the highest stream id to which a (geometry) shader > * emits vertices. It also checks whether End{Stream}Primitive is ever > called. > @@ -2355,14 +2264,6 @@ link_intrastage_shaders(void *mem_ctx, > if (ctx->Const.VertexID_is_zero_based) > lower_vertex_id(linked); > > - /* Validate correct usage of barrier() in the tess control shader */ > - if (linked->Stage == MESA_SHADER_TESS_CTRL) { > - barrier_use_visitor visitor(prog); > - foreach_in_list(ir_instruction, ir, linked->ir) { > - ir->accept(&visitor); > - } > - } > - > return linked; > } > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev