This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: FW: [PATCH,MIPS] Change the mapping for the 'move' instruction
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Simon Dardis <Simon dot Dardis at imgtec dot com>
- Cc: "binutils\ at sourceware dot org" <binutils at sourceware dot org>, Matthew Fortune <Matthew dot Fortune at imgtec dot com>, Maciej Rozycki <Maciej dot Rozycki at imgtec dot com>, "Moore\, Catherine" <Catherine_Moore at mentor dot com>
- Date: Thu, 30 Jul 2015 22:26:13 +0100
- Subject: Re: FW: [PATCH,MIPS] Change the mapping for the 'move' instruction
- Authentication-results: sourceware.org; auth=none
- References: <B83211783F7A334B926F0C0CA42E32CAF0D150 at hhmail02 dot hh dot imgtec dot org>
Simon Dardis <Simon.Dardis@imgtec.com> writes:
> This patch updates the MIPS move instruction alias so that it is 'or'
> instead of [d]addu for microMIPS, MIPS32 and MIPS64. The reasoning
> behind this change was brought up in an earlier RFC from Matthew
> Fortune:
>
> http://sourceware.org/ml/binutils/2015-03/msg00001.html
>
> "This issue was identified during performance analysis of a recent
> 64-bit design by Imagination and the use of addu for 32-bit moves can
> inhibit some pipeline forwarding optimisations as the addu has to sign
> extend in 64-bit implementations. I suspect there are ways to deal with
> this in hardware but regardless it seems sensible to use the same
> instruction for move in 32-bit and 64-bit code."
You've certainly given enough time for people to object to the RFC,
so let's go for it.
> This patch preserves existing disassembler behavior, e.g assembling
> 'daddu t7, ra, zero' and then disassembling it gives back 'move t7,ra'.
Just to check: is this tested by gas/testsuite?
> @@ -921,10 +921,7 @@ static bfd *reldyn_sorting_bfd;
> ((ABI_64_P (abfd) \
> ? 0xdf998010 /* ld t9,0x8010(gp) */ \
> : 0x8f998010)) /* lw t9,0x8010(gp) */
> -#define STUB_MOVE(abfd) \
> - ((ABI_64_P (abfd) \
> - ? 0x03e0782d /* daddu t7,ra */ \
> - : 0x03e07821)) /* addu t7,ra */
> +#define STUB_MOVE 0x03e07825 /* move t7,ra */
> #define STUB_LUI(VAL) (0x3c180000 + (VAL)) /* lui t8,VAL */
> #define STUB_JALR 0x0320f809 /* jalr t9,ra */
> #define STUB_JALRC 0xf8190000 /* jalrc t9,ra */
> @@ -941,10 +938,7 @@ static bfd *reldyn_sorting_bfd;
> ? 0xdf3c8010 /* ld t9,0x8010(gp) */ \
> : 0xff3c8010) /* lw t9,0x8010(gp) */
> #define STUB_MOVE_MICROMIPS 0x0dff /* move t7,ra */
> -#define STUB_MOVE32_MICROMIPS(abfd) \
> - (ABI_64_P (abfd) \
> - ? 0x581f7950 /* daddu t7,ra,zero */ \
> - : 0x001f7950) /* addu t7,ra,zero */
> +#define STUB_MOVE32_MICROMIPS 0x001f7a90 /* move t7,ra */
> #define STUB_LUI_MICROMIPS(abfd, VAL) /* lui t8,VAL */ \
> (MIPSR6_P (abfd) ? 0x41b80000 + (VAL) : 0x13000000 + (VAL))
> #define STUB_JALR_MICROMIPS 0x45d9 /* jalr t9 */
I think it'd be better to use the "or" instruction in the comments.
The macro name already says that it's a move.
> @@ -1043,7 +1037,7 @@ static const bfd_vma mips_o32_exec_plt0_entry[] =
> 0x8f990000, /* lw $25, %lo(&GOTPLT[0])($28) */
> 0x279c0000, /* addiu $28, $28, %lo(&GOTPLT[0]) */
> 0x031cc023, /* subu $24, $24, $28 */
> - 0x03e07821, /* move $15, $31 # 32-bit move (addu) */
> + 0x03e07825, /* move $15, $31 # 32-bit move (or) */
> 0x0018c082, /* srl $24, $24, 2 */
> 0x0320f809, /* jalr $25 */
> 0x2718fffe /* subu $24, $24, 2 */
Here and in the later hunks, I think the "32-bit move" was there to
distinguish ADDU-based moves from DADDU. Now that there's no real
concept of a 32-bit vs. 64-bit move, having just "# or" -- or the full
unaliased instruction -- might be less confusing.
OK with those changes, thanks.
Richard