On Thu, 4 Oct 2018, David Malcolm wrote: > On Thu, 2018-10-04 at 12:52 +0200, Richard Biener wrote: > > On Fri, 28 Sep 2018, David Malcolm wrote: > > [...snip...] > > > > > > > OK for trunk? > > > > OK. My question on 2/3 leaves Richard time to comment. > > > > Maybe you can tackle the issue that -fopt-info-inline does > > nothing at the moment and see if opt-problems are a good fit > > there as well (reporting the CIF strings). > > Thanks. I've committed the patch kit. > > [CCing Honza] > > I'm looking at -fopt-info-inline now (PR ipa/86395). I might look at > loop optimizations also (there are some fprintfs there also). > > Presumably the user-experience for -fopt-info-inline should be > something like: > > <CALLSITE-LOCATION>: missed: can't inline foo into bar > <REASON-LOCATION>: missed: because of <REASON> > > or: > > <CALLSITE-LOCATION>: missed: not inlining foo into bar > <REASON-LOCATION>: missed: because of <REASON> > > and: > > <CALLSITE-LOCATION>: optimized: inlined foo into bar > > (gathering all pertinent data into the -fsave-optimization-record > output). > > I suspect that the opt_problem class from the above patch kit [1] isn't > a good fit for inlining: the loop vectorizer runs one outer loop at a > time, with a deep callstack, needing to bubble up a single "problem" at > at time. > > In contrast, assuming I'm reading the code right, the inliner stores a > status in each cgraph_edge, and walks the cgraph to decide what to do > with all of the edges, with a fairly shallow callstack: I believe > decisions made for one edge can affect other edges etc. > > It might still be possible to store and/or emit a bit more per-edge > detail other than the plain enum cgraph_inline_failed_t, if that's > desirable. > > Brainstorming: > > (a) replace the "inline_failed" enum with a pointer to a class that can > contain more details (perhaps an opt_problem subclass): in a normal > dump-disabled setting, these would point to global constant singletons > (so no allocation is needed); in a dump-enabled setting, these could be > allocated objects containing additional details describing *exactly* > why an edge wasn't inlined (but this adds lots of indirections, ugh) > > (b) add a map of cgraph_edge to opt_problem instances; this would only > be populated if dumping is enabled. > > (c) if dumping is enabled, generate an opt_problem when an edge > transitions to CIF_FINAL_ERROR, and emit it... somewhere. > > I'm not sure if the above are good ideas or not. (b) and (c) feel more > credible than (a).
I guess I'd try a classical top-down approach - start by simply adding some dump_printf to where inline failed/succeeded and see if that's enough to do the job. Back to square one if it isn't ;) Richard. > > Dave > > [1] https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01747.html > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
