[updated] Make emulated TLS lto-friendly.

On 8 Jul 2010, at 20:19, Richard Henderson wrote:

> On 07/08/2010 12:07 PM, IainS wrote:
>>
>> On 8 Jul 2010, at 00:22, Richard Henderson wrote:
>>> Do you really need this kind of thing anymore, since you're exposing
>>> the use of the control variable so early?  I would have thought that
>>> varpool.c would no longer need any special-casing for !have_tls.
>>
>> this is my understanding (which might be flawed &| incomplete).
>>
>> Whilst we are on the parse side - and building varpool & cgraph, the
>> relationship is not fully exposed (that happens when gimplication  
>> is done).
>>
>> So my reasoning was that the "ghost/proxy" vars should be made to  
>> track
>> the user-land ones until then.
>
> Hmm.  So what you're saying is that there's extra cleanup work that
> needs to happen while lowering the representation.  It's not merely
> a matter of code substitution during gimplification.

Attached is an updated patch which addresses the points you raised -  
less my response above.

IMO it is not the world's most elegant solution..
... perhaps in part owing to me not know the "Best Places" to do some  
things
... but I suspect also because the whole thing doesn't sit comfortably..

I'm not sure where we sit in terms of applying this..
I guess it remains an interim solution to LTO on emuTLS targets, with  
the observation that perhaps a different approach is needed now we  
have LTO.

I've tested on i686-apple-darwin9 and cris-elf that the changes don't  
regress.
(in fact, my local tls testsuite is somewhat wider than the one in  
trunk)

If it is intended to apply this then, of course, I'd test on linux  
first (should be a nil response).

Known Minus:
  it fails the regression Jakub pointed out.

Known Plus:
   it allows lto
  it actually performs better on CSE than trunk does (test added).

I'd hazard that the CSE improvement is worth more than the lost  
trivial zero return.

> In which case I wonder if it wouldn't be better to do as Honza
> suggested and separate all of this out into a new pass_lower_emutls.
> Perhaps to be placed just after pass_lower_vector.  That placement
> is before pass_build_cgraph_edges, which I believe means you would
> not have to fix up the cgraph edges for the new function calls.  All
> you'd need to do is transform the tls variable references and fix up
> the varpool references.

My £0.02 having tangled with this for the last few weeks...

1/ a pass behind a gate of ! targetm.have_tls is a whole lot less  
intrusive to non-emutls targets than what we have
2/ it's likely more efficient for emutls targets since there's less to- 
ing and fro-ing.
3/ probably more maintainable.
4/ certainly more transparent.
5/ (probably) loses all reference to emutls from varasm, varpool,  
expr, & gimplify..

mind you, that doesn't mean I know how to write such a pass ...  ;-)

Iain.

gcc:

	PR target/44132
	* expr.c (emutls_var_address): Remove.
	(expand_expr_addr_expr_1): Remove TLS emulation hook.
	(expand_expr_real_1): Ditto.

	* gimplify.c (emutls_var_address): Add proc.
	(gimplify_decl_expr): expand TLS vars.
	(gimplify_var_or_parm_decl): Ditto.
	(omp_notice_variable): Recognize TLS_MODEL_EMULATED.

	* passes.c (rest_of_decl_compilation): Substitute TLS control vars  
for the master.

	* varasm.c (decl_needs_tls_emulation_p): New.
	(get_emutls_init_templ_addr): Adjust DECL_PRESERVE_P and DECL_INITIAL.
	(emutls_decl): Copy TREE_ADDRESSABLE, DECL_PRESERVE_P, create an
	initializer when one is present on the user-var.
	(emutls_common_1): Remove comment.
	(emutls_finalize_control_var): Copy TREE_USED, add an initializer for  
size and
	align fields for cases with un-initialized user vars.
	(emutls_find_user_var_cb): New.
	(emutls_add_base_initializer): New.
	(asm_output_bss): Assert not a tls var needing emulation.
	(asm_output_aligned_bss): Ditto.
	(assemble_variable): Remove control var init. code.  Assert not a tls  
