On Thu, 9 Feb 2012, Richard Henderson wrote:
> > From: rguenth at gcc dot gnu.org <gcc-bugzi...@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.
