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 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.
r~
