From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44793) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1hFF-0005kn-HS for qemu-devel@nongnu.org; Fri, 10 Jan 2014 13:55:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W1hF8-0004hF-FP for qemu-devel@nongnu.org; Fri, 10 Jan 2014 13:55:49 -0500 Received: from mail-qe0-x22a.google.com ([2607:f8b0:400d:c02::22a]:59267) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1hF8-0004h9-6w for qemu-devel@nongnu.org; Fri, 10 Jan 2014 13:55:42 -0500 Received: by mail-qe0-f42.google.com with SMTP id b4so4943740qen.29 for ; Fri, 10 Jan 2014 10:55:41 -0800 (PST) Sender: Richard Henderson Message-ID: <52D04229.9090003@twiddle.net> Date: Fri, 10 Jan 2014 10:55:37 -0800 From: Richard Henderson MIME-Version: 1.0 References: <1389373972-27686-1-git-send-email-peter.maydell@linaro.org> <1389373972-27686-4-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1389373972-27686-4-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/10] target-arm: A64: Add decode skeleton for SIMD data processing insns List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: Laurent Desnogues , patches@linaro.org, Michael Matz , Alexander Graf , Claudio Fontana , Dirk Mueller , Will Newton , =?UTF-8?B?QWxleCBCZW5uw6ll?= , kvmarm@lists.cs.columbia.edu, Christoffer Dall On 01/10/2014 09:12 AM, Peter Maydell wrote: > +static inline AArch64DecodeFn *lookup_disas_fn(AArch64DecodeTable *table, > + uint32_t insn) Better make table const. > +static AArch64DecodeTable data_proc_simd[] = { So that you can make this const. > +/* C3.6.1 EXT > + * 31 30 29 24 23 22 21 20 16 15 14 11 10 9 5 4 0 > + * +---+---+-------------+-----+---+------+---+------+---+------+------+ > + * | 0 | Q | 0 0 1 1 1 0 | op2 | 0 | Rm | 0 | imm4 | 0 | Rn | Rd | > + * +---+---+-------------+-----+---+------+---+------+---+------+------+ > + */ Error... 1 > +/* C3.6.16 AdvSIMD three same > + * 31 30 29 28 24 23 22 21 20 16 15 11 10 9 5 4 0 > + * +-----+---+-----------+------+---+------+--------+---+------+------+ > + * | 0 1 | U | 1 1 1 1 0 | size | 1 | Rm | opcode | 1 | Rn | Rd | > + * +-----+---+-----------+------+---+------+--------+---+------+------+ > + */ Error. Cut and paste? > + /* pattern , mask , fn */ > + { 0x0e200400, 0x9f200400, disas_simd_three_reg_same }, ok > + { 0x0e200000, 0x9f200c00, disas_simd_three_reg_diff }, ok > + { 0x0e200800, 0x9f3e0c00, disas_simd_two_reg_misc }, ok > + { 0x0e300800, 0x9f3e0c00, disas_simd_across_lanes }, ok > + { 0x0e000400, 0x9fe08400, disas_simd_copy }, ok > + { 0x0f000000, 0x9f000400, disas_simd_indexed_vector }, ok > + /* simd_mod_imm decode is a subset of simd_shift_imm, so must precede it */ > + { 0x0f000400, 0x9ff80400, disas_simd_mod_imm }, ok > + { 0x0f000400, 0x9f800400, disas_simd_shift_imm }, ok > + { 0x0e000000, 0xbf208c00, disas_simd_tb }, ok > + { 0x0e000800, 0xbf208c00, disas_simd_zip_trn }, ok > + { 0x2e000000, 0xbf208400, disas_simd_ext }, ok > + { 0x5e200400, 0xdf200400, disas_simd_scalar_three_reg_same }, ok > + { 0x5e200000, 0xdf200c00, disas_simd_scalar_three_reg_diff }, ok > + { 0x5e200800, 0xdf3e0c00, disas_simd_scalar_two_reg_misc }, ok > + { 0x5e300800, 0xdf3e0c00, disas_simd_scalar_pairwise }, ok > + { 0x5e000400, 0xdfe08400, disas_simd_scalar_copy }, ok > + { 0x5f000000, 0xdf000400, disas_simd_scalar_indexed }, ok > + { 0x5f000400, 0xdf800400, disas_simd_scalar_shift_imm }, ok > + { 0x4e280800, 0xff3e0c00, disas_crypto_aes }, ok > + { 0x5e000000, 0xff208c00, disas_crypto_three_reg_sha }, ok > + { 0x5e280800, 0xff3e0c00, disas_crypto_two_reg_sha }, ok > + { 0x00000000, 0x00000000, NULL } The errors in the comments above are not present in this table. I've verified the pattern and mask entries, but not the ordering requirements. > + (fn) (s, insn); Surely coding style sez fn(s, insn); or (*fn)(s, insn); Otherwise, Reviewed-by: Richard Henderson r~