var needing
	emulation.  Provide a trivial initializer for size and align fields  
if one is not already
	set. (do_assemble_alias): Do not handle emutls vars here.
	(var_decl_for_asm): New.
	(finish_aliases_1): Walk the alias pairs substituting emutls controls  
for the user
	counterparts.

	* varpool.c (varpool_mark_needed_node): Do not handle TLS  
substitution here.
	(decide_is_variable_needed): Or here.
	(varpool_finalize_decl): Handle TLS substitution.
	Remove early calls to varpool_assemble_pending_decls().
	Check enqueuing of vars after all tests for need are complete.
	(varpool_analyze_pending_decls): Do not record references if the  
initializer is error_mark.

testsuite:
	
	PR target/44132
	* gcc.dg/tls/thr-init-1.c: New.
	* gcc.dg/tls/thr-init-2.c: New.
	* gcc.dg/torture/tls New.
	* gcc.dg/torture/tls/tls-test.c: New.
	* gcc.dg/torture/tls/thr-init-1.c: New.
	* gcc.dg/torture/tls/tls.exp: New.
	* gcc.dg/torture/tls/thr-init-2.c: New.

	* lib/target-supports.exp (check_effective_target_tls_emulated): New.
	* gcc.dg/tls/thr-cse-1.c: New.


Index: gcc/testsuite/lib/target-supports.exp
===================================================================

--- gcc/testsuite/lib/target-supports.exp	(revision 161974)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -596,6 +596,23 @@ proc check_effective_target_tls_native {} {
     }]
 }
 
