* PR/54893: allow volatiles inside relaxed transactions
@ 2012-10-11 21:07 Aldy Hernandez
2012-10-11 21:19 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Aldy Hernandez @ 2012-10-11 21:07 UTC (permalink / raw)
To: gcc-patches, Richard Henderson; +Cc: Torvald Riegel
[-- Attachment #1: Type: text/plain, Size: 409 bytes --]
Apparently we were a bit too aggressive on disallowing volatiles inside
transactions. Torvald seems to agree that volatiles should be allowed
inside relaxed transactions, since they will go into serial irrevocable
mode anyhow.
The following patch fixes the PR.
I did not include a test that fails on a volatile in an atomic
transaction because there is already one in the testsuite.
OK pending tests?
[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 1022 bytes --]
PR middle-end/54893
* trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
transactions.
diff --git a/gcc/testsuite/c-c++-common/tm/pr54893.c b/gcc/testsuite/c-c++-common/tm/pr54893.c
new file mode 100644
index 0000000..df26f25
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tm/pr54893.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm" } */
+
+/* Test that volatiles are allowed inside relaxed transactions. */
+
+volatile int test_var = 0;
+
+int main()
+{
+ __transaction_relaxed {
+ test_var++;
+ }
+}
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index ef384ac..dc08bc6 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -561,7 +561,7 @@ diagnose_tm_1_op (tree *tp, int *walk_subtrees ATTRIBUTE_UNUSED,
if ((code == VAR_DECL
|| code == RESULT_DECL
|| code == PARM_DECL)
- && d->block_flags & (DIAG_TM_SAFE | DIAG_TM_RELAXED)
+ && d->block_flags & DIAG_TM_SAFE
&& TREE_THIS_VOLATILE (TREE_TYPE (*tp))
&& !d->saw_volatile)
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PR/54893: allow volatiles inside relaxed transactions
2012-10-11 21:07 PR/54893: allow volatiles inside relaxed transactions Aldy Hernandez
@ 2012-10-11 21:19 ` Richard Henderson
2012-10-12 11:02 ` Aldy Hernandez
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2012-10-11 21:19 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: gcc-patches, Torvald Riegel
On 10/11/2012 01:56 PM, Aldy Hernandez wrote:
> PR middle-end/54893
> * trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
> transactions.
Ok.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PR/54893: allow volatiles inside relaxed transactions
2012-10-11 21:19 ` Richard Henderson
@ 2012-10-12 11:02 ` Aldy Hernandez
2012-10-17 3:21 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Aldy Hernandez @ 2012-10-12 11:02 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, Torvald Riegel
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
On 10/11/12 17:15, Richard Henderson wrote:
> On 10/11/2012 01:56 PM, Aldy Hernandez wrote:
>> PR middle-end/54893
>> * trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
>> transactions.
>
> Ok.
>
> r~
>
Sorry for the noise, but Torvald pointed out that the transaction must
now go irrevocable if we find a volatile, and the code on mainline does
not do that.
I've rewritten the patch to go irrevocable as well.
Tested on x86-64 Linux.
OK?
[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 4090 bytes --]
PR middle-end/54893
* trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
transactions.
diff --git a/gcc/testsuite/c-c++-common/tm/pr54893.c b/gcc/testsuite/c-c++-common/tm/pr54893.c
new file mode 100644
index 0000000..8967f38
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tm/pr54893.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+/* Test that volatiles are allowed inside relaxed transactions. */
+
+volatile int test_var = 0;
+
+int main()
+{
+ __transaction_relaxed {
+ test_var++;
+ }
+}
+
+/* { dg-final { scan-ipa-dump "GTMA_DOES_GO_IRREVOCABLE" "tmipa" } } */
+/* { dg-final { cleanup-ipa-dump "tmipa" } } */
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index ef384ac..211c45e 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -548,6 +548,15 @@ struct diagnose_tm
gimple stmt;
};
+/* Return true if T is a volatile variable of some kind. */
+
+static bool
+volatile_var_p (tree t)
+{
+ return (SSA_VAR_P (t)
+ && TREE_THIS_VOLATILE (TREE_TYPE (t)));
+}
+
/* Tree callback function for diagnose_tm pass. */
static tree
@@ -556,13 +565,9 @@ diagnose_tm_1_op (tree *tp, int *walk_subtrees ATTRIBUTE_UNUSED,
{
struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
struct diagnose_tm *d = (struct diagnose_tm *) wi->info;
- enum tree_code code = TREE_CODE (*tp);
- if ((code == VAR_DECL
- || code == RESULT_DECL
- || code == PARM_DECL)
- && d->block_flags & (DIAG_TM_SAFE | DIAG_TM_RELAXED)
- && TREE_THIS_VOLATILE (TREE_TYPE (*tp))
+ if (volatile_var_p (*tp)
+ && d->block_flags & DIAG_TM_SAFE
&& !d->saw_volatile)
{
d->saw_volatile = 1;
@@ -3782,40 +3787,56 @@ ipa_tm_scan_irr_block (basic_block bb)
gimple stmt = gsi_stmt (gsi);
switch (gimple_code (stmt))
{
+ case GIMPLE_ASSIGN:
+ if (gimple_assign_single_p (stmt))
+ {
+ tree lhs = gimple_assign_lhs (stmt);
+ tree rhs = gimple_assign_rhs1 (stmt);
+ if (volatile_var_p (lhs) || volatile_var_p (rhs))
+ return true;
+ }
+ break;
+
case GIMPLE_CALL:
- if (is_tm_pure_call (stmt))
- break;
+ {
+ tree lhs = gimple_call_lhs (stmt);
+ if (lhs && volatile_var_p (lhs))
+ return true;
- fn = gimple_call_fn (stmt);
+ if (is_tm_pure_call (stmt))
+ break;
- /* Functions with the attribute are by definition irrevocable. */
- if (is_tm_irrevocable (fn))
- return true;
+ fn = gimple_call_fn (stmt);
- /* For direct function calls, go ahead and check for replacement
- functions, or transitive irrevocable functions. For indirect
- functions, we'll ask the runtime. */
- if (TREE_CODE (fn) == ADDR_EXPR)
- {
- struct tm_ipa_cg_data *d;
- struct cgraph_node *node;
+ /* Functions with the attribute are by definition irrevocable. */
+ if (is_tm_irrevocable (fn))
+ return true;
- fn = TREE_OPERAND (fn, 0);
- if (is_tm_ending_fndecl (fn))
- break;
- if (find_tm_replacement_function (fn))
- break;
+ /* For direct function calls, go ahead and check for replacement
+ functions, or transitive irrevocable functions. For indirect
+ functions, we'll ask the runtime. */
+ if (TREE_CODE (fn) == ADDR_EXPR)
+ {
+ struct tm_ipa_cg_data *d;
+ struct cgraph_node *node;
- node = cgraph_get_node(fn);
- d = get_cg_data (&node, true);
+ fn = TREE_OPERAND (fn, 0);
+ if (is_tm_ending_fndecl (fn))
+ break;
+ if (find_tm_replacement_function (fn))
+ break;
- /* Return true if irrevocable, but above all, believe
- the user. */
- if (d->is_irrevocable
- && !is_tm_safe_or_pure (fn))
- return true;
- }
- break;
+ node = cgraph_get_node(fn);
+ d = get_cg_data (&node, true);
+
+ /* Return true if irrevocable, but above all, believe
+ the user. */
+ if (d->is_irrevocable
+ && !is_tm_safe_or_pure (fn))
+ return true;
+ }
+ break;
+ }
case GIMPLE_ASM:
/* ??? The Approved Method of indicating that an inline
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PR/54893: allow volatiles inside relaxed transactions
2012-10-12 11:02 ` Aldy Hernandez
@ 2012-10-17 3:21 ` Richard Henderson
0 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2012-10-17 3:21 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: gcc-patches, Torvald Riegel
On 2012-10-12 20:42, Aldy Hernandez wrote:
> PR middle-end/54893
> * trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
> transactions.
Ok.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-17 1:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-11 21:07 PR/54893: allow volatiles inside relaxed transactions Aldy Hernandez
2012-10-11 21:19 ` Richard Henderson
2012-10-12 11:02 ` Aldy Hernandez
2012-10-17 3:21 ` 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).