This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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]

Re: [PATCH] MIPS16 TLS support for GCC


On Sat, 4 Feb 2012, Richard Sandiford wrote:

> >  I don't think _mcount has ever worked for dynamic libraries, has it? -- 
> > please correct me if I am wrong, but I believe `gprof' relies on memory 
> > segments to be contiguous which is certainly not the case for a dynamic 
> > executable where you have a separate set of mappings for the executable 
> > proper and then for each shared library loaded.
> >
> >  I didn't know it required $3 for anything either -- I've thought it was 
> > $1 only.  How has it worked for standard MIPS code then?
> 
> Sorry, I misremembered, it was $2 (for the static chain register).

 Please elaborate anyway -- if you remember the details of $2 usage 
offhand, that is.  I'm curious.

> >  We have only about now got MIPS16 shared libraries to work -- are you 
> > sure removing code to save/restore $2/$3 in the dynamic linker is going to 
> > hit anyone?
> 
> Pretty sure.  There are two separate points here.  Support for MIPS16
> shared libraries went into the FSF tools in 2008, and I was able to
> build working(!) shared libraries at that time.  (To be clear, these were
> external libraries rather than things like libgcc.)  So it's not really
> that new.
> 
> That doesn't mean that there weren't bugs in the implementation, of course.
> But I think we're just going to have to agree to disagree on the
> "working" thing.  It's probably moot anyway given the other point...

 OK, perhaps soft-float (or if you happened to avoid FP altogether) could 
have worked.  With hard float we hit problems at least with symbol 
references emitted incorrectly by GCC in the context of some MIPS16 thunks 
as well as binutils failing to emit LA25 thunks for some MIPS16 thunks.  
So it wasn't just MIPS16 shared libraries that failed, but also dynamic 
MIPS16 executables.  So it looks to me like a part of the MIPS16 ABI 
wasn't covered at all (I'd expect all the "interesting" cases to have been 
deliberately tested before claiming victory).

 Chung-Lin may remember more details; I'd have to dig out old discussions.

> > While a small piece, this is still some wasted memory and 
> > execution time for every executable on every system and whether MIPS16 
> > code is involved or not (even on systems that do not support the MIPS16 
> > ASE at all), just to cope with an ABI anomaly of literally four functions 
> > only needed for some MIPS16 code (and which had not originally been 
> > expected to be ever used for dynamic linking -- until recently nobody even 
> > considered the use of MIPS16 code on a shared library system).  And I 
> > think you can still get into trouble if you use the wrong ld.so with your 
> > MIPS16 executable -- or has symbol versioning been used to ensure that 
> > ld.so bails out in this case?
> 
> ...to be clear, I'm talking here specifically about the behaviour of
> the _PLT_ resolver function.  Only the PLT resolver function saves and
> restores $2 and $3; the resolver for SVR4-style lazy binding stubs
> doesn't.  (As I mentioned earlier, we're also not expecting the
> resolver for the SVR4-style lazy binding stubs to preserve $2 and $3.)
> 
> MIPS PLTs are an extension to the original SVR4 ABI, so we were free
> to choose the interface.  As you can tell from the glibc comments,
> including $2 and $3 in the list was a deliberate decision, based on
> the fact that there were already functions that would find it useful.
> The PLT resolver has used this interface since the day it was added to
> glibc (2008-10-01).  Its ABI has not changed, so there was no change
> that would require an .so bump.  (Adding PLTs didn't itself require
> an .so bump because old dynamic linkers would error out on them anyway.)

 Fair enough -- I didn't realise that support for MIPS PLT and MIPS16 
dynamic executables was added at the same time.

> And because we're talking about PLTs -- which for MIPS are only used in
> executables -- the question isn't really whether MIPS16 shared libraries
> work or not.  It's a question of whether partly-MIPS16 dynamic executables
> work.  And they do: this has been a regular part of my testing setup for
> at least a couple of years now.

 Yet we had some problems to fix that made me believe it was work in 
progress that went upstream.  Sorry if that was unjust.

> >> If there's still some concern that __mips16_rdhwr might not have
> >> the right ABI, then maybe we should simply emit a link-once function
> >> in each object that needs it.  We could then switch to another function
> >> (and another API) without having to keep the old one in libgcc.a for
> >> compatibility.  It would also avoid the -shared-libgcc thing.
> >> 
> >> Admittedly that's just an off-the-top-of-my-head idea. :-)
> >> What do you think?
> >
> >  Actually I had that idea of a link-once function too, but it turned out 
> > quite complicated to do without rewriting some generic parts of GCC as it 
> > is currently not prepared to emit link-once functions outside C++ 
> > compilations.  It's been a while and I did lots of other stuff meanwhile, 
> > so please excuse me if I got anything wrong here.
> 
> Hmm, OK, I wouldn't have expected that.  But if you've tried making
> __mips16_rdhwr link-once and had a bad experience with it, then yeah,
> let's go with the hidden libgcc function.  It's just a shame that we're
> having to force static linking of libgcc for this one case.

 Well, it's just this single function that's pulled from libgcc.a -- all 
the rest will come from libgcc_s.so (unless hidden as well, that is).  
The benefit is you can always change the ABI of __mips16_rdhwr without 
worrying about existing executables -- they'll continue to work using 
their private piece of code unchanged.

  Maciej


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]