Re: COPY_TWO_BYTES and C strict aliasing

Branko Čibej <brane_at_apache.org> writes:

> On 01.06.2012 14:22, Philip Martin wrote:
>> GCC gives a compiler warning where the COPY_TWO_BYTES macro is used. A
>> typical warning is:
>>
>> ../src/subversion/libsvn_subr/string.c:971:11: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
>> COPY_TWO_BYTES(dest, decimal_table[(apr_size_t)number]);
>> ^
> [...]
>
>
>> Is the warning something we can ignore? My understanding of C aliasing
>> is that writing to memory is only supposed to happen as the declared
>> type of that memory.
>>
>> Is COPY_TWO_BYTES a significant optimisation? On Linux we can avoid the
>> warning by simply using
>>
>> memcpy(dest, source, 2)
>>
>> since that memcpy call will be inlined these days.
>>
>
> I think it's OK if that macro becomes an alias for memcpy. On modern
> systems I'd even expect it to be faster than the current code, since a
> properly inlined memcpy will know when to pick word-size over byte-size
> operations, since it'll know the alignment of the source and target
> (which this macro doesn't).

With Linux/gcc using memcpy causes the generated assembler to be ordered
slightly differently but it appears to be the same instructions (to my
inexpert eye):

10,19c10,19
< jbe .L281
< .loc 1 971 0
< leaq decimal_table(%rip), %rax
< movzwl (%rax,%rsi,4), %eax
< .loc 1 972 0
< movb $0, 2(%rdi)
< .loc 1 971 0
< movw %ax, (%rdi)
< .loc 1 973 0
< movl $2, %eax

---
> 	ja	.L271
> 	.loc 1 965 0
> 	addl	$48, %esi
> 	.loc 1 966 0
> 	movb	$0, 1(%rdi)
> 	.loc 1 967 0
> 	movl	$1, %eax
> 	.loc 1 965 0
> 	movb	%sil, (%rdi)
> 	.loc 1 1019 0
183,192c183,192
< .L281:
< 	.loc 1 965 0
< 	addl	$48, %esi
< 	.loc 1 966 0
< 	movb	$0, 1(%rdi)
< 	.loc 1 967 0
< 	movl	$1, %eax
< 	.loc 1 965 0
< 	movb	%sil, (%rdi)
< 	.loc 1 1019 0
---
> .L271:
> 	.loc 1 971 0
> 	leaq	decimal_table(%rip), %rax
> 	.loc 1 972 0
> 	movb	$0, 2(%rdi)
> 	.loc 1 971 0
> 	movzwl	(%rax,%rsi,4), %eax
> 	movw	%ax, (%rdi)
> 	.loc 1 973 0
> 	movl	$2, %eax
That's replacing
  *(apr_uint16_t*)(dest) = *(apr_uint16_t*)(source)
with
  memcpy((dest), (source), 2)
-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com