On 22 September 2011 15:21, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Found by inspection. Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> --- src/glsl/opt_tree_grafting.cpp | 37 ++++++++++++++++++++++++++----------- 1 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/glsl/opt_tree_grafting.cpp b/src/glsl/opt_tree_grafting.cpp index 22a1749..d9ab2e6 100644 --- a/src/glsl/opt_tree_grafting.cpp +++ b/src/glsl/opt_tree_grafting.cpp @@ -148,18 +148,17 @@ ir_tree_grafting_visitor::visit_enter(ir_loop *ir) return visit_stop; } +/** + * Check if we can continue grafting after writing to a variable. If the + * expression we're trying to graft references the variable, we must stop. + * + * \param ir An instruction that writes to a variable. + * \param var The variable being updated. + */ ir_visitor_status -ir_tree_grafting_visitor::visit_leave(ir_assignment *ir) +ir_tree_grafting_visitor::check_graft(ir_instruction *ir, ir_variable *var) { - if (do_graft(&ir->rhs) || - do_graft(&ir->condition)) - return visit_stop; - - /* If this assignment updates a variable used in the assignment - * we're trying to graft, then we're done. - */ - if (dereferences_variable(this->graft_assign->rhs, - ir->lhs->variable_referenced())) { + if (dereferences_variable(this->graft_assign->rhs, var)) { if (debug) { printf("graft killed by: "); ir->print(); @@ -172,6 +171,19 @@ ir_tree_grafting_visitor::visit_leave(ir_assignment *ir) } ir_visitor_status +ir_tree_grafting_visitor::visit_leave(ir_assignment *ir) +{ + if (do_graft(&ir->rhs) || + do_graft(&ir->condition)) + return visit_stop; + + /* If this assignment updates a variable used in the assignment + * we're trying to graft, then we're done. + */ + return check_graft(ir, ir->lhs->variable_referenced()); +} + +ir_visitor_status ir_tree_grafting_visitor::visit_enter(ir_function *ir) { (void) ir; @@ -195,8 +207,11 @@ ir_tree_grafting_visitor::visit_enter(ir_call *ir) ir_rvalue *ir = (ir_rvalue *)iter.get(); ir_rvalue *new_ir = ir; - if (sig_param->mode != ir_var_in && sig_param->mode != ir_var_const_in) + if (sig_param->mode != ir_var_in && sig_param->mode != ir_var_const_in) { + if (check_graft(ir, sig_param) == visit_stop) + return visit_stop; continue; + } if (do_graft(&new_ir)) { ir->replace_with(new_ir); -- 1.7.6.1 _______________________________________________ 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> It looks like you need to add this line to the class declaration to get it to compile: ir_visitor_status check_graft(ir_instruction *ir, ir_variable *var); I'm assuming that you actually tried this code and it worked, and this was just a rebasing error? If so, then this code is Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> BTW, any interest in trying to craft a Piglit test that validates this fix? I'm guessing it shouldn't be too hard (famous last words, I know).