Patchwork [1/6] Thread pointer built-in functions, core parts

login
register
mail settings
Submitter Chung-Lin Tang
Date July 12, 2012, 6:52 a.m.
Message ID <4FFE741E.7080506@codesourcery.com>
Download mbox | patch
Permalink /patch/170578/
State New
Headers show

Comments

Chung-Lin Tang - July 12, 2012, 6:52 a.m.
Core parts adding the new hooks. BUILT_IN_THREAD_POINTER and
BUILT_IN_SET_THREAD_POINTER are different hooks, as some targets only
implement one of them (thread pointer read).

Thanks,
Chung-Lin

        * targhooks.c (default_expand_builtin_thread_pointer): New.
        (default_expand_builtin_set_thread_pointer): New.
        * targhooks.h (default_expand_builtin_thread_pointer): New.
        (default_expand_builtin_set_thread_pointer): New.
        * target.def (expand_builtin_thread_pointer): New target hook.
        (expand_builtin_set_thread_pointer): New target hook.
        * builtins.c (expand_builtin_thread_pointer): New.
        (expand_builtin_set_thread_pointer): New.
        (expand_builtin): Add BUILT_IN_THREAD_POINTER,
        BUILT_IN_SET_THREAD_POINTER expand cases.
        * builtins.def (BUILT_IN_THREAD_POINTER):
        New __builtin_thread_pointer builtin.
        (BUILT_IN_SET_THREAD_POINTER):
        New __builtin_set_thread_pointer builtin.
        * doc/tm.texi.in: Add BUILT_IN_THREAD_POINTER,
        BUILT_IN_SET_THREAD_POINTER hook entries.
        * doc/tm.texi: Update.
Richard Sandiford - July 12, 2012, 6:37 p.m.
Hi Chung-Lin,

Chung-Lin Tang <cltang@codesourcery.com> writes:
> Core parts adding the new hooks. BUILT_IN_THREAD_POINTER and
> BUILT_IN_SET_THREAD_POINTER are different hooks, as some targets only
> implement one of them (thread pointer read).

Thanks a lot for doing this.  It looks great to me, although I can't
approve it.  Only thing I could see was:

> +rtx
> +default_expand_builtin_thread_pointer (rtx target ATTRIBUTE_UNUSED)
> +{
> +  sorry ("__builtin_thread_pointer() not available for this target");
> +  return NULL;
> +}
> +
> +void
> +default_expand_builtin_set_thread_pointer (rtx val ATTRIBUTE_UNUSED)
> +{
> +  sorry ("__builtin_set_thread_pointer() not available for this target");
> +}

Function names should be quoted by %< %>.  But maybe we can save the
translators some work and use:

  sorry ("%qs is not available for this target", "__builtin_thread_pointer()");
...
  sorry ("%qs is not available for this target",
         "__builtin_set_thread_pointer()");

instead.  I'm no expert on the diagnostic stuff though.

Hope it goes in!

Thanks,
Richard
Mike Stump - July 13, 2012, 1:28 a.m.
On Jul 11, 2012, at 11:52 PM, Chung-Lin Tang wrote:
> Core parts adding the new hooks. BUILT_IN_THREAD_POINTER and
> BUILT_IN_SET_THREAD_POINTER are different hooks, as some targets only
> implement one of them (thread pointer read).

sorry seems a little overly dramatic.  I would have thought that error would be enough, and then for the return value, maybe a const0_rtx for Pmode.
Chung-Lin Tang - July 13, 2012, 6:47 a.m.
On 2012/7/13 09:28 AM, Mike Stump wrote:
> On Jul 11, 2012, at 11:52 PM, Chung-Lin Tang wrote:
>> Core parts adding the new hooks. BUILT_IN_THREAD_POINTER and
>> BUILT_IN_SET_THREAD_POINTER are different hooks, as some targets only
>> implement one of them (thread pointer read).
> 
> sorry seems a little overly dramatic.  I would have thought that error would be enough, 

I'll change to use error() instead.

> and then for the return value, maybe a const0_rtx for Pmode.

A little unsure what you mean.  Are you referring to return const0_rtx
for default_expand_builtin_thread_pointer() instead of NULL?

