From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1749 invoked by alias); 26 Sep 2011 03:18:19 -0000 Received: (qmail 1738 invoked by uid 22791); 26 Sep 2011 03:18:18 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,TW_TW,TW_WC,TW_XF X-Spam-Check-By: sourceware.org Received: from na3sys009aog117.obsmtp.com (HELO na3sys009aog117.obsmtp.com) (74.125.149.242) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 26 Sep 2011 03:18:02 +0000 Received: from sc-owa02.marvell.com ([65.219.4.130]) (using TLSv1) by na3sys009aob117.postini.com ([74.125.148.12]) with SMTP ID DSNKTn/u6K8HLN/nMLSDIgWZfPErNT4tMdjk@postini.com; Sun, 25 Sep 2011 20:18:02 PDT Received: from SC-vEXCH2.marvell.com ([10.93.76.134]) by sc-owa02.marvell.com ([10.93.76.22]) with mapi; Sun, 25 Sep 2011 20:15:05 -0700 From: Xinyu Qi To: Ramana Radhakrishnan CC: "gcc-patches@gcc.gnu.org" Date: Mon, 26 Sep 2011 04:31:00 -0000 Subject: RE: PING: [PATCH, ARM, iWMMXt][2/5]: intrinsic head file change Message-ID: <4737A960563B524DA805CA602BE04B30602611FB84@SC-VEXCH2.marvell.com> References: <4737A960563B524DA805CA602BE04B306010D8E699@SC-VEXCH2.marvell.com> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-09/txt/msg01550.txt.bz2 Ping http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01963.html * config/arm/mmintrin.h: Revise. At 2011-08-24 16:14:30,"Xinyu Qi" wrote: > At 2011-08-18 09:33:27,"Ramana Radhakrishnan" > wrote: > > On 6 July 2011 11:11, Xinyu Qi wrote: > > > Hi, > > > > > > It is the second part of iWMMXt maintenance. > > > > > > *config/arm/mmintrin.h: > > > =A0Revise the iWMMXt intrinsics head file. Fix some intrinsics and ad= d 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. >=20 > Hi, > The intrinsic_doc.txt is attached. It is the piece of iWMMXt intrinsic de= tails > doc picked out from "Intel Wireless MMX Technology Intrinsic Support" wit= h some > modification. >=20 > > > + > > > +/* 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. >=20 > I couldn't find any material to show why __int64 needs to be redefined. A= nd > all the tests are passed without this change. So decide to discard this c= hange. >=20 > > > > > @@ -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 > > .... >=20 > See the intrinsics doc. It says the description of _mm_cvtsi32_si64 is "T= he > integer value is zero-extended to 64 bits. > If r =3D _mm_cvtsi32_si64(i), then the action is > r [0:31] =3D i; > r[32:63] =3D 0;" >=20 > > > > What's the behaviour of wandn supposed to be ? Does wandn x, y, z > > imply x =3D y & ~z or x =3D ~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 . ? >=20 > The description of _mm_andnot_si64 is "Performs a logical NOT on the 64-b= it > value in m1 and use the result in a bitwise AND with the 64-bit value in = m2." > And, "wandn wRd, wRn, wRm" means "wRd =3D wRn & ~wRm" > I think __builtin_arm_wandn had better directly match the behavior of wan= dn. > Therefore, match _mm_andnot_si64 (m1, m2) to __builtin_arm_wandn (m2, m1). >=20 >=20 >=20 > > @@ -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. >=20 > Sorry. This comment should be removed. >=20 > The modified diff is attached. >=20 > Thanks, > Xinyu >=20