Update string_constant for new const initializer code

Hi,
while disabling initializer folding in expr.c I noticed that string-constant is
actually used by builtin folding and thus it needs to be updated for new
varppol initializer code.  Unfortunately folding also happens early from
frntend and thus we need to  behave sanely when varpool is not ready yet.  The
patch removes TREE_SIDE_EFFECTS check since we don't do it elsewhere
(and I have problems to think of volatile constant string having some meaning)
and also makes us to use DECL_REPLACEABLE_P instead of targetm.binds_local_p
like we do elsewhere now too.

Bootstrapped/regtested x86_64-linx, OK?

Honza

	* expr.c (string_constant): Use varpool to figure out if value
	is known.

On Sat, 4 Sep 2010, Jan Hubicka wrote:

> Hi,
> while disabling initializer folding in expr.c I noticed that string-constant is
> actually used by builtin folding and thus it needs to be updated for new
> varppol initializer code.  Unfortunately folding also happens early from
> frntend and thus we need to  behave sanely when varpool is not ready yet.  The
> patch removes TREE_SIDE_EFFECTS check since we don't do it elsewhere
> (and I have problems to think of volatile constant string having some meaning)
> and also makes us to use DECL_REPLACEABLE_P instead of targetm.binds_local_p
> like we do elsewhere now too.
> 
> Bootstrapped/regtested x86_64-linx, OK?
> 
> Honza
> 
> 	* expr.c (string_constant): Use varpool to figure out if value
> 	is known.
> Index: expr.c
> ===================================================================
> --- expr.c	(revision 163859)
> +++ expr.c	(working copy)
> @@ -9815,10 +9837,12 @@ string_constant (tree arg, tree *ptr_off
>  	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
>  	return 0;
>  
> -      /* If they are read-only, non-volatile and bind locally.  */
> +      /* If they are read-only and non-replaceable.  */
>        if (! TREE_READONLY (array)
> -	  || TREE_SIDE_EFFECTS (array)
> -	  || ! targetm.binds_local_p (array))
> +	  || ((!cgraph_function_flags_ready
> +	       && !DECL_REPLACEABLE_P (array))
> +	      || (cgraph_function_flags_ready
> +		  && !varpool_get_node (array)->const_value_known)))

So DECL_REPLACEABLE_P isn't kept up-to-date?  Why does

  || !DECL_REPLACEABLE_P (array)

not work?

Richard.

> On Sat, 4 Sep 2010, Jan Hubicka wrote:
> 
> > Hi,
> > while disabling initializer folding in expr.c I noticed that string-constant is
> > actually used by builtin folding and thus it needs to be updated for new
> > varppol initializer code.  Unfortunately folding also happens early from
> > frntend and thus we need to  behave sanely when varpool is not ready yet.  The
> > patch removes TREE_SIDE_EFFECTS check since we don't do it elsewhere
> > (and I have problems to think of volatile constant string having some meaning)
> > and also makes us to use DECL_REPLACEABLE_P instead of targetm.binds_local_p
> > like we do elsewhere now too.
> > 
> > Bootstrapped/regtested x86_64-linx, OK?
> > 
> > Honza
> > 
> > 	* expr.c (string_constant): Use varpool to figure out if value
> > 	is known.
> > Index: expr.c
> > ===================================================================
> > --- expr.c	(revision 163859)
> > +++ expr.c	(working copy)
> > @@ -9815,10 +9837,12 @@ string_constant (tree arg, tree *ptr_off
> >  	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
> >  	return 0;
> >  
> > -      /* If they are read-only, non-volatile and bind locally.  */
> > +      /* If they are read-only and non-replaceable.  */
> >        if (! TREE_READONLY (array)
> > -	  || TREE_SIDE_EFFECTS (array)
> > -	  || ! targetm.binds_local_p (array))
> > +	  || ((!cgraph_function_flags_ready
> > +	       && !DECL_REPLACEABLE_P (array))
> > +	      || (cgraph_function_flags_ready
> > +		  && !varpool_get_node (array)->const_value_known)))
> 
> So DECL_REPLACEABLE_P isn't kept up-to-date?  Why does
> 
>   || !DECL_REPLACEABLE_P (array)
> 
> not work?

Well, because of the WHOPR partitioning where we can turn
static const int a;
into
extern const int a;
that is replaceable yet we know that initializer is 0.
(it is also the case for "const int a;" when we realize via linker plugin that
it is not prevailed by some real value.  I will add code for this)

One option would be to make DECL_REPLACEABLE function that looks into
varpool/cgraph to give answer, but then const_value_known probably should not
be computed by means of it.  Or we can invent some extra DECL flag for this
purpose and not store it in varpool...

Honza
> 
> Richard.

On Sun, 5 Sep 2010, Jan Hubicka wrote:

> > On Sat, 4 Sep 2010, Jan Hubicka wrote:
> > 
> > > Hi,
> > > while disabling initializer folding in expr.c I noticed that string-constant is
> > > actually used by builtin folding and thus it needs to be updated for new
> > > varppol initializer code.  Unfortunately folding also happens early from
> > > frntend and thus we need to  behave sanely when varpool is not ready yet.  The
> > > patch removes TREE_SIDE_EFFECTS check since we don't do it elsewhere
> > > (and I have problems to think of volatile constant string having some meaning)
> > > and also makes us to use DECL_REPLACEABLE_P instead of targetm.binds_local_p
> > > like we do elsewhere now too.
> > > 
> > > Bootstrapped/regtested x86_64-linx, OK?
> > > 
> > > Honza
> > > 
> > > 	* expr.c (string_constant): Use varpool to figure out if value
> > > 	is known.
> > > Index: expr.c
> > > ===================================================================
> > > --- expr.c	(revision 163859)
> > > +++ expr.c	(working copy)
> > > @@ -9815,10 +9837,12 @@ string_constant (tree arg, tree *ptr_off
> > >  	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
> > >  	return 0;
> > >  
> > > -      /* If they are read-only, non-volatile and bind locally.  */
> > > +      /* If they are read-only and non-replaceable.  */
> > >        if (! TREE_READONLY (array)
> > > -	  || TREE_SIDE_EFFECTS (array)
> > > -	  || ! targetm.binds_local_p (array))
> > > +	  || ((!cgraph_function_flags_ready
> > > +	       && !DECL_REPLACEABLE_P (array))
> > > +	      || (cgraph_function_flags_ready
> > > +		  && !varpool_get_node (array)->const_value_known)))
> > 
> > So DECL_REPLACEABLE_P isn't kept up-to-date?  Why does
> > 
> >   || !DECL_REPLACEABLE_P (array)
> > 
> > not work?
> 
> Well, because of the WHOPR partitioning where we can turn
> static const int a;
> into
> extern const int a;
> that is replaceable yet we know that initializer is 0.

So if it is replaceable then why is the check necessary?  The function
looks at DECL_INITIAL anyway and I hope that is not set when
!varpool_get_node (array)->const_value_known but DECL_REPLACEABLE_P is
true?

I'd hate to have two mechanisms that can easily get out of sync.

But maybe I'm missing sth.

Richard.

> 
> So if it is replaceable then why is the check necessary?  The function
> looks at DECL_INITIAL anyway and I hope that is not set when
> !varpool_get_node (array)->const_value_known but DECL_REPLACEABLE_P is
> true?
> 
> I'd hate to have two mechanisms that can easily get out of sync.
> 
> But maybe I'm missing sth.

The problem is opposite.  You have DECL_REPLACEABLE_P false, but still
varpool_get_node (array)->const_value_known is true because we remember from
WPA stage that the initializer will be put into other ltrans partition.

Honza
> 
> Richard.

On Mon, 6 Sep 2010, Jan Hubicka wrote:

> > 
> > So if it is replaceable then why is the check necessary?  The function
> > looks at DECL_INITIAL anyway and I hope that is not set when
> > !varpool_get_node (array)->const_value_known but DECL_REPLACEABLE_P is
> > true?
> > 
> > I'd hate to have two mechanisms that can easily get out of sync.
> > 
> > But maybe I'm missing sth.
> 
> The problem is opposite.  You have DECL_REPLACEABLE_P false, but still
> varpool_get_node (array)->const_value_known is true because we remember from
> WPA stage that the initializer will be put into other ltrans partition.

Huh?  Of course DECL_REPLACEABLE_P is false - otherwise the constant
value can't be known but would be determined at dynamic link time.

Richard.

> On Mon, 6 Sep 2010, Jan Hubicka wrote:
> 
> > > 
> > > So if it is replaceable then why is the check necessary?  The function
> > > looks at DECL_INITIAL anyway and I hope that is not set when
> > > !varpool_get_node (array)->const_value_known but DECL_REPLACEABLE_P is
> > > true?
> > > 
> > > I'd hate to have two mechanisms that can easily get out of sync.
> > > 
> > > But maybe I'm missing sth.
> > 
> > The problem is opposite.  You have DECL_REPLACEABLE_P false, but still

Well I misundersood your question and made a typo here, so answer was particularly
confusing and confused ;) I meant DECL_REPLACEABLE_P to be true.

So you was asking if DECL_REPLACEABLE_P imply !DECL_INITIAL?
This is not true.  Consider:

const int a=5;

in PIC compilation.

Note that in this example I made const_value_known true as it is what we always
did for scalars (but didn't for arrays/structures and we do now). See comments
comments in varpool_decide_const_value_known.


> > varpool_get_node (array)->const_value_known is true because we remember from
> > WPA stage that the initializer will be put into other ltrans partition.
> 
> Huh?  Of course DECL_REPLACEABLE_P is false - otherwise the constant
> value can't be known but would be determined at dynamic link time.

The problem I am trying to solve with the varpool bit is folding after ltrans
partitioning. Otherwise I would just make const_value_known_p predicate that
would compute return value based on decl flags (or just use DECL_REPLACEABLE_P)
to commonize the logic I replaced by varpool_get_node
(array)->const_value_known tests.

I need extra flag is becuase where DECL_REPLACEABLE_P is too low level and
still works on ltrans unit basis (and there the decl is replaceable and will be
replaced from other ltrans where it is defined) but when folding we want to take
into account knowledge available at LTO time before we partition.

I.e.

static const int a;

at source level might get partitioned into multiple units (as result of
inlining or so).  Then it is defined in one unit where it appears as:

hidden const int a;

In all other units it apepar as:

extern hidden const int a;

Still we want to fold references to A into constant.

The motivation for putting this into varpool was that whole program stuff don't really
belongs to decls and because we already have equivalent mechanizm in cgraph that
is cgraph_body_availability.

I agree it is bit confusing to have DECL_REPLACEABLE_P, cgraph/varpool predicates
and binds_local_p.  We need to answer several questions:

1) If i see initializer of VAR, can I trust it is the value of VAR in runtime?

   This is needed by folding and it is what const_value_known should answer.
   (and cgraph_body_availability >= AVAIL_OVEERWRITABLE for functions)
2) If I am referring the variable, will the reference end up to the one I defined
   in current .s file?  

   This is what darwin needs for its indirections.
3) If I am referring the variable, will the reference end up in the same DSO file?

   This is needed when producing PIC code and is what binds_local_p should answer.

We probably don't really need DECL_REPLACEABLE_P and can just fold the logic
into the two users of it (and remove one in tree-inline.c that is confused since
it make functios uninlinable too early).

We still get wrong answers for binds_local_p.  In LTO when resolution file tells
us what is the prevailing definition we should nullify the following clause:
  /* Uninitialized COMMON variable may be unified with symbols
     resolved from other modules.  */
  else if (DECL_COMMON (exp)
           && (DECL_INITIAL (exp) == NULL
               || DECL_INITIAL (exp) == error_mark_node))
    local_p = false;
and also we unnecesarily return true for WHOPR partitioning becuase of:
  /* A variable is local if the user has said explicitly that it will
     be.  */
  else if (DECL_VISIBILITY_SPECIFIED (exp)
           && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
    local_p = true;
lto.c should set DECL_VISIBILITY_SPECIFIED for all partitioned vars.

I will need to give it bit more tought what is resonable interface for this.
Ideas are welcome ;)

Honza

On Mon, 6 Sep 2010, Jan Hubicka wrote:

> > On Mon, 6 Sep 2010, Jan Hubicka wrote:
> > 
> > > > 
> > > > So if it is replaceable then why is the check necessary?  The function
> > > > looks at DECL_INITIAL anyway and I hope that is not set when
> > > > !varpool_get_node (array)->const_value_known but DECL_REPLACEABLE_P is
> > > > true?
> > > > 
> > > > I'd hate to have two mechanisms that can easily get out of sync.
> > > > 
> > > > But maybe I'm missing sth.
> > > 
> > > The problem is opposite.  You have DECL_REPLACEABLE_P false, but still
> 
> Well I misundersood your question and made a typo here, so answer was particularly
> confusing and confused ;) I meant DECL_REPLACEABLE_P to be true.
> 
> So you was asking if DECL_REPLACEABLE_P imply !DECL_INITIAL?
> This is not true.  Consider:
> 
> const int a=5;
> 
> in PIC compilation.

No, that wasn't my question either.  My question was that when
DECL_REPLACEABLE_P is set why we care for the varpool info at all.
And if it is not set why isn't it enough to look at
DECL_INITIAL.

