From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from hedgehog.birch.relay.mailchannels.net (hedgehog.birch.relay.mailchannels.net [23.83.209.81]) by sourceware.org (Postfix) with ESMTPS id DF0B13858D35 for ; Tue, 10 Oct 2023 16:16:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DF0B13858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=eagercon.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=eagercon.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 8A023813F7; Tue, 10 Oct 2023 16:16:40 +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 080B8812E8; Tue, 10 Oct 2023 16:16:40 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1696954600; a=rsa-sha256; cv=none; b=pTjkv9lHQ2bP5ZBX+0lhH4YgiqfgI80sk2uwNwBqf514leHrUB+6abo9F3Kz2dIsjy9j7/ TswMdvkchLpMNAZ5STVaWLkwM8+oklf9jGOO2rX0BJBNHDqROyDVlP6eyeo++FGtt5b/2W bQhBCR1bmv1klxVCY+zobX9LdGzTaP0pS6p9pWI8UCKMn3IZL/AEEdQB4jQlPnPyV7ijSJ KFLpajVIB8yVqmivl6mcRokwyM+QnlxWeTHlkJ5vqOfwenGVAw/va+OKK2HB25CwIGF2WA jwpLj/8eCy17D6GBhg9kuh47dWBFM1jpHWdSIAaoLiNN8KkZG3bebLo8jD9b7g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1696954600; 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=RoTx5uq1S3dxC/3LxiYzfdIQYXC5TYangEZo3fVjDpI=; b=87qGrcgrmMlL529g+2R0klk7CY238Y2htn3XyLuzsxAOpXzSrVT0UTc6wqfeLu70uZCad6 Gml+FvsB+fAPTern4b4NlnwJZulG0ZJL/6T0eeT6bCTe4XeAxeZJxLGjJuQO1+qqvDO56F pd/7mMb0hGeWkqIl7SbLP3C5StnurE6FmgjMcFHoaQlh9g2UDTzsEoXXDWKI+3Qg8SjFd2 Y/NP/2/J7mHz6ctEBrv2IlIzMN7lX9DAooLy0ti9TrO49waJm+3dlR9Kw59h3Rs9AP4s57 HOLtzr6QwJv6K6vL10ONL5QWbcfqYVD+oYYhQbMaBpNC/RY5j7Jc0mjm7EHc8w== ARC-Authentication-Results: i=1; rspamd-7d5dc8fd68-sfxxl; auth=pass smtp.auth=dreamhost smtp.mailfrom=eager@eagercon.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-Versed-Absorbed: 605375782d744cbf_1696954600381_984932860 X-MC-Loop-Signature: 1696954600381:4205631505 X-MC-Ingress-Time: 1696954600381 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.109.167.74 (trex/6.9.1); Tue, 10 Oct 2023 16:16:40 +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 4S4gyH2NWczJl; Tue, 10 Oct 2023 09:16:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eagercon.com; s=dreamhost; t=1696954599; bh=Q0F55WonvMIAvPoRB+I/65dcWOx8Z33N8HaZuD5sq9E=; h=Date:Subject:To:Cc:From:Content-Type:Content-Transfer-Encoding; b=qX7BvxslqZ2dRwzNVeRejudKgXAwPf1r2Hh5AmijZ3rPxIMz1HnNu6Fkpt2uuoIrg 2A8EHQQXM+SyNn+1zil5fpd2w46gUDSla8mv4TBpb7s7qwhSWSqKalpuzK3keNHOmj wz7xmSfWTe2Ou74vrfj/6R+QuEyqASASqd0FzRI33CGTy5kT3oDBqAd+fF+vtYH28N l4x47kOxx7aW4H06rAEA5hCzGOSExyhdDNuoPxfWVnWYElJ3C/xA4hGb0Wzi+FsmG9 Xeyu4W14UeGJBVLtRm8kGM4C4va/mV6iMYCQdKziZVnvizK1SX1fZ0rz0XBKOgLdk+ VVv2c46KZUsXg== Message-ID: Date: Tue, 10 Oct 2023 09:16:38 -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> <4a0c359d-3651-655b-c5ef-1f92918b3e69@eagerm.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.3 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_H3,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/9/23 23:54, Frager, Neal wrote: > > Hi Michael, > > I just submitted v4 of the patch with the following updates: > - Cleaned up all of the GNU coding standard issues. > - Removed any line modifications which were unnecessary, so v4 of the patch looks much cleaner now. > - Tested this patch on the AMD functional side in that it not only builds, but the built binutils are able to build the zynqmp pmufw application correctly. > - Verified that the 32-bit host machine build issue is resolved. Being able to build and execute in your environment is not adequate. You need to verify that it does not break other targets. > As far as I can tell, the only thing missing is a test case for the gas testsuite to verify the relocation works as expected. > > Would you be ok with applying the current v4 of the patch while I work on this last part? The test suite needs to run cleanly, without regressions, before the patch can be applied. A test case to confirm that the relocation works would be good, but is not a requirement. >> 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. > > Are you sure that the problems Maciej reported can be reproduced outside of the test suite? If so, how? Build binutils using the options that Maciej used. Do not apply your latest patch. Run the test suite ("make check"). Look at testsuite/gas.log. Save it. Apply the patch, run make, run make check. Look at testsuite/gas.log. Diff with saved version. In the log, there are results of running each test, indicating failures. You can run these same commands (as-new, readelf) from the command line. FAIL: DWARF2 1 ../as-new --compress-debug-sections -o tmpdir/dwarf2-2.o binutils/gas/testsuite/gas/elf/dwarf2-2.s Executing on host: sh -c {../as-new --compress-debug-sections -o tmpdir/dwarf2-2.o binutils/gas/testsuite/gas/elf/dwarf2-2.s 2>&1} /dev/null dump.tmp (timeout = 300) spawn [open ...] build/gas/testsuite/../../binutils/readelf -w tmpdir/dwarf2-2.o > tmpdir/dump.out Executing on host: sh -c {build/gas/testsuite/../../binutils/readelf -w tmpdir/dwarf2-2.o > tmpdir/dump.out 2>dump.tmp} /dev/null (timeout = 300) spawn [open ...] exited abnormally with 1, output:readelf: Warning: Corrupt unit length (got 0x4e00004e expected at most 0x4e) in section .debug_info >> 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. Look at binutils/gas/testsuite/gas/microblaze/reloc_sym.{s,d}. > > Understood and thank you for your help. I will work on this now. > > Thank you for your support! > > Best regards, > Neal Frager > AMD -- Michael Eager