| Message ID | 5127867A.1000507@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/22/2013 09:53 AM, Aldy Hernandez wrote: > Frankly, I don't understand finish_transaction_stmt(), and why it sets > TREE_SIDE_EFFECTS only if the [noexcept] clause is set. I'm C++ > ignorant, but I would've thought the opposite to be true. It's setting that flag on the MUST_NOT_THROW_EXPR. > Anyways, I've fixed the problem by setting TREE_SIDE_EFFECTS if the > transaction body has side effects. Perhaps we should do this for > build_transaction_expr() as well? I would think managing the transaction boundaries would always have side-effects, no? If that's correct, I'd set the flag unconditionally in begin_transaction_stmt and build_transaction_expr. Jason
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 458ed26..c446cd6 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5135,6 +5135,9 @@ finish_transaction_stmt (tree stmt, tree compound_stmt, int flags, tree noex) TRANSACTION_EXPR_BODY (stmt) = body; } + if (TREE_SIDE_EFFECTS (TRANSACTION_EXPR_BODY (stmt))) + TREE_SIDE_EFFECTS (stmt) = 1; + if (compound_stmt) finish_compound_stmt (compound_stmt); finish_stmt (); diff --git a/gcc/testsuite/g++.dg/tm/pr56419.C b/gcc/testsuite/g++.dg/tm/pr56419.C new file mode 100644 index 0000000..c9a33a8 --- /dev/null +++ b/gcc/testsuite/g++.dg/tm/pr56419.C @@ -0,0 +1,13 @@ +// { dg-do compile } +// { dg-options "-fgnu-tm" } + +int x = 0; +int inc_func(int i) { + for (int j = 0; j < i; ++j) + { + __transaction_atomic { x+=1; } + } + return 0; +} + +// { dg-final { scan-assembler "ITM_commitTransaction" } }
In the following snippet, the C++ front-end drops the transaction altogether: +int x = 0; +int inc_func(int i) { + for (int j = 0; j < i; ++j) + { + __transaction_atomic { x+=1; } + } + return 0; This was caused by http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186546 The problem here is that genericize_cp_loop() calls append_to_statement_list() to add the TRANSACTION_EXPR, but in this case, TREE_SIDE_EFFECTS is not set, so it is silently ignored. Frankly, I don't understand finish_transaction_stmt(), and why it sets TREE_SIDE_EFFECTS only if the [noexcept] clause is set. I'm C++ ignorant, but I would've thought the opposite to be true. Anyways, I've fixed the problem by setting TREE_SIDE_EFFECTS if the transaction body has side effects. Perhaps we should do this for build_transaction_expr() as well? What do y'all prefer? commit 01704e6a117846458dbc11cb76284504673f2d3c Author: Aldy Hernandez <aldyh@redhat.com> Date: Fri Feb 22 08:28:14 2013 -0600 PR c++/56419 * semantics.c (finish_transaction_stmt): Set TREE_SIDE_EFFECTS if the body has side effects.