public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] MIPS16 TLS support for GCC
@ 2012-01-06 14:48 Chung-Lin Tang
  2012-01-08 11:21 ` Richard Sandiford
  0 siblings, 1 reply; 26+ messages in thread
From: Chung-Lin Tang @ 2012-01-06 14:48 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Sandiford

[-- Attachment #1: Type: text/plain, Size: 3391 bytes --]

Hi Richard,
here are the patches for MIPS16 TLS. Some highlights of the implementation:

1) For the common -mcode-readable=yes case, TP/DTP offset constants are
loaded pc-relative (this saves insn count and size). HI16/LO16
constructing is used only as a fall-back when text is not readable.

2) To support the Initial/Local-Exec TLS access models, direct access to
the TP is needed. However, MIPS16 mode cannot access the 'rdhwr $3,$29'
instruction used to do that, thus a new __mips16_rdhwr() helper routine
is added to libgcc.a

3) Uses of this __mips16_rdhwr() routine are only generated by by (a)
IE/LE accesses, and (b) calls to the (new added)
__builtin_thread_pointer() builtin function. To facilitate lower
overhead, such calls are represented as UNSPECs instead of actual
function calls, and doesn't really follow the standard ABI (e.g. value
returned in $3). The __mips16_rdhwr() function is not intended for
general use, just tightly coupled compiler runtime support; therefore,
it is only linked statically from libgcc.a, not exported from shared libgcc.

4) Due to containing such a static only support routine, mips*-linux*
targets have been adjusted to use config/t-slibgcc-libgcc in
libgcc/config.host (or else static -lgcc would not be included when
linking with -shared -shared-libgcc)

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.

Only O32 support has been attempted right now, though the code should
not have anything really old-ABI specific. We were able to build glibc
as MIPS16 code with this TLS implementation, so I believe things should
be fairly stable.

Anyways, I believe this will be going in only after 4.8-stage1 opens, so
you should have plenty time to review it :)

Thanks, and Happy New Year
Chung-Lin

2012-01-06  Chung-Lin Tang  <cltang@codesourcery.com>

	gcc/
        * config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Define versions
        for MIPS16 O32.
        * config/mips/mips-protos.h (mips_symbol_type): Add
        SYMBOL_DTPREL_HI,SYMBOL_TPREL_HI. Update in comments.
        (mips_output_tls_reloc_directive): New prototype.
        * config/mips/mips-ftypes.def: Add (0, (POINTER)) entry.
        * config/mips/predicates.md (tls_reloc_operand): New predicate.
        * config/mips/mips.md (V0_REGNUM,PIC_JUMP_REGNUM): New constant.
        (MIPS16 %dtprel_hi,%tprel_hi split pattern): New.
        (consttable_tls_reloc): New.
        (tls_get_tp_<mode>_mips16): New insn and split pattern.
        (*tls_get_tp_<mode>_mips16_rdhwr): New insn pattern.

	libgcc/
        * config.host
	(mips64*-*-linux*,mipsisa64*-*-linux*,mips*-*-linux*):
        Use config/t-slibgcc-libgcc.
        * config/mips/t-mips16 (LIB1ASMFUNCS): Add _m16rdhwr.
        * config/mips/crtfastmath.c (set_fast_math): Add 'nomips16'
        attribute.
        * config/mips/mips16.S (__mips16_rdhwr): New.

	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.


[-- Attachment #2: gcc.diff --]
[-- Type: text/plain, Size: 18818 bytes --]

Index: gcc/config/mips/mips-ftypes.def
===================================================================
--- gcc/config/mips/mips-ftypes.def	(revision 182952)
+++ gcc/config/mips/mips-ftypes.def	(working copy)
@@ -51,6 +51,8 @@ DEF_MIPS_FTYPE (2, (INT, SF, SF))
 DEF_MIPS_FTYPE (2, (INT, V2SF, V2SF))
 DEF_MIPS_FTYPE (4, (INT, V2SF, V2SF, V2SF, V2SF))
 
+DEF_MIPS_FTYPE (0, (POINTER))
+
 DEF_MIPS_FTYPE (2, (SI, DI, SI))
 DEF_MIPS_FTYPE (2, (SI, POINTER, SI))
 DEF_MIPS_FTYPE (2, (DI, POINTER, SI))
Index: gcc/config/mips/predicates.md
===================================================================
--- gcc/config/mips/predicates.md	(revision 182952)
+++ gcc/config/mips/predicates.md	(working copy)
@@ -312,6 +312,14 @@
 	  && type == SYMBOL_GOT_PAGE_OFST);
 })
 
+(define_predicate "tls_reloc_operand"
+  (match_code "const,symbol_ref,label_ref")
+{
+  enum mips_symbol_type type;
+  return (mips_symbolic_constant_p (op, SYMBOL_CONTEXT_LEA, &type)
+	  && (type == SYMBOL_DTPREL || type == SYMBOL_TPREL));
+})
+
 (define_predicate "symbol_ref_operand"
   (match_code "symbol_ref"))
 
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 182952)
+++ gcc/config/mips/mips.md	(working copy)
@@ -134,7 +134,9 @@
 ])
 
 (define_constants
-  [(TLS_GET_TP_REGNUM		3)
+  [(V0_REGNUM			2)
+   (TLS_GET_TP_REGNUM		3)
+   (PIC_JUMP_REGNUM		25)
    (RETURN_ADDR_REGNUM		31)
    (CPRESTORE_SLOT_REGNUM	76)
    (GOT_VERSION_REGNUM		79)
@@ -3933,6 +3935,23 @@
   operands[2] = mips_unspec_address (operands[1], SYMBOL_32_HIGH);
 })
 
+;; MIPS16 %dtprel_hi,%tprel_hi split pattern. Similar transform
+;; as above, for supporting MIPS16 TLS.
+(define_split
+  [(set (match_operand:SI 0 "d_operand")
+	(high:SI (match_operand:SI 1 "tls_reloc_operand")))]
+  "TARGET_MIPS16 && reload_completed"
+  [(set (match_dup 0) (match_dup 2))
+   (set (match_dup 0) (ashift:SI (match_dup 0) (const_int 16)))]
+{
+  /* SYMBOL_DTPREL_HI/TPREL_HI are ordered immediately after
+     SYMBOL_DTPREL/TPREL respectively, so use unspec_type + 1.  */
+  rtx unspec = XEXP (operands[1], 0);
+  int unspec_type = XINT (unspec, 1);
+  operands[2] = mips_unspec_address (XVECEXP (unspec, 0, 0),
+				     unspec_type + 1 - UNSPEC_ADDRESS_FIRST);
+})
+
 ;; Insns to fetch a symbol from a big GOT.
 
 (define_insn_and_split "*xgot_hi<mode>"
@@ -6492,6 +6511,17 @@
 ;;  ....................
 ;;
 
+(define_insn "consttable_tls_reloc"
+  [(unspec_volatile [(match_operand 0 "tls_reloc_operand" "")
+		     (match_operand 1 "const_int_operand" "")]
+		    UNSPEC_CONSTTABLE_INT)]
+  "TARGET_MIPS16_PCREL_LOADS"
+{
+  mips_output_tls_reloc_directive (operands[0], operands[1]);
+  return "";
+}
+  [(set (attr "length") (symbol_ref "INTVAL (operands[1])"))])
+
 (define_insn "consttable_int"
   [(unspec_volatile [(match_operand 0 "consttable_operand" "")
 		     (match_operand 1 "const_int_operand" "")]
@@ -6593,6 +6623,60 @@
    ; See tls_get_tp_<mode>
    (set_attr "can_delay" "no")
    (set_attr "mode" "<MODE>")])
+
+;; In MIPS16 mode, the TLS base pointer is accessed by a
+;; libgcc helper function __mips16_rdhwr(), as 'rdhwr' is not
+;; accessible in MIPS16.
+;;
+;; This is not represented as a call insn, to avoid the
+;; unnecesarry clobbering of caller-save registers by a
+;; function consisting only of: "rdhwr $3,$29; j $31; nop;"
+;;
+;; A $25 clobber is added to cater for a $25 load stub added by the
+;; linker to __mips16_rdhwr when the call is made from non-PIC code.
+
+(define_insn_and_split "tls_get_tp_<mode>_mips16"
+  [(set (match_operand:P 0 "register_operand" "=d")
+	(unspec:P [(const_int 0)] UNSPEC_TLS_GET_TP))
+   (clobber (reg:P TLS_GET_TP_REGNUM))
+   (clobber (reg:P PIC_JUMP_REGNUM))
+   (clobber (reg:P RETURN_ADDR_REGNUM))]
+  "HAVE_AS_TLS && TARGET_MIPS16"
+  "#"
+  "&& reload_completed"
+  [(parallel [(set (reg:P TLS_GET_TP_REGNUM)
+  	      (unspec:P [(match_dup 1)] UNSPEC_TLS_GET_TP))
+	      (clobber (reg:P PIC_JUMP_REGNUM))
+	      (clobber (reg:P RETURN_ADDR_REGNUM))])
+   (set (match_dup 0) (reg:P TLS_GET_TP_REGNUM)) ]
+  {
+    /* UNSPEC operand decides on direct/indirect pattern below.  */
+    rtx sym = gen_rtx_SYMBOL_REF (Pmode, "__mips16_rdhwr");
+    if (TARGET_ABSOLUTE_JUMPS)
+      operands[1] = sym;
+    else
+      {
+	operands[1] = gen_rtx_REG (Pmode, TLS_GET_TP_REGNUM);
+	mips_emit_move (operands[1], sym);
+      }
+  }
+  [(set_attr "type" "unknown")
+   (set_attr "can_delay" "no")
+   (set_attr "mode" "<MODE>")])
+
+(define_insn "*tls_get_tp_<mode>_mips16_rdhwr"
+  [(set (reg:P TLS_GET_TP_REGNUM)
+	(unspec:P [(match_operand:P 0 "")] UNSPEC_TLS_GET_TP))
+	(clobber (reg:P PIC_JUMP_REGNUM))
+	(clobber (reg:P RETURN_ADDR_REGNUM))]
+  "HAVE_AS_TLS && TARGET_MIPS16"
+  {
+    return MIPS_CALL ("jal", operands, 0, -1);
+  }
+  [(set_attr "type" "call")
+   (set_attr "can_delay" "no")
+   (set_attr "mode" "<MODE>")])
+
 \f
 ;; Synchronization instructions.
 
Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc/config/mips/mips-protos.h	(revision 182952)
+++ gcc/config/mips/mips-protos.h	(working copy)
@@ -89,8 +89,10 @@ enum mips_symbol_context {
    SYMBOL_TLSGD
    SYMBOL_TLSLDM
    SYMBOL_DTPREL
+   SYMBOL_DTPREL_HI
    SYMBOL_GOTTPREL
    SYMBOL_TPREL
+   SYMBOL_TPREL_HI
        UNSPEC wrappers around SYMBOL_TLS, corresponding to the
        thread-local storage relocation operators.
 
@@ -127,8 +129,10 @@ enum mips_symbol_type {
   SYMBOL_TLSGD,
   SYMBOL_TLSLDM,
   SYMBOL_DTPREL,
+  SYMBOL_DTPREL_HI,
   SYMBOL_GOTTPREL,
   SYMBOL_TPREL,
+  SYMBOL_TPREL_HI,
   SYMBOL_32_HIGH,
   SYMBOL_64_HIGH,
   SYMBOL_64_MID,
@@ -341,6 +345,7 @@ extern bool mips_epilogue_uses (unsigned int);
 extern void mips_final_prescan_insn (rtx, rtx *, int);
 extern int mips_trampoline_code_size (void);
 extern void mips_function_profiler (FILE *);
+extern void mips_output_tls_reloc_directive (rtx, rtx);
 
 typedef rtx (*mulsidi3_gen_fn) (rtx, rtx, rtx);
 #ifdef RTX_CODE
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 182952)
+++ gcc/config/mips/mips.c	(working copy)
@@ -183,6 +183,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
@@ -233,7 +234,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.  */
@@ -1763,6 +1767,8 @@ mips_symbolic_constant_p (rtx x, enum mips_symbol_
     case SYMBOL_GOTTPREL:
     case SYMBOL_TLS:
     case SYMBOL_HALF:
+    case SYMBOL_TPREL_HI:
+    case SYMBOL_DTPREL_HI:
       return false;
     }
   gcc_unreachable ();
@@ -1857,8 +1863,10 @@ mips_symbol_insns_1 (enum mips_symbol_type type, e
     case SYMBOL_TLSGD:
     case SYMBOL_TLSLDM:
     case SYMBOL_DTPREL:
+    case SYMBOL_DTPREL_HI:
     case SYMBOL_GOTTPREL:
     case SYMBOL_TPREL:
+    case SYMBOL_TPREL_HI:
     case SYMBOL_HALF:
       /* A 16-bit constant formed by a single relocation, or a 32-bit
 	 constant formed from a high 16-bit relocation and a low 16-bit
@@ -1928,14 +1936,23 @@ mips_cannot_force_const_mem (enum machine_mode mod
   if (mips_symbolic_constant_p (base, SYMBOL_CONTEXT_LEA, &type)
       && type != SYMBOL_FORCE_TO_MEM)
     {
+      if (TARGET_MIPS16_PCREL_LOADS)
+	{
+	  /* Under MIPS16, TLS DTP/TP-relative offsets are loaded from the
+	     constant pool, as this saves some code size compared to hi/lo
+	     constructing.  */
+	  if (type == SYMBOL_DTPREL || type == SYMBOL_TPREL)
+	    return false;
+
+	  /* If MIPS16 constant pools live in the text section, they should
+	     not refer to anything that might need run-time relocation.  */
+	  if (mips_got_symbol_type_p (type))
+	    return true;
+	}
+
       /* The same optimization as for CONST_INT.  */
       if (SMALL_INT (offset) && mips_symbol_insns (type, MAX_MACHINE_MODE) > 0)
 	return true;
-
-      /* If MIPS16 constant pools live in the text section, they should
-	 not refer to anything that might need run-time relocation.  */
-      if (TARGET_MIPS16_PCREL_LOADS && mips_got_symbol_type_p (type))
-	return true;
     }
 
   /* TLS symbols must be computed by mips_legitimize_move.  */
@@ -2820,11 +2837,20 @@ 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;
+  rtx tp = (target != NULL_RTX && REG_P (target)
+	    ? target : gen_reg_rtx (Pmode));
 
-  tp = gen_reg_rtx (Pmode);
+  if (TARGET_MIPS16)
+    {
+      if (Pmode == DImode)
+	emit_insn (gen_tls_get_tp_di_mips16 (tp));
+      else
+	emit_insn (gen_tls_get_tp_si_mips16 (tp));
+      return tp;
+    }
+
   if (Pmode == DImode)
     emit_insn (gen_tls_get_tp_di (tp));
   else
@@ -2842,12 +2868,6 @@ mips_legitimize_tls_address (rtx loc)
   rtx dest, insn, v0, tp, tmp1, tmp2, eqv;
   enum tls_model model;
 
-  if (TARGET_MIPS16)
-    {
-      sorry ("MIPS16 TLS");
-      return gen_reg_rtx (Pmode);
-    }
-
   model = SYMBOL_REF_TLS_MODEL (loc);
   /* Only TARGET_ABICALLS code can have more than one module; other
      code must be be static and should not use a GOT.  All TLS models
@@ -2875,13 +2895,23 @@ mips_legitimize_tls_address (rtx loc)
 			    UNSPEC_TLS_LDM);
       emit_libcall_block (insn, tmp1, v0, eqv);
 
-      tmp2 = mips_unspec_offset_high (NULL, tmp1, loc, SYMBOL_DTPREL);
-      dest = gen_rtx_LO_SUM (Pmode, tmp2,
-			     mips_unspec_address (loc, SYMBOL_DTPREL));
+      if (TARGET_MIPS16_PCREL_LOADS)
+	{
+	  tmp2 = mips_force_temporary (NULL,
+				       mips_unspec_address (loc,
+							    SYMBOL_DTPREL));
+	  dest = gen_rtx_PLUS (Pmode, tmp1, tmp2);
+	}
+      else
+	{
+	  tmp2 = mips_unspec_offset_high (NULL, tmp1, loc, SYMBOL_DTPREL);
+	  dest = gen_rtx_LO_SUM (Pmode, tmp2,
+				 mips_unspec_address (loc, SYMBOL_DTPREL));
+	}
       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)
@@ -2893,10 +2923,20 @@ mips_legitimize_tls_address (rtx loc)
       break;
 
     case TLS_MODEL_LOCAL_EXEC:
-      tp = mips_get_tp ();
-      tmp1 = mips_unspec_offset_high (NULL, tp, loc, SYMBOL_TPREL);
-      dest = gen_rtx_LO_SUM (Pmode, tmp1,
-			     mips_unspec_address (loc, SYMBOL_TPREL));
+      tp = mips_get_tp (NULL_RTX);
+      if (TARGET_MIPS16_PCREL_LOADS)
+	{
+	  tmp1 = mips_force_temporary (NULL,
+				       mips_unspec_address (loc,
+							    SYMBOL_TPREL));
+	  dest = gen_rtx_PLUS (Pmode, tp, tmp1);
+	}
+      else
+	{
+	  tmp1 = mips_unspec_offset_high (NULL, tp, loc, SYMBOL_TPREL);
+	  dest = gen_rtx_LO_SUM (Pmode, tmp1,
+				 mips_unspec_address (loc, SYMBOL_TPREL));
+	}
       break;
 
     default:
@@ -7291,16 +7331,24 @@ mips_init_relocs (void)
   mips_lo_relocs[SYMBOL_TLSGD] = "%tlsgd(";
   mips_lo_relocs[SYMBOL_TLSLDM] = "%tlsldm(";
 
-  mips_split_p[SYMBOL_DTPREL] = true;
-  mips_hi_relocs[SYMBOL_DTPREL] = "%dtprel_hi(";
-  mips_lo_relocs[SYMBOL_DTPREL] = "%dtprel_lo(";
+  if (! TARGET_MIPS16_PCREL_LOADS)
+    {
+      mips_split_p[SYMBOL_DTPREL] = true;
+      mips_hi_relocs[SYMBOL_DTPREL] = "%dtprel_hi(";
+      mips_lo_relocs[SYMBOL_DTPREL] = "%dtprel_lo(";
 
-  mips_lo_relocs[SYMBOL_GOTTPREL] = "%gottprel(";
+      mips_split_p[SYMBOL_TPREL] = true;
+      mips_hi_relocs[SYMBOL_TPREL] = "%tprel_hi(";
+      mips_lo_relocs[SYMBOL_TPREL] = "%tprel_lo(";
 
-  mips_split_p[SYMBOL_TPREL] = true;
-  mips_hi_relocs[SYMBOL_TPREL] = "%tprel_hi(";
-  mips_lo_relocs[SYMBOL_TPREL] = "%tprel_lo(";
+      if (TARGET_MIPS16)
+	{
+	  mips_lo_relocs[SYMBOL_TPREL_HI] = "%tprel_hi(";
+	  mips_lo_relocs[SYMBOL_DTPREL_HI] = "%dtprel_hi(";
+	}
+    }
 
+  mips_lo_relocs[SYMBOL_GOTTPREL] = "%gottprel(";
   mips_lo_relocs[SYMBOL_HALF] = "%half(";
 }
 
@@ -12734,7 +12782,8 @@ struct mips_builtin_description {
   /* The function's prototype.  */
   enum mips_function_type function_type;
 
-  /* Whether the function is available.  */
+  /* Whether the function is available.  A NULL pointer value here
+     means the builtin is always available.  */
   unsigned int (*avail) (void);
 };
 
@@ -12904,6 +12953,12 @@ 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, NULL }
+
 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),
@@ -13187,7 +13242,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
@@ -13258,6 +13317,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
 
@@ -13309,11 +13370,19 @@ mips_init_builtins (void)
   for (i = 0; i < ARRAY_SIZE (mips_builtins); i++)
     {
       d = &mips_builtins[i];
-      if (d->avail ())
-	mips_builtin_decls[i]
-	  = add_builtin_function (d->name,
-				  mips_build_function_type (d->function_type),
-				  i, BUILT_IN_MD, NULL, NULL);
+      if (d->avail && !d->avail ())
+	continue;
+
+      mips_builtin_decls[i]
+	= add_builtin_function (d->name,
+				mips_build_function_type (d->function_type),
+				i, BUILT_IN_MD, NULL, NULL);
+
+      if (d->builtin_type == MIPS_BUILTIN_THREAD_POINTER)
+ 	{
+ 	  TREE_NOTHROW (mips_builtin_decls[i]) = 1;
+ 	  TREE_READONLY (mips_builtin_decls[i]) = 1;
+ 	}
     }
 }
 
@@ -13544,20 +13613,21 @@ mips_expand_builtin (tree exp, rtx target, rtx sub
 		     enum machine_mode mode, int ignore)
 {
   tree fndecl;
-  unsigned int fcode, avail;
+  unsigned int fcode;
   const struct mips_builtin_description *d;
 
   fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
   fcode = DECL_FUNCTION_CODE (fndecl);
   gcc_assert (fcode < ARRAY_SIZE (mips_builtins));
   d = &mips_builtins[fcode];
-  avail = d->avail ();
-  gcc_assert (avail != 0);
-  if (TARGET_MIPS16)
+  if (d->avail && !d->avail ())
     {
-      error ("built-in function %qE not supported for MIPS16",
-	     DECL_NAME (fndecl));
-      return ignore ? const0_rtx : CONST0_RTX (mode);
+      if (TARGET_MIPS16)
+	{
+	  error ("built-in function %qE not supported for MIPS16",
+		 DECL_NAME (fndecl));
+	  return ignore ? const0_rtx : CONST0_RTX (mode);
+	}
     }
   switch (d->builtin_type)
     {
@@ -13582,6 +13652,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 ();
 }
@@ -13771,7 +13844,7 @@ mips16_rewrite_pool_refs (rtx *x, void *data)
   struct mips16_rewrite_pool_refs_info *info =
     (struct mips16_rewrite_pool_refs_info *) data;
 
-  if (force_to_mem_operand (*x, Pmode))
+  if (force_to_mem_operand (*x, Pmode) || tls_reloc_operand (*x, Pmode))
     {
       rtx mem = force_const_mem (GET_MODE (*x), *x);
       validate_change (info->insn, x, mem, false);
@@ -13783,6 +13856,11 @@ mips16_rewrite_pool_refs (rtx *x, void *data)
       return -1;
     }
 
+  if (GET_CODE (*x) == UNSPEC
+      && XINT (*x, 1) == UNSPEC_TLS_GET_TP
+      && GET_CODE (XVECEXP (*x, 0, 0)) == SYMBOL_REF)
+    return -1;
+
   if (TARGET_MIPS16_TEXT_LOADS)
     mips16_rewrite_pool_constant (info->pool, x);
 
@@ -17108,6 +17186,28 @@ mips_expand_vec_minmax (rtx target, rtx op0, rtx o
   x = gen_rtx_IOR (vmode, t0, t1);
   emit_insn (gen_rtx_SET (VOIDmode, target, x));
 }
+
+/* Output a DTP/TP-relative relocation, used in MIPS16 TLS.  */
+
+void
+mips_output_tls_reloc_directive (rtx x, rtx size)
+{
+  const char *dir = NULL;
+  if (GET_CODE (x) == CONST)
+    x = XEXP (x, 0);
+  switch (UNSPEC_ADDRESS_TYPE (x))
+    {
+    case SYMBOL_DTPREL:
+      dir = INTVAL (size) == 4 ? ".dtprelword" : ".dtpreldword";
+      break;
+    case SYMBOL_TPREL:
+      dir = INTVAL (size) == 4 ? ".tprelword" : ".tpreldword";
+      break;
+    default:
+      gcc_unreachable ();
+    }
+  fprintf (asm_out_file, "\t%s\t%s\n", dir, XSTR (UNSPEC_ADDRESS (x), 0));
+}
 \f
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	(revision 182952)
+++ gcc/config/mips/mips.h	(working copy)
@@ -2841,8 +2841,32 @@ while (0)
 	jal " USER_LABEL_PREFIX #FUNC "\n\
 	" TEXT_SECTION_ASM_OP);
 #endif
+
+#else
+#if (defined _ABIO32 && _MIPS_SIM == _ABIO32)
+#ifdef __PIC__
+/* For MIPS16 PIC, construct the GP-value in $2 using PC-relative insns.  */
+#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
+   asm (SECTION_OP "\n\
+	li $2,%hi(_gp_disp)\n\
+	addiu $3,$pc,%lo(_gp_disp)\n\
+	sll $2,16\n\
+	addu $2,$3\n\
+	lw $2,%got(" USER_LABEL_PREFIX #FUNC ")($2)\n\
+	addiu $2,%lo(" USER_LABEL_PREFIX #FUNC ")\n\
+	move $25,$2\n\
+	jalr $2\n\
+	" TEXT_SECTION_ASM_OP);
+#else
+#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
+   asm (SECTION_OP "\n\
+	jal " USER_LABEL_PREFIX #FUNC "\n\
+	" TEXT_SECTION_ASM_OP);
 #endif
+#endif
 
+#endif /* __mips16 */
+
 #ifndef HAVE_AS_TLS
 #define HAVE_AS_TLS 0
 #endif

[-- Attachment #3: libgcc.diff --]
[-- Type: text/plain, Size: 2452 bytes --]

Index: libgcc/config.host
===================================================================
--- libgcc/config.host	(revision 182952)
+++ libgcc/config.host	(working copy)
@@ -740,12 +740,12 @@ mips*-*-netbsd*)			# NetBSD/mips, either endian.
 	;;
 mips64*-*-linux* | mipsisa64*-*-linux*)
 	extra_parts="$extra_parts crtfastmath.o"
-	tmake_file="${tmake_file} t-crtfm mips/t-mips16 mips/t-tpbit"
+	tmake_file="${tmake_file} t-crtfm mips/t-mips16 mips/t-tpbit t-slibgcc-libgcc"
 	md_unwind_header=mips/linux-unwind.h
 	;;
 mips*-*-linux*)				# Linux MIPS, either endian.
 	extra_parts="$extra_parts crtfastmath.o"
-	tmake_file="${tmake_file} t-crtfm mips/t-mips16"
+	tmake_file="${tmake_file} t-crtfm mips/t-mips16 t-slibgcc-libgcc"
 	md_unwind_header=mips/linux-unwind.h
 	;;
 mips*-*-openbsd*)
Index: libgcc/config/mips/t-mips16
===================================================================
--- libgcc/config/mips/t-mips16	(revision 182952)
+++ libgcc/config/mips/t-mips16	(working copy)
@@ -36,7 +36,8 @@ LIB1ASMFUNCS = _m16addsf3 _m16subsf3 _m16mulsf3 _m
 	_m16stubsc0 _m16stubsc1 _m16stubsc2 _m16stubsc5 _m16stubsc6 \
 	_m16stubsc9 _m16stubsc10 \
 	_m16stubdc0 _m16stubdc1 _m16stubdc2 _m16stubdc5 _m16stubdc6 \
-	_m16stubdc9 _m16stubdc10
+	_m16stubdc9 _m16stubdc10 \
+	_m16rdhwr
 
 SYNC = yes
 SYNC_CFLAGS = -mno-mips16
Index: libgcc/config/mips/mips16.S
===================================================================
--- libgcc/config/mips/mips16.S	(revision 182952)
+++ libgcc/config/mips/mips16.S	(working copy)
@@ -709,4 +709,18 @@ CALL_STUB_RET (__mips16_call_stub_dc_9, 9, DC)
 CALL_STUB_RET (__mips16_call_stub_dc_10, 10, DC)
 #endif
 #endif /* !__mips_single_float */
+
+#ifdef L_m16rdhwr
+STARTFN (__mips16_rdhwr)
+	/* Forced always hidden, to avoid exporting from shared libgcc.  */
+	.hidden	__mips16_rdhwr
+	.set	push
+	.set	mips32r2
+	.set	noreorder
+	rdhwr	$3,$29
+	.set	pop
+	j	$31
+	ENDFN (__mips16_rdhwr)
 #endif
+
+#endif
Index: libgcc/config/mips/crtfastmath.c
===================================================================
--- libgcc/config/mips/crtfastmath.c	(revision 182952)
+++ 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;

[-- Attachment #4: libgomp.diff --]
[-- Type: text/plain, Size: 1341 bytes --]

Index: libgomp/config/linux/mips/futex.h
===================================================================
--- libgomp/config/linux/mips/futex.h	(revision 182952)
+++ 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");

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-01-06 14:48 [PATCH] MIPS16 TLS support for GCC Chung-Lin Tang
@ 2012-01-08 11:21 ` Richard Sandiford
  2012-01-15 18:23   ` Richard Sandiford
  2012-02-03 10:03   ` Chung-Lin Tang
  0 siblings, 2 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-01-08 11:21 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches

