Patchwork PR middle-end/51472: set DECL_GIMPLE_REG_P on TM vector saves

login
register
mail settings
Submitter Aldy Hernandez
Date Dec. 20, 2011, 4:57 p.m.
Message ID <4EF0BE74.8050107@redhat.com>
Download mbox | patch
Permalink /patch/132455/
State New
Headers show

Comments

Aldy Hernandez - Dec. 20, 2011, 4:57 p.m.
The problem here is a verify_gimple ICE because a vector store's LHS is 
not a gimple register, but the vector type is a gimple_reg_type.

      if (!is_gimple_reg (lhs)
	  && is_gimple_reg_type (TREE_TYPE (lhs)))
	{
	  error ("invalid rhs for gimple memory store");
	  debug_generic_stmt (lhs);
	  debug_generic_stmt (rhs1);
	  return true;
	}

The reason that is_gimple_reg() is false, is because the vector variable 
is not marked as DECL_GIMPLE_REG_P.  If I understand DECL_GIMPLE_REG_P 
correctly, it should be true, since the TM saves/restores are not 
aliased and the only store to them are killing assignments.

I also noticed we didn't call update_stmt() after 
gimple_assign_set_lhs(), so I have called it.  However, this part is not 
strictly necessary for the patch.

The attached patch fixes the PR, and causes no regressions to the TM 
testsuite.

OK pending further tests?
PR middle-end/51472
	* trans-mem.c (tm_log_emit_saves): Set DECL_GIMPLE_REG_P and
	update statement.
Richard Guenther - Dec. 21, 2011, 9:31 a.m.
On Tue, Dec 20, 2011 at 5:57 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> The problem here is a verify_gimple ICE because a vector store's LHS is not
> a gimple register, but the vector type is a gimple_reg_type.
>
>     if (!is_gimple_reg (lhs)
>          && is_gimple_reg_type (TREE_TYPE (lhs)))
>        {
>          error ("invalid rhs for gimple memory store");
>          debug_generic_stmt (lhs);
>          debug_generic_stmt (rhs1);
>          return true;
>        }
>
> The reason that is_gimple_reg() is false, is because the vector variable is
> not marked as DECL_GIMPLE_REG_P.  If I understand DECL_GIMPLE_REG_P
> correctly, it should be true, since the TM saves/restores are not aliased
> and the only store to them are killing assignments.
>
> I also noticed we didn't call update_stmt() after gimple_assign_set_lhs(),
> so I have called it.  However, this part is not strictly necessary for the
> patch.
>
> The attached patch fixes the PR, and causes no regressions to the TM
> testsuite.
>
> OK pending further tests?

You can't simply do

+	  if (TREE_CODE (TREE_TYPE (var)) == COMPLEX_TYPE
+	      || TREE_CODE (TREE_TYPE (var)) == VECTOR_TYPE)
+	    DECL_GIMPLE_REG_P (var) = 1;
+	  update_stmt (stmt);

where is 'var' created?  At _that_ point you should do the above
(or use create_tmp_reg instead of create_tmp_var).  Existing uses
of var might prevent DECL_GIMPLE_REG_P from being set
(which also means creating an SSA name of it is not allowed).

Richard.

Patch

Index: testsuite/gcc.dg/tm/pr51472.c
===================================================================
--- testsuite/gcc.dg/tm/pr51472.c	(revision 0)
+++ testsuite/gcc.dg/tm/pr51472.c	(revision 0)
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -O  --param tm-max-aggregate-size=32" } */
+
+typedef int __attribute__ ((vector_size (16))) vectype;
+vectype v;
+
+void
+foo (int c)
+{
+  vectype *p = __builtin_malloc (sizeof (vectype));
+  __transaction_atomic
+  {
+    *p = v;
+    if (c)
+      __transaction_cancel;
+  }
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 182542)
+++ trans-mem.c	(working copy)
@@ -1186,8 +1186,15 @@  tm_log_emit_saves (basic_block entry_blo
 	 will create a VOP for us and everything will just work.  */
       if (is_gimple_reg_type (TREE_TYPE (lp->save_var)))
 	{
+	  tree var;
+
 	  lp->save_var = make_ssa_name (lp->save_var, stmt);
+	  var = SSA_NAME_VAR (lp->save_var);
 	  gimple_assign_set_lhs (stmt, lp->save_var);
+	  if (TREE_CODE (TREE_TYPE (var)) == COMPLEX_TYPE
+	      || TREE_CODE (TREE_TYPE (var)) == VECTOR_TYPE)
+	    DECL_GIMPLE_REG_P (var) = 1;
+	  update_stmt (stmt);
 	}
 
       gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);