From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM0-obe.outbound.protection.outlook.com (mail-am0eur02on2066.outbound.protection.outlook.com [40.107.247.66]) by sourceware.org (Postfix) with ESMTPS id 3BDEA3858D37 for ; Thu, 26 Oct 2023 08:31:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3BDEA3858D37 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 3BDEA3858D37 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.107.247.66 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1698309090; cv=pass; b=fVl8PiX463Ic2U3JTi2GKIFS0lAm3wQ+uQF7hgGMSqL2D0MQife8H/Gs5lnDwZwvL+cp1dDSDDS4PTOXpj+Kw7qvzr3XJLkmdmBkSbu2+6GyZ4Oj9oHrhyJ3ndSLUjn8AG9rZiB26iNCCZfNtBKrKwDKE5SrnpAPYQhJiHm2cSs= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1698309090; c=relaxed/simple; bh=U0Qz9xudmqKQeFY7Qow/nikW7GLwN2pK8LFFJZDJCEo=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=PZr9ubFy10kli1vYhkhZX8rikRGjZrHZUX1ieA8PwY6mMoayHDW2Dqtt0JC+RpI0GPTr2Y7duKGjSDbcziU+VgLIbIQ5wuhypBN4FRHF1zspcwexuOr5ca/WCJGopKENzn0Eo20T5HooLaQmFkSFrBlnx6ugkyvOtnOMpyEMzIg= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=b0ZxmmuO7W0jXUzuEwVak+U5ExIs8X+DxyYaHuM/X68eLioHSeg8ov28CI3E2TIVePwBvEyr3Gcbea5sjnDHB6ln+hU/RCKjyNlZe1ZRVu7R4QM1rcLgypYKv5qhNtPAH7VS5FJa7gvUFQ6Xk+kJioq1x9fLqDLQv4Bb+n/sHNkKN44JFFKfewYaGo2gmzoiUX9PfHxQW5UfrUrQkNZvE8m/id00HeLPwG6mOU/W5CPuQNxQNdbUeXc7l3uNZaEcFyRZ5e1PnZwoM7oqCBBrRUPNtIcKN821OXfMteXCakO/xnc6RUPpyimei6pU/yc5ucRrb79JyW6ZndjYCDHphw== 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=RYRDhxCPMaXKJUzgQ4Gz3X/IQR9oyU57ueTaN6N5EDk=; b=Tb9GWadlLCLxe6RG45jVgeMBcTMUD0jv9Wdsq6M3iEJBe7HPqUZGD0hdJckOBgsiDpNB/HumlWNy6RVG+dutqWwufN1TYXxv01CFcAD/ahWgON6/6E3P/L2A9lW5JjafyWYT5EheJrDAtCGj4YXAbpoVzZyIKsIZM3wtSpvoMSDbEKCynt/I/1P/QQkR7hbmAnNUrN0OC9rujpfaeEN3ZJoC74FC4fJFPHBiKUyrzPKXp3kUmcEwNSWiXU4HDu37cizJ6SztEiblqyeuyinHHyd10/JVGISkcGifDEmurHy8O1MgvcNXoJMwdOU2eVkga1ypR5+jrXKR3LzcFR1lbQ== 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=RYRDhxCPMaXKJUzgQ4Gz3X/IQR9oyU57ueTaN6N5EDk=; b=2/g6cnEKIofZY47Yihz8B4qzcl2ahheIjdjZAxyOS6n9MaQo1gUfRKAwlg7+EytkaZACChZdR5px1YmKYzUpywQz1Kt8FiBqQvdpvOWJSUWhay1u5nJ2Q1a2YNY4w7fKu0ipoSdirCFEm0B3kZwFyCvXdOSbaSk/x/ODHlwFwxOGlUTre/pck2Gc2mjVWTb4dj9yfq759MI/mrbK/sC6QkHPU/a3GnYtrfx3oDSdCOFdn3jfU111VDBNaPfkmDu70GlcnSdLnTfiLtzfD/qnyUkv94CsBsxcwbxj5NN/dJqFnWhSEbdI94TeVbpVSMb0qooGTp+30G7Tjxy9JHSfCg== 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 AM9PR04MB8308.eurprd04.prod.outlook.com (2603:10a6:20b:3e3::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6933.11; Thu, 26 Oct 2023 08:31:25 +0000 Received: from DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::d924:b650:a2ad:7b25]) by DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::d924:b650:a2ad:7b25%3]) with mapi id 15.20.6954.008; Thu, 26 Oct 2023 08:31:25 +0000 Message-ID: <689f0001-ba13-cdf3-b644-f21835a83c82@suse.com> Date: Thu, 26 Oct 2023 10:31:22 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH][v4] Support Intel USER_MSR Content-Language: en-US To: "Hu, Lin1" Cc: hongjiu.lu@intel.com, binutils@sourceware.org References: <8a91a57b-9cd7-7541-557a-82c30a3debfc@suse.com> <20231026062158.3054598-1-lin1.hu@intel.com> From: Jan Beulich In-Reply-To: <20231026062158.3054598-1-lin1.hu@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR0P281CA0174.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:b4::7) To DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PR04MB8790:EE_|AM9PR04MB8308:EE_ X-MS-Office365-Filtering-Correlation-Id: eb27fa56-cd8f-4384-3e60-08dbd5fdf0e4 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: SMx+KrH0QjudZmJak6+eOrNaexb0v2U5ViTAjJtcnBqtc3WbNw2e8MyAe5pKGw6uJbF5z1g5kChKfB2WhbC3snbvPt8UlEfk7jl1YLuhr25KhKPZZBogHvQzx4DQdf4n9xV3F5/iyJhGUgysBUsWN7GQ1HJr3djuXOvfv9Jsiy5PyZRmEyWec1H0YEy9CwYaTiXWkWuFAq0+KIS4q7FCgMlMZ3b0PRyi0hzNR/RJWlm3qegEbHfEsMEeoIJx+aPa3XasgjyD/XaVz8cBfbe+gCWp0vfXvlyWs3f+AzOT2fk+hY1nCFyUGA+EcWcOhkC9TsWzbOcEa5aQHYz2NFBXZ21xgV0lixw+vcnJECs566N5j53dnaLztjyjqg5XQk61mvBmSL3H0dWEwoQ+VQbLsSLWF6JAiR/oLi4+snvHG3/ykCq+msNnzgkz+Q0o5SpQXYdRGMVQpBF6Gsscmjuz3ozeZIUmo2OrEMyrOk/KY3fCSSgLxkm0iPdIak7Ig0jrRpIaifdix2z1kjIH14+oCeJn4REM4o8AabhCUYQKvN2CrfaVh4Tk8l3ipdVY6GaZbQowEcnFctAS7/YicBRGHkTNvFR26FazelXfT0jlAtKQCUVHEHyxnHabwTALMjiVtqVCgGaDxzi6Z/SGkVaruA== 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)(366004)(346002)(136003)(396003)(39860400002)(230922051799003)(64100799003)(186009)(451199024)(1800799009)(8676002)(8936002)(2906002)(41300700001)(4326008)(5660300002)(6666004)(2616005)(36756003)(26005)(53546011)(38100700002)(6512007)(6506007)(83380400001)(31686004)(86362001)(66476007)(6916009)(66946007)(31696002)(316002)(66556008)(478600001)(6486002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZmFuWStMVlBqZHNuUGZ0M21JaVFvRUNxb0k2d0xURTZXc2k3MDNhR2JsOWQr?= =?utf-8?B?QXRKRitUZE40MUxidXlvdDJVWmNnUDdwemlnVjNId3dyWmNpalVGTEdNSndV?= =?utf-8?B?L3pNMDh2YUZBbkoveHMzS3lLNy9FaFA4TDRMSnU1UFREVFIrOU5qdElJYk40?= =?utf-8?B?QnA0cFZwaEdsL0cvTDZzaTlSVklycE42Uy96ZjhkVmE5TnhDTmt2Q3gxTldn?= =?utf-8?B?Vnk1eXY4WWVYeTE1enRPU0xza0pQbk9HRUxMbFBRT2hyM28yOVM1dVQ4YTJm?= =?utf-8?B?VUVkaG5zUzYwZmRiSCtsMzhvSUk2RGFLMUNUNXN1TXRoTUt1ckF0bzg0azBi?= =?utf-8?B?T1MxNjE5UGg0T3IzaEpZMkovZ3dIV3U1VnVnM1RZMkJmamRRZnJVdjJYQWVp?= =?utf-8?B?eHJydGF3L01COWlXbmZhTVdvWGtLbjlVUW85VUh4NWhqQTRmWGR0eGNlUEx1?= =?utf-8?B?Q2lzVUhvQmtaSXZ1Wlp4Q2wvaGFORkJMaDBmMG5aQkY5MUdvUEdJN1dqekN5?= =?utf-8?B?SXg3eHFvMjhibXZFaStEbFRqOWVZWGYwNXhCVElCQzhNaGJSZnlmN1RqdjNU?= =?utf-8?B?eERzZjhML0laY1NWTU8rYkpqTW5uUWx2KzdMTzJFeDNZRFZqOTNhN3J5Y1Av?= =?utf-8?B?NUUwdys2RlNuYXNid1JkaWxGZ3BjNWlpZzhGN2tBeHJHZyt3dE5rZ3A3YWsw?= =?utf-8?B?RWs4OCtoc2pJL29QRk1LbWRvK2U2TTZVd29UZGwyc0E0akszM292MFFYYVpW?= =?utf-8?B?ZU5BL3QrRExDZmNIK0c1Tk5QbVVsUzhscWc3KzErNzZRbkRBQnhhejJRUGd6?= =?utf-8?B?Z3Vwa3dVS2hNenBoeDQ5Vmxkc1BFTjZxR01ObE5GcmlxNjUzc2Fyd2F4V2Zh?= =?utf-8?B?WmplaDVKMTRDODNxaFluc0ppQmV1UHN6OGR1SVFDWTQycEY2bWw0dHY5YUNw?= =?utf-8?B?NnhYUU0xSFYrYldVVFdHUmJWYzBsMGxJcW1xbGlPRVVrTUFoN0ROOXhOM215?= =?utf-8?B?OWhzbGVQM0toOHliOUFQMS9qYUlIaHQreHhiUzh3UkxuckpIRVZKN2VKMW84?= =?utf-8?B?UHM3OHcvSEJNUjBmUEdGalM5TDhpTVFCL1QybXVjNzczZlk3RTJpSXArVUUz?= =?utf-8?B?UWd6bjVld1RtbmV5bGwrcThLV3JtVFhBMXpLV3RvUXBRM3dyN0ErN1BQbGwr?= =?utf-8?B?TkJKMzR5VFBKNHNlUVF3UEs0L0txeURvM3lONXAxTlZvbGFIczlOZkdVRU5N?= =?utf-8?B?ZmczbjZVRFJSU014UitKMjZGUTh5d252L3BINGxMdjVYWjl5RU5aVnRKcFZP?= =?utf-8?B?ME4zVGVWV1NSaHN1T044SFAzczFzUmN4U2xhY0NmSHc4dFNrM2ZhVm1vZkV0?= =?utf-8?B?NS9oNU16Zlo5MURHUkp5aW5uMUJsL0JzaTVqU21lcUU5c2F4bTRNQyswYVdD?= =?utf-8?B?U3lQQWE0cDY2d3NTdy9KMjZtSkxKK2tXazFEWng1WTNUOTVPeW8rbVNSWkZV?= =?utf-8?B?eVpWeUpzYVRKemdHQ3lLZm1FOXAvOGFqb3BRaDNqZTJqV0hRUGVoWC92TVdY?= =?utf-8?B?aXEva1RrVkM0SlVrZ3I5bytJUmhPN1UwZGVOK2N2dFFXYzlqbXY3SG9Vencx?= =?utf-8?B?ZG1ZN2xkVUlJZGZXSVJHTERaK2g2blBZT2xLMC9CYWZvMVJqRDQrMmgrTlRw?= =?utf-8?B?R01PU05ZT3BjT3hXWi9LeHdteWZiVSsxSHM3d3N3STRod25DSGJCUUVjUnAr?= =?utf-8?B?dUdXUGNVTkMxRUh5SHVNUUtHbUwxNkZrWG9HOHZkTlpEdW1IWGlVRkc0MHlW?= =?utf-8?B?RGlvd3cvaW1IdWpYSklVWmsxRjhrNzBGQ09FYlRTSHNjai80UmdBN0EzNTNV?= =?utf-8?B?Ymh3cUlvc3BPQXRMVXZuWWNHZXFCeU90bjRYRzlnMVd3RDVZSXloMDRPTUp1?= =?utf-8?B?RW5yVkU2bnBnVUxsNGgyS3luTUNMNngwdzRmaWJ2WG9lUUk5WG44TzJ1c3Bu?= =?utf-8?B?N0hxaVJwOTM3b2hSdDJwN1V6NmRtRDlmVFJpMW5pdTBCRHdRWnJrTXZhYUln?= =?utf-8?B?V1pITnM1emRXbGhBRHllbUJka2dDWkdXTkc1VnVCNWhibGpGVUszVy9BdXZI?= =?utf-8?Q?+BMr7hHGJT9pa6VxP4JR2L3rb?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: eb27fa56-cd8f-4384-3e60-08dbd5fdf0e4 X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8790.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2023 08:31:24.9228 (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: Q9AnHBp8lESZ0C6/5pJAtbpqJu7oFTFVEIwwda1kLzQ1BcI+D5Y0G/xWTFxng7GXk5CLbw3ain+uzPY2Xm3ACg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8308 X-Spam-Status: No, score=-3034.4 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,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 26.10.2023 08:21, Hu, Lin1 wrote: > @@ -2504,7 +2505,8 @@ smallest_imm_type (offsetT num) > t.bitfield.imm8 = 1; > t.bitfield.imm8s = 1; > t.bitfield.imm16 = 1; > - t.bitfield.imm32 = 1; > + if (fits_in_unsigned_long (num)) > + t.bitfield.imm32 = 1; > t.bitfield.imm32s = 1; > } I fear this isn't correct for 32-bit code, where all immediates are deemed fitting in both 32-bit signed and unsigned. Otoh you surely ran the testsuite, and I would have expected mistakes here to be covered by at least one testcase. > @@ -2517,12 +2519,14 @@ smallest_imm_type (offsetT num) > else if (fits_in_signed_word (num) || fits_in_unsigned_word (num)) > { > t.bitfield.imm16 = 1; > - t.bitfield.imm32 = 1; > + if (fits_in_unsigned_long (num)) > + t.bitfield.imm32 = 1; > t.bitfield.imm32s = 1; > } > else if (fits_in_signed_long (num)) > { > - t.bitfield.imm32 = 1; > + if (fits_in_unsigned_long (num)) > + t.bitfield.imm32 = 1; > t.bitfield.imm32s = 1; > } Same issue here then, if any. > @@ -5235,6 +5240,17 @@ md_assemble (char *line) > if (i.imm_operands) > optimize_imm (); > > + /* user_msr instructions can match Imm32 templates when > + guess_suffix == QWORD_MNEM_SUFFIX. */ > + if (t->mnem_off == MN_urdmsr) > + i.types[0] > + = operand_type_or (i.types[0], > + smallest_imm_type (i.op[0].imms->X_add_number)); > + if (t->mnem_off == MN_uwrmsr) > + i.types[1] > + = operand_type_or (i.types[1], > + smallest_imm_type (i.op[1].imms->X_add_number)); My respective comment on v3 was left entirely unaddressed? > @@ -6358,8 +6374,11 @@ optimize_imm (void) > smallest_imm_type (i.op[op].imms->X_add_number)); > > /* We must avoid matching of Imm32 templates when 64bit > - only immediate is available. */ > - if (guess_suffix == QWORD_MNEM_SUFFIX) > + only immediate is available. user_msr instructions can > + match Imm32 templates when guess_suffix == QWORD_MNEM_SUFFIX. > + */ > + if (guess_suffix == QWORD_MNEM_SUFFIX > + && !is_cpu(current_templates->start, CpuUSER_MSR)) > i.types[op].bitfield.imm32 = 0; > break; Taking together the changes you make to smallest_imm_type() and the change you make here, I guess - to come back to an earlier comment of yours - it would be less risky if these changes were omitted and the new insns instead bypassed optimize_imm(), as suggested before as an alternative. > @@ -7566,6 +7585,18 @@ match_template (char mnem_suffix) > break; > } > > + /* This pattern aims to put the unusually placed imm operand to a usual > + place. The constraints are currently only adapted to uwrmsr, and may > + need further tweaking when new similar instructions become available. */ > + if (i.operands > 0 As said in reply to v3, can this please be "> 1"? There's no need to ... > + && !operand_type_check (operand_types[0], imm) > + && operand_type_check (operand_types[i.operands - 1], imm)) ... rely on the combination of these two conditions to never be true when i.operands == 1. Thinking about it, since operand_type_check() may - depending on what exact code the compiler generates - be comparibly expensive, how about if (i.imm_operands > 0 && i.imm_operands < i.operands && operand_type_check (operand_types[i.operands - 1], imm)) instead? > --- /dev/null > +++ b/gas/testsuite/gas/i386/x86-64-user_msr-inval.l > @@ -0,0 +1,7 @@ > +.* Assembler messages: > +.*:6: Error: operand type mismatch for `urdmsr'. > +.*:7: Error: operand type mismatch for `urdmsr'. > +.*:8: Error: operand type mismatch for `urdmsr'. > +.*:9: Error: operand type mismatch for `urdmsr'. > +.*:10: Error: operand type mismatch for `uwrmsr'. > +.*:11: Error: operand type mismatch for `uwrmsr'. > diff --git a/gas/testsuite/gas/i386/x86-64-user_msr-inval.s b/gas/testsuite/gas/i386/x86-64-user_msr-inval.s > new file mode 100644 > index 00000000000..6aff469485b > --- /dev/null > +++ b/gas/testsuite/gas/i386/x86-64-user_msr-inval.s > @@ -0,0 +1,11 @@ > +# Check Illegal 64bit USER_MSR instructions > + > + .allow_index_reg Yet another instance of this when it's not needed? > --- /dev/null > +++ b/gas/testsuite/gas/i386/x86-64-user_msr.s > @@ -0,0 +1,43 @@ > +# Check 64bit USER_MSR instructions > + > + .allow_index_reg Iirc I did ask to remove this, for being meaningless here. Please uniformly remove this from all the new tests introduced here. > @@ -8803,7 +8872,15 @@ get_valid_dis386 (const struct dis386 *dp, instr_info *ins) > ins->need_vex = 3; > ins->codep++; > vindex = *ins->codep++; > - dp = &vex_table[vex_table_index][vindex]; > + if (vex_table_index == VEX_MAP7) > + { > + if (vindex == 0xf8) > + dp = &map7_f8_opcode; > + else > + dp = &bad_opcode; > + } > + else > + dp = &vex_table[vex_table_index][vindex]; How about if (vex_table_index != VEX_MAP7) dp = &vex_table[vex_table_index][vindex]; else if (vindex == 0xf8) dp = &map7_f8_opcode; else dp = &bad_opcode; (i.e. common case first and overall less indentation)? > @@ -11299,7 +11376,11 @@ OP_Skip_MODRM (instr_info *ins, int bytemode ATTRIBUTE_UNUSED, > > /* Skip mod/rm byte. */ > MODRM_CHECK; > - ins->codep++; > + if (!ins->has_skipped_modrm) > + { > + ins->codep++; > + ins->has_skipped_modrm = true; > + } > return true; > } I understand you need to set has_skipped_modrm here, but does this need to be conditional? Jan