In iropt, try and call flatten_BB less. Enhance the sanity checker

so that it does check for flatness at the relevant places.


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

VEX/priv/ir/irdefs.c

@@ -1853,13 +1853,17 @@ void tcStmt ( IRBB* bb, IRStmt* stmt, IRType gWordTy )
    }
 }
 
-void sanityCheckIRBB ( IRBB* bb, IRType guest_word_size )
+void sanityCheckIRBB ( IRBB* bb,          HChar* caller,
+                       Bool require_flat, IRType guest_word_size )
 {
    Int     i;
    IRStmt* stmt;
    Int     n_temps    = bb->tyenv->types_used;
    Int*    def_counts = LibVEX_Alloc(n_temps * sizeof(Int));
 
+   if (0)
+      vex_printf("sanityCheck: %s\n", caller);
+
    vassert(guest_word_size == Ity_I32
            || guest_word_size == Ity_I64);
 
@@ -1878,6 +1882,19 @@ void sanityCheckIRBB ( IRBB* bb, IRType guest_word_size )
       }
    }
 
+   /* Check for flatness, if required. */
+   if (require_flat) {
+      for (i = 0; i < bb->stmts_used; i++) {
+         stmt = bb->stmts[i];
+         if (!stmt)
+            continue;
+         if (!isFlatIRStmt(stmt))
+            sanityCheckFail(bb, stmt, "IRStmt: is not flat");
+      }
+      if (!isAtom(bb->next))
+         sanityCheckFail(bb, NULL, "bb->next is not an atom");
+   }
+
    /* Count the defs of each temp.  Only one def is allowed.
       Also, check that each used temp has already been defd. */
 

VEX/priv/ir/iropt.c

@@ -96,7 +96,6 @@
       * Redundant-GetI removal
       * Redundant-PutI removal
       * Dead code removal
-      * (redo flattening)
 
    Then the transformations are as follows, as defined by
    vex_control.iropt_level:
@@ -107,7 +106,6 @@
    Level 1: the following sequence:
       * Flatten into atomic form.
       * Cheap transformations.
-      * Flatten into atomic form.
 
    Level 2: the following sequence
       * Flatten into atomic form.
@@ -118,11 +116,9 @@
         - Unrolled a loop, and block does not contain GetI or PutI:
           Do: * CSE
               * Dead code removal
-              * Flatten into atomic form.
         - Unrolled a loop, and block contains GetI or PutI:
           Do: * Expensive transformations
               * Cheap transformations
-              * Flatten into atomic form.
 */
 
 /* Implementation notes, 29 Dec 04.
@@ -146,8 +142,6 @@
    bound to temps.  This allows them to participate in CSE, which
    is important for getting good performance for x86 guest code.
 
-   make spec_helpers_BB always return flat code
-
    CSE up F64 literals (already doing F64is)
 
    CSE: consider carefully the requirement for precise exns
@@ -1586,12 +1580,13 @@ static Bool isZeroU1 ( IRExpr* e )
 /*---------------------------------------------------------------*/
 
 static 
