On Thu, 2011-08-04 at 12:35 -0700, Richard Henderson wrote:
> On 08/04/2011 12:24 PM, Torvald Riegel wrote:
> > On Thu, 2011-08-04 at 11:55 -0700, Richard Henderson wrote:
> >> On 08/04/2011 09:22 AM, Torvald Riegel wrote:
> >>> On Thu, 2011-08-04 at 08:43 -0700, Richard Henderson wrote:
> >>>> On 08/03/2011 04:04 AM, Torvald Riegel wrote:
> >>>>> Move local_tid from gtm_thread to gtm_transaction.
> >>>>>
> >>>>> * config/generic/tls.h (gtm_thread): Move local_tid from here
> >>>>> ...
> >>>>> * libitm_i.h (local_tid): ... to here.
> >>>>> * beginend.cc (GTM::gtm_transaction::begin_transaction): Same.
> >>>>> (GTM::gtm_transaction::operator new): Set up gtm_thread if
> >>>>> necessary.
> >>>>
> >>>> This seems very wrong. Why?
> >>>
> >>> What seems wrong?
> >>>
> >>> local_tid is per thread and there is one gtm_transaction object per
> >>> thread, so moving it is correct, or not?
> >>> The purpose of this is to not having to access gtm_thread anymore in
> >>> begin. And it's a step towards merging gtm_thread and gtm_transaction
> >>> completely.
> >>>
> >>> Do you agree?
> >>
> >> If you're going to merge gtm_thread and gtm_transaction, why
> >> are you moving things away from gtm_thread?
> >
> > I believe the per-thread object should be gtm_transaction, not
> > gtm_thread.
>
> I don't. For the simple fact that *all* per-thread information
> should be in gtm_thread, not just some of it. If we ever have
> any non-transaction per-thread information, then it should go
> into gtm_thread.
>
> Suppose we did eliminate everything not directly related to the
> current transaction from gtm_thread, I think it would look like
>
> struct gtm_thread
> {
> gtm_transaction txn;
> };
>
> But even that said, the specific case of local_tid seems like
> exactly the sort of information that should remain in gtm_thread.
> It's *not* related to the current transaction, it's related to
> the current thread.
As discussed off-list, local_tid can be moved into the transaction.
The question of whether and how to merge gtm_thread and gtm_transaction
is separate from that. I have provided a first draft for that merge in
my previous email to the list.
> >> As for "not having to access gtm_thread", the non-glibc case
> >> for accessing gtm_txn is to pull the value out of gtm_thread.
> >
> > I'm aware of that, but non-glibc is not the common case we're optimizing
> > for. If so, I believe we'd have kept the tx param in all the _ITM_* load
> > and store functions.
>
> But what you're suggesting is intentionally making non-glibc worse
> for no good reason. I, and probably others, take issue with that.
That was a misunderstanding/non-issue, but it also considered in the
recent merge patch.
Torvald
