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. Regarding the name, "transaction" makes more sense to me because it has almost no uses besides transaction-related stuff. Second, I guess we'd have to keep it in libitm_i.h anyway, and not in tls.h. Therefore, it seems to be the right thing to do to put it together with the related stuff, even if we rename gtm_transaction to gtm_thread later, or move it out of libitm_i.h (why would we? where to?). > 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. Either way, when gtm_thread and gtm_transaction have been merged (and I'm thinking about merging into gtm_transaction here...), this issue should also have gone away. I'm planning to come up with a separate patch for the actual merge. I first need to think a bit more about whether the destruction can be done with just a destructor of the TLS object, what happens when other object's destructors use transactions, etc. Even with one txn per thread, we still need to deregister it and release its dynamically allocated memory. Perhaps adding a destruction-phase mode would help, in which transactions are always destructed after they've run. Torvald
