rsmith added a comment.

In https://reviews.llvm.org/D43578#1067891, @kimgr wrote:

> I disagreed up until the last paragraph :)


Can you say which things you disagree with? There seem to be two important 
facts here:

1. LLVM's timing infrastructure cannot correctly report timings when one 
function containing a timing region calls another. That will happen all the 
time with this patch applied.
2. Clang's architecture means that if you time the amount of time spent in, 
say, the "ParseTemplate" function, you have not computed the amount of time 
spent parsing templates, because the parser calls into Sema, which might 
perform tasks that are only somewhat related to parsing the template (such as 
performing other instantiations), and likewise Sema calls into other layers 
(such as AST file deserialization and code generation).

The first might have been fixed in LLVM at this point, but I can't see any 
evidence of that in LLVM's Timer implementation. And the second seems even more 
fundamental. But if there's some way around that, which would allow us to 
produce (correct!) numbers for times spent parsing / whatever else, without, 
for instance, adding a correctness requirement that we annotate every Parser -> 
Sema entry point with a timer, then that's definitely something we should 
discuss.


https://reviews.llvm.org/D43578



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to