On 11/05/2011 03:09 PM, Richard Guenther wrote:
> On Sat, Nov 5, 2011 at 10:05 PM, Aldy Hernandez <al...@redhat.com> 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);
