> > Yep, -fno-toplevel-reorder also ldisables some optimizations (as unreachable > > function removal) > > Actually it seemed like in my tests it only disables unreachable > variable removal. Might have been wrong though. > > > > /* Set when function is visible by other units. */ > > > unsigned externally_visible : 1; > > > + /* Don't reorder to other symbols having this set. */ > > > + unsigned no_reorder : 1; > > > > Is it necessary to introduce extra bit for this? It is quite rarely chcecked > > property, what it buys over lookup_attribute calls everywhere? > > There are quite a few FOR_EACH_FUNCTION/VAR loops that check it now. > So I thought about m*n cost, m:symbols, n*number of attributes per sym. > But perhaps that was premature optimization. > > Also one idea was to eventually set it implicitly for the flag, > this would allow -fno-toplevel-reorder per individual file in LTO > and also eliminate some special cases. Ok but I suppose this could > somehow add implicit attributes.
Hmm, that looks like a good argument - adding the attribute to every single declaration would be wasteful. Lets go with a flag then. > > > > --- a/gcc/cgraphclones.c > > > +++ b/gcc/cgraphclones.c > > > @@ -437,6 +437,7 @@ cgraph_node::create_clone (tree decl, gcov_type > > > gcov_count, int freq, > > > new_node->definition = definition; > > > new_node->local = local; > > > new_node->externally_visible = false; > > > + new_node->no_reorder = no_reorder; > > > > In kernel case I suppose you don't need to stick no-reorder flag on clones? > > If we produce static clone, in what case keeping noreorderness help? > > For kernel no_reorder is only ever set on variables (initcalls) > > > This makes no-reorder variables unremovable by unreachable > > function/variable removal. > > This is not documented so in extend.texi and moreover perhaps we do not > > want this, > > as user can use no_reorder && used attribute in that case? > > It was like this before, so if the flag is set implicitely it will be > convenient to use the same semantics. But I suppose implicit setting > can also add attribute used (although that may change some warnings) -fno-toplevel-reorder was done this way to immitate as closely as possible the non-unit-at-a-time mode so legacy codebases do not break. I guess -fno-topelvel-reorder can just imply no_reorder and used attribute. I still think it would be better tomake no_reorder to do only what name suggests. > > My tests also relied on it (but with Mike's trick that can be fixed) > > Given all this do you still want the flag to be removed? No, I guess it can stay. > > > > else > > > { > > > output_asm_statements (); > > > > > > expand_all_functions (); > > > output_variables (); > > > + output_in_order (true); > > > > I would expect output_in_order to come first and perhaps replace > > output_asm_statements > > (so relative order of asm statements and output in order stuff is preserved) > > good point. > > > > int current_order = -1; > > > + int noreorder_pos = 0; > > > > > > FOR_EACH_VARIABLE (vnode) > > > gcc_assert (!vnode->aux); > > > > Hmm, why this is not a simple pass over the nodes array that goes first and > > inserts all noreorder > > symbols into the first partition before the actual balancing starts? > > I guess I was more ambitious. interleaving is also better if we start > setting the flag implicitly, so the partioning with > -fno-toplevel-reorder would behave like before. I see, you want to mix toplevel/non-toplevel across partitions. In that case we also need to disable logic sorting partitions by size if no_reorder BBs exists in more than one partition. I guess this makes sense, I will look more into your implementation. Honza > > -andi