[patch] PR56572 flatten unnecessary nested transactions after inlining

Aldy Hernandez aldyh@redhat.com
Mon Jan 6 23:14:00 GMT 2014


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



More information about the Gcc-patches mailing list