Thanks,
Chung-Lin
Chung-Lin Tang - July 13, 2012, 6:52 a.m.
On 2012/7/13 02:37 AM, Richard Sandiford wrote:
>> +void
>> > +default_expand_builtin_set_thread_pointer (rtx val ATTRIBUTE_UNUSED)
>> > +{
>> > +  sorry ("__builtin_set_thread_pointer() not available for this target");
>> > +}
> Function names should be quoted by %< %>.  But maybe we can save the
> translators some work and use:
> 
>   sorry ("%qs is not available for this target", "__builtin_thread_pointer()");
> ...
>   sorry ("%qs is not available for this target",
>          "__builtin_set_thread_pointer()");

Seeing default_expand_builtin_saveregs() as a current example, using %qs
sort of diverges from the style, though I'm not sure if that should be
changed too...personally I think unquoted looks cleaner :)

Another small detail, should '()' be included with the function name in
the diagnostic message?

Thanks,
Chung-Lin
Andreas Schwab - July 13, 2012, 7:53 a.m.
Richard Sandiford <rsandifo@nildram.co.uk> writes:

> Function names should be quoted by %< %>.  But maybe we can save the
> translators some work and use:
>
>   sorry ("%qs is not available for this target", "__builtin_thread_pointer()");
> ...
>   sorry ("%qs is not available for this target",
>          "__builtin_set_thread_pointer()");
>
> instead.  I'm no expert on the diagnostic stuff though.

The function name should not be followed by parens.

Andreas.
Mike Stump - July 14, 2012, 1:58 a.m.
On Jul 12, 2012, at 11:47 PM, Chung-Lin Tang <cltang@codesourcery.com> wrote:
>> and then for the return value, maybe a const0_rtx for Pmode.
> 
> A little unsure what you mean.  Are you referring to return const0_rtx
> for default_expand_builtin_thread_pointer() instead of NULL?

Yes.  NULL has the habit of crashing things if they don't expect it.
Richard Henderson - July 19, 2012, 7:50 p.m.
On 07/11/2012 11:52 PM, Chung-Lin Tang wrote:
>         * target.def (expand_builtin_thread_pointer): New target hook.
>         (expand_builtin_set_thread_pointer): New target hook.

Is there a particular reason why you're using target hooks rather than
named patterns in the md file?  This *is* happening during rtl expansion
after all, when we're already well integrated with the backend.

E.g.

(define_expand "get_thread_pointer"
  [(set (match_operand:P 0 "...") (...))]
)

(define_expand "set_thread_pointer"
  [(set (...) (match_operand:P 0 "..."))]
)


r~

Patch

Index: target.def
===================================================================
--- target.def	(revision 189431)
+++ target.def	(working copy)
@@ -2668,6 +2668,22 @@  DEFHOOK
  enum unwind_info_type, (void),
  default_debug_unwind_info)
 
