From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on2057.outbound.protection.outlook.com [40.107.15.57]) by sourceware.org (Postfix) with ESMTPS id 674FE3858CDA for ; Mon, 6 Nov 2023 16:08:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 674FE3858CDA 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-Filter: OpenARC Filter v1.0.0 sourceware.org 674FE3858CDA Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.107.15.57 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1699286890; cv=pass; b=nebuoL8xaCfegbAbpjzoi80RiB/f/G2Z86v2vZgra7xbwr2vPubk4Eii4SqbxAt4cZ6DowXrsoXR7uFVqliKHsw3IRnyFE4V2Fv9JglEWDMNLp6GSFyv1AJZSdfy5NqKNWKPZw6q+i5lHk2oGEA3UdJgQ7l9YceWafMBtbAf768= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1699286890; c=relaxed/simple; bh=Oo/CA4LUeX+JO0BPreUuV0z0k2cS+tOfkWFex5mfWaw=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=Drd/YRTkgSZazJfbt7b0lk2juE4izkWSQ4BQHw4JxFh8S8eIyojykwKFoQgwN+KhAXC/zAf92G202dWwA2gK5nz0tk1uiRsw3FnUsjQnczPBW3NcfE5pS6p0mUvwuuvi4jChEmM8h8P6BKoMZ1YruoFxKHMnW96vg8KVBgnAgVM= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FmUrypX5nKSJFBUZZSz9mb3/u+YBEHs0K+tm9IRCu8Rhuo8+FM/9MUSaHjsYQZ5ZOLjRVgplfJPKIhLEPf+azRdX7HtR8gK+p5zNq0MscQMJRjBsWaoVtfueq+PVOoHisTRmhjr/xX7fL5Bw9/2w5rJLs+2exc4D5tfBLZtrU+LfX0xx/H2dZzSlZS6R3ZXUEzL2T3ONkSfk/EX6iUJbOSZa8TtbvxtxX/NWcERzpnrByV3QL/DA1ya68fnPafiWK3rapzLdHyAjq8/l2nb4GDblBZQh1KZV9k7NlXEvskX+h06oBMaLCIU/hvhc4Ltxgze32PpTQfixJtTaZXCCPw== 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=xxXTVwX59W/qXwJ9XEUzSm5pBRi4fVkjjZrfwYkVeKI=; b=bpOpBqHsGSAaLwKr+ZjIcB7CS5QtzdeqQDnO0QAdm7HD/cfx+AnwJvy/q0ImtUGFmzJcU1KyQSLVEw2go6G+XSaflqBiDVoL5RpC9awgK+ZF1qB2orSa6NlasO0kTtYKFEO5ls6XDH7uLtGPl1QcaYL37kSOBFEWGgiP8aeNegS6d/oXuI5rDn/qjeUzWXgOHiBiFhe+NSRYBSyFSILEEPaAyjEO7q5KbXG8XhGgno5YiPPzn237mZeQk3sNq31LzU+O+zbbUqdByqkjoKsr8irMwAUbnYhhUMuNGkWhEZPYQTo9xGs1fWV4AyXe3foMy99s02yB880zSnf4S93dTQ== 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=xxXTVwX59W/qXwJ9XEUzSm5pBRi4fVkjjZrfwYkVeKI=; b=1JUJFb6SUmCazTQOuSRIWkjOMjRXh059k82+n0xpU6AyZlQez0gMvkWJfi+vBPQx8Ch3Esv4vhk3zALeNR9Hvo8N8uISISiByfmslgfAafNIvhx+QJRSAsWzsHuzEKYY7en8exL9zV2YFi2Pol9Iwp9Rcjp7PNPRp1Schk8MPwfBnwAFxBBMS97cJ9FUijU6jNLsRy/gO24O62t49F1NxpGPqE77mRRiuTTmSy6DPhA2CbzjrWyaFDPFK2dpa54FSezPmyPx1efUsfYLe3kR+1yuwiXBA0kzN6hfS199+oufi7A/MuI0zb1F7vK/QRRpPIPoe7XFdGuaA2Me4UzX1A== 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 AS8PR04MB8561.eurprd04.prod.outlook.com (2603:10a6:20b:420::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6977.16; Mon, 6 Nov 2023 16:08:04 +0000 Received: from DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::eb8e:fa24:44c1:5d44]) by DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::eb8e:fa24:44c1:5d44%3]) with mapi id 15.20.6977.016; Mon, 6 Nov 2023 16:08:04 +0000 Message-ID: <9a6b01c3-4dd4-731e-d677-e19d092b16c4@suse.com> Date: Mon, 6 Nov 2023 17:08:02 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH 1/8] Support APX GPR32 with rex2 prefix Content-Language: en-US To: "Cui, Lili" Cc: "Lu, Hongjiu" , "ccoutant@gmail.com" , "binutils@sourceware.org" References: <20231102112911.2372810-1-lili.cui@intel.com> <20231102112911.2372810-2-lili.cui@intel.com> <9373c79d-85bb-15bd-4501-7634687d9a8e@suse.com> From: Jan Beulich In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR2P281CA0105.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:9c::17) To DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PR04MB8790:EE_|AS8PR04MB8561:EE_ X-MS-Office365-Filtering-Correlation-Id: f5b6780c-d312-424f-7f50-08dbdee28ec6 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: E6q49+DGsKFqxASCrxCuHWwgzaFvd284zFYk8Dmi4CFoT/5YMWzLA/TUqc28ULoOeZKAKYiLEmXRp5Vdgjfx4w2dHeXKvodYaZ55BSBs2YepQFE30n49LMB7Ty+KE8y10zAcxVH4K8R3lgF4qAa3Jg0EdM3ci/12ixyfs2aflWdZzbVfoOc7HJO55zSa6TV6JD+YKtQjcv+nLu4Zi/k2QeqJZDygONBY70z0uZWeXn79NOlYtDANzsxnNORrSr5849q0KWVlyAmnRHYWI4+lKzl2Kf9WBPbYoYEbqkpubg4vtqevqLcJurneQwJE5ZIVVGdUm+HZOYc5KhGg8Vq8jSDtzQHxGAtiPc40J0K5TuQROWtJTix0G/v3Nr41AbHdml5pF/blycUOoHS3yiZCx5k04WWTyccbRaQb+UUL/NhN6MOMxlVVNuSHS08HvaIUiefMx5WqVEL/dsiTrrvvOT9weT+Kjv66k9vMVxUnb0a5SOm5XkdpnCufRK0HlGbG2RKYlfwgfhPW9y07mx7AVlBnjMWyu+PPucYzy2CK0t5i34WT4TNUj8qBSVxFwvKoW3BY+RGbDVsZgUQ30b3LBY0db5tQj9QqX1P+FFysuBtBbsUfTV6yrG7+xg18DLPS8GRkezloOefz9Jixo2xHrg== 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)(376002)(396003)(366004)(136003)(39860400002)(346002)(230922051799003)(186009)(1800799009)(451199024)(64100799003)(31686004)(2616005)(6512007)(53546011)(6506007)(478600001)(6486002)(2906002)(30864003)(38100700002)(36756003)(86362001)(31696002)(66556008)(54906003)(41300700001)(66946007)(66476007)(5660300002)(83380400001)(26005)(316002)(6916009)(8676002)(4326008)(8936002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V29xbGdFd0NCelZHZ2FMMmErYVF0alpSZW9sMUhIclZXaGFWcG5JcFByb2U3?= =?utf-8?B?T244WmlobkUxY0wveW5yV3JaZWNDc2hBd1VtaE5TOVNTVTUvQng2bEtBTDVv?= =?utf-8?B?dXdnUm9QTWErOFJGcENrc1N5VmFRZVpvT3JyWjZFdTRSbTZoZG9JRU4rdkww?= =?utf-8?B?TWxNZkgxdXVDSVk1TC9FbU12by9RdXlwUWc4d1MycDB5MmZ4cjlmWFAwaE5u?= =?utf-8?B?bnBjUDdIZlVYWllnOFBObFd5bFNORDhMZjhBSU5uZm05eXZzUkc5S1lHbUxB?= =?utf-8?B?ZlV4Ujl3ckIwYUpuQmVWQkNYS1FHSkJQcXhmdmdPeVFZTkhRTmVYV1NyZlFn?= =?utf-8?B?VUx2VjA0VHd2RmtlZWNTU29wYklZU3JJM05zZmxtdEt0NExRTlZZaXBEYkp5?= =?utf-8?B?bjlSVE1qekliUjN1R3Z4ZTJqS01HU0ZZdldGTU5rUTIrSFhnSWZ2V0JhelR5?= =?utf-8?B?MmxXNW1PdFJqTExzTjhockpXbXplTm1KaEFrNkhPMnZMTkhQTUNYM1MzTUwy?= =?utf-8?B?aEN2TXh4dlBDTERrRnErR05zNFNCZ2dPME42Kzc0OW55UTUweWM3ajZnS3J2?= =?utf-8?B?QWlhRFNiR3NUbzBteVFqYzE4YXhhRm1Oc3FYWFBrdDQ2RnBpRW40UHRwY1lI?= =?utf-8?B?b01VelpvY2pyQkpZU2xHbDU2enVtV0thR1JLV2w0SnlOaEdnWlZ6MDQ4M2RE?= =?utf-8?B?MGd6aGwvWlZmeEhQdFdIeldoUGNDRmJPbzc3VEhhcWFSOWEvK2RXZ25SQkNL?= =?utf-8?B?ODNnb2c4T2x0U3lqRVVmNFdBKy9nd25iODd0VERWcjd6b1hkK0w1SnU4K0J4?= =?utf-8?B?N244U0FibEF3UWZxOVdTTEw3Ukk4L2NYNkY0bmNOU1M2OXFJeXhTMXgyb3lH?= =?utf-8?B?eXlYSXJMMENjdHYwaDRGM0tlZlNuOVhEbkRudmNpNDYreW9venNIVkgzWkNy?= =?utf-8?B?dGdpV2s1Qm5BQ3BjTDFZbER3N0FWc3k5L0ozWUgyNE1GVVV4TmVvcG96VUhG?= =?utf-8?B?M2pQQ2pWeXF6ekQ5M2xGck9zS29DeUYwVGhKMi9URmlGTXlzZ3ExSlJYdCta?= =?utf-8?B?Y3hNblhZYkQ5SEdSVUpDSFBnbGt4eE0xQjRpb0hvNjg0akJtelZsN2JCSFQ1?= =?utf-8?B?YVR1bzhKQ0lWSVpYcEFrREtKdmJneTVTQ2FaTWhTRVNtQUpGd0xNL0MwSUd1?= =?utf-8?B?c2pycWlXYVdzTFQ4Nlp0NWViZTJZbmNuaFhvWC83ME8xMk9nT1IwUEFXcVdu?= =?utf-8?B?a3pkdlBnRS9SZTFsVzZVdXRid2lqdmFYM0lpWjhtdW1mM0daZ0hNSGtnU2JX?= =?utf-8?B?VStWSjNoR3ZRVi9iNVdWN3Z3ZHhuRjdtUWw2elhxcDRseW95VENqU2dXampT?= =?utf-8?B?eG5QTmJWeU52dXpQbnJCTWxYNytwMmVQcUJLNDdMNHhiOW1uMFRBN3c2dnZM?= =?utf-8?B?MTNJSndpVmI5NnU3SXNnTmpEdGo4cGNWV1hLOUN6ZXJxK3E3UXpnYXF2Y0tL?= =?utf-8?B?VGdOYlYzNnNKUTV5ZkpyZlFWNmM2UE1xT1J4bExvYkV1cmw4cUhGY3RTazg2?= =?utf-8?B?YjlqbDZpeXVyYTJGUVg2UmlCdDlPN0k4eUVQdzgzVExEVEJ5eGZud1ZuOTBY?= =?utf-8?B?WUhBTldyVUoyQ3E1SGhKaUVlTnptMDF1bXh0NUtNMnBNUjArQ0pBSDcwRC9G?= =?utf-8?B?d3AwbHUzbVoxTHJQbDdxRWJGU2hGNFlBbXcrMjV5YmZ2NS9vOFoyZkFWb1Mz?= =?utf-8?B?WW9MSXI0ZVhlb2VwWUZaODZCZE5XVlRDbDJiUmRhUUQ1WU5hbnhEb1ArT1Zp?= =?utf-8?B?Vmxhc2V3c0g4YnQydWxlZ3ZKNm1JNXhBVXNPNTZES1c5U2ExcW9XSTNYa090?= =?utf-8?B?SXpXWTAwR2NmYjg3ZmkwQW4xNUZ1SVlaWGU4TUkzZ3ljdC9pVjNEeS9EVTNp?= =?utf-8?B?dktCODdiV1FZc25UM3RkSDVjNGVBQjBCUm4wcGtobDYwOE5ocnNMbi81OTc1?= =?utf-8?B?ODNpeWpwQUIxbWNUdVhvTSs5MWFVK3IxZitzcWU3VHR5aUtPbVpWL3VCRnFJ?= =?utf-8?B?NS85TEVUTlVuVUVqeGZBUm93Kys4dWRXaEtiTElsVXpmTmlRL3FWVWxmczJ2?= =?utf-8?Q?snt/CxjxIHigiCuvp2Ep4lZI8?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: f5b6780c-d312-424f-7f50-08dbdee28ec6 X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8790.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Nov 2023 16:08:04.3472 (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: STnoEk0+DeUsBHezwrDau5Qwvba9OjUH1yd03CnwMZY0lUlztVMepY177p17hs2e+nCgzeSBFPFXtTnhioWZ1Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8561 X-Spam-Status: No, score=-3028.1 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,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 List-Id: On 06.11.2023 16:20, Cui, Lili wrote: >> Subject: Re: [PATCH 1/8] Support APX GPR32 with rex2 prefix >> >> On 02.11.2023 12:29, Cui, Lili wrote: >>> @@ -406,6 +409,11 @@ struct _i386_insn >>> /* Compressed disp8*N attribute. */ >>> unsigned int memshift; >>> >>> + /* No CSPAZO flags update.*/ >>> + bool has_nf; >>> + >>> + bool has_zero_upper; >>> + >> >> Can both please be introduced when they're needed, not randomly ahead of >> time? > > Moved has_nf to patch 2/8 and deleted has_zero_upper. Patch 2/8? Not in this series then, I suppose? >>> @@ -4158,6 +4182,19 @@ build_evex_prefix (void) >>> i.vex.bytes[3] |= i.mask.reg->reg_num; } >>> >>> +/* Build (2 bytes) rex2 prefix. >>> + | D5h | >>> + | m | R4 X4 B4 | W R X B | >>> +*/ >>> +static void >>> +build_rex2_prefix (void) >>> +{ >>> + i.vex.length = 2; >>> + i.vex.bytes[0] = 0xd5; >>> + i.vex.bytes[1] = ((i.tm.opcode_space << 7) >>> + | (i.rex2 << 4) | i.rex); >>> +} >> >> I may have asked on v1 already: For emitting REX we don't resort to (ab)using >> i.vex. Is that really necessary? (If so, a comment next to the field declaration >> may be warranted.) >> > Added comment for it. > > /* For the W R X B bits, the variables of rex prefix will be reused. */ > i.vex.bytes[1] = ((i.tm.opcode_space << 7) > | (i.rex2 << 4) | i.rex); How does the comment relate to the (ab)use of i.vex? >> Speaking of v1: Can you please make sure you have correct version tags on >> submissions of updated patch versions? >> > I used git to send all the patches at once( git send-email --cover-letter --annotate --to="..." -8), which only has the opportunity to change the version of the cover letter patch. To change the version of each patch, I can send them one by one next time. By the way, do you have a better way? Or how did you modify them? Thanks. Well, personally I don't use git to send patches. But I know people send series with proper version tags throughout, all the time. >>> @@ -5278,6 +5319,9 @@ md_assemble (char *line) >>> case register_type_mismatch: >>> err_msg = _("register type mismatch"); >>> break; >>> + case register_type_of_address_mismatch: >>> + err_msg = _("register type of address mismatch"); >>> + break; >> >> I have a concern with wording / naming here: If I saw this in an error message, >> I wouldn't know what is meant. Maybe something along the lines of "cannot >> use an extended GPR for addressing"? And then the enumerator suitabley >> renamed as well? >> > Changed to > > + case unsupported_EGPR_for_addressing: > + err_msg = _("unsupported EGPR for addressing"); > + break; May I suggest "extended GPR" in the message text (the enumerator is fine to have EGPR)? >>> @@ -5594,6 +5641,13 @@ md_assemble (char *line) >>> return; >>> } >>> >>> + /* Check for explicit REX2 prefix. */ >>> + if (i.rex2 || i.rex2_encoding) >> >> This open-codes is_any_apx_rex2_encoding(). But read on. >> >>> + { >>> + as_bad (_("REX2 prefix invalid with `%s'"), insn_name (&i.tm)); >> >> There's no REX2 prefix; {rex2} only sets i.rex2_encoding. Question is what case >> the i.rex2 check above is intended to cover. Error message comment, and >> condition want to reflect that. >> > > Removed i.rex2 and keep i.rex2_encoding here. Added one invalid testcase for it. > > {rex} vmovaps %xmm7,%xmm2 > {rex} vmovaps %xmm17,%xmm2 > {rex} rorx $7,%eax,%ebx > + {rex2} vmovaps %xmm7,%xmm2 Right, but please see my "optional vs required" comment in the pseudo- prefix related patch I did send earlier today. I question the correctness of the {rex} related check here, which would then extend to the {rex2} one as well. >>> + for (unsigned int op = 0; op < i.operands; op++) >>> + { >>> + if (i.types[op].bitfield.class != Reg >>> + /* Special case for (%dx) while doing input/output op */ >>> + || i.input_output_operand) >> >> Why is this needed? The register table entry for %dx ... >> >>> + continue; >>> + >>> + if (i.op[op].regs->reg_flags & RegRex2) >> >> ... doesn't have this bit set anyway. >> > > For this special case i.op is empty, we need continue, otherwise r i.op[op].regs->reg_flags will cause segment fault. I vaguely recall commenting on this anomaly to H.J. - perhaps time to fix that properly (in a separate patch), to not leave this trap open any longer? (Otherwise at least a comment is needed here.) >>> + { >>> + i.error = register_type_mismatch; >>> + return 1; >>> + } >>> + } >>> + >>> + if ((i.index_reg && (i.index_reg->reg_flags & RegRex2)) >>> + || (i.base_reg && (i.base_reg->reg_flags & RegRex2))) >>> + { >>> + i.error = register_type_of_address_mismatch; >>> + return 1; >>> + } >>> + >>> + /* Check pseudo prefix {rex2} are valid. */ >>> + if (i.rex2_encoding) >>> + { >>> + i.error = invalid_pseudo_prefix; >>> + return 1; >>> + } >> >> Further up in md_assemble() {rex} or {rex2} is simply ignored when wrong to >> apply. Why would an inapplicable {rex2} be treated as an error here? This >> would then also ... >> >>> @@ -7125,7 +7230,7 @@ match_template (char mnem_suffix) >>> /* Do not verify operands when there are none. */ >>> if (!t->operands) >>> { >>> - if (VEX_check_encoding (t)) >>> + if (VEX_check_encoding (t) || check_EgprOperands (t)) >>> { >>> specific_error = progress (i.error); >>> continue; >> >> ... eliminate the need for this change, which is kind of bogus anyway: >> There are no operands here, so calling a function of the given name is at least >> suspicious. >> > > We have these tests and I'm confused whether to remove them or not. > > + #All opcodes in the row 0xf3* prefixed REX2 are illegal. > + {rex2} wrmsr > + {rex2} rdtsc > + {rex2} rdmsr > + {rex2} sysenter > + {rex2} sysexitl > + {rex2} rdpmc They should all stay. But as to my comment: There's no use of any eGPR here. If you want to abuse that function and if there's no better descriptive name for it, then once again at least a comment is needed. (Considering this, the attribute's name NoEgpr is probably also misleading in the cases here, i.e. when there are no operands. Hence, if not to be renamed, requires yet another comment in i386-opc.h.) >>> @@ -14131,6 +14258,13 @@ static bool check_register (const reg_entry *r) >>> i.vec_encoding = vex_encoding_error; >>> } >>> >>> + if (r->reg_flags & RegRex2) >>> + { >>> + if (!cpu_arch_flags.bitfield.cpuapx_f >>> + || flag_code != CODE_64BIT) >>> + return false; >>> + } >> >> Please fold the two if()s into one (unless of course you know that the outer >> one is going to be extended in a subsequent patch). >> > > Yes, other code will be added in the outer if with patch2/8. Hmm, you again say patch 2/8, yet that patch in this series clearly doesn't do anything like that. >>> --- a/gas/testsuite/gas/i386/ilp32/x86-64-opcode-inval-intel.d >>> +++ b/gas/testsuite/gas/i386/ilp32/x86-64-opcode-inval-intel.d >>> @@ -11,11 +11,11 @@ Disassembly of section .text: >>> [ ]*[a-f0-9]+: 37 \(bad\) >>> >>> 0+1 : >>> -[ ]*[a-f0-9]+: d5 \(bad\) >>> +[ ]*[a-f0-9]+: d5 rex2 >>> [ ]*[a-f0-9]+: 0a .byte 0xa >>> >>> 0+3 : >>> -[ ]*[a-f0-9]+: d5 \(bad\) >>> +[ ]*[a-f0-9]+: d5 rex2 >>> [ ]*[a-f0-9]+: 02 .byte 0x2 >>> >>> 0+5 : >>> --- a/gas/testsuite/gas/i386/ilp32/x86-64-opcode-inval.d >>> +++ b/gas/testsuite/gas/i386/ilp32/x86-64-opcode-inval.d >>> @@ -11,11 +11,11 @@ Disassembly of section .text: >>> [ ]*[a-f0-9]+: 37 \(bad\) >>> >>> 0+1 : >>> -[ ]*[a-f0-9]+: d5 \(bad\) >>> +[ ]*[a-f0-9]+: d5 rex2 >>> [ ]*[a-f0-9]+: 0a .byte 0xa >>> >>> 0+3 : >>> -[ ]*[a-f0-9]+: d5 \(bad\) >>> +[ ]*[a-f0-9]+: d5 rex2 >>> [ ]*[a-f0-9]+: 02 .byte 0x2 >>> >>> 0+5 : >> >> These expectations match the ones of the same test in the parent directory. >> Hence instead of adjusting each in both places, please have the ones here >> reference the parent directory files. >> > > They are used to test illegal opcodes for x86-64. Since D5 now makes sense, these two test cases were removed. > > # All the followings are illegal opcodes for x86-64. > aad0: > aad > aad1: > aad $2 Right, but how does this relate to my request to simply fold the expectations here with that of the same test in the parent directory? (You'll find various examples under ilp32/ where I've done such folding already, whenever I had to touch both instances anyway.) >>> @@ -1008,10 +1012,35 @@ get_element_size (char **opnd, int lineno) >>> return elem_size; >>> } >>> >>> +static bool >>> +if_entry_needs_special_handle (const unsigned long long opcode, unsigned >> int space, >>> + const char *cpu_flags) >>> +{ >>> + /* Prefixing XSAVE* and XRSTOR* instructions with REX2 triggers >>> +#UD. */ >>> + if (strcmp (cpu_flags, "XSAVES") >= 0 >>> + || strcmp (cpu_flags, "XSAVEC") >= 0 >>> + || strcmp (cpu_flags, "Xsave") >= 0 >>> + || strcmp (cpu_flags, "Xsaveopt") >= 0 >> >> Upon further thought for these (and maybe even ... >> >>> + || !strcmp (cpu_flags, "3dnow") >>> + || !strcmp (cpu_flags, "3dnowA")) >> >> ... for these, but see also below) it might be better to add the attribute right in >> the opcode table. >> >> As to the 3dnow insns - I think I'd like to revise my earlier suggestion to also >> tag those. Like e.g. FPU insns they're pretty normal GPR-wise, so allowing them >> to be used like that would appear only consistent. Otherwise, if we were >> concerned of AMD extensions in general, SSE4a insns (and maybe further >> ones) would also need excluding. (Additionally recall that there's an overlap >> between 3dnowa and SSE, which would result in another [apparent] >> inconsistency when excluding 3dnow insns here.) >> > > I see, for example I think I need to split this table into two parts, one is for SSE and one is for 3dnowA, then add noegpr to the SSE one, right? > pextrw, 0xfc5, SSE|3dnowA, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|NoRex64, { Imm8, RegMMX, Reg32|Reg64 } I'm afraid I don't understand the question. All I've asked for is that the special treatment of 3dnow insns be removed again. Unless you want to special-case further insns; it's not really clear to me what's best, as both approaches have noticable downsides (either we allow to encode something which may never become valid, or we disallow something which may become valid). In any event adding NoEgpr to any SSE insn sounds wrong to me - aiui they can all be encoded with REX2. Jan