Chung-Lin Tang <cltang@codesourcery.com> writes:
> Hi Richard,
> here are the patches for MIPS16 TLS.

Thanks for all the work.  It's great to see this hole in the MIPS16
support finally being plugged.

There seem to be three changes in the patch:

(1) Enable MIPS16 TLS (which must of course work independently
    of whether libgcc, libc, etc. were compiled as MIPS16).

(2) Allow the mips*-linux-gnu target libraries to be built as MIPS16.
    This relies on (1) but isn't required by (1).

(3) Define a built-in function for accessing the TLS pointer.  This is
    logically independent, although since the motivation was presumably
    to help build an external MIPS16 library, it's only worthwhile after (1).

I agree all three are good things to have, but I'd like to keep them as
separate patches.

Now that I've seen the implementation, (1) feels fairly low risk and
high value, so I'd like it to go in 4.7.

(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.

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.

(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 __mips16_rdhwr() function is not intended for general use, just
> tightly coupled compiler runtime support; therefore, it is only linked
> statically from libgcc.a, not exported from shared libgcc.

Well, a fair bit of libgcc is tightly-coupled compiler runtime support.
I don't really see any need to handle the new function differently from
the other __mips16 wrappers.  It's not like we're gaining any benefit in
the PIC call overhead: we can't turn JALRs into branches like we can for
nearby non-MIPS16-to-non-MIPS16 calls, so PIC calls will still go via
the GOT.  And we're not gaining any benefit in terms of ABI baggage either.
Future libgcc(.a)s would need to continue providing the function as
specified now, in order for future gccs to be able to link library
archives built with 4.7.

By making the function hidden, we lose the important ability to replace
all instances of it.  E.g. it isn't inconceivable that someone will find
a more efficient way to implement the function in cases where the RDHWR
is emulated (perhaps on a kernel other than Linux -- this support isn't
specific to *-linux-gnu).  Being able to replace all instances of a
function is useful for other things, such as profiling, debugging,
or working around processor errata.

> 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).

> 2012-01-06  Chung-Lin Tang  <cltang@codesourcery.com>
>
> 	gcc/
>         * config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Define versions
>         for MIPS16 O32.
>         * config/mips/mips-protos.h (mips_symbol_type): Add
>         SYMBOL_DTPREL_HI,SYMBOL_TPREL_HI. Update in comments.
>         (mips_output_tls_reloc_directive): New prototype.
>         * config/mips/mips-ftypes.def: Add (0, (POINTER)) entry.
>         * config/mips/predicates.md (tls_reloc_operand): New predicate.
>         * config/mips/mips.md (V0_REGNUM,PIC_JUMP_REGNUM): New constant.
>         (MIPS16 %dtprel_hi,%tprel_hi split pattern): New.
>         (consttable_tls_reloc): New.
>         (tls_get_tp_<mode>_mips16): New insn and split pattern.
>         (*tls_get_tp_<mode>_mips16_rdhwr): New insn pattern.

mips.c is missing from the changelog.

> Index: gcc/config/mips/mips.md
> ===================================================================
> --- gcc/config/mips/mips.md	(revision 182952)
> +++ gcc/config/mips/mips.md	(working copy)
> @@ -134,7 +134,9 @@
>  ])
>  
>  (define_constants
> -  [(TLS_GET_TP_REGNUM		3)
> +  [(V0_REGNUM			2)
> +   (TLS_GET_TP_REGNUM		3)
> +   (PIC_JUMP_REGNUM		25)
>     (RETURN_ADDR_REGNUM		31)
>     (CPRESTORE_SLOT_REGNUM	76)
>     (GOT_VERSION_REGNUM		79)

V0_REGNUM seems to be unused.  We should move PIC_FUNCTION_ADDR_REGNUM
here rather than add another name for it.

> @@ -3933,6 +3935,23 @@
>    operands[2] = mips_unspec_address (operands[1], SYMBOL_32_HIGH);
>  })
>  
> +;; MIPS16 %dtprel_hi,%tprel_hi split pattern. Similar transform
> +;; as above, for supporting MIPS16 TLS.
> +(define_split
> +  [(set (match_operand:SI 0 "d_operand")
> +	(high:SI (match_operand:SI 1 "tls_reloc_operand")))]
> +  "TARGET_MIPS16 && reload_completed"
> +  [(set (match_dup 0) (match_dup 2))
> +   (set (match_dup 0) (ashift:SI (match_dup 0) (const_int 16)))]
> +{
> +  /* SYMBOL_DTPREL_HI/TPREL_HI are ordered immediately after
> +     SYMBOL_DTPREL/TPREL respectively, so use unspec_type + 1.  */
> +  rtx unspec = XEXP (operands[1], 0);
> +  int unspec_type = XINT (unspec, 1);
> +  operands[2] = mips_unspec_address (XVECEXP (unspec, 0, 0),
> +				     unspec_type + 1 - UNSPEC_ADDRESS_FIRST);
> +})

mips_symbolic_constant_p allows an offset to be applied, in which case
the expression has the form:

   (const (plus (unspec [BASE] UNSPEC_FOO) (const_int OFFSET)))

I'm also not keen on hard-coding the +1 relationship.

I think I made the wrong call when adding SYMBOL_32_HIGH.  The idea
was to reuse the existing move patterns rather than add a special one.
That wasn't too bad when there was just this one simple case, but as
the patch shows, it doesn't generalise well.  Let's add a new "load
unshifted high" instruction instead.

> @@ -6593,6 +6623,60 @@
>     ; See tls_get_tp_<mode>
>     (set_attr "can_delay" "no")
>     (set_attr "mode" "<MODE>")])
> +
> +;; In MIPS16 mode, the TLS base pointer is accessed by a
> +;; libgcc helper function __mips16_rdhwr(), as 'rdhwr' is not
> +;; accessible in MIPS16.
> +;;
> +;; This is not represented as a call insn, to avoid the
> +;; unnecesarry clobbering of caller-save registers by a
> +;; function consisting only of: "rdhwr $3,$29; j $31; nop;"
> +;;
> +;; A $25 clobber is added to cater for a $25 load stub added by the
> +;; linker to __mips16_rdhwr when the call is made from non-PIC code.
> +
> +(define_insn_and_split "tls_get_tp_<mode>_mips16"
> +  [(set (match_operand:P 0 "register_operand" "=d")
> +	(unspec:P [(const_int 0)] UNSPEC_TLS_GET_TP))
> +   (clobber (reg:P TLS_GET_TP_REGNUM))
> +   (clobber (reg:P PIC_JUMP_REGNUM))
> +   (clobber (reg:P RETURN_ADDR_REGNUM))]
> +  "HAVE_AS_TLS && TARGET_MIPS16"
> +  "#"
> +  "&& reload_completed"
> +  [(parallel [(set (reg:P TLS_GET_TP_REGNUM)
> +  	      (unspec:P [(match_dup 1)] UNSPEC_TLS_GET_TP))
> +	      (clobber (reg:P PIC_JUMP_REGNUM))
> +	      (clobber (reg:P RETURN_ADDR_REGNUM))])
> +   (set (match_dup 0) (reg:P TLS_GET_TP_REGNUM)) ]
> +  {
> +    /* UNSPEC operand decides on direct/indirect pattern below.  */
> +    rtx sym = gen_rtx_SYMBOL_REF (Pmode, "__mips16_rdhwr");
> +    if (TARGET_ABSOLUTE_JUMPS)
> +      operands[1] = sym;
> +    else
> +      {
> +	operands[1] = gen_rtx_REG (Pmode, TLS_GET_TP_REGNUM);
> +	mips_emit_move (operands[1], sym);
> +      }
> +  }
> +  [(set_attr "type" "unknown")
> +   (set_attr "can_delay" "no")
> +   (set_attr "mode" "<MODE>")])

