From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7002 invoked by alias); 9 Oct 2013 07:01: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 6990 invoked by uid 89); 9 Oct 2013 07:01:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-we0-f178.google.com Received: from mail-we0-f178.google.com (HELO mail-we0-f178.google.com) (74.125.82.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 09 Oct 2013 07:01:17 +0000 Received: by mail-we0-f178.google.com with SMTP id q59so378935wes.23 for ; Wed, 09 Oct 2013 00:01:14 -0700 (PDT) X-Received: by 10.180.12.14 with SMTP id u14mr1206536wib.63.1381302074385; Wed, 09 Oct 2013 00:01:14 -0700 (PDT) Received: from localhost ([2.28.235.51]) by mx.google.com with ESMTPSA id b11sm12213601wik.1.1969.12.31.16.00.00 (version=TLSv1.2 cipher=AES128-GCM-SHA256 bits=128/128); Wed, 09 Oct 2013 00:01:13 -0700 (PDT) From: Richard Sandiford To: Chao-Ying Fu Mail-Followup-To: Chao-Ying Fu ,"binutils\@sourceware.org" , rdsandiford@googlemail.com Cc: "binutils\@sourceware.org" Subject: Re: [PATCH] MIPS SIMD Architecture (MSA) patch References: <81D57523CB07B24881D63DE650C6ED82019034B4@BADAG02.ba.imgtec.org> <87siwbr2os.fsf@talisman.default> <81D57523CB07B24881D63DE650C6ED82019039DC@BADAG02.ba.imgtec.org> Date: Wed, 09 Oct 2013 07:01:00 -0000 In-Reply-To: <81D57523CB07B24881D63DE650C6ED82019039DC@BADAG02.ba.imgtec.org> (Chao-Ying Fu's message of "Wed, 9 Oct 2013 00:06:33 +0000") Message-ID: <874n8rq66j.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-10/txt/msg00104.txt.bz2 Chao-Ying Fu writes: > Richard Sandiford wrote: >> Thanks for the patch. Looks good at first glance, although I >> agree with >> Maciej's comments. One thing that stood out though: >> >> 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 >> 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 >> >> 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. >> >> Here the operand types seem to have a single form, and I'm >> 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. >> 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 will 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. The flow is that mips_parse_arguments is first called to parse all the arguments in one go. It converts them to an array of mips_operand_tokens. This array is then used instead of string parsing when matching the operands. (The parsing only happens once per .s instruction regardless of how many opcode alternatives we have to match against. Nothing other than mips_parse_arguments calls mips_parse_argument_token.) At the moment "xx[yy]" is one token, but my patch split it into two, a normal OT_REG for the "xx" followed by an OT_INTEGER_INDEX or OT_REG_INDEX for the "[yy]". So the array will contain two tokens that can be individually matched as two operands. "xx[yy]" could then be represented as "+a+b" in the opcode/*.c table (pretending that "a" and "b" are free). "+a" would be a normal OP_REG. "+b" would be a new OP_*. And the match_operand routine for that new OP_* would just check whether the next token in the array is a OT_INTEGER_INDEX or OT_REG_INDEX. Thanks, Richard