[PATCH] Add verifier for virtual SSA form

Richard Biener rguenther@suse.de
Wed Aug 17 08:17:00 GMT 2016


On Mon, 15 Aug 2016, Richard Biener wrote:

> 
> This adds a verifier that makes sure no overlapping life-ranges occur
> for virtuals.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.

The following is what I ended up applying.  It fixes fallout found
in the vectorizer, general SSA iteration (GIMPLE_TRANSACTION was
mishandled), and CHKP-opt not updating virtual SSA form when moving
stmts.

It also works around an issue in the SSA updater which in case you
had both virtual ops to rename _and_ virtual ops to update via
old/new name mappings could end up creating two virtual PHIs in a block.
The solution is to ignore old/new name mappings when virtuals are
rewritten (disallowing this mode has too much fallout at the moment).
Note this also means that "tricks" like delaying virtual SSA rewrite
until the end of the pass (because nobody cares) but using incremental
SSA update for loop versioning like if conversion does does not work
which means you pay the updating penalty each time you do versioning.

I noticed that MPX tests are not even compiled (nor is the mpx runtime)
on non-MPX capable CPUs.  This means there might be additional fallout
on other targets or other systems I was not able to test -- note that
this is all latent wrong-code (but luckily we're rewriting virtual SSA
form completely at enough points that it may not matter in practice...
*fingers crossing*)

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2016-08-17  Richard Biener  <rguenther@suse.de>

	* tree-ssa.c: Include tree-cfg.h and tree-dfa.h.
	(verify_vssa): New function verifying virtual SSA form.
	(verify_ssa): Call it.
	* tree-ssa-loop-manip.c (slpeel_update_phi_nodes_for_guard2):
	Do not apply loop-closed SSA handling to virtuals.
	* ssa-iterators.h (op_iter_init): Handle GIMPLE_TRANSACTION.
	* tree-into-ssa.c (prepare_use_sites_for): Skip virtual SSA names
	when rewriting their symbol.
	(prepare_def_site_for): Likewise.
	* tree-chkp-opt.c (chkp_reduce_bounds_lifetime): Clear virtual
	operands of moved stmts.

Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c.orig	2016-08-16 13:21:13.006071948 +0200
+++ gcc/tree-ssa.c	2016-08-16 14:00:15.697408150 +0200
@@ -39,6 +39,8 @@ along with GCC; see the file COPYING3.
 #include "tree-ssa.h"
 #include "cfgloop.h"
 #include "cfgexpand.h"
+#include "tree-cfg.h"
+#include "tree-dfa.h"
 
 /* Pointer map of variable mappings, keyed by edge.  */
 static hash_map<edge, auto_vec<edge_var_map> > *edge_var_maps;
@@ -603,6 +605,104 @@ release_defs_bitset (bitmap toremove)
       }
 }
 
+/* Verify virtual SSA form.  */
+
+bool
+verify_vssa (basic_block bb, tree current_vdef, sbitmap visited)
+{
+  bool err = false;
+
+  if (bitmap_bit_p (visited, bb->index))
+    return false;
+
+  bitmap_set_bit (visited, bb->index);
+
+  /* Pick up the single virtual PHI def.  */
+  gphi *phi = NULL;
+  for (gphi_iterator si = gsi_start_phis (bb); !gsi_end_p (si);
+       gsi_next (&si))
+    {
+      tree res = gimple_phi_result (si.phi ());
+      if (virtual_operand_p (res))
+	{
+	  if (phi)
+	    {
+	      error ("multiple virtual PHI nodes in BB %d", bb->index);
+	      print_gimple_stmt (stderr, phi, 0, 0);
+	      print_gimple_stmt (stderr, si.phi (), 0, 0);
+	      err = true;
+	    }
+	  else
+	    phi = si.phi ();
+	}
+    }
+  if (phi)
+    {
+      current_vdef = gimple_phi_result (phi);
+      if (TREE_CODE (current_vdef) != SSA_NAME)
+	{
+	  error ("virtual definition is not an SSA name");
+	  print_gimple_stmt (stderr, phi, 0, 0);
+	  err = true;
+	}
+    }
+
+  /* Verify stmts.  */
+  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+       gsi_next (&gsi))
+    {
+      gimple *stmt = gsi_stmt (gsi);
+      tree vuse = gimple_vuse (stmt);
+      if (vuse)
+	{
+	  if (vuse != current_vdef)
+	    {
+	      error ("stmt with wrong VUSE");
+	      print_gimple_stmt (stderr, stmt, 0, TDF_VOPS);
+	      fprintf (stderr, "expected ");
+	      print_generic_expr (stderr, current_vdef, 0);
+	      fprintf (stderr, "\n");
+	      err = true;
+	    }
+	  tree vdef = gimple_vdef (stmt);
+	  if (vdef)
+	    {
+	      current_vdef = vdef;
+	      if (TREE_CODE (current_vdef) != SSA_NAME)
+		{
+		  error ("virtual definition is not an SSA name");
+		  print_gimple_stmt (stderr, phi, 0, 0);
+		  err = true;
+		}
+	    }
+	}
+    }
+
+  /* Verify destination PHI uses and recurse.  */
+  edge_iterator ei;
+  edge e;
+  FOR_EACH_EDGE (e, ei, bb->succs)
+    {
+      gphi *phi = get_virtual_phi (e->dest);
+      if (phi
+	  && PHI_ARG_DEF_FROM_EDGE (phi, e) != current_vdef)
+	{
+	  error ("PHI node with wrong VUSE on edge from BB %d",
+		 e->src->index);
+	  print_gimple_stmt (stderr, phi, 0, TDF_VOPS);
+	  fprintf (stderr, "expected ");
+	  print_generic_expr (stderr, current_vdef, 0);
+	  fprintf (stderr, "\n");
+	  err = true;
+	}
+
+      /* Recurse.  */
+      err |= verify_vssa (e->dest, current_vdef, visited);
+    }
+
+  return err;
+}
+
 /* Return true if SSA_NAME is malformed and mark it visited.
 
    IS_VIRTUAL is true if this SSA_NAME was found inside a virtual
@@ -1049,6 +1149,16 @@ verify_ssa (bool check_modified_stmt, bo
 
   free (definition_block);
 
+  if (gimple_vop (cfun)
+      && ssa_default_def (cfun, gimple_vop (cfun)))
+    {
+      auto_sbitmap visited (last_basic_block_for_fn (cfun) + 1);
+      bitmap_clear (visited);
+      if (verify_vssa (ENTRY_BLOCK_PTR_FOR_FN (cfun),
+		       ssa_default_def (cfun, gimple_vop (cfun)), visited))
+	goto err;
+    }
+
   /* Restore the dominance information to its prior known state, so
      that we do not perturb the compiler's subsequent behavior.  */
   if (orig_dom_state == DOM_NONE)