We should set "length" here, instead of "can_delay".  We should also
reuse the existing call-address handling -- call_insn_operand,
mips16_stub_function, etc. -- in order to make sure that things like
-mlong-calls are handled correctly.  It seems better to do that in
mips_get_tp (), then pass the call_insn_operand as an address.

> +(define_insn "*tls_get_tp_<mode>_mips16_rdhwr"
> +  [(set (reg:P TLS_GET_TP_REGNUM)
> +	(unspec:P [(match_operand:P 0 "")] UNSPEC_TLS_GET_TP))
> +	(clobber (reg:P PIC_JUMP_REGNUM))
> +	(clobber (reg:P RETURN_ADDR_REGNUM))]
> +  "HAVE_AS_TLS && TARGET_MIPS16"
> +  {
> +    return MIPS_CALL ("jal", operands, 0, -1);
> +  }
> +  [(set_attr "type" "call")
> +   (set_attr "can_delay" "no")
> +   (set_attr "mode" "<MODE>")])

Should be:

	(unspec:P [(match_operand:P 0 "call_insn_operand" "dS")]
		  UNSPEC_TLS_GET_TP))

because we need to tell post-reload passes what they can and can't do.
Should set "length" to 12 rather than "can_delay" to "no".

> Index: gcc/config/mips/mips.c
> ===================================================================
> --- gcc/config/mips/mips.c	(revision 182952)
> +++ gcc/config/mips/mips.c	(working copy)
> @@ -1763,6 +1767,8 @@ mips_symbolic_constant_p (rtx x, enum mips_symbol_
>      case SYMBOL_GOTTPREL:
>      case SYMBOL_TLS:
>      case SYMBOL_HALF:
> +    case SYMBOL_TPREL_HI:
> +    case SYMBOL_DTPREL_HI:
>        return false;
>      }

For the record, these types would need to have allowed the same offsets
as SYMBOL_TPREL and SYMBOL_DTPREL, otherwise we'd have ICEd after
splitting a SYMBOL + CONST_INT.  It's not important now though.

