[PATCH v2] MIPS objdump: Add `eabi32` and `eabi64` ABI options

Maciej W. Rozycki macro@orcam.me.uk
Sat Feb 15 01:37:40 GMT 2025


Hi Anghelo,

>  Thanks for the review, I applied your requested changes.
> Sorry for not going into more detail in my previous patch, this is the
> first time I use `git send-email', so I sent the email without any
> write-up.

 No worries.

> >  Thank you for linking the reference into the change description.  As it 
> > only talks about argument registers when it comes to FPRs and does not 
> > mention which are static and which are temporary I have cross-checked it 
> > with GCC sources and things are looking good in your change.
> 
> Indeed, it doesn't mention which float registers are temporary nor
> saved. What I did to ensure the names are correct was to look at the
> generated assembly from GCC in godbolt and other compilers.

 Good, and you could have noted it along with your original submission.  
It always helps when steps made to make sure a change offered is correct 
are described along with it, either in the change description or in the 
discussion below, as appropriate.

> >  We don't usually update ChangeLog files anymore, but you can include a 
> > record with your commit description if you like.  In that case please 
> > use the imperative mood though, i.e.:
> 
> Oh, I thought ChangeLog files were still being updated since there are
> changes as recent as this year.
> I updated the entry following your advice, but I can also remove the
> entry if preferred.

 Nick, our head maintainer, has indeed been marking branch points in 
ChangeLog files, but otherwise any recent activity there has been purely 
accidental.  A major problem with updates to ChangeLog files are backports 
to release branches, which cause a merge conflict that has to be resolved 
by hand almost inevitably.

 We leave it up to individual contributors as to whether include a 
ChangeLog entry in the change description of their submissions, and 
therefore I have moved yours there as well for v3.

> >  As a matter of interest this is the first mention of the MIPS EABI I 
> > have seen in decades (which I gather means things have been working 
> > perfectly all the time on that side), so do you actually have a use for 
> > this stuff?
> 
> Yes! I do have an use for EABI. It is used in the Sony PlayStation 2's
> reverse engineering community, which uses EABI for most of its games.
> Specifically we are working on matching decompilation of PS2 games.
> Using named registers for EABI allows to better think of what the
> assembly is doing.

 Excellent!  The timeline then matches my expectations for EABI use.  And 
having an enthusiast's background and striving myself to maintain support 
for several systems that are commercially no longer relevant I am always 
keen to assist such efforts.  Good luck with your project!

 As to v2 of your change there are still a couple of nits to address -- 
see below for the details -- but overall you did very well with your first 
contribution.

> diff --git a/gas/testsuite/gas/mips/gpr-names-eabi32.d b/gas/testsuite/gas/mips/gpr-names-eabi32.d
> new file mode 100644
> index 00000000000..523f883c22f
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/gpr-names-eabi32.d
> @@ -0,0 +1,42 @@
> +#objdump: -dr --prefix-addresses --show-raw-insn -M gpr-names=eabi32
> +#name: MIPS GPR disassembly (eabi32)
> +#source: gpr-names.s

 This test can reuse n64 output from gpr-names-64.d, via the #dump: tag.

> diff --git a/gas/testsuite/gas/mips/gpr-names-eabi64.d b/gas/testsuite/gas/mips/gpr-names-eabi64.d
> new file mode 100644
> index 00000000000..00a421e9acb
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/gpr-names-eabi64.d
> @@ -0,0 +1,42 @@
> +#objdump: -dr --prefix-addresses --show-raw-insn -M gpr-names=eabi64
> +#name: MIPS GPR disassembly (eabi32)
                                    ^^
 Typo here, s/eabi32/eabi64/.

> +#source: gpr-names.s

 And this test can reuse n64 output from gpr-names-64.d as well.

> diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
> index 609e621dada..84012228339 100644
> --- a/opcodes/ChangeLog
> +++ b/opcodes/ChangeLog
> @@ -1,3 +1,9 @@
> +2025-02-09  Anghelo Carvajal  <angheloalf95@gmail.com>
> +
> +	* mips-dis.c (mips_fpr_names_eabi32): New variable.
> +	(mips_fpr_names_eabi64): New variable.
> +	(mips_abi_choices): Add "eabi32" and "eabi64" options.
> +

 Moved to the change description, as noted above.

 I went ahead and have committed your patch with these updates applied.  
Thank you for your contribution.

  Maciej


More information about the Binutils mailing list