From: Florian Krohm <florian@eich-krohm.de>
Date: Sun, 30 Dec 2012 18:17:18 +0000 (+0000)
Subject: Improve handling of dirty helper calls when building trees in ado_treebuild_BB.
X-Git-Tag: svn/VALGRIND_3_9_0^2~161
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2b6c4b5a17e2cffe5db125ddfb8b42308c7798b7;p=thirdparty%2Fvalgrind.git

Improve handling of dirty helper calls when building trees in ado_treebuild_BB.
This function took an overly conservative approach and always assumed
that calling a dirty helper would modify both guest state and memory. This
patch introduces two new functions dirty_helper_stores and dirty_helper_puts,
to determine the actual side effets of a helper call. Using these functions
increases precision and allows the tree builder to move a GET past a dirty
helper call.


git-svn-id: svn://svn.valgrind.org/vex/trunk@2625
---

diff --git a/VEX/priv/ir_opt.c b/VEX/priv/ir_opt.c
index 7a879d484b..70262ea713 100644
--- a/VEX/priv/ir_opt.c
+++ b/VEX/priv/ir_opt.c
@@ -5068,6 +5068,30 @@ static IRStmt* atbSubst_Stmt ( ATmpInfo* env, IRStmt* st )
    }
 }
 
+inline
+static Bool dirty_helper_stores ( const IRDirty *d )
+{
+   return d->mFx == Ifx_Write || d->mFx == Ifx_Modify;
+}
+
+inline
+static Bool dirty_helper_puts ( const IRDirty *d )
+{
+   Int i;
+
+   /* Passing the guest state pointer opens the door to modifying the
+      guest state under the covers. It's not allowed, but let's be
+      extra conservative and assume the worst. */
+   if (d->needsBBP) return True;
+
+   /* Check the side effects on the guest state */
+   for (i = 0; i < d->nFxState; ++i) {
+      if (d->fxState[i].fx != Ifx_Read) return True;
+   }
+
+   return False;
+}
+
 /* notstatic */ Addr64 ado_treebuild_BB ( IRSB* bb )
 {
    Int      i, j, k, m;
@@ -5207,7 +5231,8 @@ static IRStmt* atbSubst_Stmt ( ATmpInfo* env, IRStmt* st )
       stmtPuts
          = toBool( st->tag == Ist_Put
                    || st->tag == Ist_PutI 
-                   || st->tag == Ist_Dirty );
+                   || (st->tag == Ist_Dirty
+                       && dirty_helper_puts(st->Ist.Dirty.details)));
 
       /* be True if this stmt writes memory or might do (==> we don't
          want to reorder other loads or stores relative to it).  Also,
@@ -5216,7 +5241,8 @@ static IRStmt* atbSubst_Stmt ( ATmpInfo* env, IRStmt* st )
          memory transactions relative to them. */
       stmtStores
          = toBool( st->tag == Ist_Store
-                   || st->tag == Ist_Dirty
+                   || (st->tag == Ist_Dirty
+                       && dirty_helper_stores(st->Ist.Dirty.details))
                    || st->tag == Ist_LLSC
                    || st->tag == Ist_CAS );