This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 1/5] MIPS/GAS: Fix o32 LD to the base register
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Maciej W. Rozycki" <macro at linux-mips dot org>
- Cc: binutils at sourceware dot org
- Date: Mon, 01 Nov 2010 21:28:06 +0000
- Subject: Re: [PATCH 1/5] MIPS/GAS: Fix o32 LD to the base register
- References: <alpine.LFD.2.00.1010130023180.15889@eddie.linux-mips.org> <g48w1w2b8a.fsf@richards-desktop.stglab.manchester.uk.ibm.com> <alpine.LFD.2.00.1010181752570.15889@eddie.linux-mips.org> <alpine.LFD.2.00.1010312127400.25426@eddie.linux-mips.org>
"Maciej W. Rozycki" <macro@linux-mips.org> writes:
> binutils-2.20.51-20100925-mips-gas-ld-o-base.patch
> Index: binutils-2.20.51/gas/config/tc-mips.c
> ===================================================================
> --- binutils-2.20.51.orig/gas/config/tc-mips.c
> +++ binutils-2.20.51/gas/config/tc-mips.c
> @@ -7346,17 +7346,29 @@ macro (struct mips_cl_insn *ip)
>
> case M_LD_OB:
> s = HAVE_64BIT_GPRS ? "ld" : "lw";
> + off = 1;
> + /* If the first load would overwrite the base register,
> + then swap the accesses. */
> + if (!HAVE_64BIT_GPRS && treg == breg && breg != ZERO)
> + {
> + offset_expr.X_add_number += 4;
> + treg += 1;
> + off = -1;
> + }
> goto sd_ob;
> +
> case M_SD_OB:
> s = HAVE_64BIT_GPRS ? "sd" : "sw";
> + off = 1;
> +
Add a fallthrough comment if you're adding whitespace. Patches 1-3
are otherwise OK.
I'm not convinced 4 is worthwhile given that these macros are really in
maintenance mode.
I still don't like the way that you're matching ECOFF and ELF offsets
in a single test, allowing two load offsets when (for each format
individually) only one offset is actually acceptable. But patch 5
is OK nonetheless.
Richard