From: Michael Eager <eager@eagercon.com>
To: "Frager, Neal" <neal.frager@amd.com>,
"Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: "binutils@sourceware.org" <binutils@sourceware.org>,
"Erkiaga Elorza, Ibai" <ibai.erkiaga-elorza@amd.com>,
"Mekala, Nagaraju" <nagaraju.mekala@amd.com>,
"Hatle, Mark" <mark.hatle@amd.com>,
"Mutyala, Sadanand" <sadanand.mutyala@amd.com>,
"Nali, Appa Rao" <appa.rao.nali@amd.com>,
"Hunsigida, Vidhumouli" <vidhumouli.hunsigida@amd.com>,
"luca.ceresoli@bootlin.com" <luca.ceresoli@bootlin.com>,
Nick Clifton <nickc@redhat.com>
Subject: Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions
Date: Mon, 9 Oct 2023 10:53:52 -0700 [thread overview]
Message-ID: <d09f1011-6a3f-fad8-d772-5e4cd37fb68e@eagercon.com> (raw)
In-Reply-To: <CH2PR12MB500468783E4E12A6AD7620FFF0CEA@CH2PR12MB5004.namprd12.prod.outlook.com>
On 10/9/23 03:53, Frager, Neal 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 <nagaraju.mekala@amd.com>
>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
>>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>>
>> 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).
>
> Hi Maciej,
>
> The patch I submitted does not modify the 'get_field_rfsl' function, but perhaps you could review
> the 'get_field_imm5width' function? Could you explain what is not following the GNU Coding
> Standards, so that I can fix it?
>
> static char *
> get_field_imm5width (struct string_buf *buf, long instr)
> {
> char *p = strbuf (buf);
>
> if (instr & 0x00004000)
> sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW))); /* bsefi */
> else
> sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW) - ((instr & IMM5_MASK) >> IMM_LOW) + 1)); /* bsifi */
> return p;
> }
Lines should be shorter than 80 characters.
>
--
Michael Eager
next prev parent reply other threads:[~2023-10-09 17:53 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-05 12:51 Neal Frager
2023-10-06 19:17 ` Michael Eager
2023-10-07 19:23 ` Maciej W. Rozycki
2023-10-07 22:33 ` Michael Eager
2023-10-07 23:11 ` Michael Eager
2023-10-09 6:32 ` Frager, Neal
2023-10-09 13:32 ` Maciej W. Rozycki
2023-10-09 15:09 ` Michael Eager
2023-10-10 6:54 ` Frager, Neal
2023-10-10 16:16 ` Michael Eager
2023-10-10 17:54 ` Michael Eager
2023-10-09 10:53 ` Frager, Neal
2023-10-09 17:53 ` Michael Eager [this message]
2023-10-07 22:01 ` [PATCH] microblaze: fix build error on 32-bit hosts Mark Wielaard
2023-10-07 22:53 ` Michael Eager
2023-10-07 23:07 ` Michael Eager
2023-10-07 23:14 ` Mark Wielaard
2023-10-09 11:58 ` Frager, Neal
2023-10-09 12:37 ` Mark Wielaard
2023-10-09 12:42 ` Frager, Neal
2023-10-09 12:48 ` Mark Wielaard
2023-10-09 12:56 ` Frager, Neal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d09f1011-6a3f-fad8-d772-5e4cd37fb68e@eagercon.com \
--to=eager@eagercon.com \
--cc=appa.rao.nali@amd.com \
--cc=binutils@sourceware.org \
--cc=ibai.erkiaga-elorza@amd.com \
--cc=luca.ceresoli@bootlin.com \
--cc=macro@orcam.me.uk \
--cc=mark.hatle@amd.com \
--cc=nagaraju.mekala@amd.com \
--cc=neal.frager@amd.com \
--cc=nickc@redhat.com \
--cc=sadanand.mutyala@amd.com \
--cc=vidhumouli.hunsigida@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).