Patchwork MIPS16 TLS support for GCC

login
register
mail settings
Submitter Chung-Lin Tang
Date July 5, 2012, 6:50 a.m.
Message ID <4FF53934.5000408@codesourcery.com>
Download mbox | patch
Permalink /patch/169071/
State New
Headers show

Comments

Chung-Lin Tang - July 5, 2012, 6:50 a.m.
Hi Richard,
picking up a yet uncommitted part of the MIPS16 changes, see below:

On 2012/2/3 11:28 PM, Richard Sandiford wrote:
> 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.

I've worked to resolve the issues raised above, notes on the patch:

(1) targetm.have_tls is now used to guard the __builtin_thread_pointer()
builtin. This adds a handwritten builtin availability predicate, and a
new BUILTIN_AVAIL_TLS code in mips.c. I believe this looks conforming to
the builtin infrastructure.

As for the issue of making __builtin_thread_pointer()
target-independent, while it's possible, it seems that most such
builtins are functionality that lends to a machine-independent
implementation when no target-hooks are provided. A thread pointer
builtin does not seem to be one of those.

(2) I have included changes in config/mips/t-linux64 to build a MIPS16
libgcc. MULTILIB_OSDIRNAMES has been modified to use the mapping style,
to make '-mabi=32 -mips16' a properly linking set of options. This
should create the appropriate conditions for committing the MIPS16
libgcc, libgomp changes.

(3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).

As you can see in the original Feb. patch, I had changes to emit a
MIPS16 version of these static calls, but with the changes in (2) above,
they will not work with the usual situation of a 32-bit MIPS built /lib
(.init/.fini will have 32/16-bit code improperly concatenated).

The CodeSourcery builds use an independent mips16 sysroot for this, so a
MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
making it 32-bit is the compatible choice.

Thanks,
Chung-Lin

2012-07-05  Chung-Lin Tang  <cltang@codesourcery.com>

	libgcc/
	* config/mips/crtfastmath.c (set_fast_math): Add 'nomips16'
        attribute.

	libgomp/
	* config/linux/mips/futex.h (sys_futex0): Change to static
        function with noinline, nomips16 attributes under MIPS16. Adjust
        asm statement to place 'li v0,SYS_futex' immediately before
        syscall insn.

	gcc/
	* config/mips/mips-ftypes.def: Add (0, (POINTER)) entry.
	* config/mips/mips.c (MIPS_FTYPE_NAME0): New.
	(mips_builtin_type): Add MIPS_BUILTIN_THREAD_POINTER.
	(mips_get_tp): Add 'target' parameter for generating to specific
	reg.
	(mips_legitimize_tls_address): Update calls to mips_get_tp().
	(BUILTIN_AVAIL_TLS): New built-in avail code.
	(mips_builtin_avail_tls): New built-in availability predicate
	function.
	(THREAD_POINTER_BUILTIN): New.
	(mips_builtins): Add __builtin_mips_thread_pointer and
	__builtin_thread_pointer using THREAD_POINTER_BUILTIN().
	(MIPS_FTYPE_ATYPES0): New.
	(mips_expand_builtin): Add code to handle built-in avail codes.
	Add MIPS_BUILTIN_THREAD_POINTER expand case.
	* config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Allow O32
	version for MIPS16, add nomips16 asm directives. Add error case
	for MIPS16 under non-O32.
	* config/mips/t-linux64 (MULTILIB_OSDIRNAMES): Change to use
	mapping style.
	(MULTILIB_OPTIONS,MULTILIB_DIRNAMES): Add mips16.
	(MULTILIB_EXCLUSIONS): Exclude mips16 when not -mabi=32.

Index: libgcc/config/mips/crtfastmath.c
===================================================================
--- libgcc/config/mips/crtfastmath.c	(revision 189224)
+++ libgcc/config/mips/crtfastmath.c	(working copy)
@@ -39,7 +39,7 @@
 #define _FPU_GETCW(cw) __asm__ ("cfc1 %0,$31" : "=r" (cw))
 #define _FPU_SETCW(cw) __asm__ ("ctc1 %0,$31" : : "r" (cw))
 
