diff mbox

Avoid unnecessary __cxa_quard_{acquire,release} (PR middle-end/54630)

Message ID 20121117123206.GM1886@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 17, 2012, 12:32 p.m. UTC 
Hi!

This PR points out that unnecessarily in two gcc spots we now use
__cxa_guard_acquire/release.  In the first case, there is no point to
make the var static, it is always created and disposed in the same function,
so making it an automatic variable is certainly cheaper, there is no need
to initialize anything in global ctors, the local initialization is just
= NULL overridden almost immediately by assignment in create anyway.

The other change is perhaps more controversial, it is tiny bit nicer to
have the variable local static, but it is more costly.

In any case, both changes passed bootstrap/regtest on x86_64-linux
and i686-linux, ok?

2012-11-16  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/54630
	* tree-ssa-coalesce.c (coalesce_ssa_name): Remove static
	keyword from ssa_name_hash var.

	* class.c (fixed_type_or_null_ref_ht): New variable.
	(fixed_type_or_null): Use it instead of local static ht.


	Jakub

Comments

Jason Merrill Nov. 17, 2012, 5:27 p.m. UTC | #1 
I suppose these changes are fine, but it might be more future-proof to 
compile with -fno-threadsafe-statics...or fix the build system so that 
the C++ library is available when linking the compiler.

Jason
Jakub Jelinek Nov. 19, 2012, 8:14 a.m. UTC | #2 
Hi!

On Sat, Nov 17, 2012 at 12:27:51PM -0500, Jason Merrill wrote:
> I suppose these changes are fine, but it might be more future-proof
> to compile with -fno-threadsafe-statics...or fix the build system so

Yeah, -fno-threadsafe-statitics might be a good idea, at least until/if ever
somebody starts playing with making gcc thread-safe again.

> that the C++ library is available when linking the compiler.

From what I understood, it was just weird configure options (and likely a
user bug in that) that resulted in C++ library not being available in the
PR.  The reason for my patch was solely that it is more costly to have local
statics.  With -fno-threadsafe-statics it will be less costly than before,
still it is about an extra guard var and need to load it/test it before
every first use in the function, right?

	Jakub
Jason Merrill Nov. 19, 2012, 1:36 p.m. UTC | #3 
On 11/19/2012 03:14 AM, Jakub Jelinek wrote:
> PR.  The reason for my patch was solely that it is more costly to have local
> statics.  With -fno-threadsafe-statics it will be less costly than before,
> still it is about an extra guard var and need to load it/test it before
> every first use in the function, right?

True.

Jason
diff mbox

Patch

--- gcc/tree-ssa-coalesce.c.jj	2012-10-26 18:05:03.000000000 +0200
+++ gcc/tree-ssa-coalesce.c	2012-11-16 16:09:47.302657256 +0100
@@ -1,5 +1,5 @@ 
 /* Coalesce SSA_NAMES together for the out-of-ssa pass.
-   Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011
+   Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
    Free Software Foundation, Inc.
    Contributed by Andrew MacLeod <amacleod@redhat.com>
 
@@ -1292,7 +1292,6 @@  coalesce_ssa_name (void)
   bitmap used_in_copies = BITMAP_ALLOC (NULL);
   var_map map;
   unsigned int i;
-  static hash_table <ssa_name_var_hash> ssa_name_hash;
 
   cl = create_coalesce_list ();
   map = create_outofssa_var_map (cl, used_in_copies);
@@ -1301,6 +1300,8 @@  coalesce_ssa_name (void)
      so debug info remains undisturbed.  */
   if (!optimize)
     {
+      hash_table <ssa_name_var_hash> ssa_name_hash;
+
       ssa_name_hash.create (10);
       for (i = 1; i < num_ssa_names; i++)
 	{
--- gcc/cp/class.c.jj	2012-11-16 12:39:15.000000000 +0100
+++ gcc/cp/class.c	2012-11-16 16:13:32.963262681 +0100
@@ -6567,6 +6567,9 @@  finish_struct (tree t, tree attributes)
   return t;
 }
 
+/* Hash table to avoid endless recursion when handling references.  */
+static hash_table <pointer_hash <tree_node> > fixed_type_or_null_ref_ht;
+
 /* Return the dynamic type of INSTANCE, if known.
    Used to determine whether the virtual function table is needed
    or not.
@@ -6682,9 +6685,8 @@  fixed_type_or_null (tree instance, int *
       else if (TREE_CODE (TREE_TYPE (instance)) == REFERENCE_TYPE)
 	{
 	  /* We only need one hash table because it is always left empty.  */
-	  static hash_table <pointer_hash <tree_node> > ht;
-	  if (!ht.is_created ())
-	    ht.create (37); 
+	  if (!fixed_type_or_null_ref_ht.is_created ())
+	    fixed_type_or_null_ref_ht.create (37); 
 
 	  /* Reference variables should be references to objects.  */
 	  if (nonnull)
@@ -6696,15 +6698,15 @@  fixed_type_or_null (tree instance, int *
 	  if (TREE_CODE (instance) == VAR_DECL
 	      && DECL_INITIAL (instance)
 	      && !type_dependent_expression_p_push (DECL_INITIAL (instance))
-	      && !ht.find (instance))
+	      && !fixed_type_or_null_ref_ht.find (instance))
 	    {
 	      tree type;
 	      tree_node **slot;
 
-	      slot = ht.find_slot (instance, INSERT);
+	      slot = fixed_type_or_null_ref_ht.find_slot (instance, INSERT);
 	      *slot = instance;
 	      type = RECUR (DECL_INITIAL (instance));
-	      ht.remove_elt (instance);
+	      fixed_type_or_null_ref_ht.remove_elt (instance);
 
 	      return type;
 	    }