Re: [PATCH][1/n] Cleanup inlining

[Jan Hubicka <hubicka at ucw dot cz>]

> 
> This is a first patch in a series to cleanup inlining, focused on
> making the code more self-documented (cleanup the predicate confusion)
> and eventually reduce the number of --params involved in inlining.
> 
> This first patch splits up cgraph_decide_inlining_incrementally with
> the goal to make cgraph_early_inlining easy to follow.  The remaining
> cgraph_decide_inlining_incrementally is still not obvious with
> respect to the cgraph_check_inline_limits and cgraph_default_inline_p
> predicates, but that is left to a followup that also includes cleanups
> in the IPA inliner.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, queued for stage1,
> feedback is always appreciated.

Looks good in general.
> 
> 2/n will focus on disentangling the various inlining related predicates
> into "can" and "want" kinds and unify the "can" kinds in one place
> and to compute all function (and not edge) related "can"s in the
> place we compute local inline parameters.

I also have similar patch in my local queue.  I didn't split the incremental
inliner implementation however since I started with the ipa-inline pass. I
refactored the tests that are done by all the inliners (incremental, small
function, called once and recursive) into more handy predicates. In my version
they was can/profiltable predicates.  The first share quite well, the profiltability
(or want) bits tends to be more specific for individual heruistics.

I also split ipa-inline.c into ipa-inline.c and ipa-inline-heuristics.c to
enforce bit of api via ipa-inline.h and moved some stuff from cgraphunit that
is inlining specific into ipa-inline.c

Your effort is very appreciated. It is good that someone else will try to make
sense of the code, too.  The code kept envolving for quite few years and I got used to
it in the current shape ;) 
My patches are old, they was written over year ago and intended for last stage1
but then I didn't find time to merge it.  Inliner changed quite a lot in stage2
so they will need heavy updating/redoing anyway)

You plan to add the `can' bits into function info?   I guess there are not too
many, but we will need to stream them for LTO and thus they would also need to
be stabe WRT optimizating settings.  
Since we want LTO format to be more stable in future, I am bit hesitant to add
new info into it that can be easily derived from what is already there.

> + 
> + /* Return true if we can inline the edge E.  Dump information to FILE
> +    if non-NULL.  */
> + 
> + static bool
> + cgraph_edge_inlinable_p (struct cgraph_edge *e, FILE *file)
> + {
> +   if (!cgraph_node_inlinable_p (e->callee, file))
> +     return false;
> +   if (!tree_can_inline_p (e)
> +       || e->call_stmt_cannot_inline_p)

I believe that since we added the edge flag with LTO branch merge, we can drop
the tree_can_inline_p tests, they should be already in the edge info.

Honza