glsl: Stop tree grafting if a variable is overwritten as an 'out' param.
authorKenneth Graunke <kenneth@whitecape.org>
Thu, 22 Sep 2011 22:04:56 +0000 (15:04 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Tue, 18 Oct 2011 19:24:48 +0000 (12:24 -0700)
While reviewing some compiler cleanups I'd sent out, Paul noticed that
tree grafting wasn't taking "out" parameters into account.

Further investigation revealed that it isn't strictly necessary: ir_call
ends basic blocks, and tree grafting currently only operates on basic
blocks.  So calls already kill grafts.

However, just to be safe, this patch makes "out" parameters explicitly
kill grafts.  Paul and I both prefer this.  It's a bit clearer.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Paul Berry <stereotype441@gmail.com>
src/glsl/opt_tree_grafting.cpp

index 22a1749b9ddf8b5af93b292ccacdb954bc18f7db..d32d14e88a901076bd978125f894aa79e417e01a 100644 (file)
@@ -76,6 +76,8 @@ public:
    virtual ir_visitor_status visit_enter(class ir_swizzle *);
    virtual ir_visitor_status visit_enter(class ir_texture *);
 
+   ir_visitor_status check_graft(ir_instruction *ir, ir_variable *var);
+
    bool do_graft(ir_rvalue **rvalue);
 
    bool progress;
@@ -148,18 +150,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();
@@ -171,6 +172,19 @@ ir_tree_grafting_visitor::visit_leave(ir_assignment *ir)
    return visit_continue;
 }
 
+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)
 {
@@ -195,8 +209,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);