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

[Aldy Hernandez <aldyh at redhat dot com>]

On 01/06/14 15:04, Aldy Hernandez wrote:
> 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.
>
> But ipa_tm still runs before the latter inliner:
>
> ...
> u.c.041i.tmipa
> u.c.043i.whole-program
> u.c.044i.profile_estimate
> u.c.045i.devirt
> u.c.046i.cp
> u.c.048i.inline
>
> So even though my proposed patch works in the supplied testcase (because
> the early inliner ran before .tmipa and inlined the nested transaction),
> if the latter inliner added a nested transaction we're in the same boat.
>   I just saw this happen with LTO, as Richi pointed out.
>
> What do you suggest?

Though, let me clarify what I saw.  LTO would only do it's cross TU 
inlining thing if the inlinee was marked as transaction_safe, because 
anything else wouldn't be valid inside an atomic transaction:

reynosa:/dev/shm/trunk/gcc/u$ cat d.c
int a;
void inc(){
    __transaction_atomic { a++; }
}
reynosa:/dev/shm/trunk/gcc/u$ cat e.c
extern void inc() __attribute__((transaction_safe));
main()
{
  __transaction_atomic {
      inc();
  }
}

So perhaps we don't care about LTO inlining, since most of what it would 
inline isn't valid within the transactional context ??.  So perhaps just 
getting rid of transactions appearing in a clone we're about to create 
in ipa-tm is all we need?  Is that what you were getting at, or am I 
running around in circles here? :)

Aldy