-void spec_helpers_BB ( IRBB* bb,
-                       IRExpr* (*specHelper) ( Char*, IRExpr**) )   
+IRBB* spec_helpers_BB ( IRBB* bb,
+                        IRExpr* (*specHelper) ( Char*, IRExpr**) )   
 {
-   Int i;
+   Int     i;
    IRStmt* st;
    IRExpr* ex;
+   Bool    any = False;
 
    for (i = bb->stmts_used-1; i >= 0; i--) {
       st = bb->stmts[i];
@@ -1608,6 +1603,7 @@ void spec_helpers_BB ( IRBB* bb,
         continue;
 
       /* We got something better.  Install it in the bb. */
+      any = True;
       bb->stmts[i]
          = IRStmt_Tmp(st->Ist.Tmp.tmp, ex);
 
@@ -1619,6 +1615,10 @@ void spec_helpers_BB ( IRBB* bb,
          vex_printf("\n");
       }
    }
+
+   if (any)
+      bb = flatten_BB(bb);
+   return bb;
 }
 
 
@@ -3342,10 +3342,8 @@ static Bool iropt_verbose = False; //True;
 /* Do a simple cleanup pass on bb.  This is: redundant Get removal,
    redundant Put removal, constant propagation, dead code removal,
    clean helper specialisation, and dead code removal (again).
+*/
 
-   Note, spec_helpers_BB destroys the 'flat' property, as the
-   expressions which replace clean helper calls can be arbitrarily
-   deep.  This should be fixed.  */
 
 static 
 IRBB* cheap_transformations ( 
@@ -3378,7 +3376,7 @@ IRBB* cheap_transformations (
       ppIRBB(bb);
    }
 
-   spec_helpers_BB ( bb, specHelper );
+   bb = spec_helpers_BB ( bb, specHelper );
    do_deadcode_BB ( bb );
    if (iropt_verbose) {
       vex_printf("\n========= SPECd \n\n" );
@@ -3400,7 +3398,7 @@ IRBB* expensive_transformations( IRBB* bb )
    do_redundant_GetI_elimination( bb );
    do_redundant_PutI_elimination( bb );
    do_deadcode_BB( bb );
-   return flatten_BB( bb );
+   return bb;
 }
 
 
@@ -3533,9 +3531,6 @@ IRBB* do_iropt_BB ( IRBB* bb0,
 
    }
 
-   /* sigh; flatten the block out (yet again) for the benefit of
-      instrumenters. */
-   bb = flatten_BB( bb );
    return bb;
 }
 

VEX/priv/main/vex_main.c

@@ -315,12 +315,14 @@ TranslateResult LibVEX_Translate (
    }
 
    /* Sanity check the initial IR. */
-   sanityCheckIRBB(irbb, guest_word_type);
+   sanityCheckIRBB( irbb, "initial IR", 
+                    False/*can be non-flat*/, guest_word_type );
 
    /* Clean it up, hopefully a lot. */
    irbb = do_iropt_BB ( irbb, specHelper, preciseMemExnsFn, 
                               guest_bytes_addr );
-   sanityCheckIRBB(irbb, guest_word_type);
+   sanityCheckIRBB( irbb, "after initial iropt", 
+                    True/*must be flat*/, guest_word_type );
 
    if (vex_traceflags & VEX_TRACE_OPT1) {
       vex_printf("\n------------------------" 
@@ -345,14 +347,16 @@ TranslateResult LibVEX_Translate (
    }
 
    if (instrument1 || instrument2)
-      sanityCheckIRBB(irbb, guest_word_type);
+      sanityCheckIRBB( irbb, "after instrumentation",
+                       True/*must be flat*/, guest_word_type );
 
    /* Do a post-instrumentation cleanup pass. */
    if (cleanup_after_instrumentation) {
       do_deadcode_BB( irbb );
       irbb = cprop_BB( irbb );
       do_deadcode_BB( irbb );
-      sanityCheckIRBB(irbb, guest_word_type);
+      sanityCheckIRBB( irbb, "after post-instrumentation cleanup",
+                       True/*must be flat*/, guest_word_type );
    }
 
    if (vex_traceflags & VEX_TRACE_OPT2) {

VEX/pub/libvex_ir.h

@@ -884,7 +884,10 @@ extern IRType typeOfIRTemp  ( IRTypeEnv*, IRTemp );
 extern IRType typeOfIRExpr  ( IRTypeEnv*, IRExpr* );
 
 /* Sanity check a BB of IR */
-extern void sanityCheckIRBB ( IRBB* bb, IRType guest_word_size );
+extern void sanityCheckIRBB ( IRBB*  bb, 
+                              HChar* caller,
+                              Bool   require_flatness, 
+                              IRType guest_word_size );
 extern Bool isFlatIRStmt ( IRStmt* );
 
 /* Is this any value actually in the enumeration 'IRType' ? */