On 05/20/2013 03:57 AM, dw wrote:
> There is a bug in the InterlockedOr function (as well as InterlockedAnd,
> InterlockedXor, InterlockedOr64, InterlockedAnd64, InterlockedXor64)
> where they do not return the "old" value as expected, but instead return
> (sort of) the new value. This code illustrates the problem:
>
> #include <stdio.h>
> #include <windows.h>
>
> #ifndef _AMD64_
> #error Must be compiled for x64
> #endif
>
> int main()
> {
> volatile LONG x = 0;
> LONG y = InterlockedXor(&x, 3);
>
> printf("old: %u new: %u\n", y, x); // Should print 0 3, prints 3 3
> }
>
> The attached patch resolves this issue.
>
> In summary, this patch:
>
> 1) Correctly returns the "old" value.
> 2) Uses builtin function instead of inline asm.
I think your implementation is less efficient than it could be. I think
it would be more efficient if you used GCC's __sync_fetch_and_OP()
built-ins instead of hand crafted compare exchange loop
(http://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/_005f_005fsync-Builtins.html#_005f_005fsync-Builtins).
Even if you decide to go with the hand crafted compare exchange loop, it
can still be made more efficient, I believe:
+#define __buildlogicali(x, y, o) __CRT_INLINE y x(volatile y
*Destination, y Value) \
+{ \
+ y lPrev; \
+ do { \
+ lPrev = *Destination; \
+ } while (__sync_val_compare_and_swap(Destination, lPrev, lPrev o
Value) != lPrev); \
+ return lPrev; \
+}
Here you are reading the *Destination memory each loop iteration once
more than should be necessary. You could leverage the
__sync_val_compare_and_swap() built-in and use its return value.
Something like this should be better:
#define __mybuildlogicali(x, y, o) \
__CRT_INLINE y x(volatile y *Destination, y Value) \
{ \
y lPrev = *Destination; \
y lTmp; \
while ((lTmp = __sync_val_compare_and_swap(Destination, \
lPrev, lPrev o Value)) != lPrev) \
lPrev = lTmp; \
return lPrev; \
}
--
VZ
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Try New Relic Now & We'll Send You this Cool Shirt New Relic is the only SaaS-based application performance monitoring service that delivers powerful full stack analytics. Optimize and monitor your browser, app, & servers with just a few lines of code. Try New Relic and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
_______________________________________________ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
