On 25/11/14 21:04, Matt Turner wrote:
On Tue, Nov 25, 2014 at 6:48 AM, Jose Fonseca <[email protected]> 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
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?
The `void` return type is the only difference between
p_atomic_{inc,dec}_return and p_atomic_{inc,dec}, even for
PIPE_ATOMIC_GCC_INTRINSIC case. In fact we should probably drop the
the void case completely, since it seems a pointless special case.
If it is the parenthesis around the (void) cast you're worried about, it
is fine. See e.g., the assert macro on Linux:
$ cat /usr/include/assert.h
...
#if defined __cplusplus && __GNUC_PREREQ (2,95)
# define __ASSERT_VOID_CAST static_cast<void>
#else
# define __ASSERT_VOID_CAST (void)
#endif
...
# define assert(expr) (__ASSERT_VOID_CAST (0))
Jose
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
