RE: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

["Frager, Neal" <neal.frager@amd.com>]

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?

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

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?

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.

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?

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 <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).
> 
>    Maciej
> 

> --
> Michael Eager