This is the mail archive of the 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

Hi Richard,

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, 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.


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