Re: [patch] PR56572 flatten unnecessary nested transactions after inlining

[Aldy Hernandez <aldyh at redhat dot com>]

On 01/06/14 13:40, Richard Henderson wrote:
> On 12/19/2013 11:06 AM, Richard Biener wrote:
> > Aldy Hernandez <aldyh@redhat.com> wrote:
> > > I'd still like to catch the common cases, like I do with this patch.
> > >
> > > Perhaps we move this code to the .tmmark pass and handle the
> > > uninstrumented case.  rth?
>
> tmmark is way way later than you'd want.  I believe that ipa_tm is the right
> place.  That's where we generate clones.  The clones know a-priori that they're
> called within a transaction and thus all internal transations may be
> eliminated.  And thus any inlining that would happen after ipa_tm would
> properly inline the clone, and thus no more need be done.

I have a patch (attached for reference) removing the nested transactions 
while we are creating the clones (as suggested), but the uninstrumented 
code path complicates things.  I'm afraid I don't have any good news.

Consider this:

inline void f() {
  __transaction_atomic {
      a = 12345;
  }
}

void g() {
  __transaction_atomic {
    f();
  }
}

The problem is that when we add the uninstrumented code path later in 
tmipa, we end up with the following for g():

g ()
{
  <bb 2>:
  __transaction_atomic  // SUBCODE=[ GTMA_HAVE_LOAD GTMA_HAVE_STORE ]
  goto <bb 3>;

  <bb 5>:
  f ();            /* <---- uninstrumented path */
  __builtin__ITM_commitTransaction ();
  goto <bb 4>;

  <bb 3>:
  f (); [tm-clone]    /* <---- instrumented path */
  __builtin__ITM_commitTransaction ();

  <bb 4>:
  return;

}

Since we only removed the transaction in the clone of f(), plain regular 
f() will still have the additional transaction, so inlining will still 
yield a g() with a nested transaction in the uninstrumented path.

So we're back to square one, needing a separate pass to remove the 
nested transactions, and this pass will unfortunately have to deal with 
the uninstrumented/instrumented paths.

This has taken longer to fix than I expected, so I'm going to put this 
aside for now and concentrate on some P1-P2's.  For the record, since 
you don't like this pass in the .tmmark pass which is WAY late, can we 
have a tree pass right after the IPA passes (thus after inlining)?

I'll add some notes to the PR so we can pick this up later.

Aldy

Attachment: curr
Description: Text document