Re: [ros-dev] [ros-diffs] [tfaber] 62266: [CRT] - Force the use of memory operands in bit test intrinsics. Bit offsets above 31 (or 63) can't behave correctly with registers (the constant case is fine because it ensures low...

[Timo Kreuzer]

While the bug was definately on our side, I still think there is a
GCC/CLANG bug here as well. Memory operands larger than register size
are a rare case (probably limited to the bittest instructions) but
allowing a register operand here should cause the compiler to either
error out or in the worst case emit something like "mov eax,
[BaseAddress]; bt eax, cl;" or "mov eax, [PointerToBaseAddress]; mov
eax, [eax]; bt eax, cl;" rather than "bt [PointerToBaseAddress], cl;",
which it actually did. Changing the constraint to something that wasn't
working changed the behaviour in a way that changed the level of
dereference! The operation was done on the pointer instead of on the
memory pointed to by the pointer!
Maybe we should file a bugreport.

Am 20.02.2014 20:28, schrieb tfaber(a)svn.reactos.org:
>   Author: tfaber
>  Date: Thu Feb 20 19:28:27 2014
>  New Revision: 62266
>
>  URL: http://svn.reactos.org/svn/reactos?rev=62266&view=rev
>  Log:
>  [CRT]
>  - Force the use of memory operands in bit test intrinsics. Bit offsets above 31 (or 63)
> can't behave correctly with registers (the constant case is fine because it ensures
> low offsets). Thanks to Timo Kreuzer and Alex Radocea.
>
>  Modified:
>      trunk/reactos/include/crt/mingw32/intrin_x86.h
>
>  Modified: trunk/reactos/include/crt/mingw32/intrin_x86.h
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/include/crt/mingw32/intrin…
>  ==============================================================================
>  --- trunk/reactos/include/crt/mingw32/intrin_x86.h	[iso-8859-1] (original)
>  +++ trunk/reactos/include/crt/mingw32/intrin_x86.h	[iso-8859-1] Thu Feb 20 19:28:27 2014
>  @@ -1032,7 +1032,7 @@
>   	if(__builtin_constant_p(b))
>   		__asm__("bt %[b], %[a]; setb %b[retval]" : [retval] "=q" (retval)
> : [a] "mr" (*(a + (b / 32))), [b] "Ir" (b % 32));
>   	else
>  -		__asm__("bt %[b], %[a]; setb %b[retval]" : [retval] "=q" (retval)
> : [a] "mr" (*a), [b] "r" (b));
>  +		__asm__("bt %[b], %[a]; setb %b[retval]" : [retval] "=q" (retval)
> : [a] "m" (*a), [b] "r" (b));
>   
>   	return retval;
>   }
>  @@ -1045,7 +1045,7 @@
>   	if(__builtin_constant_p(b))
>   		__asm__("bt %[b], %[a]; setb %b[retval]" : [retval] "=q" (retval)
> : [a] "mr" (*(a + (b / 64))), [b] "Ir" (b % 64));
>   	else
>  -		__asm__("bt %[b], %[a]; setb %b[retval]" : [retval] "=q" (retval)
> : [a] "mr" (*a), [b] "r" (b));
>  +		__asm__("bt %[b], %[a]; setb %b[retval]" : [retval] "=q" (retval)
> : [a] "m" (*a), [b] "r" (b));
>   
>   	return retval;
>   }
>  @@ -1058,7 +1058,7 @@
>   	if(__builtin_constant_p(b))
>   		__asm__("btc %[b], %[a]; setb %b[retval]" : [a] "+mr" (*(a + (b /
> 32))), [retval] "=q" (retval) : [b] "Ir" (b % 32));
>   	else
>  -		__asm__("btc %[b], %[a]; setb %b[retval]" : [a] "+mr" (*a),
> [retval] "=q" (retval) : [b] "r" (b));
>  +		__asm__("btc %[b], %[a]; setb %b[retval]" : [a] "+m" (*a),
> [retval] "=q" (retval) : [b] "r" (b));
>   
>   	return retval;
>   }
>  @@ -1070,7 +1070,7 @@
>   	if(__builtin_constant_p(b))
>   		__asm__("btr %[b], %[a]; setb %b[retval]" : [a] "+mr" (*(a + (b /
> 32))), [retval] "=q" (retval) : [b] "Ir" (b % 32));
>   	else
>  -		__asm__("btr %[b], %[a]; setb %b[retval]" : [a] "+mr" (*a),
> [retval] "=q" (retval) : [b] "r" (b));
>  +		__asm__("btr %[b], %[a]; setb %b[retval]" : [a] "+m" (*a),
> [retval] "=q" (retval) : [b] "r" (b));
>   
>   	return retval;
>   }
>  @@ -1082,7 +1082,7 @@
>   	if(__builtin_constant_p(b))
>   		__asm__("bts %[b], %[a]; setb %b[retval]" : [a] "+mr" (*(a + (b /
> 32))), [retval] "=q" (retval) : [b] "Ir" (b % 32));
>   	else
>  -		__asm__("bts %[b], %[a]; setb %b[retval]" : [a] "+mr" (*a),
> [retval] "=q" (retval) : [b] "r" (b));
>  +		__asm__("bts %[b], %[a]; setb %b[retval]" : [a] "+m" (*a),
> [retval] "=q" (retval) : [b] "r" (b));
>   
>   	return retval;
>   }
>
>
>