From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05on2069.outbound.protection.outlook.com [40.107.22.69]) by sourceware.org (Postfix) with ESMTPS id 65CF738432E3 for ; Thu, 24 Nov 2022 07:32:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 65CF738432E3 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=gZJYu2tET7qaRUSCA/cQW1oezUqRLR8GWaRraZwMqV6BZ+XqBxW4hHhKBihZgt6AehtxlMvxtUHa3UlQMoGrPdrN4oI8Gl2NyydCbu7ThpDUeR8nYTtuuXjkrs1zzrZixa2iemyxnNPfJ7Axz82hiQnVWNOGKtCPwkby7vkO7UULu3VSrjpFk4B50qG84syyb+Dx2KNMYrU2LyOuD7CVH1cWLA3dRn+ACbyNQJor65YcLdsciFEmT+UzTmJCaSo0wvQ3KFeVj5y6zm91aymmEiSo9QQIGFE8cNTewj2k6eat51cfaBuvyWrfqFXhdK3bFrXpIawEBFXRIdq5BnlATg== 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=HFE2rnC/2Gds+plZZ1/tDqf52oCJ4FYDPVbKI06/RAQ=; b=OO05kX7TcMb4cLLjrQmf28MS/GamSfAg3iCFDnYG+ipG1LuV3bwmV2zndD/icNiBJ2pztNE1jHw5PxxgbKUu30WLoxIJdx6EPDwB6IUP3PPaEtEw6UXa42ITJHoEfMdComcoKHP2uETek5EkvDQBM2GtJmPvJ6oegI8XhF+lE9s04OkDBp2ba09SIfzrg3ocWe1qNDb1A9l8Wv6Ry9PbIV4crFGSjc2lEomZBQi/k8E2sXcJZGIgfu6fJJKlyGYBRcS7QGCk/RjS+yKsjCbz6Rb/G4LUV5gYUSvB16Jdhdfwb+zZzR2BJJUHu5AXJRka9xWm9fXh54mRGUcKZDbo2A== 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=HFE2rnC/2Gds+plZZ1/tDqf52oCJ4FYDPVbKI06/RAQ=; b=yFilBn8oBu5KiUmoroEDVRnuE6UxnHfExIFeGDau9RE/4yadIPYUYPv5B6TNdCTsog7voDbp62fh9weVEORdNpLHcLeSDnlIR1qszz3en4jcf5yfkGh+K8DUGDW7nE5owcNPe2LmxQN9aJehnYJBNWLBcal8CdfXiTI1CUHbYN3lX1FxLDqGV2nfNFwwYo0SdcaxEYFOH+jk/vowIQ7IhRD4IiOMhJkwo+D0HmZzNB+WRJqHHOAOaVhGIrfsNgEh37baRpoQ+7UUA/bHFG4i6aNvyXZPwgzp5Y/vG/Ddl8nsRfhuIGfdeKC10chjeDpQOM5T0Pf/KiBc8lcUKleemQ== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from VE1PR04MB6560.eurprd04.prod.outlook.com (2603:10a6:803:122::25) 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.5857.19; Thu, 24 Nov 2022 07:31:57 +0000 Received: from VE1PR04MB6560.eurprd04.prod.outlook.com ([fe80::4da2:ea8b:e71e:b8d8]) by VE1PR04MB6560.eurprd04.prod.outlook.com ([fe80::4da2:ea8b:e71e:b8d8%4]) with mapi id 15.20.5857.019; Thu, 24 Nov 2022 07:31:57 +0000 Message-ID: <826cc496-e176-5ad7-cd74-8670a612892a@suse.com> Date: Thu, 24 Nov 2022 08:31:56 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH v2 2/2] RISC-V: Better support for long instructions Content-Language: en-US To: Tsukasa OI Cc: binutils@sourceware.org, Nelson Chu References: <0263444d-3077-8de9-6b40-5643fa60e705@irq.a4lg.com> From: Jan Beulich In-Reply-To: <0263444d-3077-8de9-6b40-5643fa60e705@irq.a4lg.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR3P281CA0127.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:94::13) To VE1PR04MB6560.eurprd04.prod.outlook.com (2603:10a6:803:122::25) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: VE1PR04MB6560:EE_|AM9PR04MB8308:EE_ X-MS-Office365-Filtering-Correlation-Id: d691e7ed-b10c-457f-b0bf-08dacdedf7ce X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: eWa41L061bVfznWdiCPLlqJqgVpv+ezG5LAAipjT+WjSh3MsQBkuoxOGaj5eNEC3ZfJuZZ14hQVHNF9XH6+w7f7ASJSasvlTnhwWI1BEB+cyUfwjt7UK3aXFXUmfaL9CkOWgfWsoJUZpGoiXPxHh0B2msgP/92jdAiYW3X+Mbg4gHNRh0/l+eZc8fYCoH0a+WjishcFfidHZ7iFs8v+Q+DUSNcx7gc0EM1GN+pX1+Q1yXnOZJFSmrNYoWc79d07+krwDB/ZE1R8HRTz4Myq5Z6Sl3/ePoi92QwwDe3Q3MTqGlMZ38fpelf0uAiZ1q6WJd03cSaIBjJVCuUpV4SSeDyMLWqfYJGH0tRhIi/ObyuPjhOipSiRF+jhnL44lDp7oHPE/E3Vt17R5EZNldv6vKhHyW+pxl4KHFY63HH2umMF3EpAqA6FTPpAApCbF61Wr//RNYudZFMvIY1y1ZGY3QNAr7nHWzByzwnojcVUphruUZRFMZh8tqoNunKlO0iYso3xah3NT6Kpg9knOYf4At2VCcOvR+g9P8S4MBnCPye1emR15HGMtc1Xx8/Qk8tgYKZng8WiBk6KjwVJi/zgh2JSmeNy05l7+aB3mFv9/SC5+Rd3Pzp+eaIL4JrhJzTtsXTW1Y9W//mlyImOD7yjr04LrLi7JHuIcg9Gu5pc1q7xUT3D+wX9EG59LsOLsMkhWY3ERhoBxIX/61XVnWVq1oNm3UX/ALDssXJ8v0DOzics= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VE1PR04MB6560.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(346002)(376002)(39860400002)(136003)(396003)(366004)(451199015)(2616005)(186003)(53546011)(5660300002)(36756003)(8936002)(6506007)(6512007)(41300700001)(26005)(31696002)(86362001)(316002)(6916009)(66476007)(8676002)(66556008)(66946007)(4326008)(6486002)(478600001)(38100700002)(2906002)(31686004)(83380400001)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bmgyYVlWMFdaY1BKdkZCSXdSaldOSnZqWDhkaUZYbyswdkJDYUVzekYyNE0x?= =?utf-8?B?THdKUnR2RVpzUlpoeTNDSXBDWWdCbE9OUnhXLytsUmh0UzZ4NjNrK29zWTkz?= =?utf-8?B?R2hTcXBXRlFKajgrNUFxWlJHNWdjWWc1RlpOcDJpZ0NYT09NejNzSGZ2NUUx?= =?utf-8?B?dzFMaDZEdzRmcjZ0Z09DTysyNlpiZUhzZkwvWmFpYXpEemxFQ2doMHVHcDRU?= =?utf-8?B?VkkxUHJ5MVBoaU9QSENFSzNyMnZzVU1PRi9PTXB3aUhaQWMrWlJSL1BZcnhp?= =?utf-8?B?OWh2WXh4Smg4RWxBSTZNeFU3amtpMlBKbGdNUWhuS0pnMXk2SWMzQVU0bzFE?= =?utf-8?B?QkNYa3d5RU9Sb293SG40cEtXYStITUFPb0RTNFF3eXlsMU81Sm8rKzhnUFl0?= =?utf-8?B?dkNVUSszZU5EZWRGMFNYOGtqci9PSll1ZTBZTmE0dFozYk5mMks5OTZGZEVF?= =?utf-8?B?bUR3Mm52SFF4SEtPTU5mczdUQVo5S05YU0d4eGhjUlpNbStyS3pFN0xzWjhH?= =?utf-8?B?bWZtczY0UXAxaWpsVTVPNjREbWZvbFVndktRYXBDQUdQQmJNb2FFU0tSbUJq?= =?utf-8?B?c2p1U09QUHN0aGxPSHFoNmhCb1lqVmlpMUpvRUdmOHk4VGowckU3bzVyalg4?= =?utf-8?B?YVRKQW1XZ294Zkl0UlN5V2FFSUVTcnY1REM4RFFiTWpyZk5jRHlDNytCekpr?= =?utf-8?B?T2hJU3MrbFZVTmpJQzR5YTBNUzBzWm5lYXladFNPV2dZNTZIdzByNVNpTjc2?= =?utf-8?B?TmNqOVNRdnc4Q2NQbE5YdjJtQVVGNUJSU3lyMm9FTFRrb01nQlVOL0JacWVN?= =?utf-8?B?Y3crc2lMMVN2aGRnQVU5SklrM1hFa3d4dUR6RzByT2huSlhhNkx0b0plNENG?= =?utf-8?B?UDdkS2tPSzdYZ2ljRWZTallGSFNsMEZpZ25OL0pCMWlpRGFHUXhIQ1JXQlBZ?= =?utf-8?B?TTlEampPK1VUemc1K0dtcjZNSlZicVFGelp6ZUZ5Z3dkVXk2VGdLaCs0WW9m?= =?utf-8?B?dkhIMXNyTlZXN2cxU0hmbGZleEVoaTZ0N2ZXNU1FZlR2SVRrOW9QdFo0NUE3?= =?utf-8?B?eTYyZ0l6NWp2UmhvdytlQmRwYXlwTWt4MWlrTTdBQ294aGlqblVyUzIyU2Vl?= =?utf-8?B?enloZ2NIZnNNOENvYlhEd21oT0ZKY0hteWJqMldxOE8vYmpILzBQdVpDRkdD?= =?utf-8?B?cVU4NDlvQ2VtTFNTTWJidlJGQ2FtYmgxcnJ3T3pWSWVzOUs3SFpoZTVoaFB2?= =?utf-8?B?NGRVQi9Hb1UrWG16V2dlTE52VVo3MjQ1aTFDNEhCZ0R4ak0xSlFUS1NrM3VY?= =?utf-8?B?SElwald4TUFIYnNxSUo3TjRBRnZJaHBqbGdSYXROMUdaYjZOdWZ0WTNObU1i?= =?utf-8?B?eHJxRE9qZGRlWTRmOVB2Y202ZGhLb0VjaEZKZlpJSEI1VXJidWluSzZnM0dI?= =?utf-8?B?N2paN1lyTkNrL2FDdE9YeElyRzZ5RTR3RXhtY20yQzloV0RIeExHMzZ3anh1?= =?utf-8?B?UjlsYU1CNzFvSFRhZklVbTc5QktVSjBWMjRlTkpQcmlzRFpCbSt5MGVTK3Fs?= =?utf-8?B?U1NNVXFhbDNZcDB4NGU3NkxRVjFLWllPMzlZWjFmMHAyc2t4UWJnaHhUTDIz?= =?utf-8?B?YVh4aHg4UTNRR0lEcWtvUEhzQUE2SHpxcURISjZtZEJqTUNHOGxxVm80UE5C?= =?utf-8?B?WVVrUFVhT04xUGczeFNkT0M2N0wwWjR4bUovM0M5TnEwa1NPdHZhdnJJbTBD?= =?utf-8?B?M1JMc2hQRFlOSXV5dGRPWUx1bEF5Z2ErRW9oSGZyQjlQdzBMNEwrTHkrUnFn?= =?utf-8?B?WHhhT0tuRk93aUpjZ2tYWlN6eDdSbHBjbml2b2xJeFRPNnR1TDRpOHNHMW5W?= =?utf-8?B?eXErSTRFbmxEajFibXVqTHNFZXBMR1NlT0ljOXcrdkhiL1lLeTN4ZExUNEdU?= =?utf-8?B?RlBEYnNrajI4N0V4d1VPdWlBL0c1RHc3SnF4RE1GeHB4SE53MDNJQUpmd29T?= =?utf-8?B?a1Jhek1rZ1VFaHlWSHcrY0R0Z3ZSTDk0aTN3ZEV2TjJleFhPQXdHaDlqb004?= =?utf-8?B?aW1VZkwyem9wWG9OMzdVV01DMUVVcWNGS1paeXl1SVRYY1ZsTEtWc0JRNFJO?= =?utf-8?Q?hjury92N7XltGk0elFmFO9/Mo?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: d691e7ed-b10c-457f-b0bf-08dacdedf7ce X-MS-Exchange-CrossTenant-AuthSource: VE1PR04MB6560.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Nov 2022 07:31:57.7307 (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: c/MLefbY/NI5NnKa8UYQw4XCoXKDyeW5vg3pYX+jOTl0KAke8l87Yy9oN7gWhjCF2IlaCB47a2wXXjfh9LNlaA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8308 X-Spam-Status: No, score=-3029.2 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 24.11.2022 03:34, Tsukasa OI wrote: > On 2022/11/23 18:04, Jan Beulich wrote: >> On 23.11.2022 09:30, Tsukasa OI wrote: >>> From: Tsukasa OI >>> >>> Commit bb996692bd96 ("RISC-V/gas: allow generating up to 176-bit >>> instructions with .insn") tried to start supporting long instructions but >>> it was insufficient. >>> >>> 1. It heavily depended on the bignum internals (radix of 2^16), >>> 2. It generates "value conflicts with instruction length" even if a big >>> number instruction encoding does not exceed its expected length, >>> 3. Because long opcode was handled separately (from struct riscv_cl_insn), >>> some information like DWARF line number correspondence was missing and >>> 4. On the disassembler, disassembler dump was limited up to 64-bit. >>> For long (unknown) instructions, instruction bits are incorrectly >>> zeroed out. >>> >>> To solve these problems, this commit: >>> >>> 1. Handles bignum (and its encodings) precisely, >>> 2. Incorporates long opcode handling into regular >>> struct riscv_cl_insn-handling functions and >>> 3. Adds packet argument to support dumping instructions >>> longer than 64-bits. >>> >>> gas/ChangeLog: >>> >>> * config/tc-riscv.c (struct riscv_cl_insn): Add long opcode field. >>> (create_insn) Clear long opcode marker. >>> (install_insn) Install longer opcode as well. >>> (s_riscv_insn) Likewise. >>> (riscv_ip_hardcode): Make big number handling stricter. Length and >>> the value conflicts only if the bignum size exceeds the expected >>> maximum length. >>> * testsuite/gas/riscv/insn.s: Add testcases such that big number >>> handling is required. >>> * testsuite/gas/riscv/insn.d: Likewise. >>> * testsuite/gas/riscv/insn-na.d: Likewise. >>> * testsuite/gas/riscv/insn-dwarf.d: Likewise. >>> >>> opcodes/ChangeLog: >>> >>> * riscv-dis.c (riscv_disassemble_insn): Print unknown instruction >>> using the new argument packet. >>> (riscv_disassemble_data): Add unused argument packet. >>> (print_insn_riscv): Pass packet to the disassemble function. >> >> The code changes look okay to me. For the testsuite additions I have >> voiced my reservations, and I've given further background in an earlier >> reply still on the v1 sub-thread. Whatever the resolution there would >> imo want to be applied here as well. > > Understood. My (minimum) opinion is, I want to keep 22-bytes patterns > corresponding PATCH v2 2/2 because that's exactly changed by the > assembler / disassembler fixes. But the assembler was rejecting the input there originally, wasn't it? At which point _extending_ the testcase is certainly wanted, but do you really need to check the ".byte ..." output to achieve the goal of the test? >> As to mixing assembler and disassembler changes in the same patch: Is >> this strictly necessary here for some reason? Generally I would suggest >> to split such, but once again I wouldn't insist on you doing so ... >> >> Jan >> > I'm okay to split: > - Assembler fix + Disassembler fix + Test > to: > 1. Assembler fix > 2. Disassembler fix + Test > but there are a good reason I did like this in this patch. > > To test fixed assembler, we need to fix disassembler as well. Although > they are not exactly the same issue, they are corresponding enough so > that merging changes might be justified. > > But since they are not the same issue (as you pointed out), I'm okay to > split to two (three might be too much) separate patches. I agree three would be too much; I wonder whether 1. Disassembler fix 2. Assembler fix + Test wouldn't be the better way to split, if you are going to in the first place. Aiui the disassembler fix doesn't need any adjustments to testcases, whereas my view is that the testcase addition is primarily about the previously wrongly rejected assembler input, and only secondarily about disassembler output. Hence keeping the testcase adjustments with the assembler fix would, to me, seem more "natural". Jan