[cfe-dev] [Templight] Templight "v2" with gdb-style debugger

[Mikael Persson]

Hi Nick,

Glad to finally get a sign of life from the clang dev team!

> The llvm project requires that patches be submitted by their author (or
authors). If you grabbed the original templight code and refactored it,
then it may be coauthored between you and the original authors. Our policy
requires that an author submit their own code, not mail in someone else's

The patches from this email are very out-dated (and I was completely
unfamiliar with the process of patch submissions... well, I still am to be
honest ;) ). I submitted a much smaller patch after successfully managing
to extract all the templight code out of the clang source, to make it a
separate tool instead, leaving only very minor "hooks" to be added to Clang
itself. That patch can be found at:

http://reviews.llvm.org/D5767

As for the authorship, there are a few things to notice.

First of all, in the new patch that I submitted, all of the code is my own
code, because all of the original templight code has been essentially
factored out from the original patch, i.e., the "real" templight code now
sits in a separate project at:

https://github.com/mikael-s-persson/templight

There is also a more up-to-date clang patch on there, which I will
re-submit eventually to Clang when there is more sign of life from the
community (I don't like wasting my valuable time throwing patch-updates
into a bottomless pit).

The original patch also lacked any specific authorship notice and
licensing. But I'm more than happy to add any co-author that is needed to
satisfy your submission requirements. All I want is the patch to go
through, and I think all other interested parties and original authors feel
the same. And after all, we are only talking about of few dozen trivial
lines of code here.

And for the record, I'm a bit annoyed by the "mail in someone else's code",
as it kind of belittles the massive refactoring work I did on the original
patch, and the fact that the majority of what it is now is my own code.

> I think that ActiveTemplateInstantiation should be changed to a form of
InstantiationRecord and held from the relevant AST nodes.

Maybe so, I don't know, and frankly I don't care. Well, I do care for
having good template instantiation code in Clang, but I cannot afford to
care too much about design decisions that are not mine to take.

> The changes to ATI to pull it out of Sema and make it part of the AST are
complicated enough, I think you should send that out as a first patch.

I think there is a confusion here. I never pulled the ATI into the AST, I
only pulled it out of the Sema class. It has literally just gone from being
a nested class within Sema to being a class at global scope (clang
namespace, of course!), that's all. There is no change whatsoever in the
code except for a change in scope and header-file placement (to avoid
including the Sema header, and just because nested classes are a bad idea
(I come from Boost library development, the design guidelines are more
strict over there)).

If you want to work on making ATIs a part of the AST, you're welcome to try
/ propose it. It is none of my concern, and in fact I would object to it
(see below).

> There's also template instantiation for different reasons which are not
currently recorded (ie., due to overload resolution, due to virtual
methods). Since that's a change to ATI, that would logically go next.

You're right, the categories of template instantiations are too broad. That
is something I would be interested in discussing and intend to bring up
after my patch has gone through. The cases of overload resolution and
virtual methods are not exactly the first ones that come to mind for me
though. The additional categories I was going to propose are for
differentiating class template instantiations, function declaration
instantiations and function definition instantiations. The lack of
categorization of those things currently cause templight traces to have
ambiguous entries, which I would like to avoid.

> I think we usually use "Callbacks" instead of "Observer", similar to
PPCallbacks?

Fine. When we get closer to getting this (D5767) patch through, that can
easily be changed, no problem.

> At a high level, I think that what we currently do in Sema by tracking
template instantiations as a stack should be available in the AST after
instantiation is complete.

I'm not sure if this makes sense in terms of the AST, after all, the stack
of template instantiations are compilation steps, not actual entities in
the code. My understanding is that the AST is only there to record all the
entities in the code. I see this as a case of semantic pollution. What
other records of compilation steps are recorded in the AST?

Also, if the AST is to keep a record of all the template instantiations
that occurred, this could significantly increase memory consumption (and
thus, speed as well). It is a good thing that the compiler throws away this
information because it would and could amount to a lot if it kept it all (I
would say approximately 5-10% increase in memory consumption, from my
experience). This would be a lot of passive overhead (and therefore, would
have to be disabled by default, of course). I certainly would not want such
overhead by default in any compiler that I use (and I'm not sure that
people who use libclang would want this either).

> Then we could query the AST to ask why something was instantiated instead
of tracing the act of instantiation. Is there any use case for templight
where this wouldn't work?

Yes, pretty much all of templight as a profiler would not work that way,
unless the AST also held records of time- and memory- consumption during
those instantiation steps. The AST is useful to the templight debugger and
is also used in conjunction with the templight traces by the metashell
utilities (that Zoltan linked to). The AST describes the code, the
templight traces describe the compilation process, they are entirely
orthogonal features (yet complementary), as far as I'm concerned.

To me, it is important to stress that, although the debugging and
interactive querying of the AST is a neat feature, the real
"production-critical" feature here is the profiling. This is my primary
interest, and those of my peers, i.e., the people who write production
template-heavy code that need to optimize the compilation time and memory.
Focusing too much on the debugging and AST querying aspects could impede
that primary goal, as far as I'm concerned.

And as Zoltan mentioned, there could be issues with the safe-mode and
dealing with failures to instantiated the templates if we are going to rely
on "successful" AST nodes to describe the compilation process.

There are basically three use-case categories for templight (from most
"novice" to most "professional"): learning, debugging and profiling.
Learning and debugging imply that things might be failing a lot, and
profiling imply that there is no need to understand the process but a need
to identify performance bottlenecks. Your approach does not serve any of
these domains particularly well.


Thanks,
Mikael.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150106/b8fcf188/attachment.html>