2007-05-30 Frank Ch. Eigler <fche@redhat.com>

	PR 4567.
	* staptree.cxx (varuse_collecting_visitor): Add side-effect
	result query functions.
	* elaborate.cxx (dead_stmtexpr_remover::visit_expr_statement):
	Switch to it.
	(dead_assignment_remover::visit_assignment): Skip elision of
	lvalues with side-effects in index exprs.
	* staptree.h: Corresponding changes.

2007-05-30  Frank Ch. Eigler  <fche@redhat.com>

	PR 4567.
	* systemtap.base/optim.stp, optim.exp: New test.

7 files changed, 79 insertions, 32 deletions

diff --git a/ChangeLog b/ChangeLog
index b250d98d..93ecc148 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2007-05-30  Frank Ch. Eigler  <fche@redhat.com>
+
+	PR 4567.
+	* staptree.cxx (varuse_collecting_visitor): Add side-effect
+	result query functions.
+	* elaborate.cxx (dead_stmtexpr_remover::visit_expr_statement):
+	Switch to it.
+	(dead_assignment_remover::visit_assignment): Skip elision of
+	lvalues with side-effects in index exprs.
+	* staptree.h: Corresponding changes.
+
 2007-05-25  Frank Ch. Eigler  <fche@redhat.com>
 
 	PR 4255 teaser.
diff --git a/elaborate.cxx b/elaborate.cxx
index b52cc4aa..82d2a5d9 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -1497,11 +1497,22 @@ dead_assignment_remover::visit_assignment (assignment* e)
       // clog << "Checking assignment to " << leftvar->name << " at " << *e->tok << endl;
       if (vut.read.find(leftvar) == vut.read.end()) // var never read?
         {
-          if (session.verbose>2)
-            clog << "Eliding assignment to " << leftvar->name 
-                 << " at " << *e->tok << endl;
-          *current_expr = e->right; // goodbye assignment*
-          relaxed_p = false;
+          // NB: Not so fast!  The left side could be an array whose
+          // index expressions may have side-effects.  This would be
+          // OK if we could replace the array assignment with a 
+          // statement-expression containing all the index expressions
+          // and the rvalue... but we can't.
+
+          varuse_collecting_visitor vut;
+          e->left->visit (& vut);
+          if (vut.side_effect_free ()) // XXX: use _wrt() once we track focal_vars
+            {
+              if (session.verbose>2)
+                clog << "Eliding assignment to " << leftvar->name 
+                     << " at " << *e->tok << endl;
+              *current_expr = e->right; // goodbye assignment*
+              relaxed_p = false;
+            }
         }
     }
 }
@@ -1581,23 +1592,7 @@ dead_stmtexpr_remover::visit_expr_statement (expr_statement *s)
 
   varuse_collecting_visitor vut;
   s->value->visit (& vut);
-
-  // Note that side-effect-freeness is not simply this test:
-  //
-  // (vut.written.empty() && !vut.embedded_seen)
-  //
-  // That's because the vut.written list may consist of local
-  // variables of called functions.  Visible side-effects occur if
-  // *our* locals, or any *globals* are written-to.
-
-
-  set<vardecl*> intersection;
-  insert_iterator<set<vardecl*> > int_it (intersection, intersection.begin());
-  set_intersection (vut.written.begin(), vut.written.end(),
-                    focal_vars.begin(), focal_vars.end(),
-                    int_it);
-
-  if (intersection.empty() && ! vut.embedded_seen)
+  if (vut.side_effect_free_wrt (focal_vars))
     {
       if (session.verbose>2)
         clog << "Eliding side-effect-free expression "
@@ -1613,15 +1608,6 @@ dead_stmtexpr_remover::visit_expr_statement (expr_statement *s)
       
       relaxed_p = false;
     }
-  else if(session.verbose>3)
-    {
-      clog << "keeping expression " << *s->tok 
-           << " because it writes: ";
-      for (std::set<vardecl*>::iterator k = intersection.begin();
-           k != intersection.end(); k++)
-        clog << (*k)->name << " ";
-      clog << "and/or embedded: " << vut.embedded_seen << endl;
-    }  
 }
 
 
diff --git a/staptree.cxx b/staptree.cxx
index daac09cb..8fb41ab6 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -1670,6 +1670,34 @@ varuse_collecting_visitor::visit_delete_statement (delete_statement* s)
   current_lrvalue = last_lrvalue;
 }
 
+bool
+varuse_collecting_visitor::side_effect_free ()
+{
+  return (written.empty() && !embedded_seen);
+}
+
+
+bool
+varuse_collecting_visitor::side_effect_free_wrt (const set<vardecl*>& vars)
+{
+  // A looser notion of side-effect-freeness with respect to a given
+  // list of variables.
+
+  // That's useful because the written list may consist of local
+  // variables of called functions.  But visible side-effects only
+  // occur if the client's locals, or any globals are written-to.
+
+  set<vardecl*> intersection;
+  insert_iterator<set<vardecl*> > int_it (intersection, intersection.begin());
+  set_intersection (written.begin(), written.end(),
+                    vars.begin(), vars.end(),
+                    int_it);
+
+  return (intersection.empty() && !embedded_seen);
+}
+
+
+
 
 // ------------------------------------------------------------------------
 
diff --git a/staptree.h b/staptree.h
index b63c6c33..0ba8c657 100644
--- a/staptree.h
+++ b/staptree.h
@@ -723,8 +723,10 @@ struct varuse_collecting_visitor: public functioncall_traversing_visitor
   void visit_pre_crement (pre_crement *e);
   void visit_post_crement (post_crement *e);
   void visit_foreach_loop (foreach_loop *s);
-};
 
+  bool side_effect_free ();
+  bool side_effect_free_wrt (const std::set<vardecl*>& vars);
+};
 
 
 
diff --git a/testsuite/ChangeLog b/testsuite/ChangeLog
index 0cad318c..9031fb14 100644
--- a/testsuite/ChangeLog
+++ b/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2007-05-30  Frank Ch. Eigler  <fche@redhat.com>
+
+	PR 4567.
+	* systemtap.base/optim.stp, optim.exp: New test.
+	
 2007-05-29  Will Cohen  <wcohen@redhat.com>
 
 	PR4540
diff --git a/testsuite/systemtap.base/optim.exp b/testsuite/systemtap.base/optim.exp
new file mode 100644
index 00000000..7241aeaf
--- /dev/null
+++ b/testsuite/systemtap.base/optim.exp
@@ -0,0 +1,3 @@
+set test "optim"
+
+stap_run $srcdir/$subdir/$test.stp no_load $all_pass_string
diff --git a/testsuite/systemtap.base/optim.stp b/testsuite/systemtap.base/optim.stp
new file mode 100644
index 00000000..711bc31b
--- /dev/null
+++ b/testsuite/systemtap.base/optim.stp
@@ -0,0 +1,12 @@
+global array
+function func(i) {
+        array[i++] = 0;
+        return i;
+}
+probe begin(1) {
+  if (func(0) == 1) log ("systemtap test success")
+  exit()
+}
+
+probe begin {  log("systemtap starting probe")  }
+probe end   {  log("systemtap ending probe")    }