On 6 July 2011 11:11, Xinyu Qi <x...@marvell.com> wrote:
> Hi,
>
> It is the second part of iWMMXt maintenance.
>
> *config/arm/mmintrin.h:
> Revise the iWMMXt intrinsics head file. Fix some intrinsics and add some new
> intrinsics
Is there a document somewhere that lists these intrinsics and what
each of these are supposed to be doing ? Missing details again . We
seem to be changing quite a few things.
> +
> +/* We will treat __int64 as a long long type
> + and __m64 as an unsigned long long type to conform to VSC++. */Is
> +typedef unsigned long long __m64;
> +typedef long long __int64;
Interesting this sort of a change with these cases where you are
changing the type to conform to VSC++ ? This just means old code that
uses this is pretty much broken. Not that I have much hope of that
happening by default - -flax-conversions appears to be needed even
with a trunk compiler.
> @@ -54,7 +63,7 @@ _mm_cvtsi64_si32 (__int64 __i)
> static __inline __int64
> _mm_cvtsi32_si64 (int __i)
> {
> - return __i;
> + return (__i & 0xffffffff);
> }
Eh ? why the & 0xffffffff before promotion rules. Is this set of
intrinsics documented some place ? What is missing and could be the
subject of a follow-up patch is a set of tests for the wMMX intrinsics
....
What's the behaviour of wandn supposed to be ? Does wandn x, y, z
imply x = y & ~z or x = ~y & z ? If the former then your intrinsic
expansion is wrong unless the meaning of this has changed ? Whats the
behaviour of the intrinsic __mm_and_not_si64 . ?
@@ -985,44 +1004,83 @@ _mm_setzero_si64 (void)
static __inline void
_mm_setwcx (const int __value, const int __regno)
{
> + /*Since gcc has the imformation of all wcgr regs
> + in arm backend, use builtin to access them instead
> + of throw asm directly. Thus, gcc could do some
> + optimization on them. */
> +
Also this comment is contradictory to what follows in the patch .
You've prima-facie replaced them with bits of inline assembler. I'm
not sure this comment makes a lot of sense on its own.
Ramana
