From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2072.outbound.protection.outlook.com [40.107.21.72]) by sourceware.org (Postfix) with ESMTPS id D804D38618D2 for ; Wed, 27 Sep 2023 14:45:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D804D38618D2 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AcENasJdBkmac/3+EiXa24nDI4QB6+6JHtAgi6KEU+5CgDYGMOuMQ0eAkGD0ylNM4aYIb9aMHXG/zNpgGr2uu9IPmtYWipNScdqrQvqUBh0khla4FtBUhKP39K7L0MVCnBeMIgPRErNmFBFmU9EDZkyieCXQBgeRoNzVoFy51kZrqYePYAyNG0NCXkY/cyZKP0iPpJ/3/apGgli9UT+Ox64R1ZVaKg0KAxmEATlNewjuialCqNfSP1+RqNgLeHKkRE/SjXPDJPZ1F3f+1SbRa8YiaAaX/IGecS1xjr8Q0cU0YJtMGSBXVkZuTRnbpM7m9G3OmrMXRKrcIChfvuN/0w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=gGDIT1HUYUbHNif9yEjajica79A7hNFuCjpLvquVxQw=; b=Xnu4t8gi9ouCKRu32ZlEoZwviZl9dMZPaBYocT4f0GKcKJOC5CMBmVVx/s7VCu3Cb9x7nqGezAJUkMk+hH5ssnKbcDq8ttYaaLytJfDuiStw58Us6tqncQ17+JG4Qd2l9Enh22bXAfUnCOfYu+PEk1SMo3w9lUCQpAzr6VmPBGoNnYaJ/+4HbdjVjDiaaFJESGqVCOu5mTIEEsb6S4NVrDRqkuy8QhQsVDZ2tr1oeguvzUeBqYQiXYuspYElTB5GzJqqKTe6mkV0PuxSoFaoGpN+caAUAw5LhoPheA/Q0sXm4P445X5pO3d9C4hyWfk3YVVWvfaDn1QR72vnnScBfQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=gGDIT1HUYUbHNif9yEjajica79A7hNFuCjpLvquVxQw=; b=jNVtT4YAZJ4aZPqX3x5IlVSgZvFCoEhELoATnE/ma0HvtI86OHpzaG2iIBRD8sEd6H2EF3EZOnkT+bdzv2aM9CXlNaEqmrTIyqOaJDD9tKC+0cfoSHX+6cbIH9DMIKGi7hHHVNv3/2sLIAYr68fjL5eubn75AD1gBnogC4rAhbAaJXRMUmBJNtAizIT2PWP1gFPwjw4328ZC/pIML5UD8f0HS64M1RRM8BcPyzW4qtuq3m6u3SOqiO3P7FrOlRvw3qkMxL51YFeP41nyciWqQz7UHxvTIgdRrNiFJC+i2ZdkvSw6eYtNUazQpRsSVm+0SdbqkssL/mr8TIOPrwQ8bA== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from AS8PR04MB8788.eurprd04.prod.outlook.com (2603:10a6:20b:42f::21) by DU0PR04MB9249.eurprd04.prod.outlook.com (2603:10a6:10:350::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6813.28; Wed, 27 Sep 2023 14:44:56 +0000 Received: from AS8PR04MB8788.eurprd04.prod.outlook.com ([fe80::afae:3fda:c84d:bcdc]) by AS8PR04MB8788.eurprd04.prod.outlook.com ([fe80::afae:3fda:c84d:bcdc%7]) with mapi id 15.20.6838.016; Wed, 27 Sep 2023 14:44:56 +0000 Message-ID: <9d317289-6d83-3f9b-ef34-af574e798a3f@suse.com> Date: Wed, 27 Sep 2023 16:44:54 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH 4/8] Support APX NDD Content-Language: en-US To: "Cui, Lili" Cc: hongjiu.lu@intel.com, konglin1 , binutils@sourceware.org References: <20230919152527.497773-1-lili.cui@intel.com> <20230919152527.497773-5-lili.cui@intel.com> From: Jan Beulich In-Reply-To: <20230919152527.497773-5-lili.cui@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR0P281CA0171.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:b4::12) To AS8PR04MB8788.eurprd04.prod.outlook.com (2603:10a6:20b:42f::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8PR04MB8788:EE_|DU0PR04MB9249:EE_ X-MS-Office365-Filtering-Correlation-Id: dad194e8-8585-4209-725c-08dbbf685130 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: TgsMpEQ0Zrb0NBa7DfAiEHZWAikCbalCrX3VN4P3yWYMJCR0VeFl3vq5/NQh4VdZSjna1zcO7H3AlM+c2i6zpuu4wE5H7LcBslTAxkbYVqDWdS6ngfCv0AaRL1Lg1hpynXjBISNphEjk7eXQXABU2XYuwdtZzahbmjpY7hZVRs4XXehzef8fD0GkO8cMsLNOHDm3lU2nSb8SlnRT+l53RCtFflB9l8dQl7qLdl0wCY4/o4jArfRzegYj2uG+Dt4EcxsIDrJa0m1TfMAAExXSwYZ0pnxjeBQGKgoHE3DUkWDMJGGdWRMpwmuI0kBzo0KGPGEW1ot8G8FnRZkebTxe/2/q8YwWo5+4Yjh/7dL+zAISxCF0t+ASqwEHpNDavbtyrvE/PafnS/qakQtqeaQlE7XXzSzz0E4naktfjt8VrKG4G2DsQ4meuFYcmhvHUkzzFF0D0Q5o2U8B+ZiIJz9Gjyx1glEX7xfjm34byyb8Vb8qSsNlo7O3bqWidXW6ZixEuV3FPlK6UfeH+C1bY+1fje6CIqxCby/FN5F26G/Fp/4zlWSjjVrU2OpOezzlwrBd+LqB950jf0Czx7LZfB51zOKg3Njhk9OsIe3rbyQaEN0kz3P29R/Xz/zuxBVWVKjKhq7HV3kpCsQVCcr8nbJMmA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR04MB8788.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(136003)(346002)(396003)(366004)(376002)(39860400002)(230922051799003)(1800799009)(451199024)(186009)(26005)(478600001)(86362001)(2616005)(36756003)(31696002)(83380400001)(6486002)(6506007)(53546011)(38100700002)(5660300002)(8676002)(6512007)(41300700001)(2906002)(8936002)(4326008)(31686004)(316002)(66556008)(66946007)(66476007)(6916009)(30864003)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QitQN2hHb2luV2pXeGVIb0hnZ2wzZHEydnd6RjZZTDFUZnFob3dwUllyMER4?= =?utf-8?B?QzJhZVVyc3BFM1dDMHZQWUk5RHdEdVBadGpNV0JaWElLYk5GQnY3T0E4M1pB?= =?utf-8?B?bnVMMDZKaGFpQVhnMExtMk92NVMyVW92NVlUaHRpRmxhVEtNQm1EWGljb3ky?= =?utf-8?B?bGlLZCt5ejdJL3JuclR6MHRDL2tDTTUySHNVQkZ6RjQ5SmRXL0lMNFZlNVdO?= =?utf-8?B?V05saHBXL2hwblJiNkp5bVlmdDdzQW1Pb2lYMXB5UGlVUUw5R25NMEdRL3dl?= =?utf-8?B?NW5PMWVpRlk5OVpHNTRCNWx4YUl4T0JJL3kweU50Z0VMQVRSUys3UFhMMHh2?= =?utf-8?B?OFprMzFDUE5XeS9KM1hJK05oY3F3RXBWdk5mcjY4UEpMejRCcWR5bEJpbXVz?= =?utf-8?B?d2Nra3lTVDBhV1hEd2F4Rmg3ZmlZdkExQ0dudFFGcGxUN2liSDJxZkhSa2NE?= =?utf-8?B?ejJnZ3pMQnltT3R6eWFSUDhQbDVLOXpDWU0xSGJQSW0yZkJqbm1zV0tVbTBq?= =?utf-8?B?QTE5akVGbGtqRld3NkpxOGoyZGFTcmlqWFFsTDNZWnJXaVp3aUljNTNSSWJX?= =?utf-8?B?ekVqL3pRRVlwc3NkY2NTZTZsRmoreE9rVmdRVmVBNklqWk5nK2F1eGFxNHFp?= =?utf-8?B?RkU2eERGYnhlSk1yb09hNkk4dE9vRk9EUXY0d25CaEpDR0c5S1RLNHZvWFpy?= =?utf-8?B?dzEzRllsMGEvZm5lV0ZiQTVwcGRTcFM1dURuWFhISVl6R2wvdTZueFl0SHpu?= =?utf-8?B?dDRrV3lJREJrQ1pyekdLWU9xemhmVElheC84eWpzUVFRQ2RXS2NMSWVzVHlY?= =?utf-8?B?a0t4UmNzL0J3WXVyYW1rYVV1aFFwQWhYS0phOVFhM1ZxbjBYQThYS2tDamhB?= =?utf-8?B?b3RXZC8yVzRMMW9yTWttUU9VanZ6UERTTE5VcW5XeTFEazVaUjZ4RjVMRmFn?= =?utf-8?B?VlpaakNrQndGYUpWYXpuczNwd1NXWWRIMmt0bmZxWG9GaW1sWmlVamRCU2dT?= =?utf-8?B?a3F1b3BsaElnV0tJdGxFYjVJakkzL3NnQjVKaDhERGJSZVNMSWcrYXNJY0Mv?= =?utf-8?B?cnFKRm80T2JzdEJ5VnVJb0JQYVNoaWtab1VLblN4ZnRSZGZKU0k2cG43STMz?= =?utf-8?B?TDI3bXBneWtsWkJiZUp1allIT25qM3E4dnhnenhJWlZVWjhOTk55WUJSUWVJ?= =?utf-8?B?Nmw5Z244bEZlYXhRTDkvWU5jTUhsUVIrUUVyTjRtMGVlYUFYY3RncjVIdTd4?= =?utf-8?B?SVZaOUxJS09wbTBxbi9HcjdxdmRhUWdNNnY2ZVlyMU5QOUp4c3dxa3lzd1JO?= =?utf-8?B?dXkrUWh5VEJDK1ZpbFRIcXNEQ1FHNHExNFJQbWUrS2N6RlpIYzJMOEh3SEk5?= =?utf-8?B?QzU1aGdkclpmSzhGRTlyTEZEYlk1ZnBCVFNjN1dwMFlWdm9nOW5hQUVxbk83?= =?utf-8?B?bENZczZpU21FSG9pdVE3NnA5d0U3bzdValJ6UkxDdWgrU3B4eFNkVER5emZq?= =?utf-8?B?bm50L1A1eFNrbXR3dkwxMDNwU0VaUUlVTkNCdUF0ZzZPcTFCT3Z4bXpoSjdB?= =?utf-8?B?TTJQUFpXdEFNQjdFS0NTMHg4bjU4V2dMR3VpdDhrNTF1S0NDOU5xT0M4b2Vh?= =?utf-8?B?d1AyNDUvTStIazFEajdPSmhZWEZnS2ZXVDc5QlNBTW1CcFVaYmRPRzlRckZZ?= =?utf-8?B?bDExWUZkckI5UUVlS1V6dmphSlZhSDRRbnJJZnJIVm55M0NneStnQTM4VkYz?= =?utf-8?B?M2tOTnFBUTZnM1FQZjV4UU1rV3NraUlXQ01PMGJZRk1INVljbWVScXNKRVhz?= =?utf-8?B?VWw0Q1Y0SGJnc1JVQWRwTmMxTlFoNGZvK0VQYzFsejY2MlBvU3dOdGZ0WWxx?= =?utf-8?B?SU5xcE0vMzFFMEh1d0dpUzNHVG5YN1k2TnRKU0pDbEtldCtMdTduZzVGRXI4?= =?utf-8?B?NGtzdElRTVo5ZUNtRUJkL2J5SlhqNmpvblFuY0pSUXBFTGlVRCtXQnl6UHNp?= =?utf-8?B?Wmc2Y2lQcFVmOG92WEwzSDNlc1NRS1BTbHcrOGxNcGxYTHIrZ1U1V3NlOW5Y?= =?utf-8?B?bU90QmFTN3ZGYzVhMnR4WW8zeThUTXk4TlkwMkptWlNLYmN4T3oxQjdISW9h?= =?utf-8?Q?yj6FbaBa6EeTfD+8+IEtbQ8Xb?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: dad194e8-8585-4209-725c-08dbbf685130 X-MS-Exchange-CrossTenant-AuthSource: AS8PR04MB8788.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Sep 2023 14:44:56.3585 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: evz2Vt7nbSeYFqhaozSG+0lMRzoVpQ9A56AkxBu+topR9mM6l3sW55v4JNyHVUUjxmiphfO+gxORPHhFb0J/gw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU0PR04MB9249 X-Spam-Status: No, score=-3033.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SCC_5_SHORT_WORD_LINES,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 19.09.2023 17:25, Cui, Lili wrote: > From: konglin1 > > opcodes/ChangeLog: > > * opcodes/i386-dis-evex-prefix.h: Add NDD decode for adox/adcx. > * opcodes/i386-dis-evex-reg.h: Handle for REG_EVEX_MAP4_80, > REG_EVEX_MAP4_81, REG_EVEX_MAP4_83, REG_EVEX_MAP4_C0, > REG_EVEX_MAP4_C1, REG_EVEX_MAP4_D0, REG_EVEX_MAP4_D1, > REG_EVEX_MAP4_D2, REG_EVEX_MAP4_D3, REG_EVEX_MAP4_F6, > REG_EVEX_MAP4_F7, REG_EVEX_MAP4_FE, REG_EVEX_MAP4_FF. > * opcodes/i386-dis-evex.h: Add NDD insn. > * opcodes/i386-dis.c (VexGb): Add new define. > (VexGv): Ditto. > (get_valid_dis386): Change for NDD decode. > (print_insn): Ditto. > (print_register): Ditto. > (intel_operand_size): Ditto. > (OP_E_memory): Ditto. > (OP_VEX): Ditto. > * opcodes/i386-opc.h (Opcode_APX_NDDD): New macro. > * opcodes/i386-opc.tbl: Add APX NDD instructions. > * opcodes/i386-tbl.h: Regenerated. > > gas/ChangeLog: > > * gas/config/tc-i386.c (is_any_apx_encoding): Add legacy insn > promote to SPACE_EVEXMAP4. > (build_legacy_insns_with_apx_encoding): Add ndd bit encode. > (md_assemble): Change for ndd encode. > (process_operands): Ditto. > (build_modrm_byte): Ditto. > (operand_size_match): > Support APX NDD that the number of operands is 3. > (match_template): Support swap the first two operands for > APX NDD that the number of operands is 3. > * gas/testsuite/gas/i386/x86-64.exp: Add x86-64-apx-ndd. > * gas/testsuite/gas/i386/x86-64-apx-ndd.d: New test. > * gas/testsuite/gas/i386/x86-64-apx-ndd.s: Ditto. > * testsuite/gas/i386/x86-64-pseudos.d: Add test. > * testsuite/gas/i386/x86-64-pseudos.s: Ditto. > --- > gas/config/tc-i386.c | 80 ++++++++---- > gas/testsuite/gas/i386/x86-64-apx-ndd.d | 165 ++++++++++++++++++++++++ > gas/testsuite/gas/i386/x86-64-apx-ndd.s | 156 ++++++++++++++++++++++ > gas/testsuite/gas/i386/x86-64-pseudos.d | 42 ++++++ > gas/testsuite/gas/i386/x86-64-pseudos.s | 43 ++++++ > gas/testsuite/gas/i386/x86-64.exp | 1 + > opcodes/i386-dis-evex-prefix.h | 4 +- > opcodes/i386-dis-evex-reg.h | 123 ++++++++++++++++++ > opcodes/i386-dis-evex.h | 124 +++++++++--------- > opcodes/i386-dis.c | 47 ++++++- > opcodes/i386-opc.h | 1 + > opcodes/i386-opc.tbl | 67 ++++++++++ > 12 files changed, 762 insertions(+), 91 deletions(-) > create mode 100644 gas/testsuite/gas/i386/x86-64-apx-ndd.d > create mode 100644 gas/testsuite/gas/i386/x86-64-apx-ndd.s > > diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c > index 48916bc3846..381e389bb04 100644 > --- a/gas/config/tc-i386.c > +++ b/gas/config/tc-i386.c > @@ -2261,8 +2261,9 @@ operand_size_match (const insn_template *t) > unsigned int given = i.operands - j - 1; > > /* For FMA4 and XOP insns VEX.W controls just the first two > - register operands. */ > - if (is_cpu (t, CpuFMA4) || is_cpu (t, CpuXOP)) > + register operands. And APX insns just swap the first operands. */ > + if (is_cpu (t, CpuFMA4) || is_cpu (t, CpuXOP) > + || (is_cpu (t,CpuAPX_F) && i.operands == 3)) > given = j < 2 ? 1 - j : j; In the comment, how about "And APX_F insns just swap the two source operands, with the 3rd one being the destination"? Is the "i.operands == 3" part of the condition really needed? I.e. are there any APX_F insns which can make it here but must not take this path? Afaict 2-operand insns are fine to go here, and more-than-3-operand insns don't come with the D attribute. Also (nit) there's a missing blank after the comma. > @@ -3876,6 +3877,7 @@ is_any_apx_encoding (void) > { > return i.rex2 > || i.rex2_encoding > + || i.tm.opcode_space == SPACE_EVEXMAP4 > || (i.vex.register_specifier > && i.vex.register_specifier->reg_flags & RegRex2); > } > @@ -4204,6 +4206,10 @@ build_legacy_insns_with_apx_encoding (void) > } > > build_evex_insns_with_extend_evex_prefix (); > + > + /* Encode the NDD bit. */ > + if (i.vex.register_specifier) > + i.vex.bytes[3] |= 0x10; > } > > static void > @@ -7383,26 +7389,31 @@ match_template (char mnem_suffix) > overlap1 = operand_type_and (operand_types[0], operand_types[1]); > if (t->opcode_modifier.d && i.reg_operands == i.operands > && !operand_type_all_zero (&overlap1)) > - switch (i.dir_encoding) > - { > - case dir_encoding_load: > - if (operand_type_check (operand_types[i.operands - 1], anymem) > - || t->opcode_modifier.regmem) > - goto check_reverse; > - break; > + { > + int isMemOperand = (t->opcode_modifier.vexvvvv > + && t->opcode_space == SPACE_EVEXMAP4) > + ? i.operands - 2 : i.operands - 1; "is" in the variable name is properly misleading. What you're determining here is which operand you want to _check_ for being the memory operand. As to the condition, the two side of && may want swapping: In such a condition it is generally desirable to have the more restricting part first. Plus this may be more neat to express without ?: anyway: i.operands - 1 - (t->opcode_space == SPACE_EVEXMAP4 && t->opcode_modifier.vexvvvv) (suitably line wrapped of course). > + switch (i.dir_encoding) > + { > + case dir_encoding_load: > + if (operand_type_check (operand_types[isMemOperand], anymem) > + || t->opcode_modifier.regmem) > + goto check_reverse; > + break; > > - case dir_encoding_store: > - if (!operand_type_check (operand_types[i.operands - 1], anymem) > - && !t->opcode_modifier.regmem) > - goto check_reverse; > - break; > + case dir_encoding_store: > + if (!operand_type_check (operand_types[isMemOperand], anymem) > + && !t->opcode_modifier.regmem) > + goto check_reverse; > + break; > > - case dir_encoding_swap: > - goto check_reverse; > + case dir_encoding_swap: > + goto check_reverse; > > - case dir_encoding_default: > - break; > - } > + case dir_encoding_default: > + break; > + } > + } > /* If we want store form, we skip the current load. */ > if ((i.dir_encoding == dir_encoding_store > || i.dir_encoding == dir_encoding_swap) > @@ -7432,11 +7443,13 @@ match_template (char mnem_suffix) > continue; > /* Try reversing direction of operands. */ > j = is_cpu (t, CpuFMA4) > - || is_cpu (t, CpuXOP) ? 1 : i.operands - 1; > + || is_cpu (t, CpuXOP) > + || is_cpu (t, CpuAPX_F) ? 1 : i.operands - 1; > overlap0 = operand_type_and (i.types[0], operand_types[j]); > overlap1 = operand_type_and (i.types[j], operand_types[0]); > overlap2 = operand_type_and (i.types[1], operand_types[1]); > - gas_assert (t->operands != 3 || !check_register); > + gas_assert (t->operands != 3 || !check_register > + || is_cpu (t,CpuAPX_F)); Nit: Missing blank again. > @@ -7471,6 +7484,12 @@ match_template (char mnem_suffix) > found_reverse_match = Opcode_VexW; > goto check_operands_345; > } > + else if (is_cpu (t,CpuAPX_F) > + && i.operands == 3) > + { > + found_reverse_match = Opcode_APX_NDDD; > + goto check_operands_345; > + } > else if (t->opcode_space != SPACE_BASE > && (t->opcode_space != SPACE_0F > /* MOV to/from CR/DR/TR, as an exception, follow > @@ -7636,6 +7655,15 @@ match_template (char mnem_suffix) > flipping VEX.W. */ > i.tm.opcode_modifier.vexw ^= VEXW0 ^ VEXW1; > > + j = i.tm.operand_types[0].bitfield.imm8; > + i.tm.operand_types[j] = operand_types[j + 1]; > + i.tm.operand_types[j + 1] = operand_types[j]; > + break; I'm not overly happy to see this code getting duplicated. Are there any encodings at all which have D and and immediate operand? I don't think so, in which case this at least wants simplifying. But read on. > + case Opcode_APX_NDDD: > + /* Only the first two register operands need reversing. */ > + i.tm.base_opcode ^= 0x2; I think you mean Opcode_D here? > j = i.tm.operand_types[0].bitfield.imm8; > i.tm.operand_types[j] = operand_types[j + 1]; > i.tm.operand_types[j + 1] = operand_types[j]; Taking both remarks together, do we need Opcode_APX_NDDD at all? Can't you use the ordinary Opcode_D, with default: /* If we found a reverse match we must alter the opcode direction bit and clear/flip the regmem modifier one. found_reverse_match holds bits to change (different for int & float insns). */ i.tm.base_opcode ^= found_reverse_match; if (i.tm.opcode_space == SPACE_EVEXMAP4 && i.operands == 3) goto swap_first_2; ... swap_first_2: j = i.tm.operand_types[0].bitfield.imm8; i.tm.operand_types[j] = operand_types[j + 1]; i.tm.operand_types[j + 1] = operand_types[j]; break; ? (I'm not convinced the i.operands == 3 part of the condition is needed; if at all possible it wants omitting.) > @@ -8462,8 +8490,8 @@ process_operands (void) > const reg_entry *default_seg = NULL; > > /* We only need to check those implicit registers for instructions > - with 3 operands or less. */ > - if (i.operands <= 3) > + with 4 operands or less. */ > + if (i.operands <= 4) > for (unsigned int j = 0; j < i.operands; j++) > if (i.types[j].bitfield.instance != InstanceNone) > i.reg_operands--; How useful is it to keep the outer if() when 4-operand insns now also need checking? There are extremely few 5-operand ones ... > @@ -8825,6 +8853,9 @@ build_modrm_byte (void) > break; > if (v >= dest) > v = ~0; > + if (i.tm.opcode_space == SPACE_EVEXMAP4 > + && i.tm.opcode_modifier.vexvvvv) > + v = dest; > if (i.tm.extension_opcode != None) > { > if (dest != source) > @@ -9088,6 +9119,9 @@ build_modrm_byte (void) > set_rex_vrex (i.op[op].regs, REX_B, false); > } > > + if (i.tm.opcode_space == SPACE_EVEXMAP4 > + && i.tm.opcode_modifier.vexvvvv) > + dest--; > if (op == dest) > dest = ~0; > if (op == source) These two changes are at the very least problematic with .insn, whose behavior may not change. I'd also prefer if we could get away with just one change to the function. Did you consider alternatives? We could re- widen VexVVVV, such that the value 2 indicates that the destination is encoded there. That then also has no chance of conflicting with .insn. > --- /dev/null > +++ b/gas/testsuite/gas/i386/x86-64-apx-ndd.s > @@ -0,0 +1,156 @@ > +# Check 64bit APX NDD instructions with evex prefix encoding > + > + .allow_index_reg > + .text > +_start: > +inc %rax,%rbx > +inc %r31,%r8 > +inc %r31,%r16 > +add %r31b,%r8b,%r16b > +addb %r31b,%r8b,%r16b > +add %r31,%r8,%r16 > +addq %r31,%r8,%r16 > +add %r31d,%r8d,%r16d > +addl %r31d,%r8d,%r16d > +add %r31w,%r8w,%r16w > +addw %r31w,%r8w,%r16w > +{store} add %r31,%r8,%r16 > +{load} add %r31,%r8,%r16 > +add %r31,(%r8),%r16 > +add (%r31),%r8,%r16 > +add 0x9090(%r31,%r16,1),%r8,%r16 > +add %r31,(%r8,%r16,8),%r16 > +add $0x34,%r13b,%r17b > +addl $0x11,(%r19,%rax,4),%r20d > +add $0x1234,%ax,%r30w > +add $0x12344433,%r15,%r16 > +addq $0x12344433,(%r15,%rcx,4),%r16 > +add $0xfffffffff4332211,%rax,%r8 > +dec %rax,%r17 > +decb (%r31,%r12,1),%r8b > +not %rax,%r17 > +notb (%r31,%r12,1),%r8b > +neg %rax,%r17 > +negb (%r31,%r12,1),%r8b > +sub %r15b,%r17b,%r18b > +sub %r15d,(%r8),%r18d > +sub (%r15,%rax,1),%r16b,%r8b > +sub (%r15,%rax,1),%r16w,%r8w > +subl $0x11,(%r19,%rax,4),%r20d > +sub $0x1234,%ax,%r30w > +sbb %r15b,%r17b,%r18b > +sbb %r15d,(%r8),%r18d > +sbb (%r15,%rax,1),%r16b,%r8b > +sbb (%r15,%rax,1),%r16w,%r8w > +sbbl $0x11,(%r19,%rax,4),%r20d > +sbb $0x1234,%ax,%r30w > +adc %r15b,%r17b,%r18b > +adc %r15d,(%r8),%r18d > +adc (%r15,%rax,1),%r16b,%r8b > +adc (%r15,%rax,1),%r16w,%r8w > +adcl $0x11,(%r19,%rax,4),%r20d > +adc $0x1234,%ax,%r30w > +or %r15b,%r17b,%r18b > +or %r15d,(%r8),%r18d > +or (%r15,%rax,1),%r16b,%r8b > +or (%r15,%rax,1),%r16w,%r8w > +orl $0x11,(%r19,%rax,4),%r20d > +or $0x1234,%ax,%r30w > +xor %r15b,%r17b,%r18b > +xor %r15d,(%r8),%r18d > +xor (%r15,%rax,1),%r16b,%r8b > +xor (%r15,%rax,1),%r16w,%r8w > +xorl $0x11,(%r19,%rax,4),%r20d > +xor $0x1234,%ax,%r30w > +and %r15b,%r17b,%r18b > +and %r15d,(%r8),%r18d > +and (%r15,%rax,1),%r16b,%r8b > +and (%r15,%rax,1),%r16w,%r8w > +andl $0x11,(%r19,%rax,4),%r20d > +and $0x1234,%ax,%r30w > +rorb (%rax),%r31b > +ror $0x2,%r12b,%r31b > +rorl $0x2,(%rax),%r31d > +rorw (%rax),%r31w > +ror %cl,%r16b,%r8b > +rorw %cl,(%r19,%rax,4),%r31w > +rolb (%rax),%r31b > +rol $0x2,%r12b,%r31b > +roll $0x2,(%rax),%r31d > +rolw (%rax),%r31w > +rol %cl,%r16b,%r8b > +rolw %cl,(%r19,%rax,4),%r31w > +rcrb (%rax),%r31b > +rcr $0x2,%r12b,%r31b > +rcrl $0x2,(%rax),%r31d > +rcrw (%rax),%r31w > +rcr %cl,%r16b,%r8b > +rcrw %cl,(%r19,%rax,4),%r31w > +rclb (%rax),%r31b > +rcl $0x2,%r12b,%r31b > +rcll $0x2,(%rax),%r31d > +rclw (%rax),%r31w > +rcl %cl,%r16b,%r8b > +rclw %cl,(%r19,%rax,4),%r31w > +shlb (%rax),%r31b > +shl $0x2,%r12b,%r31b > +shll $0x2,(%rax),%r31d > +shlw (%rax),%r31w > +shl %cl,%r16b,%r8b > +shlw %cl,(%r19,%rax,4),%r31w > +sarb (%rax),%r31b > +sar $0x2,%r12b,%r31b > +sarl $0x2,(%rax),%r31d > +sarw (%rax),%r31w > +sar %cl,%r16b,%r8b > +sarw %cl,(%r19,%rax,4),%r31w > +shlb (%rax),%r31b > +shl $0x2,%r12b,%r31b > +shll $0x2,(%rax),%r31d > +shlw (%rax),%r31w > +shl %cl,%r16b,%r8b > +shlw %cl,(%r19,%rax,4),%r31w > +shrb (%rax),%r31b > +shr $0x2,%r12b,%r31b > +shrl $0x2,(%rax),%r31d > +shrw (%rax),%r31w > +shr %cl,%r16b,%r8b > +shrw %cl,(%r19,%rax,4),%r31w > +shld $0x1,%r12,(%rax),%r31 > +shld $0x2,%r8w,%r12w,%r31w > +shld $0x2,%r15d,(%rax),%r31d > +shld %cl,%r9w,(%rax),%r31w > +shld %cl,%r12,%r16,%r8 > +shld %cl,%r13w,(%r19,%rax,4),%r31w > +shrd $0x1,%r12,(%rax),%r31 > +shrd $0x2,%r8w,%r12w,%r31w > +shrd $0x2,%r15d,(%rax),%r31d > +shrd %cl,%r9w,(%rax),%r31w > +shrd %cl,%r12,%r16,%r8 > +shrd %cl,%r13w,(%r19,%rax,4),%r31w > +adcx %r15d,%r8d,%r18d > +adcx (%r15,%r31,1),%r8d,%r18d > +adcx (%r15,%r31,1),%r8 > +adox %r15d,%r8d,%r18d > +adox (%r15,%r31,1),%r8d,%r18d > +adox (%r15,%r31,1),%r8 > +cmovo 0x90909090(%eax),%edx,%r8d > +cmovno 0x90909090(%eax),%edx,%r8d > +cmovb 0x90909090(%eax),%edx,%r8d > +cmovae 0x90909090(%eax),%edx,%r8d > +cmove 0x90909090(%eax),%edx,%r8d > +cmovne 0x90909090(%eax),%edx,%r8d > +cmovbe 0x90909090(%eax),%edx,%r8d > +cmova 0x90909090(%eax),%edx,%r8d > +cmovs 0x90909090(%eax),%edx,%r8d > +cmovns 0x90909090(%eax),%edx,%r8d > +cmovp 0x90909090(%eax),%edx,%r8d > +cmovnp 0x90909090(%eax),%edx,%r8d > +cmovl 0x90909090(%eax),%edx,%r8d > +cmovge 0x90909090(%eax),%edx,%r8d > +cmovle 0x90909090(%eax),%edx,%r8d > +cmovg 0x90909090(%eax),%edx,%r8d > +imul 0x90909(%eax),%edx,%r8d > +imul 0x909(%rax,%r31,8),%rdx,%r25 What about imul by immediate? The present spec is quite unclear there: The insn page says {ND=ZU} and the table says 0/1 in the ND column. > +.byte 0x62,0xf4,0xfc,0x08,0xff,0xc0 #inc %rax > +.byte 0x62,0xf4,0xec,0x08,0xff,0xc0 #bad As before, please avoid .byte whenever possible. And please have a more detailed comment as to what is being encoded, when .byte cannot be avoided. Plus, if at all possible, have "bad" tests live in separate testcases from "good" ones. > --- a/opcodes/i386-dis-evex-prefix.h > +++ b/opcodes/i386-dis-evex-prefix.h Once again I'll reply to disassembler changes separately. > --- a/opcodes/i386-opc.h > +++ b/opcodes/i386-opc.h > @@ -960,6 +960,7 @@ typedef struct insn_template > /* The next value is arbitrary, as long as it's non-zero and distinct > from all other values above. */ > #define Opcode_VexW 0xf /* Operand order controlled by VEX.W. */ > +#define Opcode_APX_NDDD 0x11 /* Direction bit for APX NDD insns. */ The comment talks of a single bit, but the value has two bits set. Plus in the code you also don't use this constant as described by the comment. Aiui like for Opcode_VexW the value is really arbitrary, just as long as it's different from others. In which case I'd rather suggest using e.g. 0xe (if, unlike suggested above, Opcode_D cannot be re-used). Also I don't think there's a need for three D-s in the name. > --- a/opcodes/i386-opc.tbl > +++ b/opcodes/i386-opc.tbl Comments given on the earlier patch apply here (and elsewhere) as well. Jan