On 02.12.2009 18:33, José Fonseca wrote:
> I've seen strict aliasing assumption causing bugs in other gallium
> components. It seems endemic to our code. Unless we actively decidee to
> go and chase the strict aliasing bugs now we should add
> -fno-strict-aliasing to all our builds.
Hmm, actually some of them (in mesa at least) seem to be really
unnecessary. Take the COPY_4FV macro for instance. I replaced that in a
simple test program (attached) to either just do direct assignment
without cast, or use memcpy instead.
The results are actually interesting, the comment says cast is done to
avoid going through fp registers, but looking in the assembly (at least
with optimization) that doesn't happen anyway, and the generated code is
actually nearly identical, but in fact it not only triggers
strict-aliasing warnings but doesn't work correctly (when compiled with
-O3 or similar parameters invoking -fstrict-aliasing).
assign_cast:
.LFB45:
.cfi_startproc
movl (%rsi), %edx
leaq 4(%rdi), %rax
movl %edx, 4(%rdi)
movl 4(%rsi), %edx
movl %edx, 4(%rax)
movl 8(%rsi), %edx
movl %edx, 8(%rax)
movl 12(%rsi), %edx
movl %edx, 12(%rax)
ret
.cfi_endproc
assign:
.LFB46:
.cfi_startproc
movl (%rsi), %eax
movl %eax, 4(%rdi)
movl 4(%rsi), %eax
movl %eax, 8(%rdi)
movl 8(%rsi), %eax
movl %eax, 12(%rdi)
movl 12(%rsi), %eax
movl %eax, 16(%rdi)
ret
.cfi_endproc
But clearly using memcpy the compiler does a better job:
assign_cpy:
.LFB44:
.cfi_startproc
movq (%rsi), %rax
movq %rax, 4(%rdi)
movq 8(%rsi), %rax
movq %rax, 12(%rdi)
ret
.cfi_endproc
.LFE44:
Doesn't use 128bit sse moves but looks like an improvement... When using
no optimization code certainly gets much less readable and the memcpy
version will call glibc memcpy (which itself will still be optimized
hence probably faster despite the function call).
So I'll kill at least this one and just use _mesa_memcpy there, unless
there are good reasons not to. I think pretty much all compilers should
have builtin memcpy optimizations.
Roland
#include <string.h>
#include <stdio.h>
#define COPY_4FV( DST, SRC ) \
do { \
const unsigned *_s = (const unsigned *) (SRC); \
unsigned *_d = (unsigned *) (DST); \
_d[0] = _s[0]; \
_d[1] = _s[1]; \
_d[2] = _s[2]; \
_d[3] = _s[3]; \
} while (0)
#define COPY_4FV_NOCAST( DST, SRC ) \
do { \
(DST)[0] = (SRC)[0]; \
(DST)[1] = (SRC)[1]; \
(DST)[2] = (SRC)[2]; \
(DST)[3] = (SRC)[3]; \
} while (0)
#define COPY_4FV_MEMCPY( DST, SRC ) \
do { \
memcpy(DST, SRC, sizeof(float) * 4); \
} while (0)
struct sfloat
{
unsigned unused;
float p[4];
};
void assign_cpy(struct sfloat *s, float *param)
{
COPY_4FV_MEMCPY(s->p, param);
}
void assign_cast(struct sfloat *s, float *param)
{
COPY_4FV(s->p, param);
}
void assign(struct sfloat *s, float *param)
{
COPY_4FV_NOCAST(s->p, param);
}
int main(void)
{
float fl[4] = {0.1,0.2,0.3,0.4};
struct sfloat s1;
struct sfloat s2;
struct sfloat s3;
assign(&s1, fl);
fprintf(stderr, "assigned values are %f %f %f %f\n", s1.p[0], s1.p[1], s1.p[2], s1.p[3]);
assign_cpy(&s2, fl);
fprintf(stderr, "assigned values are %f %f %f %f\n", s2.p[0], s2.p[1], s2.p[2], s2.p[3]);
assign_cast(&s3, fl);
fprintf(stderr, "assigned values are %f %f %f %f\n", s3.p[0], s3.p[1], s3.p[2], s3.p[3]);
return 0;
}------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing.
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
