Re: RFA: MIPS TLS support

[Daniel Jacobowitz <drow at false dot org>]

On Thu, Mar 10, 2005 at 09:15:12AM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> > Basically, because TLS access should be handled parallel to GOT access. 
> > It's just as much a symbolic constant as an undefined global symbol.
> 
> Not really.  A global symbol is "more of" a symbolic constant simply
> because it's a legitimate constant (in the LEGITIMATE_CONSTANT_P sense)
> whereas TLS symbols aren't.

`LEGITIMATE_CONSTANT_P (X)'
     A C expression that is nonzero if X is a legitimate constant for
     an immediate operand on the target machine.  You can assume that X
     satisfies `CONSTANT_P', so you need not check this.  In fact, `1'
     is a suitable definition for this macro on machines where anything
     `CONSTANT_P' is valid.

So, um... why the heck is a TLS symbol not a legitimate constant, if a
symbol whose address needs to be loaded from the GOT is?  I have never
grokked that.  I've been going on the assumption that GOT symbols
are legitimate constants because it makes GCC unhappy if they aren't,
and GCC doesn't mind if TLS symbols aren't, so we get something better
out this way.

> Note also that all the symbol types currently returned by
> mips_classify_symbol are legitimate move_operands under some
> configuration or other.  TLS symbols on the other hand are never
> legitimate move_operands.

That's only true via some sketchy reasoning.  Consider
SYMBOL_GOTOFF_LOADGP.  A SYMBOL_REF is a valid move_operand if it is
mips_atomic_symbolic_constant_p.  That expands to
mips_symbolic_constant_p (x, &type) && !mips_split_p[type].
SYMBOL_GOTOFF_LOADGP is mips_split_p if TARGET_NEWABI, but it's never
generated if !TARGET_NEWABI.  Ergo, it is never a valid move_operand.

Even if it were true, are you suggesting you would redo all this if
!TARGET_EXPLICIT_RELOCS were removed, which would invalidate your
statement?

> Under those circumstances, having separate symbol types for TLS symbols
> themselves just doesn't fit my mental model of how the MIPS code is
> supposed to work.  We always want to rewrite TLS symbols into "base +
> offset" form at rtl expansion time.  The rtl insns will always be
> operating on these offsets, not the original combined base+offset,
> so the new symbol types should also be attached to the offsets,
> not the original base+offset form.  That's why I suggested using
> symbol_types for each relocation type instead (which you agreed
> to try, thanks).

Eh?  I don't think that's what I agreed to try at all.  I agreed to
replace the instruction I used patterns for with use of symbol relocs,
which seems like a definite winner.  I'm not convinced that backing off
from mips_split_p is a good idea.

> > I originally handled it separately from mips_split_p; it turns out that
> > what I want to do is almost exactly the same!  The duplication
> > convinced me that I ought to be doing it this way.
> 
> I can't tell from this whether your next patch is going to go with
> the suggestion above or not. ;)  If you try it and it doesn't work,
> then fair enough, but I'd need a compelling explanation...

I think I'm just not getting your suggestion.

> 
> >> There seems to be quite a bit of cut-&-paste in the original version
> >> of mips_legitimize_tls_address:
> >
> > Only for si vs. di instructions, if that's what you mean.
> 
> No, I meant that the LOCAL_* and GLOBAL_* cases seemed to be cut-&-paste
> copies of one another with s/tls_ld/tls_gd/.  With your old set-up,
> it would have been easy to have a helper function that emits a call to
> __tls_get_addr() and pass the appropriate gen_tls_?d_di and gen_tls_?i_si
> functions to it.  With the new symbol types, you should just be able to
> pass the symbol type instead.

It's true that there are all of thirteen almost-identical lines in the
LD and GD cases.  Since it bothers you, I'll try for a helper next
time.  As I said, I consider the explicit straight-line code to be a
feature.

> >> > +#undef TARGET_HAVE_TLS
> >> > +#define TARGET_HAVE_TLS HAVE_AS_TLS
> >> 
> >> Since you don't support MIPS16 yet, wouldn't it be better to set this
> >> to "false" in override_options if TARGET_MIPS16?  Better that than:
> >
> > Oh, I can modify targetm in override_options?
> 
> Yeah ;)  I know some people don't like it much, but there's plenty of
> precendent in the MIPS port (although most of it went away at the same
> time as the MIPSpro assembler support).

OK.

-- 
Daniel Jacobowitz
CodeSourcery, LLC