-static void __attribute__((constructor))
+static void __attribute__((constructor,nomips16))
 set_fast_math (void)
 {
   unsigned int fcr;
Index: libgomp/config/linux/mips/futex.h
===================================================================
--- libgomp/config/linux/mips/futex.h	(revision 189224)
+++ libgomp/config/linux/mips/futex.h	(working copy)
@@ -28,20 +28,26 @@
 #define FUTEX_WAIT 0
 #define FUTEX_WAKE 1
 
+#ifdef __mips16
+static void __attribute__((noinline,nomips16))
+#else
 static inline void
+#endif
 sys_futex0 (int *addr, int op, int val)
 {
-  register unsigned long __v0 asm("$2") = (unsigned long) SYS_futex;
+  register unsigned long __v0 asm("$2");
   register unsigned long __a0 asm("$4") = (unsigned long) addr;
   register unsigned long __a1 asm("$5") = (unsigned long) op;
   register unsigned long __a2 asm("$6") = (unsigned long) val;
   register unsigned long __a3 asm("$7") = 0;
 
-  __asm volatile ("syscall"
+  __asm volatile ("li $2, %6\n\t"
+		  "syscall"
 		  /* returns $a3 (errno), $v0 (return value) */
 		  : "=r" (__v0), "=r" (__a3)
-		  /* arguments in v0 (syscall) a0-a3 */
-		  : "r" (__v0), "r" (__a0), "r" (__a1), "r" (__a2), "r" (__a3)
+		  /* arguments in a0-a3, and syscall number */
+		  : "r" (__a0), "r" (__a1), "r" (__a2), "r" (__a3),
+                    "IK" (SYS_futex)
 		  /* clobbers at, v1, t0-t9, memory */
 		  : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", "$14",
 		    "$15", "$24", "$25", "memory");
Richard Sandiford - July 5, 2012, 6:54 p.m.
Thanks for the update.

Chung-Lin Tang <cltang@codesourcery.com> writes:
> As for the issue of making __builtin_thread_pointer()
> target-independent, while it's possible, it seems that most such
> builtins are functionality that lends to a machine-independent
> implementation when no target-hooks are provided. A thread pointer
> builtin does not seem to be one of those.

Right.  __builtin_thread_pointer() is more like __builtin_return_address(),
__builtin_saveregs(), etc.  The fact that it belongs to the smaller set
of buitins with explicitly target-dependent implementations than the larger
set of builtins with "target-independent" implementations doesn't matter
though.  The point is that we're deliberately using the same name and
interface across targets, rather than things like __builtin_mips_thread_pointer.

Having a target hook to return the thread pointer makes perfect
sense IMO.

> (2) I have included changes in config/mips/t-linux64 to build a MIPS16
> libgcc. MULTILIB_OSDIRNAMES has been modified to use the mapping style,
> to make '-mabi=32 -mips16' a properly linking set of options. This
> should create the appropriate conditions for committing the MIPS16
> libgcc, libgomp changes.

One problem with doing this unconditionally is that, for things like
--with-arch=octeon, we'll end up trying to build MIPS16 OCTEON libraries.
It might (or might not) build without complaint, but it doesn't make
much sense.  We probably need to implement --with-multilib-list and
only include mips16 if explicitly asked.  (In future, it would be
nice to have the ABIs selected in the same way, but that's very much
a separate change.)

Just to check, with this set-up, "-mabi=32 -mips16" selects the MIPS16
multilibs for GCC libraries, but uses the normal /lib etc. directories
for system libraries.  Is that right?  (Genuine question.  I've never
tried this fancy form of MULTILIB_OSDIRNAMES myself.)

> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).
>
> As you can see in the original Feb. patch, I had changes to emit a
> MIPS16 version of these static calls, but with the changes in (2) above,
> they will not work with the usual situation of a 32-bit MIPS built /lib
> (.init/.fini will have 32/16-bit code improperly concatenated).
>
> The CodeSourcery builds use an independent mips16 sysroot for this, so a
> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
> making it 32-bit is the compatible choice.

Yeah, I agree that sounds like the right call.  Please do the same
for the n32/n64 version (i.e. explicitly make it nomips16 rather
than add the #error).

Thanks,
Richard
Richard Sandiford - July 6, 2012, 6:23 a.m.
Richard Sandiford <rdsandiford@googlemail.com> writes:
>> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
>> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).
>>
>> As you can see in the original Feb. patch, I had changes to emit a
>> MIPS16 version of these static calls, but with the changes in (2) above,
>> they will not work with the usual situation of a 32-bit MIPS built /lib
>> (.init/.fini will have 32/16-bit code improperly concatenated).
>>
>> The CodeSourcery builds use an independent mips16 sysroot for this, so a
>> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
>> making it 32-bit is the compatible choice.
>
> Yeah, I agree that sounds like the right call.  Please do the same
> for the n32/n64 version (i.e. explicitly make it nomips16 rather
> than add the #error).

BTW, doing this has removed my main concern about having dead code.
The original patch had a separate MIPS16 implementation that (as things
stood) could never be used by stock sources.  That would make it difficult
to maintain.

Now that the MIPS16 library support is purely adding nomips16 attributes
to code that is obviously nomips16, those parts are OK on their own, thanks.
(I.e. the mips.h change, the libgcc change, and the libgomp change.)
Feel free to drop the multilib thing if you don't want to implement
--with-multilib-list.

__builtin_thread_pointer is logically a separate patch anyway.
In case it isn't clear, the reason I'm pushing back about the
target-dependent thing is that you're adding a fair bit of extra
code to the general MIPS built-in infrastructure in order to
handle the set of "__builtin_thread_pointer-like functions".
And my concern is that that set probably contains just one function.

I also notice that the patch isn't marking __builtin_thread_pointer
as nothrow, whereas other targets do.  Adding the function
to builtins.def would avoid that kind of accidental difference.
(For avoidance of doubt, the fact that the TP read instruction
can trap to a kernel handler doesn't make the function throwing.)

Richard
Chung-Lin Tang - July 6, 2012, 7:20 a.m.
On 2012/7/6 02:23 PM, Richard Sandiford wrote:
> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
>>> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).
>>>
>>> As you can see in the original Feb. patch, I had changes to emit a
>>> MIPS16 version of these static calls, but with the changes in (2) above,
>>> they will not work with the usual situation of a 32-bit MIPS built /lib
>>> (.init/.fini will have 32/16-bit code improperly concatenated).
>>>
>>> The CodeSourcery builds use an independent mips16 sysroot for this, so a
>>> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
>>> making it 32-bit is the compatible choice.
>>
>> Yeah, I agree that sounds like the right call.  Please do the same
>> for the n32/n64 version (i.e. explicitly make it nomips16 rather
>> than add the #error).
> 
> BTW, doing this has removed my main concern about having dead code.
> The original patch had a separate MIPS16 implementation that (as things
> stood) could never be used by stock sources.  That would make it difficult
> to maintain.
> 
> Now that the MIPS16 library support is purely adding nomips16 attributes
> to code that is obviously nomips16, those parts are OK on their own, thanks.
> (I.e. the mips.h change, the libgcc change, and the libgomp change.)
> Feel free to drop the multilib thing if you don't want to implement
> --with-multilib-list.

Thanks, I'll probably commit those parts first, and maybe look a bit at
the --with-multilib-list thing later. Something that came to my mind
just now, is how this will mesh with micromips later... :P

> __builtin_thread_pointer is logically a separate patch anyway.
> In case it isn't clear, the reason I'm pushing back about the
> target-dependent thing is that you're adding a fair bit of extra
> code to the general MIPS built-in infrastructure in order to
> handle the set of "__builtin_thread_pointer-like functions".
> And my concern is that that set probably contains just one function.

Well, some other architectures also have __builtin_set_thread_pointer(),
as you probably also noticed. The thing is that, unlike the
__builtin_return_address(), __builtin_saveregs() you mentioned, there
seems no general concept of the TLS environment exposed by the backend
(as opposed to things like frame-pointers, return-addr), so the
machine-independent code can't properly do things at arm's length.

For a gcc/builtins.c expand function for __builtin_thread_pointer(),
it's probably either a target-hook or "sorry not implemented". That's
probably still okay, though I'm not sure it's the intended style.

> I also notice that the patch isn't marking __builtin_thread_pointer
> as nothrow, whereas other targets do.  Adding the function
> to builtins.def would avoid that kind of accidental difference.
> (For avoidance of doubt, the fact that the TP read instruction
> can trap to a kernel handler doesn't make the function throwing.)

Oh I missed that in this new patch, thanks for catching. FTR, I think I
did properly mark it in the older patch :)

Thanks,
Chung-Lin
Richard Sandiford - July 6, 2012, 9:18 a.m.
Chung-Lin Tang <cltang@codesourcery.com> writes:
>> __builtin_thread_pointer is logically a separate patch anyway.
>> In case it isn't clear, the reason I'm pushing back about the
>> target-dependent thing is that you're adding a fair bit of extra
>> code to the general MIPS built-in infrastructure in order to
>> handle the set of "__builtin_thread_pointer-like functions".
>> And my concern is that that set probably contains just one function.
>
> Well, some other architectures also have __builtin_set_thread_pointer(),
> as you probably also noticed. The thing is that, unlike the
> __builtin_return_address(), __builtin_saveregs() you mentioned, there
> seems no general concept of the TLS environment exposed by the backend
> (as opposed to things like frame-pointers, return-addr), so the
> machine-independent code can't properly do things at arm's length.
>
> For a gcc/builtins.c expand function for __builtin_thread_pointer(),
> it's probably either a target-hook or "sorry not implemented". That's
> probably still okay, though I'm not sure it's the intended style.

FWIW, I think it's both OK and the way these things are supposed
to work.  The same would apply to __builtin_set_thread_pointer()
like you say.

We certainly have other sorry()s related to TLS, such as trying to
declare TLS common data when that isn't supported.

Richard
Chung-Lin Tang - Aug. 29, 2012, 9:38 a.m.
On 2012/7/6 02:23 PM, Richard Sandiford wrote:
> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
>>> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).
>>>
>>> As you can see in the original Feb. patch, I had changes to emit a
>>> MIPS16 version of these static calls, but with the changes in (2) above,
>>> they will not work with the usual situation of a 32-bit MIPS built /lib
>>> (.init/.fini will have 32/16-bit code improperly concatenated).
>>>
>>> The CodeSourcery builds use an independent mips16 sysroot for this, so a
>>> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
>>> making it 32-bit is the compatible choice.
>>
>> Yeah, I agree that sounds like the right call.  Please do the same
>> for the n32/n64 version (i.e. explicitly make it nomips16 rather
>> than add the #error).
> 
> BTW, doing this has removed my main concern about having dead code.
> The original patch had a separate MIPS16 implementation that (as things
> stood) could never be used by stock sources.  That would make it difficult
> to maintain.
> 
> Now that the MIPS16 library support is purely adding nomips16 attributes
> to code that is obviously nomips16, those parts are OK on their own, thanks.
> (I.e. the mips.h change, the libgcc change, and the libgomp change.)
> Feel free to drop the multilib thing if you don't want to implement
> --with-multilib-list.

Hi Richard, just FYI, I just committed the said approved parts.
gcc/config/mips/t-linux64 had one additional change, adding
../lib/mips16 to the corresponding MULTILIB_OSDIRNAMES, or else we end
with a weird option-named directory for the mips16 libraries.

Thanks,
Chung-Lin
Richard Sandiford - Aug. 29, 2012, 6:44 p.m.
Chung-Lin Tang <cltang@codesourcery.com> writes:
> On 2012/7/6 02:23 PM, Richard Sandiford wrote:
>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>>> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
>>>> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).
>>>>
>>>> As you can see in the original Feb. patch, I had changes to emit a
>>>> MIPS16 version of these static calls, but with the changes in (2) above,
>>>> they will not work with the usual situation of a 32-bit MIPS built /lib
>>>> (.init/.fini will have 32/16-bit code improperly concatenated).
>>>>
>>>> The CodeSourcery builds use an independent mips16 sysroot for this, so a
>>>> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
>>>> making it 32-bit is the compatible choice.
>>>
>>> Yeah, I agree that sounds like the right call.  Please do the same
>>> for the n32/n64 version (i.e. explicitly make it nomips16 rather
>>> than add the #error).
>> 
>> BTW, doing this has removed my main concern about having dead code.
>> The original patch had a separate MIPS16 implementation that (as things
>> stood) could never be used by stock sources.  That would make it difficult
>> to maintain.
>> 
>> Now that the MIPS16 library support is purely adding nomips16 attributes
>> to code that is obviously nomips16, those parts are OK on their own, thanks.
>> (I.e. the mips.h change, the libgcc change, and the libgomp change.)
>> Feel free to drop the multilib thing if you don't want to implement
>> --with-multilib-list.
>
> Hi Richard, just FYI, I just committed the said approved parts.
> gcc/config/mips/t-linux64 had one additional change, adding
> ../lib/mips16 to the corresponding MULTILIB_OSDIRNAMES, or else we end
> with a weird option-named directory for the mips16 libraries.

Sorry, but the t-linux64 stuff wasn't approved.  It was just the mips.h
change, the libgcc change and the libgomp change.

Please revert the patch to t-linux64.  My original objection to adding
mips16 unconditionally still stands: it isn't correct for people who
configure for processors that don't have the MIPS16 ASE (such as Octeon).

Thanks,
Richard
Chung-Lin Tang - Aug. 30, 2012, 5:40 a.m.
On 2012/8/30 02:44 AM, Richard Sandiford wrote:
> Chung-Lin Tang <cltang@codesourcery.com> writes:
>> On 2012/7/6 02:23 PM, Richard Sandiford wrote:
>>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>>>> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
>>>>> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).
>>>>>
>>>>> As you can see in the original Feb. patch, I had changes to emit a
>>>>> MIPS16 version of these static calls, but with the changes in (2) above,
>>>>> they will not work with the usual situation of a 32-bit MIPS built /lib
>>>>> (.init/.fini will have 32/16-bit code improperly concatenated).
>>>>>
>>>>> The CodeSourcery builds use an independent mips16 sysroot for this, so a
>>>>> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
>>>>> making it 32-bit is the compatible choice.
>>>>
>>>> Yeah, I agree that sounds like the right call.  Please do the same
>>>> for the n32/n64 version (i.e. explicitly make it nomips16 rather
>>>> than add the #error).
>>>
>>> BTW, doing this has removed my main concern about having dead code.
>>> The original patch had a separate MIPS16 implementation that (as things
>>> stood) could never be used by stock sources.  That would make it difficult
>>> to maintain.
>>>
>>> Now that the MIPS16 library support is purely adding nomips16 attributes
>>> to code that is obviously nomips16, those parts are OK on their own, thanks.
>>> (I.e. the mips.h change, the libgcc change, and the libgomp change.)
>>> Feel free to drop the multilib thing if you don't want to implement
>>> --with-multilib-list.
>>
>> Hi Richard, just FYI, I just committed the said approved parts.
>> gcc/config/mips/t-linux64 had one additional change, adding
>> ../lib/mips16 to the corresponding MULTILIB_OSDIRNAMES, or else we end
>> with a weird option-named directory for the mips16 libraries.
> 
> Sorry, but the t-linux64 stuff wasn't approved.  It was just the mips.h
> change, the libgcc change and the libgomp change.
> 
> Please revert the patch to t-linux64.  My original objection to adding
> mips16 unconditionally still stands: it isn't correct for people who
> configure for processors that don't have the MIPS16 ASE (such as Octeon).

I have reverted that part.
Maybe a list of proper march=XXX/mips16 added to MULTILIB_EXCLUSIONS
will do what you're mentioning, though I haven't tried testing that for now.

Thanks,
Chung-Lin
Richard Sandiford - Aug. 30, 2012, 8:19 p.m.
Chung-Lin Tang <cltang@codesourcery.com> writes:
> On 2012/8/30 02:44 AM, Richard Sandiford wrote:
>> Chung-Lin Tang <cltang@codesourcery.com> writes:
>>> On 2012/7/6 02:23 PM, Richard Sandiford wrote:
>>>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>>>>> (3) Also related to libraries, I edited CRT_CALL_STATIC_FUNCTION to emit
>>>>>> a 32-bit code sequence under both MIPS/MIPS16 mode (under O32).
>>>>>>
>>>>>> As you can see in the original Feb. patch, I had changes to emit a
>>>>>> MIPS16 version of these static calls, but with the changes in (2) above,
>>>>>> they will not work with the usual situation of a 32-bit MIPS built /lib
>>>>>> (.init/.fini will have 32/16-bit code improperly concatenated).
>>>>>>
>>>>>> The CodeSourcery builds use an independent mips16 sysroot for this, so a
>>>>>> MIPS16 CRT_CALL_STATIC_FUNCTION works there. For the usual case, I think
>>>>>> making it 32-bit is the compatible choice.
>>>>>
>>>>> Yeah, I agree that sounds like the right call.  Please do the same
>>>>> for the n32/n64 version (i.e. explicitly make it nomips16 rather
>>>>> than add the #error).
>>>>
>>>> BTW, doing this has removed my main concern about having dead code.
>>>> The original patch had a separate MIPS16 implementation that (as things
>>>> stood) could never be used by stock sources.  That would make it difficult
>>>> to maintain.
>>>>
>>>> Now that the MIPS16 library support is purely adding nomips16 attributes
>>>> to code that is obviously nomips16, those parts are OK on their own, thanks.
>>>> (I.e. the mips.h change, the libgcc change, and the libgomp change.)
>>>> Feel free to drop the multilib thing if you don't want to implement
>>>> --with-multilib-list.
>>>
>>> Hi Richard, just FYI, I just committed the said approved parts.
>>> gcc/config/mips/t-linux64 had one additional change, adding
>>> ../lib/mips16 to the corresponding MULTILIB_OSDIRNAMES, or else we end
>>> with a weird option-named directory for the mips16 libraries.
>> 
>> Sorry, but the t-linux64 stuff wasn't approved.  It was just the mips.h
>> change, the libgcc change and the libgomp change.
>> 
>> Please revert the patch to t-linux64.  My original objection to adding
>> mips16 unconditionally still stands: it isn't correct for people who
>> configure for processors that don't have the MIPS16 ASE (such as Octeon).
>
> I have reverted that part.

Thanks.

> Maybe a list of proper march=XXX/mips16 added to MULTILIB_EXCLUSIONS
> will do what you're mentioning, though I haven't tried testing that for now.

TBH, I'm not sure off-hand whether MULTILIB_EXCLUSIONS takes account
of --with-arch-style defaults.  (As in: it might well do.)

Even if it does, though, I still think --with-multilib-list would be
the right way of adding a mips16 multilib.  It's just that having an
out-of-the-box way of getting a mips16 multilib seems less important
now than it did originally (because the original patch added code that
wouldn't be used without such a multilib, whereas the current patch just
adds obviously-correct nomips16 attributes).

"Not important" doesn't mean "not useful", of course.  Having
--with-multilib-list would still very nice to have if anyone
feels suitably inclined.

Richard

Patch

Index: gcc/config/mips/mips-ftypes.def
===================================================================
--- gcc/config/mips/mips-ftypes.def	(revision 189224)
+++ gcc/config/mips/mips-ftypes.def	(working copy)
@@ -46,6 +46,8 @@  DEF_MIPS_FTYPE (3, (DI, DI, V4QI, V4QI))
 DEF_MIPS_FTYPE (2, (DI, SI, SI))
 DEF_MIPS_FTYPE (2, (DI, USI, USI))
 
+DEF_MIPS_FTYPE (0, (POINTER))
+
 DEF_MIPS_FTYPE (2, (INT, DF, DF))
 DEF_MIPS_FTYPE (2, (INT, SF, SF))
 DEF_MIPS_FTYPE (2, (INT, V2SF, V2SF))
Index: gcc/config/mips/t-linux64
===================================================================
--- gcc/config/mips/t-linux64	(revision 189224)
+++ gcc/config/mips/t-linux64	(working copy)
@@ -18,4 +18,11 @@ 
 
 MULTILIB_OPTIONS = mabi=n32/mabi=32/mabi=64
 MULTILIB_DIRNAMES = n32 32 64
-MULTILIB_OSDIRNAMES = ../lib32 ../lib ../lib64
+MULTILIB_OSDIRNAMES  = mabi.n32=../lib32
+MULTILIB_OSDIRNAMES += mabi.32=../lib
+MULTILIB_OSDIRNAMES += mabi.64=../lib64
+
+# MIPS16 is currently only supported under O32
+MULTILIB_OPTIONS += mips16
+MULTILIB_DIRNAMES += mips16
+MULTILIB_EXCLUSIONS = !mabi=32/mips16
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 189224)
+++ gcc/config/mips/mips.c	(working copy)
@@ -181,6 +181,7 @@  enum mips_address_type {
 };
 
 /* Macros to create an enumeration identifier for a function prototype.  */
+#define MIPS_FTYPE_NAME0(A) MIPS_##A##_FTYPE_VOID
 #define MIPS_FTYPE_NAME1(A, B) MIPS_##A##_FTYPE_##B
 #define MIPS_FTYPE_NAME2(A, B, C) MIPS_##A##_FTYPE_##B##_##C
 #define MIPS_FTYPE_NAME3(A, B, C, D) MIPS_##A##_FTYPE_##B##_##C##_##D
@@ -231,7 +232,10 @@  enum mips_builtin_type {
   MIPS_BUILTIN_CMP_SINGLE,
 
   /* For generating bposge32 branch instructions in MIPS32 DSP ASE.  */
-  MIPS_BUILTIN_BPOSGE32
+  MIPS_BUILTIN_BPOSGE32,
+
+  /* For generating accesses to the TLS thread pointer.  */
+  MIPS_BUILTIN_THREAD_POINTER
 };
 
 /* Invoke MACRO (COND) for each C.cond.fmt condition.  */
@@ -2851,11 +2855,12 @@  mips_call_tls_get_addr (rtx sym, enum mips_symbol_
 /* Return a pseudo register that contains the current thread pointer.  */
 
 static rtx
-mips_get_tp (void)
+mips_get_tp (rtx target)
 {
-  rtx tp, fn;
+  rtx fn;
+  rtx tp = (target != NULL_RTX && REG_P (target)
+	    ? target : gen_reg_rtx (Pmode));
 
-  tp = gen_reg_rtx (Pmode);
   if (TARGET_MIPS16)
     {
       mips_need_mips16_rdhwr_p = true;
@@ -2919,7 +2924,7 @@  mips_legitimize_tls_address (rtx loc)
       break;
 
     case TLS_MODEL_INITIAL_EXEC:
-      tp = mips_get_tp ();
+      tp = mips_get_tp (NULL_RTX);
       tmp1 = gen_reg_rtx (Pmode);
       tmp2 = mips_unspec_address (loc, SYMBOL_GOTTPREL);
       if (Pmode == DImode)
@@ -2931,7 +2936,7 @@  mips_legitimize_tls_address (rtx loc)
       break;
 
     case TLS_MODEL_LOCAL_EXEC:
-      tmp1 = mips_get_tp ();
+      tmp1 = mips_get_tp (NULL_RTX);
       offset = mips_unspec_address (loc, SYMBOL_TPREL);
       if (mips_split_p[SYMBOL_TPREL])
 	{
@@ -13002,8 +13007,13 @@  mips_prefetch_cookie (rtx write, rtx locality)
 
    BUILTIN_AVAIL_NON_MIPS16
 	The function is available on the current target, but only
-	in non-MIPS16 mode.  */
+	in non-MIPS16 mode.
+
+   BUILTIN_AVAIL_TLS
+        The function is available when the current target supports TLS.
+*/
 #define BUILTIN_AVAIL_NON_MIPS16 1
+#define BUILTIN_AVAIL_TLS 2
 
 /* Declare an availability predicate for built-in functions that
    require non-MIPS16 mode and also require COND to be true.
@@ -13015,6 +13025,14 @@  mips_prefetch_cookie (rtx write, rtx locality)
    return (COND) ? BUILTIN_AVAIL_NON_MIPS16 : 0;			\
  }
 
+/* Declare availability predicate for built-in functions that are
+   available when the target has TLS.  */
+static unsigned int
+mips_builtin_avail_tls (void)
+{
+  return BUILTIN_AVAIL_TLS;
+}
+
 /* This structure describes a single built-in function.  */
 struct mips_builtin_description {
   /* The code of the main .md file instruction.  See mips_builtin_type
@@ -13203,6 +13221,13 @@  AVAIL_NON_MIPS16 (cache, TARGET_CACHE_BUILTIN)
 #define CODE_FOR_loongson_psubush CODE_FOR_ussubv4hi3
 #define CODE_FOR_loongson_psubusb CODE_FOR_ussubv8qi3
 
+/* Define a MIPS_BUILTIN_THREAD_POINTER builtin.  The parameters are fixed,
+   but we allow both __builtin* and __builtin_mips* prefixes below.  */
+#define THREAD_POINTER_BUILTIN(NAME)					\
+  { CODE_FOR_nothing, MIPS_FP_COND_f, #NAME,				\
+    MIPS_BUILTIN_THREAD_POINTER, MIPS_POINTER_FTYPE_VOID,		\
+    mips_builtin_avail_tls }
+
 static const struct mips_builtin_description mips_builtins[] = {
   DIRECT_BUILTIN (pll_ps, MIPS_V2SF_FTYPE_V2SF_V2SF, paired_single),
   DIRECT_BUILTIN (pul_ps, MIPS_V2SF_FTYPE_V2SF_V2SF, paired_single),
@@ -13486,7 +13511,11 @@  static const struct mips_builtin_description mips_
   LOONGSON_BUILTIN_SUFFIX (punpcklwd, s, MIPS_V2SI_FTYPE_V2SI_V2SI),
 
   /* Sundry other built-in functions.  */
-  DIRECT_NO_TARGET_BUILTIN (cache, MIPS_VOID_FTYPE_SI_CVPOINTER, cache)
+  DIRECT_NO_TARGET_BUILTIN (cache, MIPS_VOID_FTYPE_SI_CVPOINTER, cache),
+
+  /* TLS thread pointer built-in functions.  */
+  THREAD_POINTER_BUILTIN (__builtin_mips_thread_pointer),
+  THREAD_POINTER_BUILTIN (__builtin_thread_pointer)
 };
 
 /* Index I is the function declaration for mips_builtins[I], or null if the
@@ -13557,6 +13586,8 @@  mips_build_cvpointer_type (void)
 
 /* MIPS_FTYPE_ATYPESN takes N MIPS_FTYPES-like type codes and lists
    their associated MIPS_ATYPEs.  */
+#define MIPS_FTYPE_ATYPES0(A) MIPS_ATYPE_##A
+
 #define MIPS_FTYPE_ATYPES1(A, B) \
   MIPS_ATYPE_##A, MIPS_ATYPE_##B
 
@@ -13851,13 +13882,30 @@  mips_expand_builtin (tree exp, rtx target, rtx sub
   gcc_assert (fcode < ARRAY_SIZE (mips_builtins));
   d = &mips_builtins[fcode];
   avail = d->avail ();
-  gcc_assert (avail != 0);
-  if (TARGET_MIPS16)
+  switch (avail)
     {
-      error ("built-in function %qE not supported for MIPS16",
-	     DECL_NAME (fndecl));
-      return ignore ? const0_rtx : CONST0_RTX (mode);
+    case BUILTIN_AVAIL_NON_MIPS16:
+      if (TARGET_MIPS16)
+	{
+	  error ("built-in function %qE not supported for MIPS16",
+		 DECL_NAME (fndecl));
+	  return ignore ? const0_rtx : CONST0_RTX (mode);
+	}
+      break;
+
+    case BUILTIN_AVAIL_TLS:
+      if (!targetm.have_tls)
+	{
+	  error ("built-in function %qE not supported when TLS is not available",
+		 DECL_NAME (fndecl));
+	  return ignore ? const0_rtx : CONST0_RTX (mode);
+	}
+      break;
+
+    default:
+      gcc_unreachable ();
     }
+
   switch (d->builtin_type)
     {
     case MIPS_BUILTIN_DIRECT:
@@ -13881,6 +13929,9 @@  mips_expand_builtin (tree exp, rtx target, rtx sub
 
     case MIPS_BUILTIN_BPOSGE32:
       return mips_expand_builtin_bposge (d->builtin_type, target);
+
+    case MIPS_BUILTIN_THREAD_POINTER:
+      return mips_get_tp (target);
     }
   gcc_unreachable ();
 }
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	(revision 189224)
+++ gcc/config/mips/mips.h	(working copy)
@@ -2781,7 +2781,6 @@  while (0)
 #define STORE_BY_PIECES_P(SIZE, ALIGN) \
   mips_store_by_pieces_p (SIZE, ALIGN)
 
-#ifndef __mips16
 /* Since the bits of the _init and _fini function is spread across
    many object files, each potentially with its own GP, we must assume
    we need to load our GP.  We don't preserve $gp or $ra, since each
@@ -2790,16 +2789,22 @@  while (0)
 #if (defined _ABIO32 && _MIPS_SIM == _ABIO32)
 #define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
    asm (SECTION_OP "\n\
+	.set push\n\
+	.set nomips16\n\
 	.set noreorder\n\
 	bal 1f\n\
 	nop\n\
 1:	.cpload $31\n\
 	.set reorder\n\
 	jal " USER_LABEL_PREFIX #FUNC "\n\
+	.set pop\n\
 	" TEXT_SECTION_ASM_OP);
 #endif /* Switch to #elif when we're no longer limited by K&R C.  */
 #if (defined _ABIN32 && _MIPS_SIM == _ABIN32) \
    || (defined _ABI64 && _MIPS_SIM == _ABI64)
+#ifdef __mips16
+#error "MIPS16 support for non O32 ABIs not implemented"
+#endif
 #define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
    asm (SECTION_OP "\n\
 	.set noreorder\n\
@@ -2810,7 +2815,6 @@  while (0)
 	jal " USER_LABEL_PREFIX #FUNC "\n\
 	" TEXT_SECTION_ASM_OP);
 #endif
-#endif
 
 #ifndef HAVE_AS_TLS
 #define HAVE_AS_TLS 0