<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 24, 2021 at 2:27 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Fri, Sep 24, 2021 at 2:13 PM Arthur Eubanks <<a href="mailto:aeubanks@google.com" target="_blank">aeubanks@google.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Adding a map of mangled names to source locations (only those emitted in the IR since those are the only ones the backend can see) doesn't noticeably impact <a href="https://llvm-compile-time-tracker.com/compare.php?from=ac51ad24a75c02152f8ece943d65de9a1c4e947a&to=fd8cfa1e33bfce6421b3a088b859a240b056be5a&stat=instructions" target="_blank">compile time</a>, but does noticeably increase <a href="https://llvm-compile-time-tracker.com/compare.php?from=ac51ad24a75c02152f8ece943d65de9a1c4e947a&to=fd8cfa1e33bfce6421b3a088b859a240b056be5a&stat=max-rss" target="_blank">memory usage</a>. Of course, we'll win that back with clearing the Clang AST.</div></div></blockquote><div><br></div><div>I guess most of that is from the string data itself? Any chance that could be shared - by referencing (using StringRef) string data in the llvm::Module or otherwise? (bit more nuanced than that because presumably it'd be a problem if the string data were to go away - if a function is optimized out of an llvm::Module, etc - even if that meant the string value would never be queried for in the map anyway, it'd still violate some map invariants/etc)</div></div></div></blockquote><div>Yeah, reusing strings could have issues. However, if we use a hash of the strings as the keys then for the most part we the <a href="https://llvm-compile-time-tracker.com/compare.php?from=a2a07e8db3bf64440f24d9d6408df214886826de&to=5ca119a7c76cf38b41bd7b6d2830d130673369fc&stat=max-rss" target="_blank">memory usage</a> numbers look better.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>I found the recently implemented "dontcall" attribute which relies on the Clang AST. I have a draft patch to make it not rely on the Clang AST: <a href="https://reviews.llvm.org/D110364" target="_blank">https://reviews.llvm.org/D110364</a>.<br></div><div>But that and the changes in <a href="https://reviews.llvm.org/D109781" target="_blank">https://reviews.llvm.org/D109781</a> highlight an issue in that we don't only use the Clang AST to find source locations, we might also use it for other things.</div></div></blockquote><div><br></div><div>All the more reason to make the AST deleting mode the default (maybe the only mode, if possible)/usable everywhere - to avoid building new features that rely on the AST. (generally this makes situations like LTO better anyway - since they also break the idea that the AST is available during transformations)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>For example, we have custom naming of functions/lambdas/etc when printing out their name in a diagnostic with the Clang AST node. I've worked around this by calling LLVM's demangler, but it doesn't produce the same naming.<br></div><div>And for the "dontcall" attribute, the initial implementation looked into the Clang AST to determine whether to emit a warning or an error. I've worked around this by splitting it into two attributes, one for warning and one for error.</div><div>But overall, if we go through with this, we'll end up not having access to the Clang AST for future backend diagnostics. That's a tradeoff we'll have to decide on. Forcing this does make backend diagnostics more likely to be consistent when they don't have a Clang AST (e.g. ThinLTO post-link compiles, IR as input to Clang).</div></div></blockquote><div><br></div><div>yep!</div></div></div></blockquote><div>The tradeoff is a real tradeoff though, we may want to use the Clang AST to provide more specific diagnostics when we have access to the Clang AST/sources. I'm waiting for somebody to object, but if nobody objects then perhaps we're good.</div></div></div>