This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: trans-mem: virtual ops for gimple_transaction


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]