FDO patch -- make ic related vars TLS if target allows

FDO patch -- make ic related vars TLS if target allows

[Xinliang David Li]

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

Hi please review the trivial patch below. It reduces race conditions
in value profiling. Another trivial change (to initialize
function_list struct) is also included.

Bootstrapped and regression tested on x86-64/linux.

Thanks,

David


2011-04-27  Xinliang David Li  <davidxl@google.com>

	* tree-profile.c (init_ic_make_global_vars): Set
	tls attribute on ic vars.
	* coverage.c (coverage_end_function): Initialize
	function_list with zero.

[-- Attachment #2: tls.p --]
[-- Type: text/x-pascal, Size: 1638 bytes --]

Index: coverage.c
===================================================================
--- coverage.c	(revision 172977)
+++ coverage.c	(working copy)
@@ -608,7 +608,7 @@ coverage_end_function (void)
     {
       struct function_list *item;
 
-      item = XNEW (struct function_list);
+      item = XCNEW (struct function_list);
 
       *functions_tail = item;
       functions_tail = &item->next;
Index: tree-profile.c
===================================================================
--- tree-profile.c	(revision 172977)
+++ tree-profile.c	(working copy)
@@ -44,6 +44,8 @@ along with GCC; see the file COPYING3.  
 #include "value-prof.h"
 #include "cgraph.h"
 #include "profile.h"
+#include "target.h"
+#include "output.h"
 
 static GTY(()) tree gcov_type_node;
 static GTY(()) tree gcov_type_tmp_var;
@@ -80,6 +82,10 @@ init_ic_make_global_vars (void)
   TREE_PUBLIC (ic_void_ptr_var) = 0;
   DECL_ARTIFICIAL (ic_void_ptr_var) = 1;
   DECL_INITIAL (ic_void_ptr_var) = NULL;
+  if (targetm.have_tls)
+    DECL_TLS_MODEL (ic_void_ptr_var) =
+      decl_default_tls_model (ic_void_ptr_var);
+
   varpool_finalize_decl (ic_void_ptr_var);
   varpool_mark_needed_node (varpool_node (ic_void_ptr_var));
 
@@ -92,6 +98,10 @@ init_ic_make_global_vars (void)
   TREE_PUBLIC (ic_gcov_type_ptr_var) = 0;
   DECL_ARTIFICIAL (ic_gcov_type_ptr_var) = 1;
   DECL_INITIAL (ic_gcov_type_ptr_var) = NULL;
+  if (targetm.have_tls)
+    DECL_TLS_MODEL (ic_gcov_type_ptr_var) =
+      decl_default_tls_model (ic_gcov_type_ptr_var);
+
   varpool_finalize_decl (ic_gcov_type_ptr_var);
   varpool_mark_needed_node (varpool_node (ic_gcov_type_ptr_var));
 }

Re: FDO patch -- make ic related vars TLS if target allows

[Jan Hubicka]

> Hi please review the trivial patch below. It reduces race conditions
> in value profiling. Another trivial change (to initialize
> function_list struct) is also included.
> 
> Bootstrapped and regression tested on x86-64/linux.
OK,
thanks!
Honza

Re: FDO patch -- make ic related vars TLS if target allows

[H.J. Lu]

On Wed, Apr 27, 2011 at 10:54 AM, Xinliang David Li <davidxl@google.com> wrote:
> Hi please review the trivial patch below. It reduces race conditions
> in value profiling. Another trivial change (to initialize
> function_list struct) is also included.
>
> Bootstrapped and regression tested on x86-64/linux.
>
> Thanks,
>
> David
>
>
> 2011-04-27  Xinliang David Li  <davidxl@google.com>
>
>        * tree-profile.c (init_ic_make_global_vars): Set
>        tls attribute on ic vars.
>        * coverage.c (coverage_end_function): Initialize
>        function_list with zero.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49014


-- 
H.J.

Re: FDO patch -- make ic related vars TLS if target allows

[David Edelsohn]

Hi, David

The front-end drivers use -pthread and that often adds -lpthread. But
-pthread is not passed to cc1, etc.

I am not certain if there is a way for the compiler to ascertain that
it is being invoked to compile a file intended for a multi-threaded
application. It knows bout OpenMP and __thread, but that does not
encompass all pthreads applications.

Thanks, David

On Fri, Dec 28, 2012 at 2:08 PM, Xinliang David Li <davidxl@google.com> wrote:
> Is there a way to tell if the program is going to be multi-threaded?
> If not, it might be useful to introduce a compiler option such as -fmt
> which also enables -lpthread.  Using tricks like weakrefs can
> introduce unnecessary runtime overhead.
>
> David
>
> On Fri, Dec 28, 2012 at 8:26 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
>> David,
>>
>> Support for native TLS on AIX exposed a problem with this patch.  A
>> similar problem exists on Solaris 9.
>>
>> Some helper functions for TLS on AIX and Solaris 9 only are provided
>> by libpthread. Promoting ic related variables to TLS breaks profiling
>> of non-pthread appications.  I completely agree with reducing race
>> conditions and improving support for profiling of pthread
>> applications, but why should this change be enabled for applications
>> not built and run as multi-threaded? This feature should test more
>> than the existence of target TLS support.
>>
>> Thanks, David

Re: FDO patch -- make ic related vars TLS if target allows

[Xinliang David Li]

Is there a way to tell if the program is going to be multi-threaded?
If not, it might be useful to introduce a compiler option such as -fmt
which also enables -lpthread.  Using tricks like weakrefs can
introduce unnecessary runtime overhead.

David

On Fri, Dec 28, 2012 at 8:26 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> David,
>
> Support for native TLS on AIX exposed a problem with this patch.  A
> similar problem exists on Solaris 9.
>
> Some helper functions for TLS on AIX and Solaris 9 only are provided
> by libpthread. Promoting ic related variables to TLS breaks profiling
> of non-pthread appications.  I completely agree with reducing race
> conditions and improving support for profiling of pthread
> applications, but why should this change be enabled for applications
> not built and run as multi-threaded? This feature should test more
> than the existence of target TLS support.
>
> Thanks, David

Re: FDO patch -- make ic related vars TLS if target allows

[David Edelsohn]

David,

Support for native TLS on AIX exposed a problem with this patch.  A
similar problem exists on Solaris 9.

Some helper functions for TLS on AIX and Solaris 9 only are provided
by libpthread. Promoting ic related variables to TLS breaks profiling
of non-pthread appications.  I completely agree with reducing race
conditions and improving support for profiling of pthread
applications, but why should this change be enabled for applications
not built and run as multi-threaded? This feature should test more
than the existence of target TLS support.

Thanks, David