Re: RFA: MIPS TLS support

[Richard Sandiford <rsandifo at redhat dot com>]

This looks pretty good, thanks.  Some comments below (be warned that
the code has moved around a bit ;)

-----------------------------------------------------------------

First of all, thanks for trying to use the existing symbol
classification structure.  I'm a bit puzzled about the way you've
divided up the work though.  You've added four new symbol types:

Daniel Jacobowitz <drow@false.org> writes:
> +   SYMBOL_TLS_GD
> +   SYMBOL_TLS_LD
> +   SYMBOL_TLS_IE
> +   SYMBOL_TLS_LE
> +       The symbol refers to thread-local data, with GD, LD, IE or LE
> +       model respectively.
> +

that just mirror the existing TLS_MODEL_* classification, and handle
them in mips_classify_symbol like so:

> +  switch (SYMBOL_REF_TLS_MODEL (x))
> +    {
> +    case 0:
> +      break;
> +    case TLS_MODEL_GLOBAL_DYNAMIC:
> +      return SYMBOL_TLS_GD;
> +    case TLS_MODEL_LOCAL_DYNAMIC:
> +      return SYMBOL_TLS_LD;
> +    case TLS_MODEL_INITIAL_EXEC:
> +      return SYMBOL_TLS_IE;
> +    case TLS_MODEL_LOCAL_EXEC:
> +      return SYMBOL_TLS_LE;
> +    default:
> +      abort ();
> +    }
> +

But as I understand it, we don't ever want to treat TLS symbols
as either legitimate constants or legitimate addresses, hence:

> -#define LEGITIMATE_CONSTANT_P(X) (mips_const_insns (X) > 0)
> +#define LEGITIMATE_CONSTANT_P(X) (!mips_tls_operand_p (X) && mips_const_insns (X) > 0)

and:

> +  /* Thread-local symbols must be split.  Only LD and LE have useful
> +     LO_SUMs though.  */
> +  mips_split_p[SYMBOL_TLS_GD] = true;
> +  mips_split_p[SYMBOL_TLS_LD] = true;
> +  mips_lo_relocs[SYMBOL_TLS_LD] = "%dtprel_lo(";
> +  mips_split_p[SYMBOL_TLS_IE] = true;
> +  mips_split_p[SYMBOL_TLS_LE] = true;
> +  mips_lo_relocs[SYMBOL_TLS_LE] = "%tprel_lo(";

So mips_symbolic_constant_p seems to be lying when it returns true
for zero-offset TLS symbols.  Why not simply:

  - get mips_symbolic_constant_p to return false for TLS constants
  - get mips_const_insns to return 0 for TLS constants
  - handle TLS symbols specially in mips_legitimize_move and
    mips_legitimize_address (as opposed to TLS constants being
    special cases of mips_split_p)?

On the other hand:

