diff mbox

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

Message ID 4EF0BE74.8050107@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Dec. 20, 2011, 4:57 p.m. UTC
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.

Comments

Richard Biener Dec. 21, 2011, 9:31 a.m. UTC | #1
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.
diff mbox

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);