From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x530.google.com (mail-pg1-x530.google.com [IPv6:2607:f8b0:4864:20::530]) by sourceware.org (Postfix) with ESMTPS id 800913858D32 for ; Thu, 7 Jul 2022 16:15:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 800913858D32 Received: by mail-pg1-x530.google.com with SMTP id g4so19407262pgc.1 for ; Thu, 07 Jul 2022 09:15:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=BqmTtvkaiikzwLHlzmhLHdW4mLeq9DA2VWhkeZ1Fg4M=; b=VE1WNVe+A0wkfk7zd31v9YfwIHrouxtivZoLb6xXj7naKAN5rBjjOwS808F/Rea2Z1 xxQ8114KnL4uJSPppt3ZrrGjjPA7mOWFm+tAvDiqUFIUe1RvysA6h8Ttqvn2gjaOBlbG 08IbK4SkrOqmQV47xBzMwZkx7/24JrELinw58pWeK888GsD0J21BBNnsEehE+j9j/zaD sGMFIdEQmZYnCW9CiZpjgzVt8mhGi7sp6m54XmhSThy5FZEhtiIzAJMK6o7n2uiKtB1N 6i4Lwty4i3PKDl6KDRlmE8Z5jxQgoQ6/hEnWx9xg3znhrqA00a1z+ePZguUwPAkd5+8a GCSw== X-Gm-Message-State: AJIora8FEdU4Or809I+ond/W6YIpEPKqAtgalBGA9nzhuWV16g8Ceifr WrVAuKEcQYP2RIYuwo5r2EwMAF0SEqzMr6OgCRtk29nR X-Google-Smtp-Source: AGRyM1vW1jEnBNDLLMOncf2J+E8R5o1LAGH4jhwle8i0se9gq6qDVsL4c3MUR8gnRvR62E+NtxWZfU5e7u148LTwGZg= X-Received: by 2002:a17:90b:1e0e:b0:1ef:97f9:dfb5 with SMTP id pg14-20020a17090b1e0e00b001ef97f9dfb5mr6086631pjb.217.1657210534445; Thu, 07 Jul 2022 09:15:34 -0700 (PDT) MIME-Version: 1.0 References: <135c1ad8-9ad6-196e-4b5b-0cf8d89ea6d8@suse.com> In-Reply-To: From: "H.J. Lu" Date: Thu, 7 Jul 2022 09:14:58 -0700 Message-ID: Subject: Re: [PATCH] x86: re-order insn template fields To: Jan Beulich Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3018.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jul 2022 16:15:37 -0000 On Wed, Jul 6, 2022 at 11:47 PM Jan Beulich wrote: > > On 06.07.2022 19:10, H.J. Lu wrote: > > On Wed, Jul 6, 2022 at 6:43 AM Jan Beulich wrote: > >> > >> This saves quite a number of shift instructions: The "operands" field > >> can now be retrieved by just masking (no shift), and extracting the > >> "extension_opcode" field now only requires a (signed) right shift, with > >> no prereq left one. (Of course there may be architectures where, in a > >> cross build, there might be no difference at all, e.g. when there are > >> suitable bitfield extraction insns.) > >> > >> --- a/opcodes/i386-gen.c > >> +++ b/opcodes/i386-gen.c > >> @@ -1444,8 +1444,8 @@ output_i386_opcode (FILE *table, const c > >> fail (_("%s:%d: %s: residual opcode (0x%0*llx) too large\n"), > >> filename, lineno, name, 2 * length, opcode); > >> > >> - fprintf (table, " { \"%s\", 0x%0*llx%s, %s, %lu,\n", > >> - name, 2 * (int)length, opcode, end, extension_opcode, i); > >> + fprintf (table, " { \"%s\", 0x%0*llx%s, %lu, %s,\n", > >> + name, 2 * (int)length, opcode, end, i, extension_opcode); > >> > >> process_i386_opcode_modifier (table, opcode_modifier, space, prefix, > >> operand_types, lineno); > >> --- a/opcodes/i386-opc.h > >> +++ b/opcodes/i386-opc.h > >> @@ -929,6 +929,12 @@ typedef struct insn_template > >> from all other values above. */ > >> #define Opcode_VexW 0xf /* Operand order controlled by VEX.W. */ > >> > >> + /* how many operands */ > >> + unsigned int operands:3; > >> + > >> + /* spare bits */ > >> + unsigned int :4; > >> + > > > > These fields need some comments to explain why they are done > > this way. > > Would you mind explaining yourself a little further. "operands", after > all, ... > > >> /* (Fake) base opcode value for pseudo prefixes. */ > >> #define PSEUDO_PREFIX 0 > >> > >> @@ -952,9 +958,6 @@ typedef struct insn_template > >> #define Prefix_REX 8 /* {rex} */ > >> #define Prefix_NoOptimize 9 /* {nooptimize} */ > >> > >> - /* how many operands */ > >> - unsigned int operands:3; > >> - > > ... has been exactly the same way before, just in a different position. > IOW I guess I don't really understand what exactly you want to see > described in the comment(s) (which includes it not being clear to me > whether you want each field's comment extended, just one of them, or > a wider comment on the overall bitfield uses here). Please add some comments to describe the motivation of this change. Your commit log is nice. Thanks. -- H.J.