> Index: gcc/gcc/config/mips/mips.md
> ===================================================================
> --- gcc.orig/gcc/config/mips/mips.md	2005-03-08 17:18:48.000000000 -0500
> +++ gcc/gcc/config/mips/mips.md	2005-03-09 10:03:32.000000000 -0500
> @@ -45,6 +45,12 @@
>     (UNSPEC_LOAD_GOT		24)
>     (UNSPEC_GP			25)
>     (UNSPEC_MFHILO		26)
> +   (UNSPEC_TLS_GD		27)
> +   (UNSPEC_TLS_LD		28)
> +   (UNSPEC_TLS_DTPREL_HI16	29)
> +   (UNSPEC_TLS_IE		30)
> +   (UNSPEC_TLS_TPREL_HI16	31)
> +   (UNSPEC_TLS_GET_TP		32)
>  
>     (UNSPEC_ADDRESS_FIRST	100)
>  
> @@ -5270,6 +5276,65 @@ beq\t%2,%.,1b\;\
>    [(match_dup 0)]
>    { operands[0] = mips_rewrite_small_data (operands[0]); })
>  
> +; Thread-Local Storage
> +(define_insn "tls_gd_<mode>"
> +  [(set (match_operand:P 0 "register_operand" "=d")
> +	(unspec:P [(match_operand:P 1 "register_operand" "d")
> +		    (match_operand:P 2 "tls_symbol_ref" "")]
> +		  UNSPEC_TLS_GD))]
> +  "HAVE_AS_TLS && !TARGET_MIPS16"
> +  "<d>addiu\t%0,%1,%%tlsgd(%2)"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<MODE>")])
> +
> +(define_insn "tls_ld_<mode>"
> +  [(set (match_operand:P 0 "register_operand" "=d")
> +	(unspec:P [(match_operand:P 1 "register_operand" "d")
> +		    (match_operand:P 2 "tls_symbol_ref" "")]
> +		  UNSPEC_TLS_LD))]
> +  "HAVE_AS_TLS && !TARGET_MIPS16"
> +  "<d>addiu\t%0,%1,%%tlsldm(%2)"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<MODE>")])
> +
> +(define_insn "tls_dtprel_hi_<mode>"
> +  [(set (match_operand:P 0 "register_operand" "=d")
> +	(unspec:P [(match_operand:P 1 "tls_symbol_ref" "")]
> +		  UNSPEC_TLS_DTPREL_HI16))]
> +  "HAVE_AS_TLS && !TARGET_MIPS16"
> +  "lui\t%0,%%dtprel_hi(%1)"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<MODE>")])
> +
> +(define_insn "tls_get_tp_<mode>"
> +  [(set (match_operand:P 0 "register_operand" "=v")
> +	(unspec:P [(const_int 0)]
> +		  UNSPEC_TLS_GET_TP))]
> +  "HAVE_AS_TLS && !TARGET_MIPS16"
> +  ".set\tpush\;.set\tmips32r2\t\;rdhwr\t%0,$29\;.set\tpop"
> +  [(set_attr "type" "unknown")
> +   (set_attr "mode" "<MODE>")])
> +
> +(define_insn "tls_ie_<mode>"
> +  [(set (match_operand:P 0 "register_operand" "=d")
> +	(unspec:P [(match_operand:P 1 "register_operand" "d")
> +		   (match_operand:P 2 "tls_symbol_ref" "")]
> +		  UNSPEC_TLS_TPREL_HI16))]

(typo here btw: should be UNSPEC_TLS_IE)

> +  "HAVE_AS_TLS && !TARGET_MIPS16"
> +  "lw\t%0,%%gottprel(%2)(%1)"
> +  [(set_attr "type" "load")
> +   (set_attr "length" "4")
> +   (set_attr "mode" "<MODE>")])
> +
> +(define_insn "tls_tprel_hi_<mode>"
> +  [(set (match_operand:P 0 "register_operand" "=d")
> +	(unspec:P [(match_operand:P 1 "tls_symbol_ref" "")]
> +		  UNSPEC_TLS_TPREL_HI16))]
> +  "HAVE_AS_TLS && !TARGET_MIPS16"
> +  "lui\t%0,%%tprel_hi(%1)"
> +  [(set_attr "type" "arith")
> +   (set_attr "mode" "<MODE>")])
> +
>  ; The MIPS Paired-Single Floating Point and MIPS-3D Instructions.
>  
>  (include "mips-ps-3d.md")

the idea is that every sequence apart from tls_get_tp_<mode> should
be handled using the existing explicit reloc patterns.  There would
be special symbol types like:

    SYMBOL_TLSGD:
        lo_relocs: "%tlsgd("

    SYMBOL_TLSLDM:
        lo_relocs: "%tlsldm("

    SYMBOL_DTPREL:
        hi_relocs: "%dtprel_hi("
        lo_relocs: "%dtprel_lo("

    SYMBOL_GOTTPREL:
        lo_relocs: "%gottprel("

    SYMBOL_TPREL:
        hi_relocs: "%tprel_hi("
        lo_relocs: "%tprel_lo("

and mips_legitimize_tls_address should rewrite TLS symbolic constants
so that they use these symbol types instead.  Doing things this way
should also help any future MIPS16 port.

There seems to be quite a bit of cut-&-paste in the original version
of mips_legitimize_tls_address:

> +  switch (SYMBOL_REF_TLS_MODEL (loc))
> +    {
> +    case TLS_MODEL_GLOBAL_DYNAMIC:
> +      start_sequence ();
> +      if (Pmode == DImode)
> +	emit_insn (gen_tls_gd_di (a0, gp, loc));
> +      else
> +	emit_insn (gen_tls_gd_si (a0, gp, loc));
> +      tga = gen_rtx_MEM (Pmode, mips_tls_get_addr ());
> +      insn = gen_call_value (v0, tga, const0_rtx, const0_rtx);
> +      insn = emit_call_insn (insn);
> +      CONST_OR_PURE_CALL_P (insn) = 1;
> +      use_reg (&CALL_INSN_FUNCTION_USAGE (insn), v0);
> +      use_reg (&CALL_INSN_FUNCTION_USAGE (insn), a0);
> +      insn = get_insns ();
> +      end_sequence ();
> +      emit_libcall_block (insn, dest, v0, loc);
> +      break;
> +
> +    case TLS_MODEL_LOCAL_DYNAMIC:
> +      start_sequence ();
> +      if (Pmode == DImode)
> +	emit_insn (gen_tls_ld_di (a0, gp, loc));
> +      else
> +	emit_insn (gen_tls_ld_si (a0, gp, loc));
> +      tga = gen_rtx_MEM (Pmode, mips_tls_get_addr ());
> +      insn = gen_call_value (v0, tga, const0_rtx, const0_rtx);
> +      insn = emit_call_insn (insn);
> +      CONST_OR_PURE_CALL_P (insn) = 1;
> +      use_reg (&CALL_INSN_FUNCTION_USAGE (insn), v0);
> +      use_reg (&CALL_INSN_FUNCTION_USAGE (insn), a0);
> +      insn = get_insns ();
> +      end_sequence ();
> +      tmp1 = gen_reg_rtx (Pmode);
> +      eqv = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, const0_rtx),
> +			    UNSPEC_TLS_LD);
> +      emit_libcall_block (insn, tmp1, v0, eqv);
> +
> +      tmp2 = gen_reg_rtx (Pmode);
> +      if (Pmode == DImode)
> +	{
> +	  emit_insn (gen_tls_dtprel_hi_di (tmp2, loc));
> +	  emit_insn (gen_adddi3 (dest, tmp2, tmp1));
> +	}
> +      else
> +	{
> +	  emit_insn (gen_tls_dtprel_hi_si (tmp2, loc));
> +	  emit_insn (gen_addsi3 (dest, tmp2, tmp1));
> +	}
> +      eqv = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, loc),
> +			    UNSPEC_ADDRESS_FIRST + SYMBOL_TLS_LD);
> +      eqv = gen_rtx_CONST (Pmode, eqv);
> +      dest = gen_rtx_LO_SUM (Pmode, dest, eqv);
> +      break;
> +
> +    case TLS_MODEL_INITIAL_EXEC:
> +      tmp1 = gen_reg_rtx (Pmode);
> +      if (Pmode == DImode)
> +	{
> +	  emit_insn (gen_tls_get_tp_di (v1));
> +	  emit_insn (gen_tls_ie_di (tmp1, gp, loc));
> +	  emit_insn (gen_adddi3 (dest, tmp1, v1));
> +	}
> +      else
> +	{
> +	  emit_insn (gen_tls_get_tp_si (v1));
> +	  emit_insn (gen_tls_ie_si (tmp1, gp, loc));
> +	  emit_insn (gen_addsi3 (dest, tmp1, v1));
> +	}
> +      break;
> +
> +    case TLS_MODEL_LOCAL_EXEC:
> +      tmp1 = gen_reg_rtx (Pmode);
> +      if (Pmode == DImode)
> +	{
> +	  emit_insn (gen_tls_get_tp_di (v1));
> +	  emit_insn (gen_tls_tprel_hi_di (tmp1, loc));
> +	  emit_insn (gen_adddi3 (dest, tmp1, v1));
> +	}
> +      else
> +	{
> +	  emit_insn (gen_tls_get_tp_si (v1));
> +	  emit_insn (gen_tls_tprel_hi_si (tmp1, loc));
> +	  emit_insn (gen_addsi3 (dest, tmp1, v1));
> +	}
> +      eqv = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, loc),
> +			    UNSPEC_ADDRESS_FIRST + SYMBOL_TLS_LE);
> +      eqv = gen_rtx_CONST (Pmode, eqv);
> +      dest = gen_rtx_LO_SUM (Pmode, dest, eqv);
> +      break;
> +
> +    default:
> +      abort ();
> +    }
> +
> +  return dest;
> +}

but hopefully it should be (slightly) easier to factor everything
if you take the approach described above.

> +#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:

> +static rtx
> +mips_legitimize_tls_address (rtx loc)
> +{
> +  rtx dest, insn, v0, v1, a0, gp, tga, tmp1, tmp2, eqv;
> +
> +  if (TARGET_MIPS16)
> +    sorry ("mips16 TLS");

Richard