> @@ -1928,14 +1936,23 @@ mips_cannot_force_const_mem (enum machine_mode mod
>    if (mips_symbolic_constant_p (base, SYMBOL_CONTEXT_LEA, &type)
>        && type != SYMBOL_FORCE_TO_MEM)
>      {
> +      if (TARGET_MIPS16_PCREL_LOADS)
> +	{
> +	  /* Under MIPS16, TLS DTP/TP-relative offsets are loaded from the
> +	     constant pool, as this saves some code size compared to hi/lo
> +	     constructing.  */
> +	  if (type == SYMBOL_DTPREL || type == SYMBOL_TPREL)
> +	    return false;

As below, we should instead replace SYMBOL_FORCE_TO_MEM with a separate
predicate, since "force-to-mem"ness is now a separate property.

> @@ -2820,11 +2837,20 @@ 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)

This is part of (3), but: adjust the comment to mention the parameter.

> +  if (TARGET_MIPS16)
> +    {
> +      if (Pmode == DImode)
> +	emit_insn (gen_tls_get_tp_di_mips16 (tp));
> +      else
> +	emit_insn (gen_tls_get_tp_si_mips16 (tp));

So it looks like I missed a couple of places that should be using
PMODE_INSN.  Let's put the mode last -- tls_get_tp_mips16_{si,di} --
and use PMODE_INSN for both cases.  

> @@ -2875,13 +2895,23 @@ mips_legitimize_tls_address (rtx loc)
>  			    UNSPEC_TLS_LDM);
>        emit_libcall_block (insn, tmp1, v0, eqv);
>  
> -      tmp2 = mips_unspec_offset_high (NULL, tmp1, loc, SYMBOL_DTPREL);
> -      dest = gen_rtx_LO_SUM (Pmode, tmp2,
> -			     mips_unspec_address (loc, SYMBOL_DTPREL));
> +      if (TARGET_MIPS16_PCREL_LOADS)
> +	{
> +	  tmp2 = mips_force_temporary (NULL,
> +				       mips_unspec_address (loc,
> +							    SYMBOL_DTPREL));
> +	  dest = gen_rtx_PLUS (Pmode, tmp1, tmp2);
> +	}
> +      else
> +	{
> +	  tmp2 = mips_unspec_offset_high (NULL, tmp1, loc, SYMBOL_DTPREL);
> +	  dest = gen_rtx_LO_SUM (Pmode, tmp2,
> +				 mips_unspec_address (loc, SYMBOL_DTPREL));
> +	}

DEST is supposed to be a legitimate address (the sum of two registers isn't).
We should also use a higher-level test than TARGET_MIPS16_PCREL_LOADS.
Same for TPREL.

> @@ -17108,6 +17186,28 @@ mips_expand_vec_minmax (rtx target, rtx op0, rtx o
>    x = gen_rtx_IOR (vmode, t0, t1);
>    emit_insn (gen_rtx_SET (VOIDmode, target, x));
>  }
> +
> +/* Output a DTP/TP-relative relocation, used in MIPS16 TLS.  */
> +
> +void
> +mips_output_tls_reloc_directive (rtx x, rtx size)
> +{
> +  const char *dir = NULL;
> +  if (GET_CODE (x) == CONST)
> +    x = XEXP (x, 0);
> +  switch (UNSPEC_ADDRESS_TYPE (x))

As above, we allow small offsets to be applied to TPREL and DTPREL,
so we can't assume that we have (const (unspec ...)).

(In this case the HI/LO restriction doesn't apply, so we could actually
be a bit more relaxed in the offsets that we allow.  That's more
stage 1 material though.)

> Index: gcc/config/mips/mips.h
> ===================================================================
> --- gcc/config/mips/mips.h	(revision 182952)
> +++ gcc/config/mips/mips.h	(working copy)
> @@ -2841,8 +2841,32 @@ while (0)
>  	jal " USER_LABEL_PREFIX #FUNC "\n\
>  	" TEXT_SECTION_ASM_OP);
>  #endif
> +
> +#else
> +#if (defined _ABIO32 && _MIPS_SIM == _ABIO32)
> +#ifdef __PIC__
> +/* For MIPS16 PIC, construct the GP-value in $2 using PC-relative insns.  */
> +#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
> +   asm (SECTION_OP "\n\
> +	li $2,%hi(_gp_disp)\n\
> +	addiu $3,$pc,%lo(_gp_disp)\n\
> +	sll $2,16\n\
> +	addu $2,$3\n\
> +	lw $2,%got(" USER_LABEL_PREFIX #FUNC ")($2)\n\
> +	addiu $2,%lo(" USER_LABEL_PREFIX #FUNC ")\n\
> +	move $25,$2\n\
> +	jalr $2\n\
> +	" TEXT_SECTION_ASM_OP);
> +#else
> +#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
> +   asm (SECTION_OP "\n\
> +	jal " USER_LABEL_PREFIX #FUNC "\n\
> +	" TEXT_SECTION_ASM_OP);
>  #endif
> +#endif

This is part of (2), but: why is the !__PIC__ definition necessary for just
this one case?  If it's correct, then isn't it correct for !MIPS16 too?
And why isn't the default definition in crtstuff.c good enough?

I wanted to try this out on my set-up, so here's the modified version of (1).
I found I needed a GAS patch in order to get clean results.  I'll post
that patch soon.

The mips_init_relocs change is mostly reindentation, so it looks more
frightening that it is.

A consequence of the SYMBOL_FORCE_TO_MEM change is that -mcode-readable=pcrel
now allows non-PC-relative LEAs of absolute labels to be done via the
constant pool rather than using %hi and %lo.  I think this is a good
thing (it's shorter, and matches the corresponding symbol case)
so I've adjusted the testcase to allow it.

Tested on mips64-linux-gnu.  Does the patch look OK to you?

Thanks,
Richard


gcc/
2012-01-08  Chung-Lin Tang  <cltang@codesourcery.com>
	    Richard Sandiford  <rdsandiford@googlemail.com>

	* config/mips/mips-protos.h (SYMBOL_FORCE_TO_MEM): Delete.
	(SYMBOL_32_HIGH): Likewise.
	(mips_output_tls_reloc_directive): Declare.
	* config/mips/mips.h (PIC_FUNCTION_ADDR_REGNUM): Move to mips.md.
	(mips_use_pcrel_pool_p, mips_lo_relocs, mips_hi_relocs): Declare.
	* config/mips/mips.c (mips_use_pcrel_pool_p): New variable.
	(mips_lo_relocs, mips_hi_relocs): Make extern.
	(mips16_stub_function): Move up file.
	(mips_classify_symbol): Remove SYMBOL_FORCE_TO_MEM handling.
	(mips_symbolic_constant_p): Likewise.  Remove SYMBOL_32_HIGH too.
	(mips_symbol_insns_1): Likewise.  Check mips_use_pcrel_pool_p.
	(mips_cannot_force_const_mem): Use mips_use_pcrel_pool_p instead
	of SYMBOL_FORCE_TO_MEM.  Only check mips_tls_symbol_ref_1
	if it's false.
	(mips_get_tp): Add MIPS16 support.
	(mips_legitimize_tls_address): Remove MIPS16 sorry().
	Generalize DTPREL and TPREL handling.
	(mips_init_relocs): Initialize mips_use_pcrel_pool_p.
	Add MIPS16 TLS support.
	(mips_output_tls_reloc_directive): New function.
	(mips16_rewrite_pool_refs): Ignore UNSPEC_TLS_GET_TPs.
	* config/mips/predicates.md (symbolic_operand_with_high)
	(tls_reloc_operand): New predicates.
	(force_to_mem_operand): Use mips_use_pcrel_pool_p.
	* config/mips/mips.md (UNSPEC_UNSHIFTED_HIGH): New unspec.
	(PIC_FUNCTION_ADDR_REGNUM): Moved from mips.h.
	(*unshifted_high): New instruction.  Use it for MIPS16
	high splitter.
	(consttable_tls_reloc, tls_get_tp_mips16_<mode>): New patterns.
	(*tls_get_tp_mips16_call_<mode>): Likewise.

gcc/testsuite/
	* gcc.target/mips/code-readable-2.c: Allow the jump table address
	to be loaded from the constant pool, rather than via %hi and %lo.

libgcc/
2012-01-08  Chung-Lin Tang  <cltang@codesourcery.com>
	    Richard Sandiford  <rdsandiford@googlemail.com>

	* config/mips/libgcc-mips16.ver (__mips16_rdhwr): Add.
	* config/mips/mips16.S (__mips16_rdhwr): New function.
	* config/mips/t-mips16 (LIB1ASMFUNCS): Add _m16rdhwr.

Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc/config/mips/mips-protos.h	2012-01-08 08:43:13.000000000 +0000
+++ gcc/config/mips/mips-protos.h	2012-01-08 10:12:57.000000000 +0000
@@ -56,9 +56,6 @@ enum mips_symbol_context {
        The symbol's value will be calculated using a MIPS16 PC-relative
        calculation.
 
-   SYMBOL_FORCE_TO_MEM
-       The symbol's value must be forced to memory and loaded from there.
-
    SYMBOL_GOT_PAGE_OFST
        The symbol's value will be calculated by loading an address
        from the GOT and then applying a 16-bit offset.
@@ -94,9 +91,6 @@ enum mips_symbol_context {
        UNSPEC wrappers around SYMBOL_TLS, corresponding to the
        thread-local storage relocation operators.
 
-   SYMBOL_32_HIGH
-       For a 32-bit symbolic address X, this is the value of %hi(X).
-
    SYMBOL_64_HIGH
        For a 64-bit symbolic address X, this is the value of
        (%highest(X) << 16) + %higher(X).
@@ -116,7 +110,6 @@ enum mips_symbol_type {
   SYMBOL_ABSOLUTE,
   SYMBOL_GP_RELATIVE,
   SYMBOL_PC_RELATIVE,
-  SYMBOL_FORCE_TO_MEM,
   SYMBOL_GOT_PAGE_OFST,
   SYMBOL_GOT_DISP,
   SYMBOL_GOTOFF_PAGE,
@@ -129,7 +122,6 @@ enum mips_symbol_type {
   SYMBOL_DTPREL,
   SYMBOL_GOTTPREL,
   SYMBOL_TPREL,
-  SYMBOL_32_HIGH,
   SYMBOL_64_HIGH,
   SYMBOL_64_MID,
   SYMBOL_64_LOW,
@@ -260,6 +252,7 @@ extern void mips_push_asm_switch (struct
 extern void mips_pop_asm_switch (struct mips_asm_switch *);
 extern void mips_output_external (FILE *, tree, const char *);
 extern void mips_output_ascii (FILE *, const char *, size_t);
+extern const char *mips_output_tls_reloc_directive (rtx *);
 extern void mips_output_aligned_decl_common (FILE *, tree, const char *,
 					     unsigned HOST_WIDE_INT,
 					     unsigned int);
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	2012-01-08 08:43:13.000000000 +0000
+++ gcc/config/mips/mips.h	2012-01-08 10:12:57.000000000 +0000
@@ -1785,8 +1785,6 @@ #define GLOBAL_POINTER_REGNUM (GP_REG_FI
    from there after reload.  */
 #define PIC_OFFSET_TABLE_REGNUM \
   (reload_completed ? REGNO (pic_offset_table_rtx) : GLOBAL_POINTER_REGNUM)
-
-#define PIC_FUNCTION_ADDR_REGNUM (GP_REG_FIRST + 25)
 \f
 /* Define the classes of registers for register constraints in the
    machine description.  Also define ranges of constants.
@@ -2868,6 +2866,9 @@ struct mips_asm_switch {
 extern int mips_dwarf_regno[];
 extern bool mips_split_p[];
 extern bool mips_split_hi_p[];
+extern bool mips_use_pcrel_pool_p[];
+extern const char *mips_lo_relocs[];
+extern const char *mips_hi_relocs[];
 extern enum processor mips_arch;        /* which cpu to codegen for */
 extern enum processor mips_tune;        /* which cpu to schedule for */
 extern int mips_isa;			/* architectural level */
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2012-01-08 08:43:13.000000000 +0000
+++ gcc/config/mips/mips.c	2012-01-08 10:54:56.000000000 +0000
@@ -573,13 +573,17 @@ static GTY (()) int mips_output_filename
    can be split by mips_split_symbol.  */
 bool mips_split_hi_p[NUM_SYMBOL_TYPES];
 
+/* mips_use_pcrel_pool_p[X] is true if symbols of type X should be
+   forced into a PC-relative constant pool.  */
+bool mips_use_pcrel_pool_p[NUM_SYMBOL_TYPES];
+
 /* mips_lo_relocs[X] is the relocation to use when a symbol of type X
    appears in a LO_SUM.  It can be null if such LO_SUMs aren't valid or
    if they are matched by a special .md file pattern.  */
-static const char *mips_lo_relocs[NUM_SYMBOL_TYPES];
+const char *mips_lo_relocs[NUM_SYMBOL_TYPES];
 
 /* Likewise for HIGHs.  */
-static const char *mips_hi_relocs[NUM_SYMBOL_TYPES];
+const char *mips_hi_relocs[NUM_SYMBOL_TYPES];
 
 /* Target state for MIPS16.  */
 struct target_globals *mips16_globals;
@@ -1440,6 +1444,18 @@ mips_legitimate_constant_p (enum machine
   return mips_const_insns (x) > 0;
 }
 \f
+/* Return a SYMBOL_REF for a MIPS16 function called NAME.  */
+
+static rtx
+mips16_stub_function (const char *name)
+{
+  rtx x;
+
+  x = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (name));
+  SYMBOL_REF_FLAGS (x) |= (SYMBOL_FLAG_EXTERNAL | SYMBOL_FLAG_FUNCTION);
+  return x;
+}
+\f
 /* Return true if symbols of type TYPE require a GOT access.  */
 
 static bool
@@ -1644,9 +1660,6 @@ mips_classify_symbol (const_rtx x, enum
       return SYMBOL_GOT_PAGE_OFST;
     }
 
-  if (TARGET_MIPS16_PCREL_LOADS && context != SYMBOL_CONTEXT_CALL)
-    return SYMBOL_FORCE_TO_MEM;
-
   return SYMBOL_ABSOLUTE;
 }
 
@@ -1709,8 +1722,6 @@ mips_symbolic_constant_p (rtx x, enum mi
   switch (*symbol_type)
     {
     case SYMBOL_ABSOLUTE:
-    case SYMBOL_FORCE_TO_MEM:
-    case SYMBOL_32_HIGH:
     case SYMBOL_64_HIGH:
     case SYMBOL_64_MID:
     case SYMBOL_64_LOW:
@@ -1776,6 +1787,17 @@ mips_symbolic_constant_p (rtx x, enum mi
 static int
 mips_symbol_insns_1 (enum mips_symbol_type type, enum machine_mode mode)
 {
+  if (mips_use_pcrel_pool_p[(int) type])
+    {
+      if (mode == MAX_MACHINE_MODE)
+	/* LEAs will be converted into constant-pool references by
+	   mips_reorg.  */
+	type = SYMBOL_PC_RELATIVE;
+      else
+	/* The constant must be loaded and then dereferenced.  */
+	return 0;
+    }
+
   switch (type)
     {
     case SYMBOL_ABSOLUTE:
@@ -1809,15 +1831,6 @@ mips_symbol_insns_1 (enum mips_symbol_ty
       /* The constant must be loaded using ADDIUPC or DADDIUPC first.  */
       return 0;
 
-    case SYMBOL_FORCE_TO_MEM:
-      /* LEAs will be converted into constant-pool references by
-	 mips_reorg.  */
-      if (mode == MAX_MACHINE_MODE)
-	return 1;
-
-      /* The constant must be loaded and then dereferenced.  */
-      return 0;
-
     case SYMBOL_GOT_DISP:
       /* The constant will have to be loaded from the GOT before it
 	 is used in an address.  */
@@ -1850,7 +1863,6 @@ mips_symbol_insns_1 (enum mips_symbol_ty
     case SYMBOL_GOTOFF_DISP:
     case SYMBOL_GOTOFF_CALL:
     case SYMBOL_GOTOFF_LOADGP:
-    case SYMBOL_32_HIGH:
     case SYMBOL_64_HIGH:
     case SYMBOL_64_MID:
     case SYMBOL_64_LOW:
@@ -1925,9 +1937,12 @@ mips_cannot_force_const_mem (enum machin
     return true;
 
   split_const (x, &base, &offset);
-  if (mips_symbolic_constant_p (base, SYMBOL_CONTEXT_LEA, &type)
-      && type != SYMBOL_FORCE_TO_MEM)
+  if (mips_symbolic_constant_p (base, SYMBOL_CONTEXT_LEA, &type))
     {
+      /* See whether we explicitly want these symbols in the pool.  */
+      if (mips_use_pcrel_pool_p[(int) type])
+	return false;
+
       /* The same optimization as for CONST_INT.  */
       if (SMALL_INT (offset) && mips_symbol_insns (type, MAX_MACHINE_MODE) > 0)
 	return true;
@@ -2822,13 +2837,18 @@ mips_call_tls_get_addr (rtx sym, enum mi
 static rtx
 mips_get_tp (void)
 {
-  rtx tp;
+  rtx tp, fn;
 
   tp = gen_reg_rtx (Pmode);
-  if (Pmode == DImode)
-    emit_insn (gen_tls_get_tp_di (tp));
+  if (TARGET_MIPS16)
+    {
+      fn = mips16_stub_function ("__mips16_rdhwr");
+      if (!call_insn_operand (fn, VOIDmode))
+	fn = force_reg (Pmode, fn);
+      emit_insn (PMODE_INSN (gen_tls_get_tp_mips16, (tp, fn)));
+    }
   else
-    emit_insn (gen_tls_get_tp_si (tp));
+    emit_insn (PMODE_INSN (gen_tls_get_tp, (tp)));
   return tp;
 }
 
@@ -2839,15 +2859,9 @@ mips_get_tp (void)
 static rtx
 mips_legitimize_tls_address (rtx loc)
 {
-  rtx dest, insn, v0, tp, tmp1, tmp2, eqv;
+  rtx dest, insn, v0, tp, tmp1, tmp2, eqv, offset;
   enum tls_model model;
 
-  if (TARGET_MIPS16)
-    {
-      sorry ("MIPS16 TLS");
-      return gen_reg_rtx (Pmode);
-    }
-
   model = SYMBOL_REF_TLS_MODEL (loc);
   /* Only TARGET_ABICALLS code can have more than one module; other
      code must be be static and should not use a GOT.  All TLS models
@@ -2875,9 +2889,15 @@ mips_legitimize_tls_address (rtx loc)
 			    UNSPEC_TLS_LDM);
       emit_libcall_block (insn, tmp1, v0, eqv);
 
-      tmp2 = mips_unspec_offset_high (NULL, tmp1, loc, SYMBOL_DTPREL);
-      dest = gen_rtx_LO_SUM (Pmode, tmp2,
-			     mips_unspec_address (loc, SYMBOL_DTPREL));
+      offset = mips_unspec_address (loc, SYMBOL_DTPREL);
+      if (mips_split_p[SYMBOL_DTPREL])
+	{
+	  tmp2 = mips_unspec_offset_high (NULL, tmp1, loc, SYMBOL_DTPREL);
+	  dest = gen_rtx_LO_SUM (Pmode, tmp2, offset);
+	}
+      else
+	dest = expand_binop (Pmode, add_optab, tmp1, offset,
+			     0, 0, OPTAB_DIRECT);
       break;
 
     case TLS_MODEL_INITIAL_EXEC:
@@ -2893,10 +2913,16 @@ mips_legitimize_tls_address (rtx loc)
       break;
 
     case TLS_MODEL_LOCAL_EXEC:
-      tp = mips_get_tp ();
-      tmp1 = mips_unspec_offset_high (NULL, tp, loc, SYMBOL_TPREL);
-      dest = gen_rtx_LO_SUM (Pmode, tmp1,
-			     mips_unspec_address (loc, SYMBOL_TPREL));
+      tmp1 = mips_get_tp ();
+      offset = mips_unspec_address (loc, SYMBOL_TPREL);
+      if (mips_split_p[SYMBOL_TPREL])
+	{
+	  tmp2 = mips_unspec_offset_high (NULL, tmp1, loc, SYMBOL_TPREL);
+	  dest = gen_rtx_LO_SUM (Pmode, tmp2, offset);
+	}
+      else
+	dest = expand_binop (Pmode, add_optab, tmp1, offset,
+			     0, 0, OPTAB_DIRECT);
       break;
 
     default:
@@ -5866,18 +5892,6 @@ struct mips16_stub {
 };
 static struct mips16_stub *mips16_stubs;
 
-/* Return a SYMBOL_REF for a MIPS16 function called NAME.  */
-
-static rtx
-mips16_stub_function (const char *name)
-{
-  rtx x;
-
-  x = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (name));
-  SYMBOL_REF_FLAGS (x) |= (SYMBOL_FLAG_EXTERNAL | SYMBOL_FLAG_FUNCTION);
-  return x;
-}
-
 /* Return the two-character string that identifies floating-point
    return mode MODE in the name of a MIPS16 function stub.  */
 
@@ -7192,38 +7206,44 @@ mips_init_relocs (void)
 {
   memset (mips_split_p, '\0', sizeof (mips_split_p));
   memset (mips_split_hi_p, '\0', sizeof (mips_split_hi_p));
+  memset (mips_use_pcrel_pool_p, '\0', sizeof (mips_use_pcrel_pool_p));
   memset (mips_hi_relocs, '\0', sizeof (mips_hi_relocs));
   memset (mips_lo_relocs, '\0', sizeof (mips_lo_relocs));
 
-  if (ABI_HAS_64BIT_SYMBOLS)
+  if (TARGET_MIPS16_PCREL_LOADS)
+    mips_use_pcrel_pool_p[SYMBOL_ABSOLUTE] = true;
+  else
     {
-      if (TARGET_EXPLICIT_RELOCS)
+      if (ABI_HAS_64BIT_SYMBOLS)
 	{
-	  mips_split_p[SYMBOL_64_HIGH] = true;
-	  mips_hi_relocs[SYMBOL_64_HIGH] = "%highest(";
-	  mips_lo_relocs[SYMBOL_64_HIGH] = "%higher(";
-
-	  mips_split_p[SYMBOL_64_MID] = true;
-	  mips_hi_relocs[SYMBOL_64_MID] = "%higher(";
-	  mips_lo_relocs[SYMBOL_64_MID] = "%hi(";
-
-	  mips_split_p[SYMBOL_64_LOW] = true;
-	  mips_hi_relocs[SYMBOL_64_LOW] = "%hi(";
-	  mips_lo_relocs[SYMBOL_64_LOW] = "%lo(";
+	  if (TARGET_EXPLICIT_RELOCS)
+	    {
+	      mips_split_p[SYMBOL_64_HIGH] = true;
+	      mips_hi_relocs[SYMBOL_64_HIGH] = "%highest(";
+	      mips_lo_relocs[SYMBOL_64_HIGH] = "%higher(";
+
+	      mips_split_p[SYMBOL_64_MID] = true;
+	      mips_hi_relocs[SYMBOL_64_MID] = "%higher(";
+	      mips_lo_relocs[SYMBOL_64_MID] = "%hi(";
+
+	      mips_split_p[SYMBOL_64_LOW] = true;
+	      mips_hi_relocs[SYMBOL_64_LOW] = "%hi(";
+	      mips_lo_relocs[SYMBOL_64_LOW] = "%lo(";
 
-	  mips_split_p[SYMBOL_ABSOLUTE] = true;
-	  mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
+	      mips_split_p[SYMBOL_ABSOLUTE] = true;
+	      mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
+	    }
 	}
-    }
-  else
-    {
-      if (TARGET_EXPLICIT_RELOCS || mips_split_addresses_p () || TARGET_MIPS16)
+      else
 	{
-	  mips_split_p[SYMBOL_ABSOLUTE] = true;
-	  mips_hi_relocs[SYMBOL_ABSOLUTE] = "%hi(";
-	  mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
-
-	  mips_lo_relocs[SYMBOL_32_HIGH] = "%hi(";
+	  if (TARGET_EXPLICIT_RELOCS
+	      || mips_split_addresses_p ()
+	      || TARGET_MIPS16)
+	    {
+	      mips_split_p[SYMBOL_ABSOLUTE] = true;
+	      mips_hi_relocs[SYMBOL_ABSOLUTE] = "%hi(";
+	      mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
+	    }
 	}
     }
 
@@ -7291,16 +7311,23 @@ mips_init_relocs (void)
   mips_lo_relocs[SYMBOL_TLSGD] = "%tlsgd(";
   mips_lo_relocs[SYMBOL_TLSLDM] = "%tlsldm(";
 
-  mips_split_p[SYMBOL_DTPREL] = true;
-  mips_hi_relocs[SYMBOL_DTPREL] = "%dtprel_hi(";
-  mips_lo_relocs[SYMBOL_DTPREL] = "%dtprel_lo(";
-
-  mips_lo_relocs[SYMBOL_GOTTPREL] = "%gottprel(";
+  if (TARGET_MIPS16_PCREL_LOADS)
+    {
+      mips_use_pcrel_pool_p[SYMBOL_DTPREL] = true;
+      mips_use_pcrel_pool_p[SYMBOL_TPREL] = true;
+    }
+  else
+    {
+      mips_split_p[SYMBOL_DTPREL] = true;
+      mips_hi_relocs[SYMBOL_DTPREL] = "%dtprel_hi(";
+      mips_lo_relocs[SYMBOL_DTPREL] = "%dtprel_lo(";
 
-  mips_split_p[SYMBOL_TPREL] = true;
-  mips_hi_relocs[SYMBOL_TPREL] = "%tprel_hi(";
-  mips_lo_relocs[SYMBOL_TPREL] = "%tprel_lo(";
+      mips_split_p[SYMBOL_TPREL] = true;
+      mips_hi_relocs[SYMBOL_TPREL] = "%tprel_hi(";
+      mips_lo_relocs[SYMBOL_TPREL] = "%tprel_lo(";
+    }
 
+  mips_lo_relocs[SYMBOL_GOTTPREL] = "%gottprel(";
   mips_lo_relocs[SYMBOL_HALF] = "%half(";
 }
 
@@ -8082,6 +8109,29 @@ mips_output_ascii (FILE *stream, const c
   fprintf (stream, "\"\n");
 }
 
+/* Return the pseudo-op for full SYMBOL_(D)TPREL address *ADDR.
+   Update *ADDR with the operand that should be printed.  */
+
+const char *
+mips_output_tls_reloc_directive (rtx *addr)
+{
+  enum mips_symbol_type type;
+
+  type = mips_classify_symbolic_expression (*addr, SYMBOL_CONTEXT_LEA);
+  *addr = mips_strip_unspec_address (*addr);
+  switch (type)
+    {
+    case SYMBOL_DTPREL:
+      return Pmode == SImode ? ".dtprelword\t%0" : ".dtpreldword\t%0";
+
+    case SYMBOL_TPREL:
+      return Pmode == SImode ? ".tprelword\t%0" : ".tpreldword\t%0";
+
+    default:
+      gcc_unreachable ();
+    }
+}
+
 /* Emit either a label, .comm, or .lcomm directive.  When using assembler
    macros, mark the symbol as written so that mips_asm_output_external
    won't emit an .extern for it.  STREAM is the output file, NAME is the
@@ -13783,6 +13833,10 @@ mips16_rewrite_pool_refs (rtx *x, void *
       return -1;
     }
 
+  /* Don't rewrite the __mips16_rdwr symbol.  */
+  if (GET_CODE (*x) == UNSPEC && XINT (*x, 1) == UNSPEC_TLS_GET_TP)
+    return -1;
+
   if (TARGET_MIPS16_TEXT_LOADS)
     mips16_rewrite_pool_constant (info->pool, x);
 
Index: gcc/config/mips/predicates.md
===================================================================
--- gcc/config/mips/predicates.md	2012-01-08 08:43:13.000000000 +0000
+++ gcc/config/mips/predicates.md	2012-01-08 10:12:57.000000000 +0000
@@ -288,12 +288,20 @@ (define_predicate "absolute_symbolic_ope
 	  && type == SYMBOL_ABSOLUTE);
 })
 
+(define_predicate "symbolic_operand_with_high"
+  (match_code "const,symbol_ref,label_ref")
+{
+  enum mips_symbol_type type;
+  return (mips_symbolic_constant_p (op, SYMBOL_CONTEXT_LEA, &type)
+	  && mips_hi_relocs[(int) type]);
+})
+
 (define_predicate "force_to_mem_operand"
   (match_code "const,symbol_ref,label_ref")
 {
   enum mips_symbol_type symbol_type;
   return (mips_symbolic_constant_p (op, SYMBOL_CONTEXT_LEA, &symbol_type)
-	  && symbol_type == SYMBOL_FORCE_TO_MEM);
+	  && mips_use_pcrel_pool_p[(int) symbol_type]);
 })
 
 (define_predicate "got_disp_operand"
@@ -312,6 +320,14 @@ (define_predicate "got_page_ofst_operand
 	  && type == SYMBOL_GOT_PAGE_OFST);
 })
 
+(define_predicate "tls_reloc_operand"
+  (match_code "const,symbol_ref,label_ref")
+{
+  enum mips_symbol_type type;
+  return (mips_symbolic_constant_p (op, SYMBOL_CONTEXT_LEA, &type)
+	  && (type == SYMBOL_DTPREL || type == SYMBOL_TPREL));
+})
+
 (define_predicate "symbol_ref_operand"
   (match_code "symbol_ref"))
 
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	2012-01-08 08:43:13.000000000 +0000
+++ gcc/config/mips/mips.md	2012-01-08 10:45:39.000000000 +0000
@@ -103,6 +103,7 @@ (define_c_enum "unspec" [
   UNSPEC_LOAD_GOT
   UNSPEC_TLS_LDM
   UNSPEC_TLS_GET_TP
+  UNSPEC_UNSHIFTED_HIGH
 
   ;; MIPS16 constant pools.
   UNSPEC_ALIGN
@@ -135,6 +136,7 @@ (define_c_enum "unspec" [
 
 (define_constants
   [(TLS_GET_TP_REGNUM		3)
+   (PIC_FUNCTION_ADDR_REGNUM	25)
    (RETURN_ADDR_REGNUM		31)
    (CPRESTORE_SLOT_REGNUM	76)
    (GOT_VERSION_REGNUM		79)
@@ -3924,14 +3926,19 @@ (define_insn_and_split "*lea64"
 ;;
 ;; on MIPS16 targets.
 (define_split
-  [(set (match_operand:SI 0 "d_operand")
-	(high:SI (match_operand:SI 1 "absolute_symbolic_operand")))]
+  [(set (match_operand:P 0 "d_operand")
+	(high:P (match_operand:P 1 "symbolic_operand_with_high")))]
   "TARGET_MIPS16 && reload_completed"
-  [(set (match_dup 0) (match_dup 2))
-   (set (match_dup 0) (ashift:SI (match_dup 0) (const_int 16)))]
-{
-  operands[2] = mips_unspec_address (operands[1], SYMBOL_32_HIGH);
-})
+  [(set (match_dup 0) (unspec:P [(match_dup 1)] UNSPEC_UNSHIFTED_HIGH))
+   (set (match_dup 0) (ashift:P (match_dup 0) (const_int 16)))])
+
+(define_insn "*unshifted_high"
+  [(set (match_operand:P 0 "d_operand" "=d")
+	(unspec:P [(match_operand:P 1 "symbolic_operand_with_high")]
+		  UNSPEC_UNSHIFTED_HIGH))]
+  ""
+  "li\t%0,%h1"
+  [(set_attr "extended_mips16" "yes")])
 
 ;; Insns to fetch a symbol from a big GOT.
 
@@ -6492,6 +6499,14 @@ (define_expand "mov<mode>cc"
 ;;  ....................
 ;;
 
+(define_insn "consttable_tls_reloc"
+  [(unspec_volatile [(match_operand 0 "tls_reloc_operand" "")
+		     (match_operand 1 "const_int_operand" "")]
+		    UNSPEC_CONSTTABLE_INT)]
+  "TARGET_MIPS16_PCREL_LOADS"
+  { return mips_output_tls_reloc_directive (&operands[0]); }
+  [(set (attr "length") (symbol_ref "INTVAL (operands[1])"))])
+
 (define_insn "consttable_int"
   [(unspec_volatile [(match_operand 0 "consttable_operand" "")
 		     (match_operand 1 "const_int_operand" "")]
@@ -6593,6 +6608,49 @@ (define_insn "*tls_get_tp_<mode>_split"
    ; See tls_get_tp_<mode>
    (set_attr "can_delay" "no")
    (set_attr "mode" "<MODE>")])
+
+;; In MIPS16 mode, the TLS base pointer is accessed by a
+;; libgcc helper function __mips16_rdhwr(), as 'rdhwr' is not
+;; accessible in MIPS16.
+;;
+;; This is not represented as a call insn, to avoid the
+;; unnecesarry clobbering of caller-save registers by a
+;; function consisting only of: "rdhwr $3,$29; j $31; nop;"
+;;
+;; A $25 clobber is added to cater for a $25 load stub added by the
+;; linker to __mips16_rdhwr when the call is made from non-PIC code.
+
+(define_insn_and_split "tls_get_tp_mips16_<mode>"
+  [(set (match_operand:P 0 "register_operand" "=d")
+	(unspec:P [(match_operand:P 1 "call_insn_operand" "dS")]
+		  UNSPEC_TLS_GET_TP))
+   (clobber (reg:P TLS_GET_TP_REGNUM))
+   (clobber (reg:P PIC_FUNCTION_ADDR_REGNUM))
+   (clobber (reg:P RETURN_ADDR_REGNUM))]
+  "HAVE_AS_TLS && TARGET_MIPS16"
+  "#"
+  "&& reload_completed"
+  [(parallel [(set (reg:P TLS_GET_TP_REGNUM)
+	  	   (unspec:P [(match_dup 1)] UNSPEC_TLS_GET_TP))
+	      (clobber (reg:P PIC_FUNCTION_ADDR_REGNUM))
+	      (clobber (reg:P RETURN_ADDR_REGNUM))])
+   (set (match_dup 0) (reg:P TLS_GET_TP_REGNUM))]
+  ""
+  [(set_attr "type" "multi")
+   (set_attr "length" "16")
+   (set_attr "mode" "<MODE>")])
+
+(define_insn "*tls_get_tp_mips16_call_<mode>"
+  [(set (reg:P TLS_GET_TP_REGNUM)
+	(unspec:P [(match_operand:P 0 "call_insn_operand" "dS")]
+		  UNSPEC_TLS_GET_TP))
+   (clobber (reg:P PIC_FUNCTION_ADDR_REGNUM))
+   (clobber (reg:P RETURN_ADDR_REGNUM))]
+  "HAVE_AS_TLS && TARGET_MIPS16"
+  { return MIPS_CALL ("jal", operands, 0, -1); }
+  [(set_attr "type" "call")
+   (set_attr "length" "12")
+   (set_attr "mode" "<MODE>")])
 \f
 ;; Synchronization instructions.
 
Index: gcc/testsuite/gcc.target/mips/code-readable-2.c
===================================================================
--- gcc/testsuite/gcc.target/mips/code-readable-2.c	2012-01-08 10:13:04.000000000 +0000
+++ gcc/testsuite/gcc.target/mips/code-readable-2.c	2012-01-08 10:15:24.000000000 +0000
@@ -26,8 +26,7 @@ bar (void)
 
 /* { dg-final { scan-assembler-not "\tla\t" } } */
 /* { dg-final { scan-assembler-not "\t\\.half\t" } } */
-/* { dg-final { scan-assembler "%hi\\(\[^)\]*L" } } */
-/* { dg-final { scan-assembler "%lo\\(\[^)\]*L" } } */
+/* { dg-final { scan-assembler "\t\\.word\t\[^\n\]*L" } } */
 
 /* { dg-final { scan-assembler "\t\\.word\tk\n" } } */
 /* { dg-final { scan-assembler-not "%hi\\(k\\)" } } */
Index: libgcc/config/mips/libgcc-mips16.ver
===================================================================
--- libgcc/config/mips/libgcc-mips16.ver	2012-01-08 08:43:13.000000000 +0000
+++ libgcc/config/mips/libgcc-mips16.ver	2012-01-08 10:12:57.000000000 +0000
@@ -84,3 +84,7 @@ GCC_4.4.0 {
   __mips16_call_stub_dc_9
   __mips16_call_stub_dc_10
 }
+
+GCC_4.7.0 {
+  __mips16_rdhwr
+}
Index: libgcc/config/mips/mips16.S
===================================================================
--- libgcc/config/mips/mips16.S	2012-01-08 08:43:13.000000000 +0000
+++ libgcc/config/mips/mips16.S	2012-01-08 10:12:57.000000000 +0000
@@ -709,4 +709,15 @@ CALL_STUB_RET (__mips16_call_stub_dc_9,
 CALL_STUB_RET (__mips16_call_stub_dc_10, 10, DC)
 #endif
 #endif /* !__mips_single_float */
+
+#ifdef L_m16rdhwr
+STARTFN (__mips16_rdhwr)
+	.set	push
+	.set	mips32r2
+	.set	noreorder
+	rdhwr	$3,$29
+	.set	pop
+	j	$31
+	ENDFN (__mips16_rdhwr)
+#endif
 #endif
Index: libgcc/config/mips/t-mips16
===================================================================
--- libgcc/config/mips/t-mips16	2012-01-08 08:43:13.000000000 +0000
+++ libgcc/config/mips/t-mips16	2012-01-08 10:12:57.000000000 +0000
@@ -36,7 +36,8 @@ LIB1ASMFUNCS = _m16addsf3 _m16subsf3 _m1
 	_m16stubsc0 _m16stubsc1 _m16stubsc2 _m16stubsc5 _m16stubsc6 \
 	_m16stubsc9 _m16stubsc10 \
 	_m16stubdc0 _m16stubdc1 _m16stubdc2 _m16stubdc5 _m16stubdc6 \
-	_m16stubdc9 _m16stubdc10
+	_m16stubdc9 _m16stubdc10 \
+	_m16rdhwr
 
 SYNC = yes
 SYNC_CFLAGS = -mno-mips16

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-01-08 11:21 ` Richard Sandiford
@ 2012-01-15 18:23   ` Richard Sandiford
  2012-02-03 10:03   ` Chung-Lin Tang
  1 sibling, 0 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-01-15 18:23 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches

Richard Sandiford <rdsandiford@googlemail.com> writes:
> gcc/
> 2012-01-08  Chung-Lin Tang  <cltang@codesourcery.com>
> 	    Richard Sandiford  <rdsandiford@googlemail.com>
>
> 	* config/mips/mips-protos.h (SYMBOL_FORCE_TO_MEM): Delete.
> 	(SYMBOL_32_HIGH): Likewise.
> 	(mips_output_tls_reloc_directive): Declare.
> 	* config/mips/mips.h (PIC_FUNCTION_ADDR_REGNUM): Move to mips.md.
> 	(mips_use_pcrel_pool_p, mips_lo_relocs, mips_hi_relocs): Declare.
> 	* config/mips/mips.c (mips_use_pcrel_pool_p): New variable.
> 	(mips_lo_relocs, mips_hi_relocs): Make extern.
> 	(mips16_stub_function): Move up file.
> 	(mips_classify_symbol): Remove SYMBOL_FORCE_TO_MEM handling.
> 	(mips_symbolic_constant_p): Likewise.  Remove SYMBOL_32_HIGH too.
> 	(mips_symbol_insns_1): Likewise.  Check mips_use_pcrel_pool_p.
> 	(mips_cannot_force_const_mem): Use mips_use_pcrel_pool_p instead
> 	of SYMBOL_FORCE_TO_MEM.  Only check mips_tls_symbol_ref_1
> 	if it's false.
> 	(mips_get_tp): Add MIPS16 support.
> 	(mips_legitimize_tls_address): Remove MIPS16 sorry().
> 	Generalize DTPREL and TPREL handling.
> 	(mips_init_relocs): Initialize mips_use_pcrel_pool_p.
> 	Add MIPS16 TLS support.
> 	(mips_output_tls_reloc_directive): New function.
> 	(mips16_rewrite_pool_refs): Ignore UNSPEC_TLS_GET_TPs.
> 	* config/mips/predicates.md (symbolic_operand_with_high)
> 	(tls_reloc_operand): New predicates.
> 	(force_to_mem_operand): Use mips_use_pcrel_pool_p.
> 	* config/mips/mips.md (UNSPEC_UNSHIFTED_HIGH): New unspec.
> 	(PIC_FUNCTION_ADDR_REGNUM): Moved from mips.h.
> 	(*unshifted_high): New instruction.  Use it for MIPS16
> 	high splitter.
> 	(consttable_tls_reloc, tls_get_tp_mips16_<mode>): New patterns.
> 	(*tls_get_tp_mips16_call_<mode>): Likewise.
>
> gcc/testsuite/
> 	* gcc.target/mips/code-readable-2.c: Allow the jump table address
> 	to be loaded from the constant pool, rather than via %hi and %lo.
>
> libgcc/
> 2012-01-08  Chung-Lin Tang  <cltang@codesourcery.com>
> 	    Richard Sandiford  <rdsandiford@googlemail.com>
>
> 	* config/mips/libgcc-mips16.ver (__mips16_rdhwr): Add.
> 	* config/mips/mips16.S (__mips16_rdhwr): New function.
> 	* config/mips/t-mips16 (LIB1ASMFUNCS): Add _m16rdhwr.

This wouldn't normally be "stage 4" material, but since it was posted
just before stage 3 closed, I went ahead and applied it.  I'll update
the web pages "soon".

Richard

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-01-08 11:21 ` Richard Sandiford
  2012-01-15 18:23   ` Richard Sandiford
@ 2012-02-03 10:03   ` Chung-Lin Tang
  2012-02-03 11:57     ` Maciej W. Rozycki
  2012-02-03 15:28     ` Richard Sandiford
  1 sibling, 2 replies; 26+ messages in thread
From: Chung-Lin Tang @ 2012-02-03 10:03 UTC (permalink / raw)
  To: gcc-patches, rdsandiford; +Cc: Maciej W. Rozycki

Hi Richard,

Sorry for the delay to respond, and thanks for revising and committing
the changes to trunk. The revised changes look much cleaner :)

About the other concerns:

> (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;
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)

> 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)

> (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...

>> The __mips16_rdhwr() function is not intended for general use, just
>> tightly coupled compiler runtime support; therefore, it is only linked
>> statically from libgcc.a, not exported from shared libgcc.
> 
> Well, a fair bit of libgcc is tightly-coupled compiler runtime support.
> I don't really see any need to handle the new function differently from
> the other __mips16 wrappers.  It's not like we're gaining any benefit in
> the PIC call overhead: we can't turn JALRs into branches like we can for
> nearby non-MIPS16-to-non-MIPS16 calls, so PIC calls will still go via
> the GOT.  And we're not gaining any benefit in terms of ABI baggage either.
> Future libgcc(.a)s would need to continue providing the function as
> specified now, in order for future gccs to be able to link library
> archives built with 4.7.
> 
> By making the function hidden, we lose the important ability to replace
> all instances of it.  E.g. it isn't inconceivable that someone will find
> a more efficient way to implement the function in cases where the RDHWR
> is emulated (perhaps on a kernel other than Linux -- this support isn't
> specific to *-linux-gnu).  Being able to replace all instances of a
> function is useful for other things, such as profiling, debugging,
> or working around processor errata.

We touched this internally as well, but interestingly came to the
opposite conclusion :)  We're not sure the __mips16_rdhwr() is
ultimately the best choice of API, plus the non-standardness of the
calling convention, hence simply decided to hide it from libgcc_s.so, in
case we need to change the MIPS16 TLS interface.

On the issue of hiding stuff in mips16.S, it may be suitable to raise
this issue here: there is a problem when MIPS16 hard-float calls go
through PLTs, because the hard-float stub functions in mips16.S use
v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated
by BFD. (I think I described the scenario mostly correct, CCing Maciej
here who worked on this issue)  I think the solution was to mark a large
portion of the mips16.S functions as all hidden.

>> 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.

Thanks,
Chung-Lin

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-03 10:03   ` Chung-Lin Tang
@ 2012-02-03 11:57     ` Maciej W. Rozycki
  2012-02-03 16:20       ` Richard Sandiford
  2012-02-03 15:28     ` Richard Sandiford
  1 sibling, 1 reply; 26+ messages in thread
From: Maciej W. Rozycki @ 2012-02-03 11:57 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Richard Sandiford

On Fri, 3 Feb 2012, Chung-Lin Tang wrote:

> On the issue of hiding stuff in mips16.S, it may be suitable to raise
> this issue here: there is a problem when MIPS16 hard-float calls go
> through PLTs, because the hard-float stub functions in mips16.S use
> v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated
> by BFD. (I think I described the scenario mostly correct, CCing Maciej
> here who worked on this issue)  I think the solution was to mark a large
> portion of the mips16.S functions as all hidden.

 Not quite a large portion, just the function return stubs (which I reckon 
there are exactly four) that process values in $v0/$v1 as if they were 
arguments.  This is of course a non-standard calling convention that 
breaks the ABI and shouldn't be exposed to the dynamic linker and the PLT.  
Not even mentioning that three of these four stubs are not bigger than a 
PLT entry they would need, and the fourth is two or so instructions 
larger, so making them dynamic hardly makes point (the cost of the 
indirect call from PLT also matters).

 Given that this code (to support MIPS16 shared libraries) was obviously 
never fully functional before this effort, I think it is about the right 
time to remove some hacks I saw in glibc's dynamic linker that are meant 
to avoid clobbering $v0/$v1 before someone tries to rely on them for real.  

 And there's no reasonable way to support MIPS16 PLT entries without 
clobbering $v0/$v1 (to preempt your question -- MIPS16 code can of course 
work just fine with standard MIPS PLT entries (no pure-MIPS16 processors 
are possible), but on some processors the cost of flipping the ISA bit is 
prohibitive as it requires draining the pipeline to reconfigure the 
instruction decoder and as such should best be avoided in pure-MIPS16 
dynamic binaries).

  Maciej

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-03 10:03   ` Chung-Lin Tang
  2012-02-03 11:57     ` Maciej W. Rozycki
@ 2012-02-03 15:28     ` Richard Sandiford
  2012-07-05  6:50       ` Chung-Lin Tang
  1 sibling, 1 reply; 26+ messages in thread
From: Richard Sandiford @ 2012-02-03 15:28 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Maciej W. Rozycki

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.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-03 11:57     ` Maciej W. Rozycki
@ 2012-02-03 16:20       ` Richard Sandiford
  2012-02-03 17:34         ` Maciej W. Rozycki
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Sandiford @ 2012-02-03 16:20 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Chung-Lin Tang, gcc-patches

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Fri, 3 Feb 2012, Chung-Lin Tang wrote:
>> On the issue of hiding stuff in mips16.S, it may be suitable to raise
>> this issue here: there is a problem when MIPS16 hard-float calls go
>> through PLTs, because the hard-float stub functions in mips16.S use
>> v0/v1 as scratch registers, the same as MIPS16 PLT sequences generated
>> by BFD. (I think I described the scenario mostly correct, CCing Maciej
>> here who worked on this issue)  I think the solution was to mark a large
>> portion of the mips16.S functions as all hidden.
>
>  Not quite a large portion, just the function return stubs (which I reckon 
> there are exactly four) that process values in $v0/$v1 as if they were 
> arguments.  This is of course a non-standard calling convention that 
> breaks the ABI and shouldn't be exposed to the dynamic linker and the PLT.  
> Not even mentioning that three of these four stubs are not bigger than a 
> PLT entry they would need, and the fourth is two or so instructions 
> larger, so making them dynamic hardly makes point (the cost of the 
> indirect call from PLT also matters).

But that isn't the only reason to use shared libraries.

>  Given that this code (to support MIPS16 shared libraries) was obviously 
> never fully functional before this effort, I think it is about the right 
> time to remove some hacks I saw in glibc's dynamic linker that are meant 
> to avoid clobbering $v0/$v1 before someone tries to rely on them for real.

FSF BFD doesn't generate MIPS16 PLTs though.  Is this a CS-local change?
If so, the criticism seems a little unfair.  The FSF code is self-
consistent: PLTs can be used for these stubs because the resolver
function preserves the necessary registers.  Traditional lazy binding
stubs are avoided by not using %call16.

I don't think changing the resolver function so that it preserves
fewer registers would be a good idea.  As well as the MIPS16 thing,
$3 is needed for _mcount.

You're right of course that PLTs can't be used for __mips16_rdhwr.
I'd forgotten about that, sorry.  Will fix this weekend.

If there's still some concern that __mips16_rdhwr might not have
the right ABI, then maybe we should simply emit a link-once function
in each object that needs it.  We could then switch to another function
(and another API) without having to keep the old one in libgcc.a for
compatibility.  It would also avoid the -shared-libgcc thing.

Admittedly that's just an off-the-top-of-my-head idea. :-)
What do you think?

Richard

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-03 16:20       ` Richard Sandiford
@ 2012-02-03 17:34         ` Maciej W. Rozycki
  2012-02-04 10:07           ` Richard Sandiford
  0 siblings, 1 reply; 26+ messages in thread
From: Maciej W. Rozycki @ 2012-02-03 17:34 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: Chung-Lin Tang, gcc-patches

On Fri, 3 Feb 2012, Richard Sandiford wrote:

> >  Not quite a large portion, just the function return stubs (which I reckon 
> > there are exactly four) that process values in $v0/$v1 as if they were 
> > arguments.  This is of course a non-standard calling convention that 
> > breaks the ABI and shouldn't be exposed to the dynamic linker and the PLT.  
> > Not even mentioning that three of these four stubs are not bigger than a 
> > PLT entry they would need, and the fourth is two or so instructions 
> > larger, so making them dynamic hardly makes point (the cost of the 
> > indirect call from PLT also matters).
> 
> But that isn't the only reason to use shared libraries.

 That is true in the general case, but these MIPS16 stubs have essentially 
been cast in stone and there's little room if any for change here.  I 
can't imagine a case where static linking of these four stubs would cause 
trouble, but certainly I am open to any examples.

> >  Given that this code (to support MIPS16 shared libraries) was obviously 
> > never fully functional before this effort, I think it is about the right 
> > time to remove some hacks I saw in glibc's dynamic linker that are meant 
> > to avoid clobbering $v0/$v1 before someone tries to rely on them for real.
> 
> FSF BFD doesn't generate MIPS16 PLTs though.  Is this a CS-local change?

 A patch is in the queue, together with microMIPS PLT entries (that you 
have expected, haven't you?).  I can't give you a specific schedule, but I 
expect to propose it as soon as I've got the current batch of GDB changes 
followed by the outstanding binutils clean-ups I got your feedback on that 
I haven't finished processing yet, off my plate.

> If so, the criticism seems a little unfair.  The FSF code is self-
> consistent: PLTs can be used for these stubs because the resolver
> function preserves the necessary registers.  Traditional lazy binding
> stubs are avoided by not using %call16.

 Please don't take it as criticism but as a starting point for discussion. 
And certainly I did not intend to sound negative, I'm sorry if I did.  
I'm just raising points I noted as MIPS16 PLT entries were being 
developed.  I do think though that it's a bit unfortunate that such a 
creeping ABI change has been made here.  I think in case of doubt it's 
always safe to stick to the ABI.

 Lazy binding stubs are less of concern for some reason that is not 
entirely clear to me and we have no MIPS16 implementation in the queue 
(but we do have microMIPS stubs, as you might have expected -- to 
facilitate pure-microMIPS processors that may eventually appear).

> I don't think changing the resolver function so that it preserves
> fewer registers would be a good idea.  As well as the MIPS16 thing,
> $3 is needed for _mcount.

 I don't think _mcount has ever worked for dynamic libraries, has it? -- 
please correct me if I am wrong, but I believe `gprof' relies on memory 
segments to be contiguous which is certainly not the case for a dynamic 
executable where you have a separate set of mappings for the executable 
proper and then for each shared library loaded.

 I didn't know it required $3 for anything either -- I've thought it was 
$1 only.  How has it worked for standard MIPS code then?

 We have only about now got MIPS16 shared libraries to work -- are you 
sure removing code to save/restore $2/$3 in the dynamic linker is going to 
hit anyone?  While a small piece, this is still some wasted memory and 
execution time for every executable on every system and whether MIPS16 
code is involved or not (even on systems that do not support the MIPS16 
ASE at all), just to cope with an ABI anomaly of literally four functions 
only needed for some MIPS16 code (and which had not originally been 
expected to be ever used for dynamic linking -- until recently nobody even 
considered the use of MIPS16 code on a shared library system).  And I 
think you can still get into trouble if you use the wrong ld.so with your 
MIPS16 executable -- or has symbol versioning been used to ensure that 
ld.so bails out in this case?

> You're right of course that PLTs can't be used for __mips16_rdhwr.

 Unless you want to waste text (and possibly stack) space and time to save 
and restore call-clobbered registers in the caller that is. ;)

> I'd forgotten about that, sorry.  Will fix this weekend.
> 
> If there's still some concern that __mips16_rdhwr might not have
> the right ABI, then maybe we should simply emit a link-once function
> in each object that needs it.  We could then switch to another function
> (and another API) without having to keep the old one in libgcc.a for
> compatibility.  It would also avoid the -shared-libgcc thing.
> 
> Admittedly that's just an off-the-top-of-my-head idea. :-)
> What do you think?

 Actually I had that idea of a link-once function too, but it turned out 
quite complicated to do without rewriting some generic parts of GCC as it 
is currently not prepared to emit link-once functions outside C++ 
compilations.  It's been a while and I did lots of other stuff meanwhile, 
so please excuse me if I got anything wrong here.

  Maciej

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-03 17:34         ` Maciej W. Rozycki
@ 2012-02-04 10:07           ` Richard Sandiford
  2012-02-05 15:03             ` Richard Sandiford
                               ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-02-04 10:07 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Chung-Lin Tang, gcc-patches

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> I don't think changing the resolver function so that it preserves
>> fewer registers would be a good idea.  As well as the MIPS16 thing,
>> $3 is needed for _mcount.
>
>  I don't think _mcount has ever worked for dynamic libraries, has it? -- 
> please correct me if I am wrong, but I believe `gprof' relies on memory 
> segments to be contiguous which is certainly not the case for a dynamic 
> executable where you have a separate set of mappings for the executable 
> proper and then for each shared library loaded.
>
>  I didn't know it required $3 for anything either -- I've thought it was 
> $1 only.  How has it worked for standard MIPS code then?

Sorry, I misremembered, it was $2 (for the static chain register).
But you're probably right that it's not relevant.

>  We have only about now got MIPS16 shared libraries to work -- are you 
> sure removing code to save/restore $2/$3 in the dynamic linker is going to 
> hit anyone?

Pretty sure.  There are two separate points here.  Support for MIPS16
shared libraries went into the FSF tools in 2008, and I was able to
build working(!) shared libraries at that time.  (To be clear, these were
external libraries rather than things like libgcc.)  So it's not really
that new.

That doesn't mean that there weren't bugs in the implementation, of course.
But I think we're just going to have to agree to disagree on the
"working" thing.  It's probably moot anyway given the other point...

> While a small piece, this is still some wasted memory and 
> execution time for every executable on every system and whether MIPS16 
> code is involved or not (even on systems that do not support the MIPS16 
> ASE at all), just to cope with an ABI anomaly of literally four functions 
> only needed for some MIPS16 code (and which had not originally been 
> expected to be ever used for dynamic linking -- until recently nobody even 
> considered the use of MIPS16 code on a shared library system).  And I 
> think you can still get into trouble if you use the wrong ld.so with your 
> MIPS16 executable -- or has symbol versioning been used to ensure that 
> ld.so bails out in this case?

...to be clear, I'm talking here specifically about the behaviour of
the _PLT_ resolver function.  Only the PLT resolver function saves and
restores $2 and $3; the resolver for SVR4-style lazy binding stubs
doesn't.  (As I mentioned earlier, we're also not expecting the
resolver for the SVR4-style lazy binding stubs to preserve $2 and $3.)

MIPS PLTs are an extension to the original SVR4 ABI, so we were free
to choose the interface.  As you can tell from the glibc comments,
including $2 and $3 in the list was a deliberate decision, based on
the fact that there were already functions that would find it useful.
The PLT resolver has used this interface since the day it was added to
glibc (2008-10-01).  Its ABI has not changed, so there was no change
that would require an .so bump.  (Adding PLTs didn't itself require
an .so bump because old dynamic linkers would error out on them anyway.)

And because we're talking about PLTs -- which for MIPS are only used in
executables -- the question isn't really whether MIPS16 shared libraries
work or not.  It's a question of whether partly-MIPS16 dynamic executables
work.  And they do: this has been a regular part of my testing setup for
at least a couple of years now.

>> If there's still some concern that __mips16_rdhwr might not have
>> the right ABI, then maybe we should simply emit a link-once function
>> in each object that needs it.  We could then switch to another function
>> (and another API) without having to keep the old one in libgcc.a for
>> compatibility.  It would also avoid the -shared-libgcc thing.
>> 
>> Admittedly that's just an off-the-top-of-my-head idea. :-)
>> What do you think?
>
>  Actually I had that idea of a link-once function too, but it turned out 
> quite complicated to do without rewriting some generic parts of GCC as it 
> is currently not prepared to emit link-once functions outside C++ 
> compilations.  It's been a while and I did lots of other stuff meanwhile, 
> so please excuse me if I got anything wrong here.

Hmm, OK, I wouldn't have expected that.  But if you've tried making
__mips16_rdhwr link-once and had a bad experience with it, then yeah,
let's go with the hidden libgcc function.  It's just a shame that we're
having to force static linking of libgcc for this one case.

I'll take the relevant parts from Chung-Lin's patch and test them
over the weekend.

Richard

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-04 10:07           ` Richard Sandiford
@ 2012-02-05 15:03             ` Richard Sandiford
  2012-02-06 16:27             ` Maciej W. Rozycki
  2012-02-11  2:44             ` Richard Henderson
  2 siblings, 0 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-02-05 15:03 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Chung-Lin Tang, gcc-patches

Richard Sandiford <rdsandiford@googlemail.com> writes:
> "Maciej W. Rozycki" <macro@codesourcery.com> writes:
>>> If there's still some concern that __mips16_rdhwr might not have
>>> the right ABI, then maybe we should simply emit a link-once function
>>> in each object that needs it.  We could then switch to another function
>>> (and another API) without having to keep the old one in libgcc.a for
>>> compatibility.  It would also avoid the -shared-libgcc thing.
>>> 
>>> Admittedly that's just an off-the-top-of-my-head idea. :-)
>>> What do you think?
>>
>>  Actually I had that idea of a link-once function too, but it turned out 
>> quite complicated to do without rewriting some generic parts of GCC as it 
>> is currently not prepared to emit link-once functions outside C++ 
>> compilations.  It's been a while and I did lots of other stuff meanwhile, 
>> so please excuse me if I got anything wrong here.
>
> Hmm, OK, I wouldn't have expected that.  But if you've tried making
> __mips16_rdhwr link-once and had a bad experience with it, then yeah,
> let's go with the hidden libgcc function.  It's just a shame that we're
> having to force static linking of libgcc for this one case.
>
> I'll take the relevant parts from Chung-Lin's patch and test them
> over the weekend.

Here's what I committed after testing on mips64-linux-gnu, both with
and without PLTs.

Richard


libgcc/
2012-02-05  Chung-Lin Tang  <cltang@codesourcery.com>

	* config.host (mips64*-*-linux*, mipsisa64*-*-linux*, mips*-*-linux*):
	Add t-slibgcc-libgcc to tmake_file.
	* config/mips/libgcc-mips16.ver: Revert previous patch.
	* config/mips/mips16.S (__mips16_rdhwr): Hide.

Index: libgcc/config.host
===================================================================
--- libgcc/config.host	2012-02-04 10:27:18.000000000 +0000
+++ libgcc/config.host	2012-02-04 11:34:03.000000000 +0000
@@ -746,12 +746,12 @@ mips*-*-netbsd*)			# NetBSD/mips, either
 	;;
 mips64*-*-linux* | mipsisa64*-*-linux*)
 	extra_parts="$extra_parts crtfastmath.o"
-	tmake_file="${tmake_file} t-crtfm mips/t-mips16 mips/t-tpbit"
+	tmake_file="${tmake_file} t-crtfm mips/t-mips16 mips/t-tpbit t-slibgcc-libgcc"
 	md_unwind_header=mips/linux-unwind.h
 	;;
 mips*-*-linux*)				# Linux MIPS, either endian.
 	extra_parts="$extra_parts crtfastmath.o"
-	tmake_file="${tmake_file} t-crtfm mips/t-mips16"
+	tmake_file="${tmake_file} t-crtfm mips/t-mips16 t-slibgcc-libgcc"
 	md_unwind_header=mips/linux-unwind.h
 	;;
 mips*-*-openbsd*)
Index: libgcc/config/mips/libgcc-mips16.ver
===================================================================
--- libgcc/config/mips/libgcc-mips16.ver	2012-02-04 11:12:01.000000000 +0000
+++ libgcc/config/mips/libgcc-mips16.ver	2012-02-04 11:34:03.000000000 +0000
@@ -84,7 +84,3 @@ GCC_4.4.0 {
   __mips16_call_stub_dc_9
   __mips16_call_stub_dc_10
 }
-
-GCC_4.7.0 {
-  __mips16_rdhwr
-}
Index: libgcc/config/mips/mips16.S
===================================================================
--- libgcc/config/mips/mips16.S	2012-02-04 10:27:18.000000000 +0000
+++ libgcc/config/mips/mips16.S	2012-02-04 11:34:03.000000000 +0000
@@ -712,6 +712,9 @@ CALL_STUB_RET (__mips16_call_stub_dc_10,
 
 #ifdef L_m16rdhwr
 STARTFN (__mips16_rdhwr)
+	/* Forced always hidden, because the PLT resolver function would
+	   not preserve all necessary registers.  */
+	.hidden	__mips16_rdhwr
 	.set	push
 	.set	mips32r2
 	.set	noreorder

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-04 10:07           ` Richard Sandiford
  2012-02-05 15:03             ` Richard Sandiford
@ 2012-02-06 16:27             ` Maciej W. Rozycki
  2012-02-06 19:39               ` Richard Sandiford
  2012-02-06 19:43               ` Richard Sandiford
  2012-02-11  2:44             ` Richard Henderson
  2 siblings, 2 replies; 26+ messages in thread
From: Maciej W. Rozycki @ 2012-02-06 16:27 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: Chung-Lin Tang, gcc-patches

On Sat, 4 Feb 2012, Richard Sandiford wrote:

> >  I don't think _mcount has ever worked for dynamic libraries, has it? -- 
> > please correct me if I am wrong, but I believe `gprof' relies on memory 
> > segments to be contiguous which is certainly not the case for a dynamic 
> > executable where you have a separate set of mappings for the executable 
> > proper and then for each shared library loaded.
> >
> >  I didn't know it required $3 for anything either -- I've thought it was 
> > $1 only.  How has it worked for standard MIPS code then?
> 
> Sorry, I misremembered, it was $2 (for the static chain register).

 Please elaborate anyway -- if you remember the details of $2 usage 
offhand, that is.  I'm curious.

> >  We have only about now got MIPS16 shared libraries to work -- are you 
> > sure removing code to save/restore $2/$3 in the dynamic linker is going to 
> > hit anyone?
> 
> Pretty sure.  There are two separate points here.  Support for MIPS16
> shared libraries went into the FSF tools in 2008, and I was able to
> build working(!) shared libraries at that time.  (To be clear, these were
> external libraries rather than things like libgcc.)  So it's not really
> that new.
> 
> That doesn't mean that there weren't bugs in the implementation, of course.
> But I think we're just going to have to agree to disagree on the
> "working" thing.  It's probably moot anyway given the other point...

 OK, perhaps soft-float (or if you happened to avoid FP altogether) could 
have worked.  With hard float we hit problems at least with symbol 
references emitted incorrectly by GCC in the context of some MIPS16 thunks 
as well as binutils failing to emit LA25 thunks for some MIPS16 thunks.  
So it wasn't just MIPS16 shared libraries that failed, but also dynamic 
MIPS16 executables.  So it looks to me like a part of the MIPS16 ABI 
wasn't covered at all (I'd expect all the "interesting" cases to have been 
deliberately tested before claiming victory).

 Chung-Lin may remember more details; I'd have to dig out old discussions.

> > While a small piece, this is still some wasted memory and 
> > execution time for every executable on every system and whether MIPS16 
> > code is involved or not (even on systems that do not support the MIPS16 
> > ASE at all), just to cope with an ABI anomaly of literally four functions 
> > only needed for some MIPS16 code (and which had not originally been 
> > expected to be ever used for dynamic linking -- until recently nobody even 
> > considered the use of MIPS16 code on a shared library system).  And I 
> > think you can still get into trouble if you use the wrong ld.so with your 
> > MIPS16 executable -- or has symbol versioning been used to ensure that 
> > ld.so bails out in this case?
> 
> ...to be clear, I'm talking here specifically about the behaviour of
> the _PLT_ resolver function.  Only the PLT resolver function saves and
> restores $2 and $3; the resolver for SVR4-style lazy binding stubs
> doesn't.  (As I mentioned earlier, we're also not expecting the
> resolver for the SVR4-style lazy binding stubs to preserve $2 and $3.)
> 
> MIPS PLTs are an extension to the original SVR4 ABI, so we were free
> to choose the interface.  As you can tell from the glibc comments,
> including $2 and $3 in the list was a deliberate decision, based on
> the fact that there were already functions that would find it useful.
> The PLT resolver has used this interface since the day it was added to
> glibc (2008-10-01).  Its ABI has not changed, so there was no change
> that would require an .so bump.  (Adding PLTs didn't itself require
> an .so bump because old dynamic linkers would error out on them anyway.)

 Fair enough -- I didn't realise that support for MIPS PLT and MIPS16 
dynamic executables was added at the same time.

> And because we're talking about PLTs -- which for MIPS are only used in
> executables -- the question isn't really whether MIPS16 shared libraries
> work or not.  It's a question of whether partly-MIPS16 dynamic executables
> work.  And they do: this has been a regular part of my testing setup for
> at least a couple of years now.

 Yet we had some problems to fix that made me believe it was work in 
progress that went upstream.  Sorry if that was unjust.

> >> If there's still some concern that __mips16_rdhwr might not have
> >> the right ABI, then maybe we should simply emit a link-once function
> >> in each object that needs it.  We could then switch to another function
> >> (and another API) without having to keep the old one in libgcc.a for
> >> compatibility.  It would also avoid the -shared-libgcc thing.
> >> 
> >> Admittedly that's just an off-the-top-of-my-head idea. :-)
> >> What do you think?
> >
> >  Actually I had that idea of a link-once function too, but it turned out 
> > quite complicated to do without rewriting some generic parts of GCC as it 
> > is currently not prepared to emit link-once functions outside C++ 
> > compilations.  It's been a while and I did lots of other stuff meanwhile, 
> > so please excuse me if I got anything wrong here.
> 
> Hmm, OK, I wouldn't have expected that.  But if you've tried making
> __mips16_rdhwr link-once and had a bad experience with it, then yeah,
> let's go with the hidden libgcc function.  It's just a shame that we're
> having to force static linking of libgcc for this one case.

 Well, it's just this single function that's pulled from libgcc.a -- all 
the rest will come from libgcc_s.so (unless hidden as well, that is).  
The benefit is you can always change the ABI of __mips16_rdhwr without 
worrying about existing executables -- they'll continue to work using 
their private piece of code unchanged.

  Maciej

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-06 16:27             ` Maciej W. Rozycki
@ 2012-02-06 19:39               ` Richard Sandiford
  2012-02-06 19:43               ` Richard Sandiford
  1 sibling, 0 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-02-06 19:39 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Chung-Lin Tang, gcc-patches

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> >  We have only about now got MIPS16 shared libraries to work -- are you 
>> > sure removing code to save/restore $2/$3 in the dynamic linker is going to 
>> > hit anyone?
>> 
>> Pretty sure.  There are two separate points here.  Support for MIPS16
>> shared libraries went into the FSF tools in 2008, and I was able to
>> build working(!) shared libraries at that time.  (To be clear, these were
>> external libraries rather than things like libgcc.)  So it's not really
>> that new.
>> 
>> That doesn't mean that there weren't bugs in the implementation, of course.
>> But I think we're just going to have to agree to disagree on the
>> "working" thing.  It's probably moot anyway given the other point...
>
>  OK, perhaps soft-float (or if you happened to avoid FP altogether) could 
> have worked.  With hard float we hit problems at least with symbol 
> references emitted incorrectly by GCC in the context of some MIPS16 thunks 
> as well as binutils failing to emit LA25 thunks for some MIPS16 thunks.  
> So it wasn't just MIPS16 shared libraries that failed, but also dynamic 
> MIPS16 executables.  So it looks to me like a part of the MIPS16 ABI 
> wasn't covered at all (I'd expect all the "interesting" cases to have been 
> deliberately tested before claiming victory).

It was hard-float too FWIW.  As with all these things, some applications
just have a knack of avoiding certain bugs :-)

>> >> If there's still some concern that __mips16_rdhwr might not have
>> >> the right ABI, then maybe we should simply emit a link-once function
>> >> in each object that needs it.  We could then switch to another function
>> >> (and another API) without having to keep the old one in libgcc.a for
>> >> compatibility.  It would also avoid the -shared-libgcc thing.
>> >> 
>> >> Admittedly that's just an off-the-top-of-my-head idea. :-)
>> >> What do you think?
>> >
>> >  Actually I had that idea of a link-once function too, but it turned out 
>> > quite complicated to do without rewriting some generic parts of GCC as it 
>> > is currently not prepared to emit link-once functions outside C++ 
>> > compilations.  It's been a while and I did lots of other stuff meanwhile, 
>> > so please excuse me if I got anything wrong here.
>> 
>> Hmm, OK, I wouldn't have expected that.  But if you've tried making
>> __mips16_rdhwr link-once and had a bad experience with it, then yeah,
>> let's go with the hidden libgcc function.  It's just a shame that we're
>> having to force static linking of libgcc for this one case.
>
>  Well, it's just this single function that's pulled from libgcc.a -- all 
> the rest will come from libgcc_s.so (unless hidden as well, that is).  
> The benefit is you can always change the ABI of __mips16_rdhwr without 
> worrying about existing executables -- they'll continue to work using 
> their private piece of code unchanged.

We can't change the ABI of __mips16_rdhwr.  As I was saying before,
we need to keep this function around from now on so that static
libraries that contain objects built with gcc 4.7 can be linked
by later compilers (which will link against their own libgcc.a).
If we come up with another interface, the function will need to
have a new name, and we'll need to continue to provide the current
__mips16_rdhwr in libgcc.a for compatibility reasons.

Richard

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-06 16:27             ` Maciej W. Rozycki
  2012-02-06 19:39               ` Richard Sandiford
@ 2012-02-06 19:43               ` Richard Sandiford
  1 sibling, 0 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-02-06 19:43 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Chung-Lin Tang, gcc-patches

[forking for different topics]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Sat, 4 Feb 2012, Richard Sandiford wrote:
>> >  I don't think _mcount has ever worked for dynamic libraries, has it? -- 
>> > please correct me if I am wrong, but I believe `gprof' relies on memory 
>> > segments to be contiguous which is certainly not the case for a dynamic 
>> > executable where you have a separate set of mappings for the executable 
>> > proper and then for each shared library loaded.
>> >
>> >  I didn't know it required $3 for anything either -- I've thought it was 
>> > $1 only.  How has it worked for standard MIPS code then?
>> 
>> Sorry, I misremembered, it was $2 (for the static chain register).
>
>  Please elaborate anyway -- if you remember the details of $2 usage 
> offhand, that is.  I'm curious.

$2 was the traditional static chain pointer, and _mcount would preserve it
so that calls from nested functions would work correctly.  We since changed
GCC's own static pointer to $15, so we now have to move $15 to $2 before
calling _mcount and restore it afterwards, since there's no guarantee
that _mcount itself will preserve $15.

Or at least, that's the theory.  I have to admit to never using it in
"real world" situations.

Richard

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-04 10:07           ` Richard Sandiford
  2012-02-05 15:03             ` Richard Sandiford
  2012-02-06 16:27             ` Maciej W. Rozycki
@ 2012-02-11  2:44             ` Richard Henderson
  2012-02-11 10:38               ` Maciej W. Rozycki
  2012-02-19 17:21               ` Richard Sandiford
  2 siblings, 2 replies; 26+ messages in thread
From: Richard Henderson @ 2012-02-11  2:44 UTC (permalink / raw)
  To: Maciej W. Rozycki, Chung-Lin Tang, gcc-patches, rdsandiford

On 02/04/2012 02:06 AM, Richard Sandiford wrote:
>> >  Actually I had that idea of a link-once function too, but it turned out 
>> > quite complicated to do without rewriting some generic parts of GCC as it 
>> > is currently not prepared to emit link-once functions outside C++ 
>> > compilations.  It's been a while and I did lots of other stuff meanwhile, 
>> > so please excuse me if I got anything wrong here.
> Hmm, OK, I wouldn't have expected that.  But if you've tried making
> __mips16_rdhwr link-once and had a bad experience with it, then yeah,
> let's go with the hidden libgcc function.  It's just a shame that we're
> having to force static linking of libgcc for this one case.

The i386 target does it all the time for its __x86.get_pc_thunk.?x thingys.
So I really don't believe the "not prepared" comment.


r~

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-11  2:44             ` Richard Henderson
@ 2012-02-11 10:38               ` Maciej W. Rozycki
  2012-02-19 17:21               ` Richard Sandiford
  1 sibling, 0 replies; 26+ messages in thread
From: Maciej W. Rozycki @ 2012-02-11 10:38 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Chung-Lin Tang, gcc-patches, rdsandiford

On Sat, 11 Feb 2012, Richard Henderson wrote:

> On 02/04/2012 02:06 AM, Richard Sandiford wrote:
> >> >  Actually I had that idea of a link-once function too, but it turned out 
> >> > quite complicated to do without rewriting some generic parts of GCC as it 
> >> > is currently not prepared to emit link-once functions outside C++ 
> >> > compilations.  It's been a while and I did lots of other stuff meanwhile, 
> >> > so please excuse me if I got anything wrong here.
> > Hmm, OK, I wouldn't have expected that.  But if you've tried making
> > __mips16_rdhwr link-once and had a bad experience with it, then yeah,
> > let's go with the hidden libgcc function.  It's just a shame that we're
> > having to force static linking of libgcc for this one case.
> 
> The i386 target does it all the time for its __x86.get_pc_thunk.?x thingys.
> So I really don't believe the "not prepared" comment.

 I'm confused then, sorry -- Chung-Lin, can you look into it again then 
please?

  Maciej

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-11  2:44             ` Richard Henderson
  2012-02-11 10:38               ` Maciej W. Rozycki
@ 2012-02-19 17:21               ` Richard Sandiford
  2012-02-21 17:23                 ` Maciej W. Rozycki
  1 sibling, 1 reply; 26+ messages in thread
From: Richard Sandiford @ 2012-02-19 17:21 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Maciej W. Rozycki, Chung-Lin Tang, gcc-patches

Richard Henderson <rth@redhat.com> writes:
> On 02/04/2012 02:06 AM, Richard Sandiford wrote:
>>> >  Actually I had that idea of a link-once function too, but it turned out 
>>> > quite complicated to do without rewriting some generic parts of GCC as it 
>>> > is currently not prepared to emit link-once functions outside C++ 
>>> > compilations.  It's been a while and I did lots of other stuff meanwhile, 
>>> > so please excuse me if I got anything wrong here.
>> Hmm, OK, I wouldn't have expected that.  But if you've tried making
>> __mips16_rdhwr link-once and had a bad experience with it, then yeah,
>> let's go with the hidden libgcc function.  It's just a shame that we're
>> having to force static linking of libgcc for this one case.
>
> The i386 target does it all the time for its __x86.get_pc_thunk.?x thingys.

Thanks for the pointer.  Here's a patch that takes the same approach.
There's no problem relying on comdat groups here, since MIPS16 TLS
requires 2.22 gas and ld anyway.

Tested on mips64-linux-gnu, mips-sde-elf and various other mips*-elf targets.
Applied.

Richard


gcc/
	* config/mips/mips.c (mips_need_mips16_rdhwr_p): New variable.
	(mips_get_tp): Set it.  Record that __mips16_rdhwr binds locally.
	(mips_start_unique_function, mips_output_mips16_rdhwr)
	(mips_code_end): New functions.
	(TARGET_ASM_CODE_END): Define.

libgcc/
	* config.host (mips64*-*-linux*, mipsisa64*-*-linux*)
	(mips*-*-linux*): Remove t-slibgcc-libgcc.
	* config/mips/t-mips16 (LIB1ASMFUNCS): Remove __mips16_rdhwr.
	* config/mips/mips16.S (__mips16_rdhwr): Delete.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2012-02-18 18:11:13.000000000 +0000
+++ gcc/config/mips/mips.c	2012-02-19 16:27:26.198924095 +0000
@@ -592,6 +592,9 @@ struct target_globals *mips16_globals;
    and returned from mips_sched_reorder2.  */
 static int cached_can_issue_more;
 
+/* True if the output uses __mips16_rdhwr.  */
+static bool mips_need_mips16_rdhwr_p;
+
 /* Index R is the smallest register class that contains register R.  */
 const enum reg_class mips_regno_to_class[FIRST_PSEUDO_REGISTER] = {
   LEA_REGS,	LEA_REGS,	M16_REGS,	V1_REG,
@@ -2842,7 +2845,9 @@ mips_get_tp (void)
   tp = gen_reg_rtx (Pmode);
   if (TARGET_MIPS16)
     {
+      mips_need_mips16_rdhwr_p = true;
       fn = mips16_stub_function ("__mips16_rdhwr");
+      SYMBOL_REF_FLAGS (fn) |= SYMBOL_FLAG_LOCAL;
       if (!call_insn_operand (fn, VOIDmode))
 	fn = force_reg (Pmode, fn);
       emit_insn (PMODE_INSN (gen_tls_get_tp_mips16, (tp, fn)));
@@ -5827,6 +5832,33 @@ mips_gimplify_va_arg_expr (tree valist,
   return addr;
 }
 \f
+/* Declare a unique, locally-binding function called NAME, then start
+   its definition.  */
+
+static void
+mips_start_unique_function (const char *name)
+{
+  tree decl;
+
+  decl = build_decl (BUILTINS_LOCATION, FUNCTION_DECL,
+		     get_identifier (name),
+		     build_function_type_list (void_type_node, NULL_TREE));
+  DECL_RESULT (decl) = build_decl (BUILTINS_LOCATION, RESULT_DECL,
+				   NULL_TREE, void_type_node);
+  TREE_PUBLIC (decl) = 1;
+  TREE_STATIC (decl) = 1;
+
+  DECL_COMDAT_GROUP (decl) = DECL_ASSEMBLER_NAME (decl);
+
+  targetm.asm_out.unique_section (decl, 0);
+  switch_to_section (get_named_section (decl, NULL, 0));
+
+  targetm.asm_out.globalize_label (asm_out_file, name);
+  fputs ("\t.hidden\t", asm_out_file);
+  assemble_name (asm_out_file, name);
+  putc ('\n', asm_out_file);
+}
+
 /* Start a definition of function NAME.  MIPS16_P indicates whether the
    function contains MIPS16 code.  */
 
@@ -5865,6 +5897,26 @@ mips_end_function_definition (const char
     }
 }
 \f
+/* Output a definition of the __mips16_rdhwr function.  */
+
+static void
+mips_output_mips16_rdhwr (void)
+{
+  const char *name;
+
+  name = "__mips16_rdhwr";
+  mips_start_unique_function (name);
+  mips_start_function_definition (name, false);
+  fprintf (asm_out_file,
+	   "\t.set\tpush\n"
+	   "\t.set\tmips32r2\n"
+	   "\t.set\tnoreorder\n"
+	   "\trdhwr\t$3,$29\n"
+	   "\t.set\tpop\n"
+	   "\tj\t$31\n");
+  mips_end_function_definition (name);
+}
+\f
 /* Return true if calls to X can use R_MIPS_CALL* relocations.  */
 
 static bool
@@ -8467,6 +8519,15 @@ mips_file_start (void)
 	     ASM_COMMENT_START,
 	     mips_small_data_threshold, mips_arch_info->name, mips_isa);
 }
+
+/* Implement TARGET_ASM_CODE_END.  */
+
+static void
+mips_code_end (void)
+{
+  if (mips_need_mips16_rdhwr_p)
+    mips_output_mips16_rdhwr ();
+}
 \f
 /* Make the last instruction frame-related and note that it performs
    the operation described by FRAME_PATTERN.  */
@@ -17357,6 +17418,8 @@ #define TARGET_PREFERRED_RELOAD_CLASS mi
 #define TARGET_ASM_FILE_START mips_file_start
 #undef TARGET_ASM_FILE_START_FILE_DIRECTIVE
 #define TARGET_ASM_FILE_START_FILE_DIRECTIVE true
+#undef TARGET_ASM_CODE_END
+#define TARGET_ASM_CODE_END mips_code_end
 
 #undef TARGET_INIT_LIBFUNCS
 #define TARGET_INIT_LIBFUNCS mips_init_libfuncs
Index: libgcc/config.host
===================================================================
--- libgcc/config.host	2012-02-18 18:10:21.000000000 +0000
+++ libgcc/config.host	2012-02-19 07:58:16.415998843 +0000
@@ -746,12 +746,12 @@ mips*-*-netbsd*)			# NetBSD/mips, either
 	;;
 mips64*-*-linux* | mipsisa64*-*-linux*)
 	extra_parts="$extra_parts crtfastmath.o"
-	tmake_file="${tmake_file} t-crtfm mips/t-mips16 mips/t-tpbit t-slibgcc-libgcc"
+	tmake_file="${tmake_file} t-crtfm mips/t-mips16 mips/t-tpbit"
 	md_unwind_header=mips/linux-unwind.h
 	;;
 mips*-*-linux*)				# Linux MIPS, either endian.
 	extra_parts="$extra_parts crtfastmath.o"
-	tmake_file="${tmake_file} t-crtfm mips/t-mips16 t-slibgcc-libgcc"
+	tmake_file="${tmake_file} t-crtfm mips/t-mips16"
 	md_unwind_header=mips/linux-unwind.h
 	;;
 mips*-*-openbsd*)
Index: libgcc/config/mips/t-mips16
===================================================================
--- libgcc/config/mips/t-mips16	2012-02-18 18:10:21.000000000 +0000
+++ libgcc/config/mips/t-mips16	2012-02-19 07:58:16.455998841 +0000
@@ -36,8 +36,7 @@ LIB1ASMFUNCS = _m16addsf3 _m16subsf3 _m1
 	_m16stubsc0 _m16stubsc1 _m16stubsc2 _m16stubsc5 _m16stubsc6 \
 	_m16stubsc9 _m16stubsc10 \
 	_m16stubdc0 _m16stubdc1 _m16stubdc2 _m16stubdc5 _m16stubdc6 \
-	_m16stubdc9 _m16stubdc10 \
-	_m16rdhwr
+	_m16stubdc9 _m16stubdc10
 
 SYNC = yes
 SYNC_CFLAGS = -mno-mips16
Index: libgcc/config/mips/mips16.S
===================================================================
--- libgcc/config/mips/mips16.S	2012-02-18 18:10:40.000000000 +0000
+++ libgcc/config/mips/mips16.S	2012-02-19 08:00:15.012998552 +0000
@@ -718,17 +718,4 @@ CALL_STUB_RET (__mips16_call_stub_dc_10,
 #endif
 #endif /* !__mips_single_float */
 
-#ifdef L_m16rdhwr
-STARTFN (__mips16_rdhwr)
-	/* Forced always hidden, because the PLT resolver function would
-	   not preserve all necessary registers.  */
-	.hidden	__mips16_rdhwr
-	.set	push
-	.set	mips32r2
-	.set	noreorder
-	rdhwr	$3,$29
-	.set	pop
-	j	$31
-	ENDFN (__mips16_rdhwr)
-#endif
 #endif

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-19 17:21               ` Richard Sandiford
@ 2012-02-21 17:23                 ` Maciej W. Rozycki
  0 siblings, 0 replies; 26+ messages in thread
From: Maciej W. Rozycki @ 2012-02-21 17:23 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: Richard Henderson, Chung-Lin Tang, gcc-patches

On Sun, 19 Feb 2012, Richard Sandiford wrote:

> > The i386 target does it all the time for its __x86.get_pc_thunk.?x thingys.
> 
> Thanks for the pointer.  Here's a patch that takes the same approach.
> There's no problem relying on comdat groups here, since MIPS16 TLS
> requires 2.22 gas and ld anyway.
> 
> Tested on mips64-linux-gnu, mips-sde-elf and various other mips*-elf targets.
> Applied.

 Excellent!  Thanks for working on this and insisting on the most 
reasonable solution.

  Maciej

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-02-03 15:28     ` Richard Sandiford
@ 2012-07-05  6:50       ` Chung-Lin Tang
  2012-07-05 18:55         ` Richard Sandiford
  0 siblings, 1 reply; 26+ messages in thread
From: Chung-Lin Tang @ 2012-07-05  6:50 UTC (permalink / raw)
  To: gcc-patches, Maciej W. Rozycki, rdsandiford

[-- Attachment #1: Type: text/plain, Size: 5934 bytes --]

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.

[-- Attachment #2: gcc.diff --]
[-- Type: text/plain, Size: 8258 bytes --]

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)
 \f
-#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

[-- Attachment #3: libgcc.diff --]
[-- Type: text/plain, Size: 495 bytes --]

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;

[-- Attachment #4: libgomp.diff --]
[-- Type: text/plain, Size: 1341 bytes --]

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");

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-07-05  6:50       ` Chung-Lin Tang
@ 2012-07-05 18:55         ` Richard Sandiford
  2012-07-06  6:24           ` Richard Sandiford
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Sandiford @ 2012-07-05 18:55 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Maciej W. Rozycki

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-07-05 18:55         ` Richard Sandiford
@ 2012-07-06  6:24           ` Richard Sandiford
  2012-07-06  7:41             ` Chung-Lin Tang
  2012-08-29  9:38             ` Chung-Lin Tang
  0 siblings, 2 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-07-06  6:24 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Maciej W. Rozycki

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-07-06  6:24           ` Richard Sandiford
@ 2012-07-06  7:41             ` Chung-Lin Tang
  2012-07-06  9:18               ` Richard Sandiford
  2012-08-29  9:38             ` Chung-Lin Tang
  1 sibling, 1 reply; 26+ messages in thread
From: Chung-Lin Tang @ 2012-07-06  7:41 UTC (permalink / raw)
  To: gcc-patches, Maciej W. Rozycki, rdsandiford

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-07-06  7:41             ` Chung-Lin Tang
@ 2012-07-06  9:18               ` Richard Sandiford
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-07-06  9:18 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Maciej W. Rozycki

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-07-06  6:24           ` Richard Sandiford
  2012-07-06  7:41             ` Chung-Lin Tang
@ 2012-08-29  9:38             ` Chung-Lin Tang
  2012-08-29 18:45               ` Richard Sandiford
  1 sibling, 1 reply; 26+ messages in thread
From: Chung-Lin Tang @ 2012-08-29  9:38 UTC (permalink / raw)
  To: gcc-patches, Maciej W. Rozycki, rdsandiford

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-08-29  9:38             ` Chung-Lin Tang
@ 2012-08-29 18:45               ` Richard Sandiford
  2012-08-30  5:41                 ` Chung-Lin Tang
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Sandiford @ 2012-08-29 18:45 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Maciej W. Rozycki

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-08-29 18:45               ` Richard Sandiford
@ 2012-08-30  5:41                 ` Chung-Lin Tang
  2012-08-30 20:20                   ` Richard Sandiford
  0 siblings, 1 reply; 26+ messages in thread
From: Chung-Lin Tang @ 2012-08-30  5:41 UTC (permalink / raw)
  To: gcc-patches, Maciej W. Rozycki, rdsandiford

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] MIPS16 TLS support for GCC
  2012-08-30  5:41                 ` Chung-Lin Tang
@ 2012-08-30 20:20                   ` Richard Sandiford
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Sandiford @ 2012-08-30 20:20 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Maciej W. Rozycki

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2012-08-30 20:20 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-06 14:48 [PATCH] MIPS16 TLS support for GCC Chung-Lin Tang
2012-01-08 11:21 ` Richard Sandiford
2012-01-15 18:23   ` Richard Sandiford
2012-02-03 10:03   ` Chung-Lin Tang
2012-02-03 11:57     ` Maciej W. Rozycki
2012-02-03 16:20       ` Richard Sandiford
2012-02-03 17:34         ` Maciej W. Rozycki
2012-02-04 10:07           ` Richard Sandiford
2012-02-05 15:03             ` Richard Sandiford
2012-02-06 16:27             ` Maciej W. Rozycki
2012-02-06 19:39               ` Richard Sandiford
2012-02-06 19:43               ` Richard Sandiford
2012-02-11  2:44             ` Richard Henderson
2012-02-11 10:38               ` Maciej W. Rozycki
2012-02-19 17:21               ` Richard Sandiford
2012-02-21 17:23                 ` Maciej W. Rozycki
2012-02-03 15:28     ` Richard Sandiford
2012-07-05  6:50       ` Chung-Lin Tang
2012-07-05 18:55         ` Richard Sandiford
2012-07-06  6:24           ` Richard Sandiford
2012-07-06  7:41             ` Chung-Lin Tang
2012-07-06  9:18               ` Richard Sandiford
2012-08-29  9:38             ` Chung-Lin Tang
2012-08-29 18:45               ` Richard Sandiford
2012-08-30  5:41                 ` Chung-Lin Tang
2012-08-30 20:20                   ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).