Re: PATCH: break MIPS16 TLS

[Richard Sandiford <richard at codesourcery dot com>]

Sandra Loosemore <sandra@codesourcery.com> writes:
> Richard and I have previously been discussing this change in the context 
> of the MIPS16 function attributes patch.  Yesterday we had some internal 
> discussion and realized we were talking about doing different things; 
> so, for avoidance of ambiguity, this patch is my current understanding 
> of the change Richard wants me to make.  It's actually completely 
> independent of the MIPS16 function attributes patch, so I've split it 
> off for separate consideration.

Well, I wouldn't say it was "completely independent" exactly.  You're
moving the MIPS16-related override_options code to mips_set_mips16_mode,
so leaving bits behind seemed conceptually wrong.  And just leaving things
as they were, without any of this patch, introduced obscure failures.

But yes, it can certainly be done first, so thanks for splitting it out.

> I'm still trying to figure out what to do with the testsuite, since this 
> change intentionally breaks all the TLS tests with -mips16.

FWIW, I stand by my original suggestion of adding (working versions of ;)):

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"]
}

to gcc/testsuite/lib/target-supports.exp, then either: (a) adding
FAILs to individual tests for combinations of mips16 and/or flip_mips16,
such as "{ target { mips16 || flip_mips16 } }" or (b) if that's too hairy,
short-cutting TLS altogether with:

    if { [check_effective_target_mips16]
         || [check_effective_target_flip_mips16] } {
        return
    }

in tls.exp.  I'm not sure if you're asking about this or something else.

> Index: gcc/config/mips/mips.c
> ===================================================================
> *** gcc/config/mips/mips.c	(revision 128065)
> --- gcc/config/mips/mips.c	(working copy)
> *************** mips_cannot_force_const_mem (rtx x)
> *** 1778,1784 ****
>       }
>   
>     if (TARGET_HAVE_TLS && for_each_rtx (&x, &mips_tls_symbol_ref_1, 0))
> !     return true;
>   
>     return false;
>   }
> --- 1778,1791 ----
>       }
>   
>     if (TARGET_HAVE_TLS && for_each_rtx (&x, &mips_tls_symbol_ref_1, 0))
> !     {
> !       if (TARGET_MIPS16)
> ! 	{
> ! 	  sorry ("MIPS16 TLS");
> ! 	  return false;
> ! 	}
> !       return true;
> !     }
>   
>     return false;
>   }

Is this part needed?  I was hoping the mips_legitimize_tls_address part:

> *************** mips_legitimize_tls_address (rtx loc)
> *** 2326,2331 ****
> --- 2333,2344 ----
>     rtx dest, insn, v0, v1, tmp1, tmp2, eqv;
>     enum tls_model model;
>   
> +   if (TARGET_MIPS16)
> +     {
> +       sorry ("MIPS16 TLS");
> +       return gen_reg_rtx (Pmode);
> +     }
> + 
>     v0 = gen_rtx_REG (Pmode, GP_RETURN);
>     v1 = gen_rtx_REG (Pmode, GP_RETURN + 1);

would be enough on its own.

The mips_legitimize_tls_address and override_options parts look good, thanks.

Richard