Re: [PATCH] MIPS16 TLS support for GCC

[Richard Sandiford <rdsandiford at googlemail dot com>]

Chung-Lin Tang <cltang@codesourcery.com> writes:
>> (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;

That, plus the CRT_CALL_STATIC_FUNCTION and crtfastmath.c changes.

> 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)

Vendor stuff would be fine.  But to be clear, (2) isn't OK without
some way of getting MIPS16 multilibs out of the box, since it would
otherwise be dead code.

>> 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)

All the more reason to make it target-independent. :-)

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

This stuff is inherently guarded by TARGET_HAVE_TLS.

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

Please don't!  I was unnecessarily confusing here, sorry.  I was trying
to say earlier that (2) isn't OK as-is because people have no way of
testing it without changing the sources locally.  We need to add some
way of configuring MIPS16 multilibs at the same time.  The libgomp
change is OK as part of that patch, but not on its own.

Thanks,
Richard