From: Xinyu Qi <xyqi@marvell.com>
To: Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: PING: [PATCH, ARM, iWMMXt][2/5]: intrinsic head file change
Date: Mon, 26 Sep 2011 04:31:00 -0000 [thread overview]
Message-ID: <4737A960563B524DA805CA602BE04B30602611FB84@SC-VEXCH2.marvell.com> (raw)
In-Reply-To: <CACUk7=W8Hg14jw+dPQ+nodOeWiX-v=h9oG8uVXhb5hU4MZ3wNg@mail.gmail.com>
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" <xyqi@marvell.com> wrote:
> At 2011-08-18 09:33:27,"Ramana Radhakrishnan"
> <ramana.radhakrishnan@linaro.org> wrote:
> > On 6 July 2011 11:11, Xinyu Qi <xyqi@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.
>
> Hi,
> The intrinsic_doc.txt is attached. It is the piece of iWMMXt intrinsic details
> doc picked out from "Intel Wireless MMX Technology Intrinsic Support" with some
> modification.
>
> > > +
> > > +/* 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.
>
> I couldn't find any material to show why __int64 needs to be redefined. And
> all the tests are passed without this change. So decide to discard this change.
>
> >
> > > @@ -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
> > ....
>
> See the intrinsics doc. It says the description of _mm_cvtsi32_si64 is "The
> integer value is zero-extended to 64 bits.
> If r = _mm_cvtsi32_si64(i), then the action is
> r [0:31] = i;
> r[32:63] = 0;"
>
> >
> > 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 . ?
>
> The description of _mm_andnot_si64 is "Performs a logical NOT on the 64-bit
> 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 = wRn & ~wRm"
> I think __builtin_arm_wandn had better directly match the behavior of wandn.
> Therefore, match _mm_andnot_si64 (m1, m2) to __builtin_arm_wandn (m2, m1).
>
>
>
> > @@ -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.
>
> Sorry. This comment should be removed.
>
> The modified diff is attached.
>
> Thanks,
> Xinyu
>
next prev parent reply other threads:[~2011-09-26 3:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-06 10:15 Xinyu Qi
2011-08-18 2:35 ` Ramana Radhakrishnan
2011-08-24 9:07 ` Xinyu Qi
2011-09-26 4:31 ` Xinyu Qi [this message]
2011-10-20 8:05 ` PING: " Xinyu Qi
2011-12-29 6:26 ` Xinyu Qi
2012-02-03 2:08 ` Xinyu Qi
2012-03-13 8:55 ` Xinyu Qi
2011-07-29 4:04 Xinyu Qi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4737A960563B524DA805CA602BE04B30602611FB84@SC-VEXCH2.marvell.com \
--to=xyqi@marvell.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ramana.radhakrishnan@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).