* [trans-mem] Fix instantiation of transaction expressions.
@ 2011-11-07 14:24 Torvald Riegel
2011-11-07 16:41 ` Richard Henderson
0 siblings, 1 reply; 5+ messages in thread
From: Torvald Riegel @ 2011-11-07 14:24 UTC (permalink / raw)
To: GCC Patches; +Cc: Aldy Hernandez, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 580 bytes --]
I stumbled upon this when working on noexcept. When txn expressions were
processed in tsubst_expr(), the previous code incorrectly created empty
transaction statements because no statements got added to the statement
list used by (begin|finish)_transaction_stmt(). Also,
"return __transaction_atomic (x+1);" incorrectly returned an error that
this wouldn't return a value.
With the patch, if RECUR(...) returns a value, then we assume that this
is an expression and create a txn expression instead.
No regressions for the TM tests, running a bootstrap currently.
OK for branch?
[-- Attachment #2: patch3 --]
[-- Type: text/plain, Size: 2174 bytes --]
commit 437bd75328aa06561cb84f5e419431f979776115
Author: Torvald Riegel <triegel@redhat.com>
Date: Mon Nov 7 14:51:50 2011 +0100
Fix instantiation of transaction expressions.
* cp/pt.c (tsubst_expr) [TRANSACTION_EXPR]: If body is not a
statement, create an expression instead.
* testsuite/g++.dg/tm/template-1.C: New.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b1593fe..6e6dcdb 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12960,6 +12960,16 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
stmt = begin_transaction_stmt (input_location, NULL, flags);
tmp = RECUR (TRANSACTION_EXPR_BODY (t));
+ if (tmp)
+ {
+ /* No statements; handle this like an expression. */
+ pop_stmt_list (TRANSACTION_EXPR_BODY (stmt));
+ stmt = build1 (TRANSACTION_EXPR, TREE_TYPE (tmp), tmp);
+ TRANSACTION_EXPR_OUTER (stmt) = TRANSACTION_EXPR_OUTER (t);
+ TRANSACTION_EXPR_RELAXED (stmt) = TRANSACTION_EXPR_RELAXED (t);
+ SET_EXPR_LOCATION (stmt, EXPR_LOCATION (t));
+ return stmt;
+ }
finish_transaction_stmt (stmt, NULL, flags);
}
break;
diff --git a/gcc/testsuite/g++.dg/tm/template-1.C b/gcc/testsuite/g++.dg/tm/template-1.C
new file mode 100644
index 0000000..b93828a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tm/template-1.C
@@ -0,0 +1,35 @@
+// { dg-do compile }
+// { dg-options "-fgnu-tm -O -fdump-tree-tmmark" }
+
+struct TrueFalse
+{
+ static bool v() { return true; }
+};
+
+int global;
+
+template<typename T> int foo()
+{
+ __transaction_atomic { global += 2; }
+ return __transaction_atomic (global + 1);
+}
+
+template<typename T> int bar() __transaction_atomic
+{
+ return global + 3;
+}
+
+template<typename T> void bar2() __transaction_atomic
+{
+ global += 4;
+}
+
+int f1()
+{
+ bar2<TrueFalse>();
+ return foo<TrueFalse>() + bar<TrueFalse>();
+}
+
+/* 4 transactions overall, two of the write to global: */
+/* { dg-final { scan-tree-dump-times "ITM_RU4\\s*\\(&global" 4 "tmmark" } } */
+/* { dg-final { scan-tree-dump-times "ITM_WU4\\s*\\(&global" 2 "tmmark" } } */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [trans-mem] Fix instantiation of transaction expressions.
2011-11-07 14:24 [trans-mem] Fix instantiation of transaction expressions Torvald Riegel
@ 2011-11-07 16:41 ` Richard Henderson
2011-11-07 19:09 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2011-11-07 16:41 UTC (permalink / raw)
To: Torvald Riegel; +Cc: GCC Patches, Aldy Hernandez
On 11/07/2011 06:12 AM, Torvald Riegel wrote:
> stmt = begin_transaction_stmt (input_location, NULL, flags);
> tmp = RECUR (TRANSACTION_EXPR_BODY (t));
> + if (tmp)
> + {
> + /* No statements; handle this like an expression. */
In which case I'm pretty sure you ought to check for non-null
TRANSACTION_EXPR_BODY first and not call begin_transaction_stmt.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [trans-mem] Fix instantiation of transaction expressions.
2011-11-07 16:41 ` Richard Henderson
@ 2011-11-07 19:09 ` Jason Merrill
2011-11-07 19:47 ` Torvald Riegel
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2011-11-07 19:09 UTC (permalink / raw)
To: Richard Henderson; +Cc: Torvald Riegel, GCC Patches, Aldy Hernandez
On 11/07/2011 11:31 AM, Richard Henderson wrote:
> On 11/07/2011 06:12 AM, Torvald Riegel wrote:
>> stmt = begin_transaction_stmt (input_location, NULL, flags);
>> tmp = RECUR (TRANSACTION_EXPR_BODY (t));
>> + if (tmp)
>> + {
>> + /* No statements; handle this like an expression. */
>
> In which case I'm pretty sure you ought to check for non-null
> TRANSACTION_EXPR_BODY first and not call begin_transaction_stmt.
I believe TRANSACTION_EXPR_BODY is always non-null, but when tsubst_expr
is called for a statement it adds the new statement via add_stmt and
returns null. This approach is a bit funny, but ought to work fine, and
there doesn't seem to be a simple test to distinguish between statements
and expressions currently.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [trans-mem] Fix instantiation of transaction expressions.
2011-11-07 19:09 ` Jason Merrill
@ 2011-11-07 19:47 ` Torvald Riegel
2011-11-07 20:28 ` Richard Henderson
0 siblings, 1 reply; 5+ messages in thread
From: Torvald Riegel @ 2011-11-07 19:47 UTC (permalink / raw)
To: Jason Merrill; +Cc: Richard Henderson, GCC Patches, Aldy Hernandez
[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]
On Mon, 2011-11-07 at 14:04 -0500, Jason Merrill wrote:
> On 11/07/2011 11:31 AM, Richard Henderson wrote:
> > On 11/07/2011 06:12 AM, Torvald Riegel wrote:
> >> stmt = begin_transaction_stmt (input_location, NULL, flags);
> >> tmp = RECUR (TRANSACTION_EXPR_BODY (t));
> >> + if (tmp)
> >> + {
> >> + /* No statements; handle this like an expression. */
> >
> > In which case I'm pretty sure you ought to check for non-null
> > TRANSACTION_EXPR_BODY first and not call begin_transaction_stmt.
>
> I believe TRANSACTION_EXPR_BODY is always non-null, but when tsubst_expr
> is called for a statement it adds the new statement via add_stmt and
> returns null. This approach is a bit funny
I didn't claim it was pretty ... :)
> but ought to work fine, and
> there doesn't seem to be a simple test to distinguish between statements
> and expressions currently.
As suggested by Richard, here's an updated patch that uses
TREE_LANG_FLAG_0 to keep track of whether a TRANSACTION_EXPR contains
statements or an expression. I also split out the code to create a
transaction expression.
TM tests seem to run fine. OK for branch pending bootstrap?
[-- Attachment #2: patch3 --]
[-- Type: text/plain, Size: 6732 bytes --]
commit 6f0835817e263d2b28d609096305f94459799af2
Author: Torvald Riegel <triegel@redhat.com>
Date: Mon Nov 7 14:51:50 2011 +0100
Fix instantiation of transaction expressions.
* cp/pt.c (tsubst_expr) [TRANSACTION_EXPR]: If body is not a
statement, create an expression instead.
* cp/cp-tree.h (TRANSACTION_EXPR_IS_STMT, build_transaction_expr): New.
* cp/parser.c (cp_parser_transaction_expression): Use
build_transaction_expr.
* cp/semantics.c (build_transaction_expr): New.
(finish_transaction_stmt): Set TRANSACTION_EXPR_IS_STMT.
* testsuite/g++.dg/tm/template-1.C: New.
--- a/gcc/cp/ChangeLog.tm-merge
+++ b/gcc/cp/ChangeLog.tm-merge
@@ -10,7 +10,9 @@
(look_for_tm_attr_overrides, set_one_vmethod_tm_attributes,
set_method_tm_attributes): New.
(finish_struct_1): Call set_method_tm_attributes.
- * cp-tree.h (begin_transaction_stmt, finish_transaction_stmt): Declare.
+ * cp-tree.h (begin_transaction_stmt, finish_transaction_stmt,
+ build_transaction_expr): Declare.
+ (TRANSACTION_EXPR_IS_STMT): New.
* decl.c (push_cp_library_fn): Set attribute to transaction_safe.
* except.c (do_get_exception_ptr): Apply transaction_pure.
(do_begin_catch): Mark _ITM_cxa_begin_catch transaction_pure and
@@ -30,4 +32,5 @@
cp_parser_token_starts_function_definition_p): Same.
(cp_parser_required_error): Handle RT_TRANSACTION*.
* pt.c (tsubst_expr): Handle TRANSACTION_EXPR.
- * semantics.c (begin_transaction_stmt, finish_transaction_stmt): New.
\ No newline at end of file
+ * semantics.c (begin_transaction_stmt, finish_transaction_stmt,
+ build_transaction_expr): New.
\ No newline at end of file
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b6468e6..1bb6d98 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -73,6 +73,7 @@ c-common.h, not after.
VEC_INIT_EXPR_IS_CONSTEXPR (in VEC_INIT_EXPR)
DECL_OVERRIDE_P (in FUNCTION_DECL)
IMPLICIT_CONV_EXPR_DIRECT_INIT (in IMPLICIT_CONV_EXPR)
+ TRANSACTION_EXPR_IS_STMT (in TRANSACTION_EXPR)
1: IDENTIFIER_VIRTUAL_P (in IDENTIFIER_NODE)
TI_PENDING_TEMPLATE_FLAG.
TEMPLATE_PARMS_FOR_INLINE.
@@ -3862,6 +3863,10 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
TREE_TYPE (OMP_CLAUSE_RANGE_CHECK (NODE, OMP_CLAUSE_PRIVATE, \
OMP_CLAUSE_COPYPRIVATE))
+/* Nonzero if this transaction expression's body contains statements. */
+#define TRANSACTION_EXPR_IS_STMT(NODE) \
+ TREE_LANG_FLAG_0 (TRANSACTION_EXPR_CHECK (NODE))
+
/* These macros provide convenient access to the various _STMT nodes
created when parsing template declarations. */
#define TRY_STMTS(NODE) TREE_OPERAND (TRY_BLOCK_CHECK (NODE), 0)
@@ -5527,6 +5532,7 @@ extern void finish_omp_flush (void);
extern void finish_omp_taskwait (void);
extern tree begin_transaction_stmt (location_t, tree *, int);
extern void finish_transaction_stmt (tree, tree, int);
+extern tree build_transaction_expr (location_t, tree, int);
extern void finish_omp_taskyield (void);
extern bool cxx_omp_create_clause_info (tree, tree, bool, bool, bool);
extern tree baselink_for_fns (tree);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index b6ac918..334ed22 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -26682,10 +26682,7 @@ cp_parser_transaction_expression (cp_parser *parser, enum rid keyword)
if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN))
{
tree expr = cp_parser_expression (parser, /*cast_p=*/false, NULL);
- ret = build1 (TRANSACTION_EXPR, TREE_TYPE (expr), expr);
- if (this_in & TM_STMT_ATTR_RELAXED)
- TRANSACTION_EXPR_RELAXED (ret) = 1;
- SET_EXPR_LOCATION (ret, token->location);
+ ret = build_transaction_expr (token->location, expr, this_in);
}
else
{
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b1593fe..d4df8de 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12958,9 +12958,19 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
flags |= (TRANSACTION_EXPR_OUTER (t) ? TM_STMT_ATTR_OUTER : 0);
flags |= (TRANSACTION_EXPR_RELAXED (t) ? TM_STMT_ATTR_RELAXED : 0);
- stmt = begin_transaction_stmt (input_location, NULL, flags);
- tmp = RECUR (TRANSACTION_EXPR_BODY (t));
- finish_transaction_stmt (stmt, NULL, flags);
+ if (TRANSACTION_EXPR_IS_STMT (t))
+ {
+ stmt = begin_transaction_stmt (input_location, NULL, flags);
+ tmp = RECUR (TRANSACTION_EXPR_BODY (t));
+ finish_transaction_stmt (stmt, NULL, flags);
+ }
+ else
+ {
+ stmt = build_transaction_expr (EXPR_LOCATION (t),
+ RECUR (TRANSACTION_EXPR_BODY (t)),
+ flags);
+ return stmt;
+ }
}
break;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index d1967ee..068754a 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -5003,11 +5003,25 @@ finish_transaction_stmt (tree stmt, tree compound_stmt, int flags)
TRANSACTION_EXPR_BODY (stmt) = pop_stmt_list (TRANSACTION_EXPR_BODY (stmt));
TRANSACTION_EXPR_OUTER (stmt) = (flags & TM_STMT_ATTR_OUTER) != 0;
TRANSACTION_EXPR_RELAXED (stmt) = (flags & TM_STMT_ATTR_RELAXED) != 0;
+ TRANSACTION_EXPR_IS_STMT (stmt) = 1;
if (compound_stmt)
finish_compound_stmt (compound_stmt);
finish_stmt ();
}
+
+/* Build a __transaction_atomic or __transaction_relaxed expression. */
+
+tree
+build_transaction_expr (location_t loc, tree expr, int flags)
+{
+ tree ret;
+ ret = build1 (TRANSACTION_EXPR, TREE_TYPE (expr), expr);
+ if (flags & TM_STMT_ATTR_RELAXED)
+ TRANSACTION_EXPR_RELAXED (ret) = 1;
+ SET_EXPR_LOCATION (ret, loc);
+ return ret;
+}
\f
void
init_cp_semantics (void)
diff --git a/gcc/testsuite/g++.dg/tm/template-1.C b/gcc/testsuite/g++.dg/tm/template-1.C
new file mode 100644
index 0000000..b93828a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tm/template-1.C
@@ -0,0 +1,35 @@
+// { dg-do compile }
+// { dg-options "-fgnu-tm -O -fdump-tree-tmmark" }
+
+struct TrueFalse
+{
+ static bool v() { return true; }
+};
+
+int global;
+
+template<typename T> int foo()
+{
+ __transaction_atomic { global += 2; }
+ return __transaction_atomic (global + 1);
+}
+
+template<typename T> int bar() __transaction_atomic
+{
+ return global + 3;
+}
+
+template<typename T> void bar2() __transaction_atomic
+{
+ global += 4;
+}
+
+int f1()
+{
+ bar2<TrueFalse>();
+ return foo<TrueFalse>() + bar<TrueFalse>();
+}
+
+/* 4 transactions overall, two of the write to global: */
+/* { dg-final { scan-tree-dump-times "ITM_RU4\\s*\\(&global" 4 "tmmark" } } */
+/* { dg-final { scan-tree-dump-times "ITM_WU4\\s*\\(&global" 2 "tmmark" } } */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [trans-mem] Fix instantiation of transaction expressions.
2011-11-07 19:47 ` Torvald Riegel
@ 2011-11-07 20:28 ` Richard Henderson
0 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2011-11-07 20:28 UTC (permalink / raw)
To: Torvald Riegel; +Cc: Jason Merrill, GCC Patches, Aldy Hernandez
On 11/07/2011 11:33 AM, Torvald Riegel wrote:
> Fix instantiation of transaction expressions.
>
> * cp/pt.c (tsubst_expr) [TRANSACTION_EXPR]: If body is not a
> statement, create an expression instead.
> * cp/cp-tree.h (TRANSACTION_EXPR_IS_STMT, build_transaction_expr): New.
> * cp/parser.c (cp_parser_transaction_expression): Use
> build_transaction_expr.
> * cp/semantics.c (build_transaction_expr): New.
> (finish_transaction_stmt): Set TRANSACTION_EXPR_IS_STMT.
> * testsuite/g++.dg/tm/template-1.C: New.
Looks good to me.
> + tmp = RECUR (TRANSACTION_EXPR_BODY (t));
Dead store now.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-07 19:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-07 14:24 [trans-mem] Fix instantiation of transaction expressions Torvald Riegel
2011-11-07 16:41 ` Richard Henderson
2011-11-07 19:09 ` Jason Merrill
2011-11-07 19:47 ` Torvald Riegel
2011-11-07 20:28 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).