PATCH: mips16 function attributes, version N+1

Richard Sandiford richard@codesourcery.com
Mon Sep 3 08:23:00 GMT 2007


Sandra Loosemore <sandra@codesourcery.com> writes:
> Richard Sandiford wrote:
>> If so, does it work if you remove:
>> 
>>   static int mips_base_have_tls;  /* targetm.have_tls */
>>   ...
>>     targetm.have_tls = mips_base_have_tls;
>>   ...
>>         /* We don't have a thread pointer access instruction on MIPS16, or
>>            appropriate TLS relocations.  */
>>         targetm.have_tls = false;
>>   ...
>>     mips_base_have_tls = targetm.have_tls;
>> 
>> and simply put something like:
>> 
>>   if (TARGET_MIPS16)
>>     {
>>       /* We don't have an instruction to access the thread pointer on MIPS16,
>>          or appropriate TLS relocations.  */
>>       sorry ("MIPS16 TLS);
>>       return gen_reg_rtx (Pmode);
>>     }
>> 
>> in mips_legitimize_tls_address?  We don't want to use different TLS models
>> (emulated vs. non-emulated) for MIPS16 and non-MIPS16 code.
>> 
>> Unfortunately, this will probably mean a bit of testsuite work, since
>> we've now removed dg-effective-target-tls (or whatever it was called).
>
> I tried this, and yes, it does cause about half a dozen of the tls tests 
> to fail with -mflip-mips16.  Rather than trying to mess with 
> conditionalizing those, how about if I just leave the sorry message in, 
> and document that this is one of the things that doesn't work yet?  In 
> extend.texi I've already written:
>
>    Mixed MIPS16 and non-MIPS16 code may interact badly with some GCC
>    extensions such as @code{__builtin_apply} (@pxref{Constructing
>    Calls}).
>
> so I could just add:
>
>    and thread-local variables (@pxref{Thread-Local}).

My understanding is that __builtin_apply is only going to be troublesome
for calls between non-MIPS16 and MIPS16 hard-float code.  Each side is
abiding by the correct ABI for its own mode, but the linker's stubs
support can't help us.  Although it is of course a solvable problem,
it's not a particularly worrying one, since __builtin_apply is one of
those things we keep meaning to get rid of...

TLS is different because emultls is not supposed to be used for MIPS
at all.  4.2 and earlier would have reported an error about TLS not being
available for MIPS16, so the sorry() behaviour is ABI compatible with
earlier compilers, and fixes an ABI compatibility regression.  So let's
go ahead with the sorry() patch and add the testsuite stuff.  I suggest
adding two new target procedures:

proc check_effective_target_mips16 { } {
    return [check_no_compiler_messages mips16 assembly {
	#ifndef __mips16
	#error NO
	#endif
    }]
}

proc check_effective_target_flip_mips16 { } {
    set selector { target { { mips*-*-* } { "-mflip-mips16" } { "" } } }
    return [expr [dg-process-target $selector] == "S"]
}

(untested!)  Then use a combination of the target selectors "mips16",
"flip_mips16" and "mips*-*-*" to either skip the tests, xfail the tests,
or mark the sorry() message as expected.  If there's still a way to do the
last of the three (now that the error type is part of the thing we check),
then that's probably the best option.  Any of the three would do though.

Richard



More information about the Gcc-patches mailing list