[Pixman] Basic infrastructure for MIPS32r2 and DSPr1 optimizations, and patch for mips* asm exports symbols.
Søren Sandmann
soren.sandmann at gmail.com
Sat Aug 23 11:44:31 PDT 2014
Nemanja Lukic <nemanja.lukic at rt-rk.com> writes:
> This patch set should address all previous code review remarks.
>From looking at this patch set fairly briefly,
- There is some confusion about the type of the new memcpy
function. Does it return bool or void*? And what precisely does the
return value mean? It seems to be used both as "this call succeeded"
(boolean) and "copy of the destination pointer" (void *).
In my opinion, the memcpy routines should take a
pixman_implementation_t * as the first argument, and have no return
value. This is similar to the other routines exported by
implementations.
- I'm not sure the exported pixman_memcpy() and the
s/memcpy/pixman_memcpy/g sed job make sense. The new memcpy()
infrastructure should primarily be considered a way to support the
implementations that wish to provide a faster memcpy() for the various
graphics operations, and the various ancillary uses of memcpy() don't
really need to super optimized.
Also, as mentioned in the last review:
- The blt() function in pixman-mips32r2.c is not MIPS specific. It can
live in pixman-general and rely on the normal fallback mechanism. And
instead of calling the new pixman_memcpy() it could just call
implementation->toplevel->memcpy(), which is the usual way to call a
lower-level function in pixman.
- The fill_buff pointers should be static and defined inside the C files
that use them. It's not generally a good idea to define variables in a
header file.
Søren
More information about the Pixman
mailing list