[PATCH] Template Instantiation Observer + a few other templight-related changes

[Sean Silva]

On Thu, Jan 8, 2015 at 11:03 AM, Sven Mikael Persson <
mikael.s.persson at gmail.com> wrote:

> Thanks for the feedback Nick!
>
> I fixed all the minor cosmetic issues you pointed out, and I also fixed a
> few "more than 80 char" lines in the original diff.
>
> I changed the name "Observer" to "Callbacks" as requested in the mailing
> list discussion:
>
> http://clang-developers.42468.n3.nabble.com/Templight-Templight-quot-v2-quot-with-gdb-style-debugger-td4038413.html
>
> Here the new diff (how do I replace the original diff with this one in
> this llvm-reviews system? I'm new to this system).
>
> F330648: templight_clang_patch.diff <http://reviews.llvm.org/F330648>
>
> To your comments:
>
> > I think we'll want a way to test out this interface; it should probably
> come with a command-line tool that has a very simple template observer that
> prints out the callbacks as they're called, and a test which runs this tool
> to exercise it. This might also be a FrontendAction in clang, similar to
> -ast-dump, instead of a separate tool. (This is not a trivial amount of
> work, you may want to wait for another clang developer to chime in before
> spending time on it.)
>
>
> That is essentially what I have done for the templight tool (
> https://github.com/mikael-s-persson/templight). I created a new
> FrontendAction that creates the templight tracer / debugger as a
> TemplateInstCallbacks. The templight tracer is essentially what you
> describe, it dumps a trace of all the callbacks (plus a bit of optional
> filtering and gathering / pretty-formatting of the information). If you
> want me to distil it down to a more "raw" tracer that only dumps everything
> to stdout, then that can easily be done.
>
> But there is one hick-up. If the idea is to use this as a kind of
> unit-test for the tool, that is, to verify that the trace of callbacks are
> "correct" (as in, later changes to clang should preserve the same trace),
> then I think that is a great idea in principle. However, I see two problems
> with that.
>
> First, it's hard to be sure that the current traces are "correct", as in,
> maybe this patch has some loop-holes that miss some instantiations, or
> maybe clang is currently incorrect in some ways that might potentially be
> fixed in the future (at least, I have seen several "FIXME" notes all over
> the template instantiation code of the Sema and Parser classes).
>
> Second, according to my tests so far, the traces produced by clang are
> **non-deterministic**. Different runs on the same source code will produce
> different trace files. They only differ in inconsequential ways (different
> ordering of nested instantiations and memoizations). But the point is that
> a simple comparison of the trace file with some reference trace file would
> most certainly always fail, unless the test is really trivial.
>

I'm wondering where this non-determinism comes from. I would certain
ephemeral computations inside clang to proceed non-deterministically (but
are supposed to have a deterministic final result) due to ASLR and the
pervasive use of pointers as keys. If you turn off ASLR, does the
non-determinism go away?

-- Sean Silva


>
> Therefore, the notion of what makes a trace "correct" for a given source
> file is not well-defined, and might never be. But if such a notion could be
> defined (for example, from the C++ standard), this could end up as a great
> way to evaluate the correctness of Clang (e.g., "does clang instantiate
> templates in a standard-compliant way?").
>
> But this is far beyond the scope of this patch.
>
> > Alphabetize.
>
>
> Fixed in new patch.
>
> > What about a callback when clang transitions from parsing the TU to
> actOnEndOfTranslationUnit? Template instantiation is different before and
> after we reach the end-of-TU.
>
>
> Thanks for pointing that out. I'm not sure what impact this really has.
> The initialize / finalize pair of callbacks bracket the entire TU parsing,
> as far as I know. Therefore, all instantiations are captured by the trace,
> again, AFAIK. After this comment, I know that there are some late implicit
> instantiations being instantiated when EOF is reached, basically, right?
> AFAIK, these are still captured by the callbacks.
>
> I don't know if there is any meaning to a callback at this point. If I
> were debugging Clang, I might see some value in being able to distinguish
> the instantiations happening before and after this point. But from the
> perspective of profiling the template instantiations, I don't really need
> to know this. In any case, I think that this can be the subject of a future
> patch if a good concrete use-case / motivation is brought forward. And this
> also raises the question (maybe more appropriate for cfe-dev mailing list)
> of whether Clang developers would be interested in using this
> TemplateInstCallbacks as a means to evaluate / debug the clang compiler
> itself, which might motivate the addition of more callbacks in it.
>
> > Is there a guarantee that this is a template? Offhand it looks like this
> could fire on any struct, couldn't it?
>
>
> You are correct in that this callback (for memoizations) picks up a lot of
> things that are not templates. I am very well aware of this issue and it
> has been pointed out before, but there is really not a lot I can do to
> change this. Firstly, memoizations are really important (which I am just
> beginning to realize after making plans for a call-graph like visualization
> of the instantiations). Secondly, much of the clang template instantiation
> code is structured as a "check if instantiated already, if not, instantiate
> it", which means that there are many many places in clang where some kind
> of "check if instantiated already" logic exists and it varies greatly from
> context to context. This makes it very hard to insert the memoization
> callbacks in proper places. So, as far as I know, this particular
> placements is not too bad because it picks up most, if not all, checks, and
> generates noise that can be dealt with (I don't do it currently in
> templight, but I plan to add that soon, an!
>  d this is logic that could go into the TemplateInstCallbacks if needed),
> i.e., it's easier to filter out spurious callbacks than to miss some
> important ones.
>
> But at the end of the day, the only way that this can be fixed is for
> someone from the Clang development team who is very much familiar with the
> complete template instantiation code to provide his/her hands-on assistance
> for this. I am simply not in a position to be able to figure out how to
> appropriately trace the memoizations reliably and without too much noise.
>
>
> http://reviews.llvm.org/D5767
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150108/162f3691/attachment.html>