From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-vi1eur04on2052.outbound.protection.outlook.com [40.107.8.52]) by sourceware.org (Postfix) with ESMTPS id CD1DA3858D28 for ; Fri, 22 Sep 2023 10:12:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD1DA3858D28 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=evKugHUegG4Vdodr6DnMTp2PGEzb2IdLquPD+YBAg0Xiht6kIwxvTNLQOVbCASEHvfhcheWCO8lvk4qJQqwv5Me2io+qIi24ef8wekHhSBzT3v111gKCZGmYXdDQS9gceV54igPq4JFpufT/aGEybiygVIhD33J+le+Mx/l+Rw5zj+mE9PkQTeHgJR9zgsj/DWLh66rBKvJ2jsfb0OdwL2jYKx4jERreRRG97quIYjSZndGWHZ0Qi9bYzwFSbtsBovNUc/L5aZB5qy1MxdGDOGyTutS9+tvF+kpCjT28zTOBUb+UfIm9e7gi0wZcO3o5S2ciAJgu5Eyy+8htQ+gNqQ== 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=O5OF7zaInsmsL2AxxNK+OkfeqA4qjMvHvIubqdyCCXg=; b=GmbrQ+u1e0kb7hxft7Nqsmw/+ZktajS2XpGcZ4eWm/x8FaAvOYc/27YfpKA82mvvDffl5ZIDqRTAXcQbTdpH2XYcJO7WaM0qxUIhQh4KB7xUTj4+CDahOko3/DxWegVH+UtsCQt8rf6zD2LiqnMS5r8NV3dVBoyykAX+HudgeFwEutIjFPpy1T/ScMxTxMfm9u0GFBAF1yXlgpltHb7x2WMNGmc/2rY2eMIIWId8KaDurO53nezkHEDA0Iy5cj/VDL0ol2j498cyJL7FTDeB7gYQMd+1Ov8x64MdM+rPsryu24Eu+rxsz27/PvQYgsS2stANI+wPMJa1pSIwtT4MZw== 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=O5OF7zaInsmsL2AxxNK+OkfeqA4qjMvHvIubqdyCCXg=; b=ClhLfd1mmENow21pJBUi3clBVqrw9lg/IT7ay5gW9pO4UbiV2uITfMNcPee0dhpVsQqFFit6s9Bk8CLHYg29RFJsTiOr13dlHq3LWjKfi3epDr60qp0nZvv/t90+hxu2DeboQbcHwHtGdmY6jDh+igsvo4IkWORPyitkSaF01i6WqAymiIeFsi/MZveL2lrtQyI765JWFeF70A/c2J2Dsv7kTag3dXYkEpt/N0h07BEGPyvNbb1KDcDJDMIExdP8sOUK8CAtwPSCd9CMGhkd3boDmkr9BOSD+KSL0PjgROrRu4u034v3Jn0L151dUIRirhp1Y+POPEWBsjlBYJm1yg== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) by AM7PR04MB6965.eurprd04.prod.outlook.com (2603:10a6:20b:104::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6813.20; Fri, 22 Sep 2023 10:12:18 +0000 Received: from DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::f749:b27f:2187:6654]) by DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::f749:b27f:2187:6654%6]) with mapi id 15.20.6813.017; Fri, 22 Sep 2023 10:12:18 +0000 Message-ID: Date: Fri, 22 Sep 2023 12:12:15 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH 2/8] Support APX GPR32 with extend evex prefix Content-Language: en-US To: "Cui, Lili" Cc: hongjiu.lu@intel.com, binutils@sourceware.org References: <20230919152527.497773-1-lili.cui@intel.com> <20230919152527.497773-3-lili.cui@intel.com> From: Jan Beulich In-Reply-To: <20230919152527.497773-3-lili.cui@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR3P281CA0200.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a5::15) To DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PR04MB8790:EE_|AM7PR04MB6965:EE_ X-MS-Office365-Filtering-Correlation-Id: 14305140-63b2-4b97-7768-08dbbb5466d6 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: f81AupMzaGWZbgqhGgtqnHmpgtDnYzjp+r9y5Ime9JP8TPfAU9+3fUzbBGpibTWGqS8twiUvmIR1nhnspLWrC+hG0s0P0M/ielMAdSrkPjO2Bp2z85I9g74eZatNS+gC7PrbnXqScidYE5e4RUjszdJfQewcOQWJ2veFJ6V9GnM95OqjWD7Z+7/58ZmsA18awDJuV24OzfoCs0u1wDknFUe6hKQAX90Ik/q6HG2Orjv2HEZuJD/hSfoQALmQJGC5jW9vsrNtql2T/oGS5sD84ck3yrtZC70tMXdxW9kgSpJldudaX9MWAuBKMG1qQynMQ7TEigGs+KgzXOjztqOBFOZ3aDadtiSTqIVhB/NCj9dfCczVkJrJkn46H3McKq0Zpt4gtMGGgLiwXzmtaeB9FGCU+UuTLAs+7Q3WOpDhVOTnOg18AZ+O/ZNjVRN1sajwHddtV9ceyVWYv84mGkGg3YCcQb26F3GYMcJq2EcUZwuzfuVyKfHc0OvYY1qIgTA081F94emzSARj2oI8AnMchhbuzVDuDVZ0WP7f1iWnp/DyQH2vH7WUA3n/LjqQc8U/pD3G6JCjhKLn8neIrErAZTfCi2Gwlm3uYkQYfTGBVN7/pscTjxiOd12O7piCv97vxQcRQgfkOKBZJuTKxkuBOA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DU2PR04MB8790.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(366004)(376002)(346002)(136003)(39860400002)(396003)(1800799009)(451199024)(186009)(2906002)(30864003)(478600001)(6666004)(26005)(6512007)(6506007)(2616005)(38100700002)(36756003)(31686004)(66556008)(66476007)(6916009)(66946007)(8676002)(41300700001)(83380400001)(6486002)(53546011)(5660300002)(4326008)(8936002)(31696002)(86362001)(316002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?R2gxY2c0dVRkbml3aU1jUzVFUVR0N0RGV3N4emo3cGlnV3Q2NEE3UTZ2L01S?= =?utf-8?B?QkFqTjFWcnEvczhYYUdJVkVVZ0lvMno0MVdsWnpuUE9YQU9ub05BSTNBRXFv?= =?utf-8?B?RGV0anVEQ2czbWJqZHY0WGErNElTbnBZMkgybnBWSXZkTVptSGNYRE5wUlZt?= =?utf-8?B?eFBhOWVxTXVDc2Fpb1hjLzRYTXN4bjJUejJSTXBUaXF6cGt1Vkx3Q1oyQTJP?= =?utf-8?B?RnBqNzlIMDhldjdiVXl0RlhSb3dPaWVScmpIUWRvSEVhRUQ5WXVYMkRiVnhi?= =?utf-8?B?YnB2cXZoOWhhMS9iZmxCWWNoNWtGdGQ4MUYyY2VIWVorN0hHbnhJSjRiZ1pY?= =?utf-8?B?NzFrMmZ5NDd6dEl2VlhXU3BIbk14NXJndmRNeWlxK3BVdXJmSERHVUNtTmZS?= =?utf-8?B?bWMvanFjSzdncHlxUWRieGRzWk95MU91WVZuWTVOcUxRSE9DZVF1WHFES244?= =?utf-8?B?V0FVMU85OGZ2eU1yLy9Tc3lXNDBxU0hPRUxxbzRvYWtTU0JUM2UyRHIyc3Ir?= =?utf-8?B?RHdZNEZrL3lxV0JUcUZsQWNWbUlNVmo2SUhkc0NnbVlrZXNhNW10RGRLb3pB?= =?utf-8?B?MHkrYTlqQ3pJa0FlMDRRZlZUb29wK29rUWI2Vnl3QjlsdGhlbSszOWlWMWJi?= =?utf-8?B?UjFLaG9uNXdYZklBY1JoNzBtVWw2QmJhRmNXOEhWTlg2RWJvNGloaEllQjZa?= =?utf-8?B?K1lrTXZialVteG5ybUVsczltUG82L2RkQlZ0VFZ4eVA4WnRFU3NPVlVxa05n?= =?utf-8?B?RzVXZlRYeHpUNC9lUER3ejdEQkttOVE5ekRNQ0xreVZNRXFIaEZIelBDcW1r?= =?utf-8?B?SmpMQVZtVGEyb1I3ekRIM2lkNTQxbk1tM25RQ2FrY1hhUGRqSWd6bUJTaVFz?= =?utf-8?B?UDFDT05RSWxOMmd5b1NJOVFCRUJscFE1TUNrR2dDdzJ5NnhEanlQV3BPRVEr?= =?utf-8?B?R21XWTd5dmI4Mm1MVEhwdDlST1YwUm5qQlhYUWVWU2E2ZXdXVzlOWXdhYUZt?= =?utf-8?B?a1pwa3hzNHZSZkRFRytoVWJiWUJENy83emo0bEtNMENHa3ZhLzl6OVJNVHR4?= =?utf-8?B?eFdlSkZza0NQbHl6OFNvdWJjbmtvNlFtWmEwSDdmWVdYL0VwWitrZXc4MDEv?= =?utf-8?B?RG82M0V5MDlkWGw2Wk9ZNXNRck16aTNxcnVoam9IRHFsWnpocTJGK2pQdFdo?= =?utf-8?B?RUxGTHRUVlBXVHljMmdFUlByMk5wWmgxS3gvUXpJeWNoZjJJdGU4cE8zUHQx?= =?utf-8?B?NENDOG5wTFlLUXp4TElrRGZYVWkzaEhuWTRzRG9wRHZsdmtpNzFjRDBvaWpB?= =?utf-8?B?Vzk2TWF4bDkvQ0dHVFVtOWxLYTd5eERUOHc5OVkrRm9IY0daRUFicTNxcy9n?= =?utf-8?B?clp0ZHoxRWdhY2V0N1NTTEZBUW9kazhDQlRqMDlSQ2ZoazhNRkhYSXE5Njd6?= =?utf-8?B?aFlqSUtESFBzVEgrYzljSytFazNia2VKc2dyN1RFZUNQeS9pS0U1V0NCQ2o5?= =?utf-8?B?ekxkay8zc3JqMUdseG45S1YwTXc1UGE2YTlWL290MFo2UFZERkk1MFBDcU14?= =?utf-8?B?dHY2RTNKRlkydTkvcnRKZExhaFJTdGd5QjJzY3ZPamxwcjIwZzY5Wm0yblJK?= =?utf-8?B?dm05NlAzRkdzK1diQlpWMW1XOFJId3dvTkFNWi9UQ2RXVXJNZmJhMnc1NFNE?= =?utf-8?B?UDBBTXQyVjNhcHluREJkSllDbGpoc0ZHOUZLQi8xMFpEVDkrV1ROeDZFNVNW?= =?utf-8?B?a0ZrZ1dSNFlMZ2ZEUVJROGJJM0UxUDc0YTZTQVB4d1BWdCt3a21kbHgwbGF0?= =?utf-8?B?T3hCY05KaHJUenQ3dDJhV3JyQkh5eVBwbkNrTjNxTDBnSDVOU0orWTdiaDBK?= =?utf-8?B?SHhsVXJPaDB6aEN4QTdJOTFQTGJ1dyt4ZkZweEJMUkFZeGU0eXBudEJsNHZU?= =?utf-8?B?Y25jYVBiNnIvOVdGei9VU01heW1RQ1IwMlEvNmNYVTMweU13Tm1yVGRzczlS?= =?utf-8?B?NWlvZDhrVit2d1ZsN3g5cUdtaU5mRktWQi85UFB2K2JaakNNRjRObE8yTXdW?= =?utf-8?B?SmVBc2ErazNvY2pwTzJndmtIMW1nSERySmIydWJBbGxUK1BMSTZub3pVOGtP?= =?utf-8?Q?0Poo7WsQ/kQKZlDrQmR4V6q9P?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: 14305140-63b2-4b97-7768-08dbbb5466d6 X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8790.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2023 10:12:18.1284 (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: fTGNBUSIPPP9RzTFtOSEzWWphKXoVeM8yGYTSOYHyCU5WtwFaAgklIhpEgCpLFfdJl/aoK4LrF1dec8kdIGBNw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR04MB6965 X-Spam-Status: No, score=-3027.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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: > --- a/gas/config/tc-i386.c > +++ b/gas/config/tc-i386.c > @@ -1945,6 +1945,30 @@ cpu_flags_match (const insn_template *t) > && (!x.bitfield.cpuvpclmulqdq || cpu.bitfield.cpuvpclmulqdq)) > match |= CPU_FLAGS_ARCH_MATCH; > } > + else if (x.bitfield.cpuapx_f) > + { > + if (cpu.bitfield.cpuapx_f > + && (!x.bitfield.cpumovbe || cpu.bitfield.cpumovbe) > + && (!x.bitfield.cpuept || cpu.bitfield.cpuept) > + && (!x.bitfield.cpuinvpcid || cpu.bitfield.cpuinvpcid) > + && (!x.bitfield.cpusse4_2 || cpu.bitfield.cpusse4_2) > + && (!x.bitfield.cpubmi2 || cpu.bitfield.cpubmi2) > + && (!x.bitfield.cpubmi || cpu.bitfield.cpubmi) > + && (!x.bitfield.cpuadx || cpu.bitfield.cpuadx) > + && (!x.bitfield.cpusha || cpu.bitfield.cpusha) > + && (!x.bitfield.cpuavx512bw || cpu.bitfield.cpuavx512bw) > + && (!x.bitfield.cpuavx512dq || cpu.bitfield.cpuavx512dq) > + && (!x.bitfield.cpuavx512f || cpu.bitfield.cpuavx512f) > + && (!x.bitfield.cpushstk || cpu.bitfield.cpushstk) > + && (!x.bitfield.cpumovdir64b || cpu.bitfield.cpumovdir64b) > + && (!x.bitfield.cpumovdiri || cpu.bitfield.cpumovdiri) > + && (!x.bitfield.cpuenqcmd || cpu.bitfield.cpuenqcmd) > + && (!x.bitfield.cpukl || cpu.bitfield.cpukl) > + && (!x.bitfield.cpuwidekl || cpu.bitfield.cpuwidekl) > + && (!x.bitfield.cpucmpccxadd || cpu.bitfield.cpucmpccxadd) > + && (!x.bitfield.cpurao_int || cpu.bitfield.cpurao_int)) > + match |= CPU_FLAGS_ARCH_MATCH; > + } > else > match |= CPU_FLAGS_ARCH_MATCH; > This is getting unwieldy, so I think we will need to think of a better way of expressing both "multiple ISAs need to be enabled" and "one of a set of ISAs needs to be enabled". It's only the mix of these expressed in a uniform way in the insn table that requires these extra conditionals. With the size of i386_cpu_attr greatly shrunk as of recently, I wonder if we couldn't simply add a 2nd instance of it to insn_template. One would be "all of these are required", while the other would be "any one of these is sufficient". > @@ -3850,7 +3874,10 @@ is_any_vex_encoding (const insn_template *t) > static INLINE bool > is_any_apx_encoding (void) > { > - return i.rex2 || i.rex2_encoding; > + return i.rex2 > + || i.rex2_encoding > + || (i.vex.register_specifier > + && i.vex.register_specifier->reg_flags & RegRex2); Nit: For readability as well as for consistency this wants indenting differently: return i.rex2 || i.rex2_encoding || (i.vex.register_specifier && i.vex.register_specifier->reg_flags & RegRex2); or possibly (slightly shorter) return i.rex2 || i.rex2_encoding || (i.vex.register_specifier && i.vex.register_specifier->reg_flags & RegRex2); In any event you want to avoid trailing blanks on any line. > @@ -3859,6 +3886,12 @@ is_any_apx_rex2_encoding (void) > return (i.rex2 && i.vex.length == 2) || i.rex2_encoding; > } > > +static INLINE bool > +is_any_apx_evex_encoding (void) > +{ > + return i.rex2 && i.vex.length == 4; > +} This doesn't feel right: {evex} use would demand this encoding even if i.rex2 is still zero. Also - what is "any" in the name (also of the earlier predicate) intending to express? is_any_vex_encoding() is named the way it is because it covers both VEX and EVEX. > @@ -4129,6 +4162,50 @@ build_rex2_prefix (void) > | (i.rex2 << 4) | i.rex); > } > > +/* Build the EVEX prefix (4-byte) for evex insn > + | 62h | > + | `R`X`B`R' | B'mmm | > + | W | v`v`v`v | `x' | pp | > + | z| L'L | b | `v | aaa | > +*/ > +static void > +build_evex_insns_with_extend_evex_prefix (void) The name is somewhat odd and doesn't fit that of other similar functions. In particular this function doesn't build an entire insn, but still just the prefix. So perhaps build_apx_evex_prefix()? > +{ > + build_evex_prefix (); > + if (i.rex2 & REX_R) > + i.vex.bytes[1] &= 0xef; > + if (i.vex.register_specifier > + && register_number (i.vex.register_specifier) > 0xf) > + i.vex.bytes[3] &=0xf7; Nit: Missing blank. But: Is this needed? Doesn't build_evex_prefix() fill this bit already, which isn't new in APX? > + if (i.rex2 & REX_B) > + i.vex.bytes[1] |= 0x08; > + if (i.rex2 & REX_X) > + i.vex.bytes[2] &= 0xfb; > +} > + > +/* Build the EVEX prefix (4-byte) for legacy insn > + | 62h | > + | `R`X`B`R' | B'100 | > + | W | v`v`v`v | `x' | pp | > + | 000 | ND | `v | NF | 00 | > + For legacy insn without ndd nor nf, [vvvvv] must be all zero. */ > +static void > +build_legacy_insns_with_apx_encoding (void) As per above, maybe build_extended_evex_prefix()? Or, ... > +{ > + /* map{0,1} of legacy space without ndd or nf could use rex2 prefix. */ > + if (i.tm.opcode_space <= SPACE_0F > + && !i.vex.register_specifier && !i.has_nf && !i.has_zero_upper) > + return build_rex2_prefix (); ... because of this, build_apx_prefix()? Yet I think the call to this function might better remain in the caller. > + if (i.prefix[DATA_PREFIX] != 0) > + { > + i.tm.opcode_modifier.opcodeprefix = PREFIX_0X66; > + i.prefix[DATA_PREFIX] = 0; > + } While this looks to be correct for the case when the prefix was derived from an insn template and the use of 16-bit operands, I don't think it is uniformly correct when "data16" was used as a prefix explicitly. In such a case either REX2 encoding needs to be used, or an error needs emitting. You may further want to assert that i.tm.opcode_modifier.opcodeprefix is still zero ahead of the assignment. > @@ -10057,7 +10136,7 @@ output_insn (void) > > /* Since the VEX/EVEX prefix contains the implicit prefix, we > don't need the explicit prefix. */ > - if (!is_any_vex_encoding (&i.tm)) > + if (!is_any_vex_encoding (&i.tm) && !is_any_apx_evex_encoding ()) > { > switch (i.tm.opcode_modifier.opcodeprefix) I'm not convinced the use of this predicate is appropriate here. I'd generally have expected is_any_vex_encoding() to be extended to also detect all cases of EVEX encodings in APX. > --- a/opcodes/i386-dis-evex-len.h > +++ b/opcodes/i386-dis-evex-len.h As for the earlier patch, I'll look at the disassembler changes separately. > @@ -1121,6 +1122,15 @@ process_i386_opcode_modifier (FILE *table, char *mod, unsigned int space, > fprintf (stderr, > "%s: %d: W modifier without Word/Dword/Qword operand(s)\n", > filename, lineno); > + if (modifiers[Vex].value > + || (space > SPACE_0F > + && !(space == SPACE_EVEXMAP4 > + || modifiers[EVex].value > + || modifiers[Disp8MemShift].value > + || modifiers[Broadcast].value > + || modifiers[Masking].value > + || modifiers[SAE].value))) First of all, this wants simplifying to if (modifiers[Vex].value || (space > SPACE_0F && space != SPACE_EVEXMAP4 && !modifiers[EVex].value && !modifiers[Disp8MemShift].value && !modifiers[Broadcast].value && !modifiers[Masking].value && !modifiers[SAE].value)) which helps readability and makes more obvious that this parallels tc-i386.c:is_evex_encoding(). Such a connection, where updates need to be made in sync, needs pointing out in code comments at both sites. Yet of course this condition won't hold anymore for combined VEX/EVEX templates. > + modifiers[No_egpr].value = 1; > } And then - shouldn't at least part of this already be put in place in patch 1? Finally, to avoid the split between where this attribute gets set, wouldn't it be possible to also handle the XSAVE/XRSTOR variants here rather than directly in the opcode table? > @@ -187,6 +188,7 @@ mov, 0xf24, i386|No64, D|RegMem|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf, { Te > > // Move after swapping the bytes > movbe, 0x0f38f0, Movbe, D|Modrm|CheckOperandSize|No_bSuf|No_sSuf, { Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 } > +movbe, 0x60, Movbe|APX_F|x64, D|Modrm|CheckOperandSize|No_bSuf|No_sSuf|EVex128|EVexMap4, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 } In new code please omit redundant Word, Dword, and alike. I further wonder if it wouldn't help if i386-gen inserted the x64 for all APX templates, rather than open-coding that on every single template. Or alternatively put #define APX_F APX_F|x64 earlier in the file. > @@ -300,6 +302,9 @@ sbb, 0x18, 0, D|W|CheckOperandSize|Modrm|No_sSuf|HLEPrefixLock, { Reg8|Reg16|Reg > sbb, 0x83/3, 0, Modrm|No_bSuf|No_sSuf|HLEPrefixLock, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex } > sbb, 0x1c, 0, W|No_sSuf, { Imm8|Imm16|Imm32|Imm32S, Acc|Byte|Word|Dword|Qword } > sbb, 0x80/3, 0, W|Modrm|No_sSuf|HLEPrefixLock, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > +sbb, 0x18, APX_F|x64, D|W|CheckOperandSize|Modrm|EVex128|EVexMap4|No_sSuf, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > +sbb, 0x83/3, APX_F|x64, Modrm|EVex128|EVexMap4|No_bSuf|No_sSuf, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex } > +sbb, 0x80/3, APX_F|x64, W|Modrm|EVex128|EVexMap4|No_sSuf, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > > cmp, 0x38, 0, D|W|CheckOperandSize|Modrm|No_sSuf, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > cmp, 0x83/7, 0, Modrm|No_bSuf|No_sSuf, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex } > @@ -332,9 +337,14 @@ adc, 0x10, 0, D|W|CheckOperandSize|Modrm|No_sSuf|HLEPrefixLock, { Reg8|Reg16|Reg > adc, 0x83/2, 0, Modrm|No_bSuf|No_sSuf|HLEPrefixLock, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex } > adc, 0x14, 0, W|No_sSuf, { Imm8|Imm16|Imm32|Imm32S, Acc|Byte|Word|Dword|Qword } > adc, 0x80/2, 0, W|Modrm|No_sSuf|HLEPrefixLock, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > +adc, 0x10, APX_F|x64, D|W|CheckOperandSize|Modrm|EVex128|EVexMap4|No_sSuf, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > +adc, 0x83/2, APX_F|x64, Modrm|EVex128|EVexMap4|No_bSuf|No_sSuf, { Imm8S, Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex } > +adc, 0x80/2, APX_F|x64, W|Modrm|EVex128|EVexMap4|No_sSuf, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > > neg, 0xf6/3, 0, W|Modrm|No_sSuf|HLEPrefixLock, { Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > + > not, 0xf6/2, 0, W|Modrm|No_sSuf|HLEPrefixLock, { Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } > +not, 0xf6/2, APX_F|x64, W|Modrm|No_sSuf|EVex128|EVexMap4, { Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex } Looking at just the additions up to here, I'm getting the impression that in this patch - despite its title - you only add non-ND, non-NF insn forms for previously non-VEX-encoded insns. This could do with clarifying, by both making the title more concise and by stating the exact scope of the work done in the description. > @@ -1312,13 +1330,16 @@ getsec, 0xf37, SMX, NoSuf, {} > > invept, 0x660f3880, EPT|No64, Modrm|IgnoreSize|NoSuf, { Oword|Unspecified|BaseIndex, Reg32 } > invept, 0x660f3880, EPT|x64, Modrm|NoSuf|NoRex64, { Oword|Unspecified|BaseIndex, Reg64 } > +invept, 0xf3f0, APX_F|EPT|x64, Modrm|NoSuf|NoRex64|EVex128|EVexMap4, { Oword|Unspecified|BaseIndex, Reg64 } > invvpid, 0x660f3881, EPT|No64, Modrm|IgnoreSize|NoSuf, { Oword|Unspecified|BaseIndex, Reg32 } > invvpid, 0x660f3881, EPT|x64, Modrm|NoSuf|NoRex64, { Oword|Unspecified|BaseIndex, Reg64 } > +invvpid, 0xf3f1, APX_F|EPT|x64, Modrm|NoSuf|NoRex64|EVex128|EVexMap4, { Oword|Unspecified|BaseIndex, Reg64 } > > // INVPCID instruction > > invpcid, 0x660f3882, INVPCID|No64, Modrm|IgnoreSize|NoSuf, { Oword|Unspecified|BaseIndex, Reg32 } > invpcid, 0x660f3882, INVPCID|x64, Modrm|NoSuf|NoRex64, { Oword|Unspecified|BaseIndex, Reg64 } > +invpcid, 0xf3f2, APX_F|INVPCID|x64, Modrm|NoSuf|NoRex64|EVex128|EVexMap4, { Oword|Unspecified|BaseIndex, Reg64 } I don't think NoRex64 belongs in any EVEX template. > @@ -1418,7 +1439,9 @@ pcmpestrm, 0x660f3a60, SSE4_2|x64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf, { I > pcmpistri, 0x660f3a63, , Modrm||NoSuf, { Imm8, RegXMM|Unspecified|BaseIndex, RegXMM } > pcmpistrm, 0x660f3a62, , Modrm||NoSuf, { Imm8, RegXMM|Unspecified|BaseIndex, RegXMM } > crc32, 0xf20f38f0, SSE4_2, W|Modrm|No_sSuf|No_qSuf, { Reg8|Reg16|Reg32|Unspecified|BaseIndex, Reg32 } > +crc32, 0xf0, APX_F|x64, W|Modrm|No_sSuf|No_qSuf|EVex128|EVexMap4, { Reg8|Reg16|Reg32|Unspecified|BaseIndex, Reg32 } > crc32, 0xf20f38f0, SSE4_2|x64, W|Modrm|No_wSuf|No_lSuf|No_sSuf, { Reg8|Reg64|Unspecified|BaseIndex, Reg64 } > +crc32, 0xf0, APX_F|x64, W|Modrm|No_wSuf|No_lSuf|No_sSuf|EVex128|EVexMap4, { Reg8|Reg64|Unspecified|BaseIndex, Reg64 } There's quite a bit of logic in tc-i386.c to get CRC32 right. I wonder if you can really get away without adjusting that logic to also take effect on the EVEX encodings. > @@ -3408,3 +3487,4 @@ erets, 0xf20f01ca, FRED|x64, NoSuf, {} > eretu, 0xf30f01ca, FRED|x64, NoSuf, {} > > // FRED instructions end. > + Nit: Stray change. Jan