Message ID | 1419379661.27606.88.camel@ubuntu-sellcey |
---|---|
State | New |
Headers | show |
Steve Ellcey <Steve.Ellcey@imgtec.com> writes: > On Fri, 2014-12-19 at 13:54 -0800, Matthew Fortune wrote: > > > I never got to reviewing this internally. We should switch this to > > using ADDIUPC. > > > > The goal of this code I believe is to determine the relative offset > > from the static and dynamic locations of ld.so. > > > > " STRINGXP (PTR_LA) " %0, 0f\n" > > 0: addiupc <reg>, 0 > > " STRINGXP (PTR_SUBU) " %0, <reg>, %0\n" > > > > We should also do something similar with the other case. > > > > Thanks, > > Matthew > > How does this look Matthew? There were no regressions in testing. I > have also fixed the preprocessor indentation issues that Joseph pointed > out and I added an include of <sysdep.h> so that __mips_isa_rev is > always defined. This looks correct from a read through. I think it would be good to have another technical review of the code though as well if anyone else is willing to comment. I think the first hunk is easy to see that it is correct, the second hunk looks correct for R6 but I am unsure how the pre-existing code actually works. I assume it is in a reorder block otherwise $31 will not get the runtime address of .Lcoff (now .Lcof2) it would be 4-bytes off if in a noreorder section. The bltzal using $8 is also weird, I don't support it matters whether it is taken or not if the code is in a reorder section. I don't have time to check what the objdump of this code looks like for pre-r6 but if it either shows a NOP after the bltzal or a reordering of the instruction before it into the DS then it all makes sense. On a related note... There is a general plan to go through and update any pre-R6 code that needs to obtain a pc-relative runtime address by using bltzal $0, <whatever> which has been given the name 'nal' in the latest binutils source. This is being promoted because it is statically predictable as not-taken and avoids the problems with BAL where a BAL .+8 may interfere with hardware return predictors. Thanks, Matthew > 2014-12-23 Steve Ellcey <sellcey@imgtec.com> > > * sysdeps/mips/dl-machine.h (elf_machine_load_address): Replace > bltzal with addiupc. > (RTLD_START): Ditto. > > > diff --git a/sysdeps/mips/dl-machine.h b/sysdeps/mips/dl-machine.h index > 5000d2a..c69fdd8 100644 > --- a/sysdeps/mips/dl-machine.h > +++ b/sysdeps/mips/dl-machine.h > @@ -30,6 +30,7 @@ > #endif > > #include <sgidefs.h> > +#include <sysdep.h> > #include <sys/asm.h> > #include <dl-tls.h> > > @@ -138,9 +139,14 @@ elf_machine_load_address (void) #ifndef __mips16 > asm (" .set noreorder\n" > " " STRINGXP (PTR_LA) " %0, 0f\n" > +# if __mips_isa_rev < 6 > " bltzal $0, 0f\n" > " nop\n" > "0: " STRINGXP (PTR_SUBU) " %0, $31, %0\n" > +# else > + "0: addiupc $31, 0\n" > + " " STRINGXP (PTR_SUBU) " %0, $31, %0\n" > +# endif > " .set reorder\n" > : "=r" (addr) > : /* No inputs */ > @@ -241,6 +247,13 @@ do { \ > and not just plain _start. */ > > #ifndef __mips16 > +# if __mips_isa_rev < 6 > +# define LCOFF STRINGXP(.Lcof2) > +# define LOAD_31 STRINGXP(bltzal $8) "," STRINGXP(.Lcof2) # else # > +define LCOFF STRINGXP(.Lcof1) # define LOAD_31 "addiupc $31, 0" > +# endif > # define RTLD_START asm (\ > ".text\n\ > " _RTLD_PROLOGUE(ENTRY_POINT) "\ > @@ -255,9 +268,9 @@ do { \ > move $4, $29\n\ > " STRINGXP(PTR_SUBIU) " $29, 16\n\ > \n\ > - " STRINGXP(PTR_LA) " $8, .Lcoff\n\ > - bltzal $8, .Lcoff\n\ > -.Lcoff: " STRINGXP(PTR_SUBU) " $8, $31, $8\n\ > + " STRINGXP(PTR_LA) " $8, " LCOFF "\n\ > +.Lcof1: " LOAD_31 "\n\ > +.Lcof2: " STRINGXP(PTR_SUBU) " $8, $31, $8\n\ > \n\ > " STRINGXP(PTR_LA) " $25, _dl_start\n\ > " STRINGXP(PTR_ADDU) " $25, $8\n\ >
On Tue, 23 Dec 2014, Steve Ellcey wrote: > 2014-12-23 Steve Ellcey <sellcey@imgtec.com> > > * sysdeps/mips/dl-machine.h (elf_machine_load_address): Replace > bltzal with addiupc. > (RTLD_START): Ditto. OK.
diff --git a/sysdeps/mips/dl-machine.h b/sysdeps/mips/dl-machine.h index 5000d2a..c69fdd8 100644 --- a/sysdeps/mips/dl-machine.h +++ b/sysdeps/mips/dl-machine.h @@ -30,6 +30,7 @@ #endif #include <sgidefs.h> +#include <sysdep.h> #include <sys/asm.h> #include <dl-tls.h> @@ -138,9 +139,14 @@ elf_machine_load_address (void) #ifndef __mips16 asm (" .set noreorder\n" " " STRINGXP (PTR_LA) " %0, 0f\n" +# if __mips_isa_rev < 6 " bltzal $0, 0f\n" " nop\n" "0: " STRINGXP (PTR_SUBU) " %0, $31, %0\n" +# else + "0: addiupc $31, 0\n" + " " STRINGXP (PTR_SUBU) " %0, $31, %0\n" +# endif " .set reorder\n" : "=r" (addr) : /* No inputs */ @@ -241,6 +247,13 @@ do { \ and not just plain _start. */ #ifndef __mips16 +# if __mips_isa_rev < 6 +# define LCOFF STRINGXP(.Lcof2) +# define LOAD_31 STRINGXP(bltzal $8) "," STRINGXP(.Lcof2) +# else +# define LCOFF STRINGXP(.Lcof1) +# define LOAD_31 "addiupc $31, 0" +# endif # define RTLD_START asm (\ ".text\n\ " _RTLD_PROLOGUE(ENTRY_POINT) "\ @@ -255,9 +268,9 @@ do { \ move $4, $29\n\ " STRINGXP(PTR_SUBIU) " $29, 16\n\ \n\ - " STRINGXP(PTR_LA) " $8, .Lcoff\n\ - bltzal $8, .Lcoff\n\ -.Lcoff: " STRINGXP(PTR_SUBU) " $8, $31, $8\n\ + " STRINGXP(PTR_LA) " $8, " LCOFF "\n\ +.Lcof1: " LOAD_31 "\n\ +.Lcof2: " STRINGXP(PTR_SUBU) " $8, $31, $8\n\ \n\ " STRINGXP(PTR_LA) " $25, _dl_start\n\ " STRINGXP(PTR_ADDU) " $25, $8\n\