This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] MIPS16 TLS support for GCC
- From: Chung-Lin Tang <cltang at codesourcery dot com>
- To: gcc-patches <gcc-patches at gcc dot gnu dot org>, <rdsandiford at googlemail dot com>
- Cc: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Date: Fri, 3 Feb 2012 18:02:41 +0800
- Subject: Re: [PATCH] MIPS16 TLS support for GCC
- References: <4F0709B3.firstname.lastname@example.org> <email@example.com>
Sorry for the delay to respond, and thanks for revising and committing
the changes to trunk. The revised changes look much cleaner :)
About the other concerns:
> (2) is interesting if there is also a way to build those MIPS16 libraries
> out of the box. I'd like such a mechanism to be added at the same time,
> so that the new support is easy to test. This is still a 4.7 candidate
> if it can be done in time, although it's probably a little tight.
I assume you mean the libgomp patch? That's mostly a potential bug fix;
as for MIP16 multilib additions, so far I haven't added any to the
default mips*-linux* configs, as I guess this should be vendor-stuff
(though the current additions should clear away any obstacles)
> You've given the built-in function the generic name __builtin_thread_pointer.
> I agree that's a good idea, but I think we should also _treat_ it as a generic
> function, and make it available on all targets. The actual implementation
> can be delegated to a target hook. That makes (3) 4.8 material.
Actually, I named and added __builtin_thread_pointer() only because it
was needed for building glibc (to avoid using 32-bit asm in MIPS16
mode), and because some other targets also have it (I'm sure ARM also
has it, and at least one other)
> (Incidentally, I don't think it's correct to define the builtin if TLS
> isn't available, so if we did keep it as a MIPS-specific function,
> d->avail would need to be nonnull. There would need to be some other
> way of indicating that MIPS16 was OK.)
The function is essentially a wrapper for mips_get_tp(), which I don't
see any guarding for the no TLS case? FWIW, TARGET_HAVE_TLS is just
defined as HAVE_AS_TLS for MIPS...
>> The __mips16_rdhwr() function is not intended for general use, just
>> tightly coupled compiler runtime support; therefore, it is only linked
>> statically from libgcc.a, not exported from shared libgcc.
> Well, a fair bit of libgcc is tightly-coupled compiler runtime support.
> I don't really see any need to handle the new function differently from
> the other __mips16 wrappers. It's not like we're gaining any benefit in
> the PIC call overhead: we can't turn JALRs into branches like we can for
> nearby non-MIPS16-to-non-MIPS16 calls, so PIC calls will still go via
> the GOT. And we're not gaining any benefit in terms of ABI baggage either.
> Future libgcc(.a)s would need to continue providing the function as
> specified now, in order for future gccs to be able to link library
> archives built with 4.7.
> By making the function hidden, we lose the important ability to replace
> all instances of it. E.g. it isn't inconceivable that someone will find
> a more efficient way to implement the function in cases where the RDHWR
> is emulated (perhaps on a kernel other than Linux -- this support isn't
> specific to *-linux-gnu). Being able to replace all instances of a
> function is useful for other things, such as profiling, debugging,
> or working around processor errata.
We touched this internally as well, but interestingly came to the
opposite conclusion :) We're not sure the __mips16_rdhwr() is
ultimately the best choice of API, plus the non-standardness of the
calling convention, hence simply decided to hide it from libgcc_s.so, in
case we need to change the MIPS16 TLS interface.
On the issue of hiding stuff in mips16.S, it may be suitable to raise
this issue here: there is a problem when MIPS16 hard-float calls go
through PLTs, because the hard-float stub functions in mips16.S use
v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated
by BFD. (I think I described the scenario mostly correct, CCing Maciej
here who worked on this issue) I think the solution was to mark a large
portion of the mips16.S functions as all hidden.
>> 5) The libgomp MIPS futex.h header has been adjusted; sys_futex0() has
>> been modified to be static, non-inlined, nomips16 under MIPS16 mode (for
>> sake of using 'syscall'). The inline assembly has also been fixed, as
>> Maciej noticed a possible violation of the MIPS syscall restart
>> convention; the 'li $2, #syscall_number' must be right before the
>> syscall insn.
> This change is OK as part of (2).
Okay thanks, I commit this part soon.