From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9363 invoked by alias); 9 Oct 2013 00:07:19 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 9353 invoked by uid 89); 9 Oct 2013 00:07:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_JMF_BL,RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: multi.imgtec.com Received: from multi.imgtec.com (HELO multi.imgtec.com) (194.200.65.239) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 09 Oct 2013 00:07:15 +0000 From: Chao-Ying Fu To: 'Richard Sandiford' CC: "binutils@sourceware.org" Subject: RE: [PATCH] MIPS SIMD Architecture (MSA) patch Date: Wed, 09 Oct 2013 00:07:00 -0000 Message-ID: <81D57523CB07B24881D63DE650C6ED82019039DC@BADAG02.ba.imgtec.org> References: <81D57523CB07B24881D63DE650C6ED82019034B4@BADAG02.ba.imgtec.org> <87siwbr2os.fsf@talisman.default> In-Reply-To: <87siwbr2os.fsf@talisman.default> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-SEF-Processed: 7_3_0_01192__2013_10_09_01_06_44 X-SW-Source: 2013-10/txt/msg00098.txt.bz2 Richard Sandiford wrote: > Thanks for the patch. Looks good at first glance, although I=20 > agree with > Maciej's comments. One thing that stood out though: >=20 > Chao-Ying Fu writes: > > + /* MSA register element $w0[0]-$w31[31]. $w is at bit 11. > > + Index is at bit 16. */ > > + OP_MSA_ELEM0_31, > > + > > + /* MSA register element $w0[0]-$w31[15]. $w is at bit 11. > > + Index is at bit 16. */ > > + OP_MSA_ELEM0_15, > > + > > + /* MSA register element $w0[0]-$w31[7]. $w is at bit 11. > > + Index is at bit 16. */ > > + OP_MSA_ELEM0_7, > > + > > + /* MSA register element $w0[0]-$w31[3]. $w is at bit 11. > > + Index is at bit 16. */ > > + OP_MSA_ELEM0_3, > > + > > + /* MSA register element $w0[0]-$w31[0]. $w is at bit=20 > 11. Index is 0. */ > > + OP_MSA_ELEM0, > > + > > + /* MSA register element $w0[$0]-$w31[$31]. $w is at bit 11. > > + Index register is at bit 16. */ > > + OP_MSA_ELEM_REG, > > + > > + /* MSA register element $w0[0]-$w31[31]. $w is at bit 6. > > + Index is at bit 16. */ > > + OP_MSA2_ELEM0_31, > > + > > + /* MSA register element $w0[0]-$w31[15]. $w is at bit 6. > > + Index is at bit 16. */ > > + OP_MSA2_ELEM0_15, > > + > > + /* MSA register element $w0[0]-$w31[7]. $w is at bit 6. > > + Index is at bit 16. */ > > + OP_MSA2_ELEM0_7, > > + > > + /* MSA register element $w0[0]-$w31[3]. $w is at bit 6. > > + Index is at bit 16. */ > > + OP_MSA2_ELEM0_3 >=20 > The idea was to try to treat individual fields as individual operand > types where possible. MDMX was an exception because the two 5-bit > fields effectively formed a single 10-bit field that specified a > full register, a broadcast element, or a constant. I.e. the fields > together could have three different forms. >=20 > Here the operand types seem to have a single form, and I'm=20 > guessing you > treated the two fields as a single operand type because OT_REG_ELEMENT > is a single token. Is that right? I think I'd prefer to split the > tokens instead. E.g. something like the attached.=20=20 > OT_REG_INDEX is unused > until your patch. Thanks a lot for your patch! However, the issue is that after matching a first register, mips_parse_argument_token() continues to match an '[' and= an index and ']'. This implies that if an instruction has a format of "xx[yy]", xx and yy wil= l be parsed together as one operand. I am not sure splitting to two tokens can help to parse xx and yy via calling mips_parse_argument_token() twice. So, for MSA instructions, I combine "xx[yy]" into one operand. Or, I may misunderstand the parsing code. > With that change, I'm hoping the registers could just be=20 > normal OP_REGs > and the indices could use just two operand types, one for=20 > constant indices > and one for register indices. The range of the constant=20 > indices would then > be obvious from their size, so we wouldn't need different OP_s for > 0..3, 0..7, etc. The "always 0" case would be a 0-sized operand. >=20 > BTW: >=20 > > - my_getExpression (&element, s); > > - if (element.X_op !=3D O_constant) > > + if (*s =3D=3D '$') > > { > > - set_insn_error (0, _("vector element must be constant")); > > - return 0; > > + token.u.reg_element.is_reg_index_p =3D TRUE; > > + if (!mips_parse_register (&s, ®no3, NULL)) > > + { > > + set_insn_error (0, _("invalid register")); > > + return 0; > > + } > > + temp_index =3D regno3; > > + } >=20 > $ can be used for general symbols too, in something like: >=20 > .equ $my_index, 1 > ...[$my_index] >=20 > which should be the same as "...[1]". The patch below tries=20 > parsing the > register first and falls back to the expression on failure. >=20 Thanks for pointing out this example! Yes, my patch fails on this exampl= e. Regards, Chao-ying