<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Jan 27, 2017 at 11:54 AM Dehao Chen <<a href="mailto:danielcdh@gmail.com">danielcdh@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg">On Fri, Jan 27, 2017 at 11:45 AM, David Blaikie <span dir="ltr" class="gmail_msg"><<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><span class="gmail_msg"><div dir="ltr" class="gmail_msg">On Fri, Jan 27, 2017 at 11:35 AM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="gmail_msg" target="_blank">mehdi.amini@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><blockquote type="cite" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">On Jan 27, 2017, at 11:29 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="m_4431236286031313409m_7207517522773178981m_-2799873086343754503Apple-interchange-newline m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div dir="ltr" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="gmail_quote m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div dir="ltr" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">On Fri, Jan 27, 2017 at 11:15 AM Dehao Chen via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div><blockquote class="gmail_quote m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">danielcdh added a comment.<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
After a lengthy discussion with @dblaikie (Thanks David for the input), let me add some more context to this patch.<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
- What is the impact of adding -fdebug-info-for-profiling<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
  - It will not affect the generated code<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
  - It will increase the debug info size in -gmlt by ~17%<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
  - It will not increase the debug info size in -g binary<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></blockquote><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">^ I take it that's true for the initial implementation, but not true shortly after (once discriminators and the newer discriminator feature (duplication factor, etc) are triggered off this flag as well), right? (but the relative impact on -g binaries, even under -gsplit-dwarf, will probably be pretty small in the grand scheme of things (there's still a lot of stuff to clean up in </div><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"> </div><blockquote class="gmail_quote m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  - The compile time change should be negligible<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
- What we use a binary built with out -fdebug-info-for-profiling to collect profile<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
  - The profile will be inaccurate and will yield sub-optimal performance when used to drive SamplePGO<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
- What is the motivating example that you want to compile some sources with -fdebug-info-for-profiling and some without<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">
  - If the debug info size is a concern for -gmlt binary, then one would want to only enable this flag for sources that is performance-critical<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></blockquote><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">To put my 2c in to this & descirbe some of the higher level design decisions going on here:<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">* Generally it's seemed like LTO is made to behave like non-LTO where possible (eg: put attributes on functions or the DICompileUnit so that the effects are respected in the same functions with and without LTO).</div><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">* in cases where that isn't practical with the current implementation (eg: we emit a single line table, so we can't respect things that change the line table format on a per-function basis - the design could be changed to emit more than one line table, but the benefit doesn't seem worthwhile right now) a module flag is used and when modules are merged some rule for resolving the conflict (taking, say, the highest DWARF version) is chosen and possibly warnings are emitted<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">A deeper philosophical point arises as well: there are cases where "LTO behaves like non-LTO" can be supported but maybe /shouldn't/ be. The mental model I have for this situation is something like: If this situation represents a latent bug that could not be diagnosed in a traditional build (such as an ODR violation) then it's acceptable to break or change the behavior of LTO compared to non-LTO to address or depend on these conditions that should be satisfied but sometimes are not. (preferably with warnings, but sometimes not - no effort is made to diagnose debug info type definitions that differ/violate the ODR, one is chosen and the others discarded, for example)<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">So one of the questions here is whether -fdebug-info-for-profiling fits into this "any case of mismatches is really a user mistake that should be addressed, not a case that LLVM needs to respect".<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">After that question is answered - if it's decided that it is best to respect the non-LTO behavior in LTO, then there's a question of how to implement this to respect the perf-function or per-CU behavior.<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">I'd say having it as a flag/bit on the DICompileUnit's probably the way to go - and it can be consulted whenever choices about a particular Function are being made.</div></div></div></div></blockquote><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div></div></div><div style="word-wrap:break-word" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">Don’t we merge DICompileUnit? Like if a.cpp and b.cpp includes the same header: will the DICompileUnit for this header be merged? </div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></span><div class="gmail_msg">There is no DICompileUnit for the header. There's only a DICompileUnit for a.cpp and one for b.cpp.<br class="gmail_msg"><br class="gmail_msg">Say both have an instance of the same inline function.<br class="gmail_msg"><br class="gmail_msg">In a non-LTO build both the CUs would have a description of the function, and both descriptions would point to their respective addresses in the object files. Once linked, they'd both end up pointing to the same code (so you'll have two descriptions of the same function pointing at the same code - heaven forbid you optimized those differently and one of your descriptions says the variable X is in memory and the other says the variable X is in a register... ;) )<br class="gmail_msg"><br class="gmail_msg">In an LTO build the llvm::Function points to the DISubprogram which points to the CU. When merging modules, one of the llvm::Functions is dropped. Once that happens the debug info no longer appears in the CU of the Function that was dropped. So one CU has no description/subprogram/function, the other CU does have one.</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">If a.cc and b.cc both includes c.h, which has foo() defined. If a.cc is built with -g and b.cc is built with -gmlt:</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">* will LTO pick foo built with "-g" or "-gmlt"</div></div></div></div></blockquote><div><br></div><div>I'm not sure of the specific logic that the LTO linker uses, but essentially "arbitrary" (I suspect it's based on the order of the arguments - I think we merge things into the first module, so whichever comes first will keep its inline definition and others will be discarded)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">* will nonLTO linker think this is ODR violation? if not, which one would the linker choose</div></div></div></div></blockquote><div><br></div><div>Non LTO linking won't know the difference between the two (it doesn't look at/understand debug info at all, really) & doesn't detect ODR violations anyway (well, there's that flag... but that's a more complicated thing where it does look at debug info - we've turned that off due to incompatibilities between GCC and Clang for debug info).<br><br>The linker would choose arbitrarily & any debug info for each of the original descirpitons will point to the one chosen by the linker.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Dehao</div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><span class="gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">If so what if the flag mismatch? Do we want it to prevent merging for instance?</div><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">(I have really no idea the consequences, so these are really naive questions right now).</div></div></div><div style="word-wrap:break-word" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><blockquote type="cite" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div dir="ltr" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="gmail_quote m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"> When it comes to the discriimnator features that will be associated with this flag soon maybe that gets a bit trickier - can they be respected once inlining occurs? (eg: a -fdebug-info-for-profiling function inlined into a plain/non-debug-info-for-profiling function, can the bits of the inlined function have all the good stuff but the outer function can still get the normal treatment? Probably? maybe?)<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">If the decision goes the other way (use a module flag) then it's probably fairly simple - the merge choice is probably "if any module has -fdebug-info-for-profiling, the resulting module does".<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">One extra wrinkle: ThinLTO doesn't merge modules, it imports functions from them. Does it import/resolve conflicts in module flags too? If not, it probably should (even for the exsiting module flags, regardless of this new one (whether or not it becomes a module flag or not))<br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div></div></div></div></blockquote><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div></div></div><div style="word-wrap:break-word" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div>We materialize the module flags, and we run the IRLinker, I’m not sure if the logic to resolve conflicts is honored the same way (I hope so).</div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></span><div class="gmail_msg">*fingers crossed* :)</div><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">— </div></div><div style="word-wrap:break-word" class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg">Mehdi</div><div class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"><br class="m_4431236286031313409m_7207517522773178981gmail_msg gmail_msg"></div></div></blockquote></div></div>
</blockquote></div></div></div></blockquote></div></div>