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