Index: gcc/tree-vect-loop-manip.c
===================================================================
--- gcc/tree-vect-loop-manip.c.orig	2016-08-16 13:21:13.006071948 +0200
+++ gcc/tree-vect-loop-manip.c	2016-08-16 14:00:15.761408894 +0200
@@ -607,6 +607,9 @@ slpeel_update_phi_nodes_for_guard2 (edge
 
       /** 2. Handle loop-closed-ssa-form phis  **/
 
+      if (virtual_operand_p (PHI_RESULT (orig_phi)))
+	continue;
+
       /* 2.1. Generate new phi node in NEW_EXIT_BB:  */
       new_res = copy_ssa_name (PHI_RESULT (orig_phi));
       new_phi = create_phi_node (new_res, *new_exit_bb);
Index: gcc/ssa-iterators.h
===================================================================
--- gcc/ssa-iterators.h.orig	2016-08-16 13:21:13.006071948 +0200
+++ gcc/ssa-iterators.h	2016-08-16 14:00:15.777409080 +0200
@@ -607,6 +607,10 @@ op_iter_init (ssa_op_iter *ptr, gimple *
 	  case GIMPLE_ASM:
 	    ptr->numops = gimple_asm_noutputs (as_a <gasm *> (stmt));
 	    break;
+	  case GIMPLE_TRANSACTION:
+	    ptr->numops = 0;
+	    flags &= ~SSA_OP_DEF;
+	    break;
 	  default:
 	    ptr->numops = 0;
 	    flags &= ~(SSA_OP_DEF | SSA_OP_VDEF);
Index: gcc/tree-into-ssa.c
===================================================================
--- gcc/tree-into-ssa.c.orig	2016-08-16 13:21:13.006071948 +0200
+++ gcc/tree-into-ssa.c	2016-08-16 15:06:27.767575733 +0200
@@ -2596,6 +2596,11 @@ prepare_use_sites_for (tree name, bool i
   use_operand_p use_p;
   imm_use_iterator iter;
 
+  /* If we rename virtual operands do not update them.  */
+  if (virtual_operand_p (name)
+      && cfun->gimple_df->rename_vops)
+    return;
+
   FOR_EACH_IMM_USE_FAST (use_p, iter, name)
     {
       gimple *stmt = USE_STMT (use_p);
@@ -2631,6 +2636,11 @@ prepare_def_site_for (tree name, bool in
 		       || !bitmap_bit_p (names_to_release,
 					 SSA_NAME_VERSION (name)));
 
+  /* If we rename virtual operands do not update them.  */
+  if (virtual_operand_p (name)
+      && cfun->gimple_df->rename_vops)
+    return;
+
   stmt = SSA_NAME_DEF_STMT (name);
   bb = gimple_bb (stmt);
   if (bb)
Index: gcc/tree-chkp-opt.c
===================================================================
--- gcc/tree-chkp-opt.c.orig	2016-01-15 11:59:29.902823725 +0100
+++ gcc/tree-chkp-opt.c	2016-08-16 15:04:43.802371674 +0200
@@ -1236,6 +1236,8 @@ chkp_reduce_bounds_lifetime (void)
 		  gsi_move_before (&i, &gsi);
 		}
 
+	      gimple_set_vdef (stmt, NULL_TREE);
+	      gimple_set_vuse (stmt, NULL_TREE);
 	      update_stmt (stmt);
 	    }
 	}



More information about the Gcc-patches mailing list