From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from snail.cherry.relay.mailchannels.net (snail.cherry.relay.mailchannels.net [23.83.223.170]) by sourceware.org (Postfix) with ESMTPS id 615343858002 for ; Mon, 9 Oct 2023 15:09:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 615343858002 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=eagerm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=eagerm.com X-Sender-Id: dreamhost|x-authsender|eager@eagerm.com Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 601B4142322; Mon, 9 Oct 2023 15:09:46 +0000 (UTC) Received: from pdx1-sub0-mail-a246.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id B79FE142299; Mon, 9 Oct 2023 15:09:45 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1696864185; a=rsa-sha256; cv=none; b=s8eBVcptmMco/nD9V7mf05z9zRaqxIHVgaqyYv1GEObom4riGaTqUgu1SRN8Oqr8MJ/Wzb /T69+B+XHw4W0l32p/qHYIbxqFoqKSpS9VJIJPilsvjTGfA16MWD5NnM/y+f4FpOVJKa9f cABmwpS4hOHntuIwkM30g2oVcNJvyxzmuRQcgBK6nXHI7mAs1ghuPlJIvvdK9UUEQDM60g eCLBC5MtVY2HLI8L5P23dWzGO9ul59lTZfwBwKYLOdCpTN1YLWYyPbFplMlIsg0wlfRWgm jnBU5H196shd6/Vv4R10ybW1XUb/mLuXeDAAxNeP2DKFP5zT0hehQIsG1VEMHw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1696864185; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=V8b2/dVnKHuR9A2oi5Cs9Zh6fUlvEHsY21FQR7DvJOQ=; b=7Wro6gmUn0vnbOLsW+Zz0ChA/B0nvp6JfCvnstbsLf9I+JbJQ+Tu3PGHmT67zERb+Jhr38 AfA9UsYz7VlzE8to7bkBjY3bXrz5A9eCV+5NzAugRPrCynrG9v5h5vz7MqA21hQqOgVmQd wfaN/avNcYV7vDVxFxDTxZ+mS1Qq8WGpwUteSK0R9PQxersTRRL8n0WVmS9PXzqjka8eHd 39V+xDtjD5duHAzTDPgO8i2Rksipz1YxaR9QRbHnosFw+jaNqBBZA1cFVmh/HFGRiIGkMF QiOycHjiCrsBXgyMuXtsMNvx4NirfTxmuIDihe0REl3YAVEdBXJ0hFbyo6wQ0w== ARC-Authentication-Results: i=1; rspamd-7c449d4847-glgx7; auth=pass smtp.auth=dreamhost smtp.mailfrom=eager@eagerm.com X-Sender-Id: dreamhost|x-authsender|eager@eagerm.com X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|eager@eagerm.com X-MailChannels-Auth-Id: dreamhost X-Gusty-Reign: 4d066cf653691dcf_1696864186141_4282367585 X-MC-Loop-Signature: 1696864186141:4213975721 X-MC-Ingress-Time: 1696864186141 Received: from pdx1-sub0-mail-a246.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.99.108.167 (trex/6.9.1); Mon, 09 Oct 2023 15:09:46 +0000 Received: from [192.168.20.10] (c-73-170-238-207.hsd1.ca.comcast.net [73.170.238.207]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) (Authenticated sender: eager@eagerm.com) by pdx1-sub0-mail-a246.dreamhost.com (Postfix) with ESMTPSA id 4S42WY0dKSz6p; Mon, 9 Oct 2023 08:09:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eagerm.com; s=dreamhost; t=1696864185; bh=V8b2/dVnKHuR9A2oi5Cs9Zh6fUlvEHsY21FQR7DvJOQ=; h=Date:Subject:To:Cc:From:Content-Type:Content-Transfer-Encoding; b=inq0Tlz91tTIsMtMpNNyE2jWfQjQTMQSiDJNYLoCqvQOC1WpxZV/bZhuis4xOdDp0 fqeDLf/Lti/HzAKuRFl8yPyEpM95ZH8fEPRfUWbdpyNDZVos/dyTQQw9pRCfWUu9eE 4mUqrUfZjc93ZIWL1BpkPKi4sE5xcbdqp9F/BsgnEl8SFsIJZcXk62V9XZbAuC2gx3 rRT+LYLwlPcQPiSnHVFtqaOF47c06WdlhbOu/39nNW6wHsoIBPsIKwH6sCNXwGqbTh FKxqcAr4sgH4oF6rqNj4Z/LsyG9ZIsT/9lwhsZuCjQZ0Lhj75rzXpGqnh84A/vO3lB saKo40OGOzo6g== Message-ID: <4a0c359d-3651-655b-c5ef-1f92918b3e69@eagerm.com> Date: Mon, 9 Oct 2023 08:09:44 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions Content-Language: en-US To: "Frager, Neal" , Michael Eager , "Maciej W. Rozycki" Cc: "binutils@sourceware.org" , "Erkiaga Elorza, Ibai" , "Mekala, Nagaraju" , "Hatle, Mark" , "Mutyala, Sadanand" , "Nali, Appa Rao" , "Hunsigida, Vidhumouli" , "luca.ceresoli@bootlin.com" , Nick Clifton References: <20231005125103.1330807-1-neal.frager@amd.com> <5d2ce973-6287-db3d-fc82-966914f765a7@eagercon.com> <66d26b55-0457-9ed6-50af-0f5feeb057b0@eagercon.com> From: Michael Eager In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.5 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_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,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 10/8/23 23:32, Frager, Neal wrote: > Hi Michael, Maciej, > >> Neal -- > >> Please resubmit this patch. > >> Add test cases for new instructions. >> Fix GNU Coding Standard issues. >> Run binutils test suite. > > Thank you for feedback. Could you possibly help me with resolving the GNU coding standard issues and the build issue Maciej is reporting? See other messages for GNU Coding Standards. Maciej did not report a build issue: there are test case regressions. See his instructions on running the binutils test suite. > > Currently, the problem we are facing is that the upstream binutils is unable to build the zynqmp pmufw application. I am trying to resolve this by upstreaming the toolchain patches AMD is currently using in its microblaze toolchain release. > > This is the only patch remaining that is preventing the zynqmp pmufw application from building, as the application needs the microblaze bsefi and bsifi instructions. > > My patch for adding these instructions is coming from the following two patches which AMD currently applies on its own microblaze toolchain that comes with our yocto releases: > https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/recipes-devtools/binutils/binutils/0004-LOCAL-Fix-relaxation-of-assembler-resolved-reference.patch > https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/recipes-devtools/binutils/binutils/0008-Add-new-bit-field-instructions.patch I don't have time to research patches in a out-of-tree repository. I did look at the other patches you submitted to see if there was some information about what problem the patch was solving and found nothing. If there is relevant information in these other patches, include it with your patch. > > As I am far away from being a toolchain or GNU binutils expert, could you please help me in the simplest of terms understand what is not following the GNU coding standard in these patches, so that I can fix it? See other message. > > My current way of testing is verifying that the master branch build of GNU binutils is able to build the zynqmp pmufw application and that it executes correctly on zynqmp hardware. So I have verified on hardware that the v2 version of the patch I submitted is indeed working. If you could help me with resolving these regressions, so that this patch can solve the problem I am trying to solve without causing another problem, I would appreciate any support you can provide. That may be satisfactory for AMD's requirements, but your patch has to work with other targets. I'm happy to help you, but my time is limited. You (or someone else) will have to do the heavy lifting to resolve regressions. For each of the test suite failures which Maciej reported, reproduce the problem outside of the test suite. I believe there are error messages. > > Also, are there any instructions for coding standard for generating test cases for the bsefi and bsifi instructions? As I have never written GNU binutils test cases before, perhaps you can point me to something that will help me to save time with this as well? To test the functions in your patch, you will need to create a small assembler test case which invokes the correct relocations. The bare bones "is the opcode correct" test which you added will not do this. I'll take a look at the MB test cases and see if there is one you can adapt. > > Thank you for your support! > > Best regards, > Neal Frager > AMD > >> On Fri, 6 Oct 2023, Michael Eager wrote: >> >>>> This patch has been tested for years of AMD Xilinx Yocto releases as >>>> part of the following patch set: >>>> >>>> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/re >>>> cipes-devtools/binutils/binutils >>>> >>>> Signed-off-by: nagaraju >>>> Signed-off-by: Ibai Erkiaga >>>> Signed-off-by: Neal Frager >>> >>> Committed. >> >> Yet it has caused numerous regressions: >> >> microblaze-elf +FAIL: unordered .debug_info references to >> .debug_ranges microblaze-elf +FAIL: binutils-all/pr26548 >> microblaze-elf +FAIL: readelf -Wwi pr26548e (reason: unexpected >> output) microblaze-elf +FAIL: readelf --debug-dump=loc locview-1 >> (reason: unexpected output) microblaze-elf +FAIL: readelf >> --debug-dump=loc locview-2 (reason: unexpected output) microblaze-elf >> +FAIL: readelf -wiaoRlL dw5 microblaze-elf +FAIL: readelf -wi >> dwarf-attributes (reason: unexpected output) microblaze-elf +FAIL: >> readelf -wKis -P debuglink (reason: unexpected output) microblaze-elf >> +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo >> microblaze-elf +FAIL: DWARF2 1 microblaze-elf +FAIL: DWARF2 2 >> microblaze-elf +FAIL: DWARF2 3 microblaze-elf +FAIL: DWARF2 5 >> microblaze-elf +FAIL: DWARF2 6 microblaze-elf +FAIL: DWARF2 7 >> microblaze-elf +FAIL: DWARF2 11 microblaze-elf +FAIL: DWARF2 12 >> microblaze-elf +FAIL: DWARF2 13 microblaze-elf +FAIL: DWARF2 14 >> microblaze-elf +FAIL: DWARF2 15 microblaze-elf +FAIL: DWARF2 16 >> microblaze-elf +FAIL: DWARF2 17 microblaze-elf +FAIL: DWARF2 18 >> microblaze-elf +FAIL: DWARF2 19 microblaze-elf +FAIL: DWARF2_20: >> debug ranges ignore non-code sections microblaze-elf +FAIL: DWARF2 21 >> microblaze-elf +FAIL: DWARF5 .loc 0 microblaze-elf +FAIL: DWARF4 CU >> microblaze-elf +FAIL: DWARF5 CU microblaze-elf +FAIL: Check line >> table is produced with .nops microblaze-elf +FAIL: line number >> entries for section changes inside .irp microblaze-elf +FAIL: line >> number entries for .macro expansions microblaze-elf +FAIL: line >> number entries for expansions of .macro coming from .include >> microblaze-elf +FAIL: lns-duplicate microblaze-elf +FAIL: >> lns-common-1 microblaze-elf +FAIL: ld-elf/pr22450 >> >> They all seem to follow a similar pattern, e.g: >> >> exited abnormally with 1, output:readelf: Warning: Corrupt unit length >> (got 0x4e00004e expected at most 0x4e) in section .debug_info >> >> FAIL: DWARF2 1 >> >> or: >> >> exited abnormally with 0, output:readelf: Warning: Corrupt unit length >> (got 0x20000020 expected at most 0x20) in section .debug_info >> >> FAIL: Check line table is produced with .nops >> >> Configured with: >> >> $ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver >> --disable-gprofng --disable-libbacktrace --disable-libdecnumber >> --disable-readline --disable-sim --enable-obsolete --enable-plugins >> --build=powerpc64le-linux --target=microblaze-elf >> >> Just reporting in case it's useful as it has popped up in unrelated >> verification; I won't do anything else here. >> >> NB I've skimmed over the change and noticed it does not follow the >> GNU Coding Standards in many places, most prominently in `get_field_imm5width' >> and `get_field_rfsl', but also elsewhere (which is easy to spot >> looking through the diff). >> >> Maciej >> > >> -- >> Michael Eager