On Tue, Nov 25, 2014 at 1:42 PM, Jose Fonseca <jfons...@vmware.com> wrote: > On 25/11/14 21:04, Matt Turner wrote: >> >> On Tue, Nov 25, 2014 at 6:48 AM, Jose Fonseca <jfons...@vmware.com> wrote: >>> >>> On 25/11/14 00:39, Matt Turner wrote: >>>> >>>> >>>> I've got some thread-safety fixes queued up after this and thought I'd >>>> be a good Mesa citizen and pull some code into src/util. >>> >>> >>> >>> Thanks for going the extra mile! >>> >>>> I did some clean ups like replacing INLINE (MSVC knows about "inline" >>>> these days, right?) and used stdbool.h instead of the "boolean" type. >>> >>> >>> >>> No, at least MSVC 2012 doesn't have `inline` keyword when compiling C >>> files, >>> and requires a >>> >>> #if !defined(__cplusplus) && !defined(inline) >>> #define inline __inline >>> #endif >>> >>> somewhere. Anyway, there are no `INLINES` nor `inlines` left after your >>> series, so we're good. >>> >>> stdbool.h is fine -- we include our own when MSVC doesn't >>> >>>> I also removed the inline assembly implementations because they were >>>> either dead code, or only allowed *ancient* gcc to build Mesa and >>>> because I didn't want to update them for the next patch, which makes >>>> the API consist of some macros that internally do the right operation >>>> based on the type. >>>> >>>> The last patch looks funky, but I think it's actually a reasonable >>>> solution. I don't have MSVC or Sun Studio, so please give this a >>>> test. >>> >>> >>> >>> I had to do a few tweaks to get things building on MSVC properly. >>> >>> I pushed my changes to >>> >>> >>> https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_-7Ejrfonseca_mesa_log_-3Fh-3Du-5Fatomic&d=AAIFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=8QJFF29dW3a3z8eyBjuTu3qPR2fyjPrA74i0KYdqcfY&s=VuQNqF0RPRZ7KG-MqTPY5S8_b_yShiJ6ddvx_9Y9LT0&e= >>> >>> I need to do a few more tests, but all looks feasible so far -- I don't >>> get >>> any warnings with MSVC and I believe that the generated code quality >>> should >>> be exactly the same. >>> >>> And it is indeed a nice cleanup. >> >> >> Excellent, thanks a lot José! >> >> I'll merge your patches into my series and wire up the test into >> automake. I'll also put parentheses around the nested ternary in the >> Sun Studio case, like you noticed was necessary for MSVC. >> >> My thread-safety fixes actually do a compare-and-swap on a bool, so I >> do need an 8-bit CAS. I found this [0] page lists >> _InterlockedCompareExchange8. I don't see a non-intrinsic version >> though. Can we use this? GCC will generate 8-bit atomic ops on 32- and >> 64-bit x86, so I don't know of a technical reason it can't work. >> >> [0] >> https://urldefense.proofpoint.com/v2/url?u=http-3A__msdn.microsoft.com_en-2Dus_library_ttk2z1ws.aspx&d=AAIFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=8QJFF29dW3a3z8eyBjuTu3qPR2fyjPrA74i0KYdqcfY&s=Qkw_NXkDNtKz2k3ULg5JWRJNL0PhrQFGAAbtcNrjz-0&e= > > > You're right. I was confused because I didn't find a corresponding > InterlockedCompareExchange8() in the Win32 API, but the underscore-prefixed > intrinsic itself exists and works fine. I've brought it back with: > > http://cgit.freedesktop.org/~jrfonseca/mesa/commit/?h=u_atomic
Awesome, thanks! >> Also, I'm a little surprised that >> >> +#define p_atomic_dec(_v) \ >> + ((void) p_atomic_dec_return(_v)) >> >> is sufficient. Are you sure? > > > Could you elaborate your concern? Oh, I'm sorry -- I totally misread. Yeah, that seems completely fine. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
