Patchwork section anchors and weak hidden symbols

login
register
mail settings
Submitter Nathan Sidwell
Date May 13, 2013, 9:41 a.m.
Message ID <5190B55C.7060303@acm.org>
Download mbox | patch
Permalink /patch/243343/
State New
Headers show

Comments

Nathan Sidwell - May 13, 2013, 9:41 a.m.
On 05/09/13 07:02, Nathan Sidwell wrote:
> On 05/08/13 18:47, Jan Hubicka wrote:

>> Thinking about it again, isn't decl_replaceable_p the thing you are looking for
>> here?
>
> that looks promising.  I'll try !decl_replaceable_p in the section anchor hook.

It does indeed seem to be the right predicate.
tested with a ppc-linux target, ok?

nathan
2013-05-13  Nathan Sidwell  <nathan@codesourcery.com>

	gcc/
	* varasm.c (default_use_anchors_for_symbol_p): Use decl_replaceable_p.

	gcc/testsuite/
	* gcc.dg/visibility-21.c: New.

+int __attribute__((weak, visibility("hidden"))) weak_hidden[3];
+
+int *f_weak_hidden ()
+{
+  return weak_hidden;
+}
Jan Hubicka - May 13, 2013, 10:10 a.m.
> On 05/09/13 07:02, Nathan Sidwell wrote:
> >On 05/08/13 18:47, Jan Hubicka wrote:
> 
> >>Thinking about it again, isn't decl_replaceable_p the thing you are looking for
> >>here?
> >
> >that looks promising.  I'll try !decl_replaceable_p in the section anchor hook.
> 
> It does indeed seem to be the right predicate.
> tested with a ppc-linux target, ok?

Looks good to me.  We eventually ought to do some renaming and move all the predicates
to single place.  It is really hard to make sense of them as they are organized currently.

Honza
> 
> nathan

> 2013-05-13  Nathan Sidwell  <nathan@codesourcery.com>
> 
> 	gcc/
> 	* varasm.c (default_use_anchors_for_symbol_p): Use decl_replaceable_p.
> 
> 	gcc/testsuite/
> 	* gcc.dg/visibility-21.c: New.
> 
> Index: varasm.c
> ===================================================================
> --- varasm.c	(revision 198771)
> +++ varasm.c	(working copy)
> @@ -6582,10 +6582,18 @@ default_use_anchors_for_symbol_p (const_
>      {
>        /* Don't use section anchors for decls that might be defined by
>  	 other modules.  */
> -      if (!targetm.binds_local_p (decl))
> +      if (decl_replaceable_p (decl))
>  	return false;
>  
>        /* Don't use section anchors for decls that will be placed in a
> Index: testsuite/gcc.dg/visibility-21.c
> ===================================================================
> --- testsuite/gcc.dg/visibility-21.c	(revision 0)
> +++ testsuite/gcc.dg/visibility-21.c	(revision 0)
> @@ -0,0 +1,13 @@
> +/* Test visibility attribute on function definition. */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fsection-anchors" } */
> +/* { dg-require-visibility "" } */
> +/* { dg-require-weak "" } */
> +/* { dg-final { scan-assembler-not "ANCHOR" } } */
> +
> +int __attribute__((weak, visibility("hidden"))) weak_hidden[3];
> +
> +int *f_weak_hidden ()
> +{
> +  return weak_hidden;
> +}
Jan Hubicka - May 13, 2013, 1:09 p.m.
> Index: varasm.c
> ===================================================================
> --- varasm.c	(revision 198771)
> +++ varasm.c	(working copy)
> @@ -6582,10 +6582,18 @@ default_use_anchors_for_symbol_p (const_
>      {
>        /* Don't use section anchors for decls that might be defined by
>  	 other modules.  */
> -      if (!targetm.binds_local_p (decl))
> +      if (decl_replaceable_p (decl))
>  	return false;

Actually looking more into this, I think decl_replaceable_p is still not correct
predicate:
bool
decl_replaceable_p (tree decl)
{
  gcc_assert (DECL_P (decl));
  if (!TREE_PUBLIC (decl) || DECL_COMDAT (decl))
    return false;
  return !decl_binds_to_current_def_p (decl);
}

I think DECL_COMDAT is not what you really want to return true for.  So perhaps
you really want (TREE_PUBLIC (decl) && decl_binds_to_current_def_p)?

Honza

Patch

Index: varasm.c
===================================================================
--- varasm.c	(revision 198771)
+++ varasm.c	(working copy)
@@ -6582,10 +6582,18 @@  default_use_anchors_for_symbol_p (const_
     {
       /* Don't use section anchors for decls that might be defined by
 	 other modules.  */
-      if (!targetm.binds_local_p (decl))
+      if (decl_replaceable_p (decl))
 	return false;
 
       /* Don't use section anchors for decls that will be placed in a
Index: testsuite/gcc.dg/visibility-21.c
===================================================================
--- testsuite/gcc.dg/visibility-21.c	(revision 0)
+++ testsuite/gcc.dg/visibility-21.c	(revision 0)
@@ -0,0 +1,13 @@ 
+/* Test visibility attribute on function definition. */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fsection-anchors" } */
+/* { dg-require-visibility "" } */
+/* { dg-require-weak "" } */
+/* { dg-final { scan-assembler-not "ANCHOR" } } */
+