+/* Expand builtin function for returning TLS thread pointer.  */
+DEFHOOK
+(expand_builtin_thread_pointer,
+ "This hook expands the built-in function for reading\
+ the TLS thread pointer, if supported on the target.",
+ rtx, (rtx),
+ default_expand_builtin_thread_pointer)
+
+/* Expand builtin function for setting TLS thread pointer.  */
+DEFHOOK
+(expand_builtin_set_thread_pointer,
+ "This hook expands the built-in function for setting\
+ the TLS thread pointer, if supported on the target.",
+ void, (rtx),
+ default_expand_builtin_set_thread_pointer)
+
 DEFHOOKPOD
 (atomic_test_and_set_trueval,
  "This value should be set if the result written by\
Index: targhooks.c
===================================================================
--- targhooks.c	(revision 189431)
+++ targhooks.c	(working copy)
@@ -1456,4 +1456,17 @@  default_pch_valid_p (const void *data_p, size_t le
   return NULL;
 }
 
+rtx
+default_expand_builtin_thread_pointer (rtx target ATTRIBUTE_UNUSED)
+{
+  sorry ("__builtin_thread_pointer() not available for this target");
+  return NULL;
+}
+
+void
+default_expand_builtin_set_thread_pointer (rtx val ATTRIBUTE_UNUSED)
+{
+  sorry ("__builtin_set_thread_pointer() not available for this target");
+}
+
 #include "gt-targhooks.h"
Index: targhooks.h
===================================================================
--- targhooks.h	(revision 189431)
+++ targhooks.h	(working copy)
@@ -179,5 +179,8 @@  extern enum machine_mode default_get_reg_raw_mode(
 extern void *default_get_pch_validity (size_t *);
 extern const char *default_pch_valid_p (const void *, size_t);
 
+extern rtx default_expand_builtin_thread_pointer (rtx);
+extern void default_expand_builtin_set_thread_pointer (rtx);
+
 extern void default_asm_output_ident_directive (const char*);
 
Index: builtins.c
===================================================================
--- builtins.c	(revision 189431)
+++ builtins.c	(working copy)
@@ -5760,6 +5760,27 @@  expand_builtin_sync_synchronize (void)
   expand_mem_thread_fence (MEMMODEL_SEQ_CST);
 }
 
+static rtx
+expand_builtin_thread_pointer (tree exp, rtx target)
+{
+  if (!validate_arglist (exp, VOID_TYPE))
+    return const0_rtx;
+  if (!REG_P (target) || GET_MODE (target) != Pmode)
+    target = gen_reg_rtx (Pmode);
+  target = targetm.expand_builtin_thread_pointer (target);
+  return (target ? target : const0_rtx);
+}
+
+static void
+expand_builtin_set_thread_pointer (tree exp)
+{
+  rtx val;
+  if (!validate_arglist (exp, POINTER_TYPE, VOID_TYPE))
+    return;
+  val = expand_expr (CALL_EXPR_ARG (exp, 0), NULL_RTX, Pmode, EXPAND_NORMAL);
+  targetm.expand_builtin_set_thread_pointer (val);
+}
+
 
 /* Expand an expression EXP that calls a built-in function,
    with result going to TARGET if that's convenient
@@ -6825,6 +6846,13 @@  expand_builtin (tree exp, rtx target, rtx subtarge
 	maybe_emit_free_warning (exp);
       break;
 
+    case BUILT_IN_THREAD_POINTER:
+      return expand_builtin_thread_pointer (exp, target);
+
+    case BUILT_IN_SET_THREAD_POINTER:
+      expand_builtin_set_thread_pointer (exp);
+      return const0_rtx;
+
     default:	/* just do library call, if unknown builtin */
       break;
     }
Index: builtins.def
===================================================================
--- builtins.def	(revision 189431)
+++ builtins.def	(working copy)
@@ -782,6 +782,17 @@  DEF_BUILTIN (BUILT_IN_PROFILE_FUNC_ENTER, "__cyg_p
 DEF_BUILTIN (BUILT_IN_PROFILE_FUNC_EXIT, "__cyg_profile_func_exit", BUILT_IN_NORMAL, BT_FN_VOID_PTR_PTR, BT_LAST,
 	     false, false, false, ATTR_NULL, true, true)
 
+/* TLS thread pointer related builtins.  */
+DEF_BUILTIN (BUILT_IN_THREAD_POINTER, "__builtin_thread_pointer",
+	     BUILT_IN_NORMAL, BT_FN_PTR, BT_LAST,
+	     false, false, true, ATTR_CONST_NOTHROW_LIST, true,
+	     targetm.have_tls)
+
+DEF_BUILTIN (BUILT_IN_SET_THREAD_POINTER, "__builtin_set_thread_pointer",
+	     BUILT_IN_NORMAL, BT_FN_VOID_PTR, BT_LAST,
+	     false, false, true, ATTR_NOTHROW_LIST, true,
+	     targetm.have_tls)
+
 /* TLS emulation.  */
 DEF_BUILTIN (BUILT_IN_EMUTLS_GET_ADDRESS, targetm.emutls.get_address,
 	     BUILT_IN_NORMAL,
Index: doc/tm.texi
===================================================================
--- doc/tm.texi	(revision 189431)
+++ doc/tm.texi	(working copy)
@@ -11293,3 +11293,11 @@  memory model bits are allowed.
 @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL
 This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}.
 @end deftypevr
+
+@deftypefn {Target Hook} rtx TARGET_EXPAND_BUILTIN_THREAD_POINTER (rtx)
+This hook expands the built-in function for reading the TLS thread pointer, if supported on the target.
+@end deftypefn
+
+@deftypefn {Target Hook} void TARGET_EXPAND_BUILTIN_SET_THREAD_POINTER (rtx)
+This hook expands the built-in function for setting the TLS thread pointer, if supported on the target.
+@end deftypefn
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 189431)
+++ doc/tm.texi.in	(working copy)
@@ -11165,3 +11165,7 @@  memory model bits are allowed.
 @end deftypefn
 
 @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL
+
+@hook TARGET_EXPAND_BUILTIN_THREAD_POINTER
+
+@hook TARGET_EXPAND_BUILTIN_SET_THREAD_POINTER