This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: trans-mem: virtual ops for gimple_transaction
- From: Richard Guenther <rguenther at suse dot de>
- To: Richard Henderson <rth at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Diego Novillo <dnovillo at google dot com>, Aldy Hernandez <aldyh at redhat dot com>
- Date: Fri, 10 Feb 2012 10:44:24 +0100 (CET)
- Subject: Re: trans-mem: virtual ops for gimple_transaction
- References: <bug-51752-119-dJucciRs1z@http.gcc.gnu.org/bugzilla/> <4F345153.8090602@redhat.com>
On Thu, 9 Feb 2012, Richard Henderson wrote:
> > From: rguenth at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>
> > the __transaction_atomic // SUBCODE=[ GTMA_HAVE_STORE ] statement
> > looks like an overly optimistic way to start a transaction in my quick view.
>
> Indeed. At some point this worked, but this may have gotten lost
> during one of the merges. Now,
>
> # .MEM_8 = VDEF <.MEM_7(D)>
> __transaction_relaxed // SUBCODE=[ ... ]
>
> Bootstrapped and tested on x86_64. Ok?
Yes.
But ...
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index ccdf14a..ace9ef9 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -965,6 +965,13 @@ propagate_necessity (struct edge_list *el)
mark_aliased_reaching_defs_necessary (stmt, op);
}
}
+ else if (gimple_code (stmt) == GIMPLE_TRANSACTION)
+ {
+ /* The beginning of a transaction is a memory barrier. */
+ /* ??? If we were really cool, we'd only be a barrier
+ for the memories touched within the transaction. */
+ mark_all_reaching_defs_necessary (stmt);
+ }
else
gcc_unreachable ();
hints at that code might not expect virtual operands on this ...
What is the reason to keep a GIMPLE_TRANSACTION stmt after
TM lowering and not lower it to a builtin function call? It seems
the body is empty after lowering (what's the label thing?)
I'd have arranged it do lower
__transaction_atomic
{
{
x[9] = 1;
}
}
to
__builtin_transaction_atomic (GTMA_HAVE_STORE, &label);
try
{
x[9] = 1;
}
finally
{
__builtin__ITM_commitTransaction ();
}
Eventually returning/passing a token to link a transaction start
to its commit / abort.
That said, I expect some more fallout from the patch, but I suppose
TM is still experimental enough that we can fixup things later.
Richard.