[ bkoz, see <bkoz> below ] On 02/14/2012 04:48 AM, Torvald Riegel wrote: > BTW, why can we assume that the dispatch objects are initialized before > we run any transactions (e.g., from within a constructor of another > static object)? Or is this the library initialization problem that we > still have to deal with at some point?
ELF shared libraries are initialized in dependency order. Static libraries simply must be careful to be linked in the proper order on the command line. It should be fine unless the user really tries to do something wrong. > libitm/ > * beginend.cc (GTM::gtm_thread::begin_transaction): Move serial lock > acquisition to ... > * retry.cc (GTM::gtm_thread::decide_begin_dispatch): ... here. > (default_dispatch): Make atomic. > (GTM::gtm_thread::set_default_dispatch): Access atomically. > (GTM::gtm_thread::decide_retry_strategy): Access atomically and > use decide_begin_dispatch() if default_dispatch might have changed. > (GTM::gtm_thread::number_of_threads_changed): Initialize > default_dispatch here. I discussed this with Torvald at length on IRC. For the record, I think the change to use atomic<> obscures the actual bug fix, that of rearranging decide_begin_dispatch. The change to use atomic<> is at best a theoretical bug fix, for some embedded target that doesn't load/store pointers atomically by default. Or for consumption by some static race detector. I'd have been happier with the one interesting reference to default_dispatch being changed to use __atomic_load. Rather than having special markup for all of the non-interesting references, and spending cycles figuring out whether the use of memory_order_relaxed is really correct at each instance. All that said the patch is ok, Torvald. <bkoz> I wonder if the atomic<> template wouldn't have been better with another (defaulted) template argument to set the default memory access mode for the object. In this case we could have used static std::atomic<GTM::abi_dispatch*, memory_order_relaxed> default_dispatch; and then not have had to worry about ugly-ing up the source with .load/.store accessors to change all of the accesses back to relaxed. Is a change like this something that might get us in trouble somehow with the c++ standard, or cause us ABI problems vs 4.7 if we changed this in 4.8? Or is the ABI thing a non-issue since we expose the entire template to the user? </bkoz> r~
