This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH v4] Fixups of Loongson2F
- From: Wu Zhangjin <wuzhangjin at gmail dot com>
- To: Nick Clifton <nickc at redhat dot com>, Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: Andreas Barth <aba at not dot so dot argh dot org>, zhangfx at lemote dot com, binutils at sourceware dot org, "Maciej W. Rozycki" <macro at linux-mips dot org>, yanh at lemote dot com, admin <admin at heiher dot info>, = <liushiwei at gmail dot com>, Zhang Le <r0bertz at gentoo dot org>
- Date: Sat, 17 Apr 2010 00:04:19 +0800
- Subject: Re: [PATCH v4] Fixups of Loongson2F
- References: <1258617915-9563-1-git-send-email-wuzhangjin@gmail.com> <20100224000937.GA21981@mails.so.argh.org> <4B865C31.3060501@redhat.com> <877hq1nksl.fsf@firetop.home> <4B87C7B8.6010209@redhat.com> <871vg79fwy.fsf@firetop.home> <87hbomd2a5.fsf@firetop.home> <4BC82EEE.2090001@redhat.com>
- Reply-to: wuzhangjin at gmail dot com
Hi, Nick and Richard
Heihaier Just helped to test it and got the building errors:
arch/mips/kernel/genex.S:39: Error: Macro used $at after ".set noat"
arch/mips/kernel/genex.S:43: Error: Macro used $at after ".set noat"
> OK, how's this? If it looks OK, could someone give it a spin on
> affected Loongson 2F hardware and let me know if it works? I'd
> especially like to know if the ".set noat" behaviour I was advocating
> breaks current kernel builds.
As the above error shows, this patch really broke the current kernel
builds for lots of places used the following stuff:
.set noat
// use the at register
.set at
for example:
1. arch/mips/kernel/genex.S
.set noat; \
PTR_LA AT, panic; \
jr AT; \
9: b 9b;
2. arch/mips/kernel/mcount.S
...
more:
$ grep ".set[[:space:]]noat" -ur arch/mips/ | ...
> If so, I think the right fix is to let
> the workarounds be switched on and off using ".set", just like you can
> for many other options:
>
> .set push
> .set no-fix-loongson2f
> ...stuff that's known to be safe...
> .set pop
>
> We could then remove the special case for $26 and $27.
>
> I think this is better than making ".set noat" disable the workarounds.
> ".set noat" is a general feature that can be used by people who don't
> know about this errata, so there's no guarantee that their code is safe.
The ".set no-fix-loongson2f" is a good idea, but since lots of places
have used the ".set noat; /* use AT */; .set at" and we don't know which
places have used this stuff ".set noat; jr AT; .set at", it will be
hard(perhaps we can grep ... and change it) to find out these places to
add the ".set no-fix-loongson2f" and even if we find out them but it
will add lots of loongson specific stuff to the existing source code
which will be more or less ugly.
Best Regards,
Wu Zhangjin
On Fri, 2010-04-16 at 10:33 +0100, Nick Clifton wrote:
> Hi Richard,
>
> [Oops - sorry for taking so long to review this patch].
>
> > OK, how's this? If it looks OK, could someone give it a spin on
> > affected Loongson 2F hardware and let me know if it works?
>
> It looks OK to me, but I do not have access to Loongson hardware to
> verify it. Since there has been no response from Wu however I think
> that you should go ahead and apply it. If there turns out to be a
> problem the the ".set noat" behaviour then I am sure that someone will
> eventually notice it and complain.
>
> Cheers
> Nick
>
> > include/opcode/
> > * mips.h (M_JALR_1, M_JALR_2, M_JR, M_NOP): New macro enums.
> >
> > opcodes/
> > * mips-opc.c (nop, jr, jalr): Treat as macros.
> >
> > gas/
> > * doc/c-mips.texi (-mfix-loongson2f-jump): Be more specific.
> > * config/tc-mips.c (mips_fix_loongson2f): Delete.
> > (fix_loongson2f_nop, fix_loongson2f_jump, fix_loongson2f): Likewise.
> > (append_insn): Don't call mips_fix_loongson2f.
> > (append_simple_insn): New function, extracted from...
> > (md_assemble): ...here.
> > (macro_build_nop, modify_jump_target): New functions.
> > (macro_build_jalr, macro_build_jr): Likewise.
> > (macro_build_jalr): Rename existing function to...
> > (macro_build_pic_jalr): ...this. Add a used_at parameter and use
> > the new macro_build_jalr.
> > (load_delay_nop): Use macro_build_nop.
> > (load_address): Likewise.
> > (macro): Use macro_build_nop, macro_build_jr and macro_build_jalr.
> > Handle M_NOP, M_JR, M_JALR_1 and M_JALR_2. Update calls to what
> > is now macro_build_pic_jalr.
> > (macro2): Likewise.
> > (md_parse_option): Don't set mips_fix_loongson2f.
> >
> > gas/testsuite/
> > * gas/mips/loongson-2f-2.s: Swap the explicit and implicit cases.
> > Add tests for various macro modes.
> > * gas/mips/loongson-2f-3.s: Test J, JR, single-operand JA{L,}R and
> > double-operand JA{L,}R for both the "fix" and "no fix" cases.
> > * gas/mips/loongson-2f-3.d: Require -mips1 -mabi=32. Update after
> > above changes.
> > * gas/mips/loongson-2f-4.s, gas/mips/loongson-2f-4.d,
> > gas/mips/loongson-2f-4.l, gas/mips/loongson-2f-5.s,
> > gas/mips/loongson-2f-5.l: New tests.
> > * gas/mips/mips.exp: Run them.