From patchwork Mon Nov 7 19:01:03 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 124166 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 0DD291007D4 for ; Tue, 8 Nov 2011 06:01:32 +1100 (EST) Received: (qmail 6025 invoked by alias); 7 Nov 2011 19:01:28 -0000 Received: (qmail 5997 invoked by uid 22791); 7 Nov 2011 19:01:23 -0000 X-SWARE-Spam-Status: No, hits=-7.0 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:01:08 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pA7J16KR000787 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 7 Nov 2011 14:01:07 -0500 Received: from anchor.twiddle.net (vpn-225-162.phx2.redhat.com [10.3.225.162]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id pA7J14Ln018021; Mon, 7 Nov 2011 14:01:04 -0500 Message-ID: <4EB82AEF.7000208@redhat.com> Date: Mon, 07 Nov 2011 11:01:03 -0800 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110927 Thunderbird/7.0 MIME-Version: 1.0 To: Richard Guenther CC: Aldy Hernandez , gcc-patches Subject: Re: [patch] 19/n: trans-mem: compiler tree/gimple stuff References: <4EB2EACC.8050307@redhat.com> <4EB5A537.1090204@redhat.com> In-Reply-To: X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On 11/05/2011 03:09 PM, Richard Guenther wrote: > On Sat, Nov 5, 2011 at 10:05 PM, Aldy Hernandez wrote: >> [rth, see below] >> >>>> local_define_builtin ("__builtin_eh_pointer", ftype, >>>> BUILT_IN_EH_POINTER, >>>> "__builtin_eh_pointer", ECF_PURE | ECF_NOTHROW | >>>> ECF_LEAF); >>>> + if (flag_tm) >>>> + apply_tm_attr (builtin_decl_explicit (BUILT_IN_EH_POINTER), >>>> + get_identifier ("transaction_pure")); >>> >>> I think this should use a new ECF_TM_PURE flag, unconditionally set >>> with handling in the functions that handle/return ECF flags so that >>> transitioning this to a tree node flag instead of an attribute is easier. >> >> I could add a ECF_TM_PURE flag and attach it to the BUILT_IN_EH_POINTER in >> the local_define_builtin above, but we still need the attribute for function >> decl's as in: >> >> __attribute__((transaction_pure)) void foo(); >> >> Attributes seem like a clean way to approach this. > > The middle-end interfacing is supposed to be via ECF_ flags, the user interface > via attributes. What's the semantic of transaction-pure vs. ... > >> I don't see what the flag buys us. Or am I misunderstanding something? >> >>>> +/* Nonzero if this call performs a transactional memory operation. */ >>>> +#define ECF_TM_OPS (1<< 11) >>> >>> What's this flag useful for? Isn't it the case that you want to >>> conservatively >>> know whether a call might perform a tm operation? Thus, the flag >>> should be inverted? Is this the same as "TM pure"? > > ... this? > >> Richard? > >> Richi, I have fixed or addressed all the issues in this thread, with the >> exception of your EFC_TM_PURE and ECF_TM_OPS questions, which I am deferring >> to rth and then fixing if required. > > Yeah, seems to be still an open question. I hope this cleanup both addresses the above questions and tidies things up as indicated. Please ask if you've got more questions. Ok, Richi? r~ diff --git a/gcc/calls.c b/gcc/calls.c index 515ab97..382de7f 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -707,13 +707,28 @@ flags_from_decl_or_type (const_tree exp) if (TREE_NOTHROW (exp)) flags |= ECF_NOTHROW; - if (is_tm_builtin (exp)) - flags |= ECF_TM_OPS; + if (flag_tm) + { + if (is_tm_builtin (exp)) + flags |= ECF_TM_BUILTIN; + else if ((flags & ECF_CONST) != 0 + || lookup_attribute ("transaction_pure", + TYPE_ATTRIBUTES (TREE_TYPE (exp)))) + flags |= ECF_TM_PURE; + } flags = special_function_p (exp, flags); } - else if (TYPE_P (exp) && TYPE_READONLY (exp)) - flags |= ECF_CONST; + else if (TYPE_P (exp)) + { + if (TYPE_READONLY (exp)) + flags |= ECF_CONST; + + if (flag_tm + && ((flags & ECF_CONST) != 0 + || lookup_attribute ("transaction_pure", TYPE_ATTRIBUTES (exp)))) + flags |= ECF_TM_PURE; + } if (TREE_THIS_VOLATILE (exp)) { diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c index ba25fd8..be399a0 100644 --- a/gcc/trans-mem.c +++ b/gcc/trans-mem.c @@ -172,14 +172,8 @@ get_attrs_for (const_tree x) bool is_tm_pure (const_tree x) { - if (flag_tm) - { - tree attrs = get_attrs_for (x); - if (attrs) - return lookup_attribute ("transaction_pure", attrs) != NULL; - return false; - } - return false; + unsigned flags = flags_from_decl_or_type (x); + return (flags & ECF_TM_PURE) != 0; } /* Return true if X has been marked TM_IRREVOCABLE. */ @@ -229,10 +223,6 @@ static bool is_tm_pure_call (gimple call) { tree fn = gimple_call_fn (call); - unsigned flags; - - if (is_tm_pure (TREE_TYPE (fn))) - return true; if (TREE_CODE (fn) == ADDR_EXPR) { @@ -241,9 +231,8 @@ is_tm_pure_call (gimple call) } else fn = TREE_TYPE (fn); - flags = flags_from_decl_or_type (fn); - return (flags & ECF_CONST) != 0; + return is_tm_pure (fn); } /* Return true if X has been marked TM_CALLABLE. */ @@ -2484,7 +2473,7 @@ make_tm_edge (gimple stmt, basic_block bb, struct tm_region *region) } -/* Split block BB as necessary for every TM_OPS function we added, and +/* Split block BB as necessary for every builtin function we added, and wire up the abnormal back edges implied by the transaction restart. */ static void @@ -2496,15 +2485,16 @@ expand_block_edges (struct tm_region *region, basic_block bb) { gimple stmt = gsi_stmt (gsi); - /* ??? TM_COMMIT (and any other ECF_TM_OPS function) in a nested + /* ??? TM_COMMIT (and any other tm builtin function) in a nested transaction has an abnormal edge back to the outer-most transaction (there are no nested retries), while a TM_ABORT also has an abnormal backedge to the inner-most transaction. We haven't actually saved the inner-most transaction here. We should be able to get to it via the region_nr saved on STMT, and read the transaction_stmt from that, and find the first region block from there. */ + /* ??? Shouldn't we split for any non-pure, non-irrevocable function? */ if (gimple_code (stmt) == GIMPLE_CALL - && (gimple_call_flags (stmt) & ECF_TM_OPS) != 0) + && (gimple_call_flags (stmt) & ECF_TM_BUILTIN) != 0) { if (gsi_one_before_end_p (gsi)) make_tm_edge (stmt, bb, region); @@ -3934,11 +3924,18 @@ ipa_tm_mayenterirr_function (struct cgraph_node *node) { struct tm_ipa_cg_data *d = get_cg_data (node); tree decl = node->decl; + unsigned flags = flags_from_decl_or_type (decl); + + /* Handle some TM builtins. Ordinarily these aren't actually generated + at this point, but handling these functions when written in by the + user makes it easier to build unit tests. */ + if (flags & ECF_TM_BUILTIN) + return false; /* Filter out all functions that are marked. */ - if (is_tm_safe_or_pure (decl)) + if (flags & ECF_TM_PURE) return false; - if ((flags_from_decl_or_type (decl) & ECF_CONST) != 0) + if (is_tm_safe (decl)) return false; if (is_tm_irrevocable (decl)) return true; @@ -3947,11 +3944,6 @@ ipa_tm_mayenterirr_function (struct cgraph_node *node) if (find_tm_replacement_function (decl)) return true; - /* Handle some TM builtins. */ - if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL - && (flags_from_decl_or_type (decl) & ECF_TM_OPS) != 0) - return false; - /* If we aren't seeing the final version of the function we don't know what it will contain at runtime. */ if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE) @@ -4394,8 +4386,10 @@ ipa_tm_transform_calls_redirect (struct cgraph_node *node, return; } - /* If the call is to the TM runtime, do nothing further. */ - if (flags_from_decl_or_type (fndecl) & ECF_TM_OPS) + /* Handle some TM builtins. Ordinarily these aren't actually generated + at this point, but handling these functions when written in by the + user makes it easier to build unit tests. */ + if (flags_from_decl_or_type (fndecl) & ECF_TM_BUILTIN) return; /* Fixup recursive calls inside clones. */ diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 61e4476..7cb4a3d 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -2298,9 +2298,9 @@ is_ctrl_altering_stmt (gimple t) return true; /* TM ending statements have backedges out of the transaction. - Return true so we split the basic block containing - them. */ - if ((flags & ECF_TM_OPS) + Return true so we split the basic block containing them. + Note that the TM_BUILTIN test is merely an optimization. */ + if ((flags & ECF_TM_BUILTIN) && is_tm_ending_fndecl (gimple_call_fndecl (t))) return true; diff --git a/gcc/tree.c b/gcc/tree.c index 30c6bb8..ba6c2e1 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -9428,6 +9428,8 @@ local_define_builtin (const char *name, tree type, enum built_in_function code, if (ecf_flags & ECF_LEAF) DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("leaf"), NULL, DECL_ATTRIBUTES (decl)); + if ((ecf_flags & ECF_TM_PURE) && flag_tm) + apply_tm_attr (decl, get_identifier ("transaction_pure")); set_builtin_decl (code, decl, true); } @@ -9593,10 +9595,8 @@ build_common_builtin_nodes (void) ftype = build_function_type_list (ptr_type_node, integer_type_node, NULL_TREE); local_define_builtin ("__builtin_eh_pointer", ftype, BUILT_IN_EH_POINTER, - "__builtin_eh_pointer", ECF_PURE | ECF_NOTHROW | ECF_LEAF); - if (flag_tm) - apply_tm_attr (builtin_decl_explicit (BUILT_IN_EH_POINTER), - get_identifier ("transaction_pure")); + "__builtin_eh_pointer", + ECF_PURE | ECF_NOTHROW | ECF_LEAF | ECF_TM_PURE); tmp = lang_hooks.types.type_for_mode (targetm.eh_return_filter_mode (), 0); ftype = build_function_type_list (tmp, integer_type_node, NULL_TREE); diff --git a/gcc/tree.h b/gcc/tree.h index 23f3d69..ab20272 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -5601,8 +5601,10 @@ extern tree build_duplicate_type (tree); #define ECF_NOVOPS (1 << 9) /* The function does not lead to calls within current function unit. */ #define ECF_LEAF (1 << 10) -/* Nonzero if this call performs a transactional memory operation. */ -#define ECF_TM_OPS (1 << 11) +/* Nonzero if this call does not affect transactions. */ +#define ECF_TM_PURE (1 << 11) +/* Nonzero if this call is into the transaction runtime library. */ +#define ECF_TM_BUILTIN (1 << 12) extern int flags_from_decl_or_type (const_tree); extern int call_expr_flags (const_tree);