This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
> -----Original Message----- > From: Maciej W. Rozycki [mailto:macro@imgtec.com] > Sent: Saturday, January 16, 2016 10:42 AM > To: Moore, Catherine > Cc: binutils@sourceware.org > Subject: Re: [RFA] [MIPS] Don't use $gp if abicalls are in effect > > Hi Catherine, > > > This patch prevents the assembler from using $gp if abicalls are in > > effect. OK to install? > > Please use a more accurate patch headline for the GIT commit, e.g. > "Disallow gp-relative addressing if abicalls are in effect" or "Disallow the use > of -G if...", etc. -- at first I thought: why would you want to stop GAS using > that register? > Okay, sure. > The change conceptually looks good to me, however I see a few small issues > with the patch itself: > > > diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c index > > 859ddc6..eec725b 100644 > > --- a/gas/config/tc-mips.c > > +++ b/gas/config/tc-mips.c > > @@ -3466,7 +3466,13 @@ md_begin (void) > > as_bad (_("-G may not be used in position-independent code")); > > g_switch_value = 0; > > } > > - > > + else if (mips_abicalls) > > + { > > + if (g_switch_seen && g_switch_value != 0) > > + as_bad (_("-G may not be used with abicalls")); > > + g_switch_value = 0; > > + } > > + > > Trailing space here, please remove. Done. > > > @@ -14318,6 +14324,8 @@ md_parse_option (int c, char *arg) > > case OPTION_CALL_SHARED: > > mips_pic = SVR4_PIC; > > mips_abicalls = TRUE; > > + if (!g_switch_seen) > > + g_switch_value = 0; > > break; > > This change makes code depend on the ordering between -G and - > call_shared on the command line, and also affects the situation where GAS is > called like this: > > $ as -call_shared -non_shared ... > > i.e. where a later option cancels an earlier one -- one would expect the > setting of -G to remain at its default then. > > However this change is also not needed AFAICT, because with your patch in > place `g_switch_value' will be reset as required in `md_begin'. So please > either just drop this part or otherwise tell me why it is needed. I've dropped this part of the patch. As you pointed out, it's not necessary. > > > diff --git a/gas/testsuite/gas/mips/mips.exp > > b/gas/testsuite/gas/mips/mips.exp index 6645e83..c0a65f1 100644 > > --- a/gas/testsuite/gas/mips/mips.exp > > +++ b/gas/testsuite/gas/mips/mips.exp > > @@ -741,6 +741,7 @@ if { [istarget mips*-*-vxworks*] } { > > run_dump_test_arches "rol64-hw" [mips_arch_list_matching > gpr64 ror] > > > > run_dump_test "sb" > > + run_dump_test_arches "sdata-gp" [mips_arch_list_matching > mips3] > > Why is running this test limited to MIPS III+ ISAs only? I was avoiding errors from the mips64-linux configuration: Error: -march=mips1 is not compatible with the selected ABI /scratch/cmoore/9101/patched2/binutils-gdb/gas/testsuite/gas/mips/sdata-gp.s:7: Error: `gp=32' used with a 64-bit ABI /scratch/cmoore/9101/patched2/binutils-gdb/gas/testsuite/gas/mips/sdata-gp.s:7: Warning: `fp=32' used with a 64-bit ABI This version of the patch adds -32 to the assembler command and runs the test for MIPS1 + I take it you prefer this? > > > diff --git a/gas/testsuite/gas/mips/sdata-gp.s > > b/gas/testsuite/gas/mips/sdata-gp.s > > new file mode 100755 > > index 0000000..fa6fc08 > > --- /dev/null > > +++ b/gas/testsuite/gas/mips/sdata-gp.s > > @@ -0,0 +1,7 @@ > > + .sdata > > +c0101: .word 0xabcd > > + > > + .text > > + .align 4 > > +test: > > + lw $2, c0101 > > Wrong indentation in the last line, please replace spaces with tabs. > Done. Okay, now? Thanks, Catherine
Attachment:
gprel.cl
Description: gprel.cl
Attachment:
gprel.patch
Description: gprel.patch
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |