On Mon, Sep 2, 2013 at 5:19 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>
>> But we still refer to the local entity from TREE_TYPE of the function decl,
>> no?
>
> depending on definition of local entity.  I tought we was discussing if 
> PARM_DECL
> is local or not.
>
> I spent some time thining about the whole streaming scheme and I think we do 
> have
> important side cases handled incorrectly.  Lets try to discuss things.  I 
> will try
> to summarize my understanding to make sure we are on sync and I do not miss 
> something
> important.
>
> So we now have two types of streams
>   - the global stream
>     this stream is one per compilation unit and it gets load into WPA
>     and merged with other streams by tree mergina and LTO-symtab resolution.
>   - local stream.
>     this stream is local for function (and hope for variable initializer 
> soon, too),
>     it is read when we need the function body and it is not in any way merged 
> with
>     global stream, except that the references to global streams are resolved
>     after merging
>
> We do have way to express references from local stream to global stream.  We 
> however
> can not express
>   A) references from global stream to local stream
>   B) references in between two different local streams.

Correct.

> The decision what should go to local or global stream is basically motivated 
> by
>   1) everything needed for interprocedural optimization has to be global
>   2) everything related to function bodies should be local.

I'd rather formulate it as "everything not needed at WPA time should be
local if it can be local"

> For tree nodes, the decision about where the node goes is made by
> tree_is_indexable.  The current set of rules is
>  - parm_decl/result_decl is local
>    (before my change resut_decl was global and parm_decl was local, why those
>     was not the same escapes me, but it needed ugly fixup in ipa-prop that
>     needed to reconnect global DECL pointers to local DECL pointers after
>     streaming in)
>  - function variables are local if they are not static.
>    (Even external declarations are local, not only automatic vars.)
>  - Types are global unless they are variably modified types
>    (The variably modified type code walks into subtypes, so even pointers
>    to variably modified type are local or function declarations mentioning
>    these.)
>  - FIELD_DECL is global unless it is part of variably modified type.
>  - SSA_NAME is local despite logic of tree_is_indexable because it is handled
>    separately as part of gimple and gimple is always local.
>  - LABEL_DECLs are global even when their address is not taken and it would 
> make
>    more sense to hide it in the body sections
>
> Now I think we are safe only when our references never violate A) and B) and 
> moreover
> C) same entity that needs to be unique (such as declarations by one decl rule
>    or BLOCK) is not streamed into multiple streams.  We never merge things
>    between streams and duplicating those is forbidden by design.
>
> It is an irritating property of our streaming machinery that it is not
> diagnozing violations of A/B/C. Violations of type A and B are basically 
> hidden
> by introducing violations of type C and compiler tends to be resistant to 
> those
> in a sense that is usually does not ICE.

Yes, and with the re-organized streaming we can now do much better and
much faster!  This is because with the SCC finding we can record in O(SCC size)
whether a) the SCC contains entities that are local (and to what function) or
global, b) the SCC refers to other SCCs that are local (and in what function)
We can then detect whether we run into the situation that the new SCC refers
to SCCs that are in different local sections (which would mean those
SCCs in turn
would need to be globalized or that the current SCC needs to be duplicated into
all local sections it refers to).

I would not be surprised if our current tree structures, when strictly following
restrictions A) and B) would need to be either all global or all duplicated into
the local sections :/

At least we can now more easily formalize this (and can compute
"scc_is_indexable" which is the property we need now)

> Now I only once added sanity check for C and larnt that
>
>  1) variably sized types may end up being twice - in global stream because 
> they are
>     mentioned by FUNCTION_DECL/VARIABLE_DECL or TYPE_DECL
>     and local stream because of tree_is_indexable rule
>
>     Moreover they violate A after landing in global stream because they refer
>     to temporaries in the gimple body.
>     As you mentioned, we may want to clear TYPE_SIZE and variable 
> DECL_FIELD_OFFSET.
>     But sadly I think dwarf2out needs both and gimple land needs 
> DECL_FIELD_OFFSET.
>  2) TYPE_DECLS of local types (common in C++) tends to drag into global stream
>     BLOCK that later brings to global stream the local variables.
>
>     Making function local TYPE_DECLs local seems cool, but it break A because
>     function/local static variable types may reffer to them.
>  3) _ORIGIN pointers violate B by design.
>  4) LABEL_DECL with address taken violate A and B.
>
> I am sure this list is not complette.
>
> Now we can play about what can be global or local.  Like putting PARM_DECL 
> into global
> stream only, or returning RESULT_DECL into global decl stream.  But
> I do not really see how much chance it has to fix 1-4.  I think the general 
> flaw
> is that functions/static variables (we absolutely need in global stream) may 
> reffer
> to local entities by contextes and variably sized types.
>
> Either we need to consistently not stream pointers causing the violations 
> above
> - i.e. clear them in free_lang_data and make backend not dependent on them. Or
> we need to find way to make references of type A and perhaps B possible.  I 
> can
> think of <local_ref function_decl localid> tree construct that we patch in and
> replace by actual declaration after reading function_decl body when it is
> needed (by dwarf2out or tree optimizers), but it does not seem cool to me 
> either.
> We will like soon notice that with comdats localid needs to be unique across
> the merged comdat bodies or somehting similarly nasty.
>
> Another alternative I can think of is to declare duplication in between 
> streams
> sane and allowed, but teach streaming in code to patch in the in-memory
> representation from global stream by local stream one when this happens.  It
> seems bit tricky to implement - the decl states of functions would need to
> explicitly record the duplications in between local and global streams.

I think duplication is what we do right now, otherwise we couldn't fill in all
tree references at streaming time.

What we need to do is compute the decision once per SCC during the
SCC build and record statistics on how many trees we'll duplicate that
way (not sure if we even properly duplicate them now with the SCC streaming ...)

Richard.

> Honza

Reply via email to