+# Return 1 if *emulated* thread local storage (TLS) is supported, 0 otherwise.
+
+proc check_effective_target_tls_emulated {} {
+    # VxWorks uses emulated TLS machinery, but with non-standard helper
+    # functions, so we fail to automatically detect it.
+    global target_triplet
+    if { [regexp ".*-.*-vxworks.*" $target_triplet] } {
+	return 1
+    }
+    
+    return [check_no_messages_and_pattern tls_emulated "emutls" assembly {
+	__thread int i;
+	int f (void) { return i; }
+	void g (int j) { i = j; }
+    }]
+}
+
 # Return 1 if TLS executables can run correctly, 0 otherwise.
 
 proc check_effective_target_tls_runtime {} {
Index: gcc/testsuite/gcc.dg/tls/thr-cse-1.c
===================================================================
--- gcc/testsuite/gcc.dg/tls/thr-cse-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tls/thr-cse-1.c	(revision 0)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+/* { dg-require-effective-target tls_emulated } */
+
+/* Test that we only get one call to emutls_get_address when CSE is
+   active.  Note that the var _must_ be initialized for the scan asm
+   to work, since otherwise there will be an initializer which will,
+   correctly, call emutls_get_address.  */
+int foo (int b, int c, int d)
+{
+  static __thread int a=1;
+  a += b;
+  a -= c;
+  a += d;
+  return a;
+}
+
+/* { dg-final { scan-assembler-not "emutls_get_address.*emutls_get_address.*" { target { ! *-wrs-vxworks } } } } */
+/* { dg-final { scan-assembler-not "tls_lookup.*tls_lookup.*" { target *-wrs-vxworks } } } */
+
Index: gcc/testsuite/gcc.dg/tls/thr-init-1.c
===================================================================
--- gcc/testsuite/gcc.dg/tls/thr-init-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tls/thr-init-1.c	(revision 0)
@@ -0,0 +1,8 @@
+/* { dg-require-effective-target tls } */
+/* { dg-do compile } */
+
+static __thread int fstat ;
+static __thread int fstat = 1 ;
+static __thread int fstat ;
+static __thread int fstat = 2; /* { dg-error "redefinition of 'fstat'" } */
+				/* { dg-message "note: previous definition of 'fstat' was here" "" { target *-*-* } 5 } */
Index: gcc/testsuite/gcc.dg/tls/thr-init-2.c
===================================================================
--- gcc/testsuite/gcc.dg/tls/thr-init-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tls/thr-init-2.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-require-effective-target tls } */
+/* { dg-do run } */
+
+extern void abort() ;
+
+static __thread int fstat ;
+static __thread int fstat = 1;
+
+int test_code(int b)
+{
+  fstat += b ;
+  return fstat;
+}
+
+int main (int ac, char *av[])
+{
+  int a = test_code(1);
+  
+  if ((a != 2) || (fstat != 2))
+    abort () ;
+  
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/tls/tls-test.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/tls/tls-test.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/tls/tls-test.c	(revision 0)
@@ -0,0 +1,52 @@
+/* { dg-do run }  */
+/* { dg-require-effective-target tls  }  */
+/* { dg-require-effective-target pthread } */
+/* { dg-options "-pthread" } */
+
+#include <pthread.h>
+extern int printf (char *,...);
+__thread int a = 5; 
+int *volatile a_in_other_thread = (int *) 12345;
+
+static void *
+thread_func (void *arg)
+{
+  a_in_other_thread = &a;
+  a+=5;
+  *((int *) arg) = a;
+  return (void *)0;
+}
+
+int
+main ()
+{
+  pthread_t thread;
+  void *thread_retval;
+  int *volatile a_in_main_thread;
+  int *volatile again ;
+  int thr_a;
+
+  a_in_main_thread = &a;
+
+  if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, &thr_a))
+    return 0;
+
+  if (pthread_join (thread, &thread_retval))
+    return 0;
+
+  again = &a;
+  if (again != a_in_main_thread)
+    {
+      printf ("FAIL: main thread addy changed from 0x%0x to 0x%0x\n", 
+		a_in_other_thread, again);
+      return 1;
+    }
+
+  if (a != 5 || thr_a != 10 || (a_in_other_thread == a_in_main_thread))
+    {
+      printf ("FAIL: a= %d, thr_a = %d Addr = 0x%0x\n", 
+		a, thr_a, a_in_other_thread);
+      return 1;
+    }
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/tls/thr-init-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/tls/thr-init-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/tls/thr-init-1.c	(revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do run } */
+/* { dg-require-effective-target tls } */
+
+extern int printf (char *,...);
+extern void abort() ;
+
+int test_code(int b)
+{
+static __thread int fstat = 1;
+  fstat += b ;
+  return fstat;
+}
+
+int main (int ac, char *av[])
+{
+  int a = test_code(1);
+  
+  if ( a != 2 )
+    {
+      printf ("a=%d\n", a) ;
+      abort ();
+    }
+  
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/tls/tls.exp
===================================================================
--- gcc/testsuite/gcc.dg/torture/tls/tls.exp	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/tls/tls.exp	(revision 0)
@@ -0,0 +1,36 @@
+#   Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Load support procs.
+load_lib gcc-dg.exp
+
+# If a testcase doesn't have special options, use these.
+global DEFAULT_CFLAGS
+if ![info exists DEFAULT_CFLAGS] then {
+    set DEFAULT_CFLAGS " -ansi -pedantic-errors"
+}
+
+# Initialize `dg'.
+dg-init
+
+# Main loop.
+gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
+        $DEFAULT_CFLAGS
+
+# All done.
+dg-finish
Index: gcc/testsuite/gcc.dg/torture/tls/thr-init-2.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/tls/thr-init-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/tls/thr-init-2.c	(revision 0)
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-require-effective-target tls } */
+
+extern int printf (char *,...);
+extern void abort() ;
+
+static __thread int fstat ;
+static __thread int fstat = 1;
+static __thread int fstat ;
+
+int test_code(int b)
+{
+  fstat += b ;
+  return fstat;
+}
+
+int main (int ac, char *av[])
+{
+  int a = test_code(1);
+  
+  if ( a != 2 || fstat != 2 )
+    {
+    printf ("a=%d fstat=%d\n", a, fstat) ;
+    abort ();
+    }
+  
+  return 0;
+}


[updated] Make emulated TLS lto-friendly.

Commit Message

Comments

Patch