> Note that in this example I made const_value_known true as it is what we always
> did for scalars (but didn't for arrays/structures and we do now). See comments
> comments in varpool_decide_const_value_known.
> 
> 
> > > varpool_get_node (array)->const_value_known is true because we remember from
> > > WPA stage that the initializer will be put into other ltrans partition.
> > 
> > Huh?  Of course DECL_REPLACEABLE_P is false - otherwise the constant
> > value can't be known but would be determined at dynamic link time.
> 
> The problem I am trying to solve with the varpool bit is folding after ltrans
> partitioning. Otherwise I would just make const_value_known_p predicate that
> would compute return value based on decl flags (or just use DECL_REPLACEABLE_P)
> to commonize the logic I replaced by varpool_get_node
> (array)->const_value_known tests.
> 
> I need extra flag is becuase where DECL_REPLACEABLE_P is too low level and
> still works on ltrans unit basis (and there the decl is replaceable and will be
> replaced from other ltrans where it is defined) but when folding we want to take
> into account knowledge available at LTO time before we partition.
> 
> I.e.
> 
> static const int a;
> 
> at source level might get partitioned into multiple units (as result of
> inlining or so).  Then it is defined in one unit where it appears as:
> 
> hidden const int a;
> 
> In all other units it apepar as:
> 
> extern hidden const int a;
> 
> Still we want to fold references to A into constant.

But both will have DECL_INITIAL set and both will be !DECL_REPLACEABLE_P
(at least I hope so).

> The motivation for putting this into varpool was that whole program stuff don't really
> belongs to decls and because we already have equivalent mechanizm in cgraph that
> is cgraph_body_availability.
> 
> I agree it is bit confusing to have DECL_REPLACEABLE_P, cgraph/varpool predicates
> and binds_local_p.  We need to answer several questions:

Is binds_local_p equivalent to !DECL_REPLACEABLE_P?

> 1) If i see initializer of VAR, can I trust it is the value of VAR in runtime?
> 
>    This is needed by folding and it is what const_value_known should answer.
>    (and cgraph_body_availability >= AVAIL_OVEERWRITABLE for functions)
> 2) If I am referring the variable, will the reference end up to the one I defined
>    in current .s file?  
> 
>    This is what darwin needs for its indirections.
> 3) If I am referring the variable, will the reference end up in the same DSO file?
> 
>    This is needed when producing PIC code and is what binds_local_p should answer.
> 
> We probably don't really need DECL_REPLACEABLE_P and can just fold the logic
> into the two users of it (and remove one in tree-inline.c that is confused since
> it make functios uninlinable too early).

Oh, now I see DECL_REPLACEABLE_P is just

#define DECL_REPLACEABLE_P(NODE) \
  (!DECL_COMDAT (NODE) && (DECL_WEAK (NODE) || !targetm.binds_local_p 
(NODE)))

> We still get wrong answers for binds_local_p.  In LTO when resolution file tells
> us what is the prevailing definition we should nullify the following clause:
>   /* Uninitialized COMMON variable may be unified with symbols
>      resolved from other modules.  */
>   else if (DECL_COMMON (exp)
>            && (DECL_INITIAL (exp) == NULL
>                || DECL_INITIAL (exp) == error_mark_node))
>     local_p = false;
> and also we unnecesarily return true for WHOPR partitioning becuase of:
>   /* A variable is local if the user has said explicitly that it will
>      be.  */
>   else if (DECL_VISIBILITY_SPECIFIED (exp)
>            && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
>     local_p = true;
> lto.c should set DECL_VISIBILITY_SPECIFIED for all partitioned vars.
> 
> I will need to give it bit more tought what is resonable interface for this.
> Ideas are welcome ;)

I don't have good ideas here but just notice that the recent patches
here add, not remove, from the confusion of mine.

Richard.

> No, that wasn't my question either.  My question was that when
> DECL_REPLACEABLE_P is set why we care for the varpool info at all.
> And if it is not set why isn't it enough to look at
> DECL_INITIAL.
Ah, hopefully I gave answer to that too ;)
> > at source level might get partitioned into multiple units (as result of
> > inlining or so).  Then it is defined in one unit where it appears as:
> > 
> > hidden const int a;
> > 
> > In all other units it apepar as:
> > 
> > extern hidden const int a;
> > 
> > Still we want to fold references to A into constant.
> 
> But both will have DECL_INITIAL set and both will be !DECL_REPLACEABLE_P
> (at least I hope so).

No, those don't have DECL_INITIAL (they are initialized to 0). Moreover even
extern const int a=5; (i.e. with DECL_INITIAL) is still DECL_REPLACEABLE
because it is !binds_local_p.
> 
> Is binds_local_p equivalent to !DECL_REPLACEABLE_P?

No, as you found later yourself ;)
> 
> I don't have good ideas here but just notice that the recent patches
> here add, not remove, from the confusion of mine.

Yep, it is all surprisingly stubble ;(

Honza
> 
> Richard.