From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25420 invoked by alias); 7 Nov 2011 19:33:55 -0000 Received: (qmail 25404 invoked by uid 22791); 7 Nov 2011 19:33:54 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_TM X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 07 Nov 2011 19:33:38 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pA7JXcvt013527 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 7 Nov 2011 14:33:38 -0500 Received: from [10.36.5.84] (vpn1-5-84.ams2.redhat.com [10.36.5.84]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id pA7JXY1H009882; Mon, 7 Nov 2011 14:33:35 -0500 Subject: Re: [trans-mem] Fix instantiation of transaction expressions. From: Torvald Riegel To: Jason Merrill Cc: Richard Henderson , GCC Patches , Aldy Hernandez In-Reply-To: <4EB82BD3.1030507@redhat.com> References: <1320675134.18023.349.camel@triegel.csb> <4EB807FF.3070300@redhat.com> <4EB82BD3.1030507@redhat.com> Content-Type: multipart/mixed; boundary="=-n5+TbseQD7pOAl/Wrh7u" Date: Mon, 07 Nov 2011 19:47:00 -0000 Message-ID: <1320694411.32515.6.camel@triegel.csb> Mime-Version: 1.0 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-11/txt/msg01045.txt.bz2 --=-n5+TbseQD7pOAl/Wrh7u Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Content-length: 1196 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? --=-n5+TbseQD7pOAl/Wrh7u Content-Disposition: attachment; filename="patch3" Content-Type: text/plain; name="patch3"; charset="UTF-8" Content-Transfer-Encoding: 7bit Content-length: 6732 commit 6f0835817e263d2b28d609096305f94459799af2 Author: Torvald Riegel 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; +} 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 int foo() +{ + __transaction_atomic { global += 2; } + return __transaction_atomic (global + 1); +} + +template int bar() __transaction_atomic +{ + return global + 3; +} + +template void bar2() __transaction_atomic +{ + global += 4; +} + +int f1() +{ + bar2(); + return foo() + bar(); +} + +/* 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" } } */ --=-n5+TbseQD7pOAl/Wrh7u--