Izaron added a comment.
In D136022#3861245 <https://reviews.llvm.org/D136022#3861245>, @jloser wrote:
> I'd like to see some tests through before I approve.
Thanks for the greenlight!
I added a test that compiles a chunk of code and then checks the time trace
graph in a human-readable form.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1740-1746
+ llvm::TimeTraceScope TimeScope("CheckConstexprFunctionDefinition", [&] {
+ std::string Name;
+ llvm::raw_string_ostream OS(Name);
+ NewFD->getNameForDiagnostic(OS, Context.getPrintingPolicy(),
+ /*Qualified=*/true);
+ return Name;
+ });
----------------
aaron.ballman wrote:
> Huh, I'm a bit surprised that the checking in this function isn't dominated
> by the time spent in `Expr::isPotentialConstantExpr()` -- if you add the time
> tracing to all of the constant expression evaluation functions, do you still
> need a time trace here in Sema?
Thanks! Removed trace here, added to `Expr::isPotentialConstantExpr()`. This
method is called inside of `CheckConstexprFunctionDefinition` and really takes
almost all the time.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:17544
+ {
+ llvm::TimeTraceScope TimeScope("EvaluateAsConstantExpr", [&] {
+ return CE->getSourceRange().printToString(SemaRef.getSourceManager());
----------------
aaron.ballman wrote:
> Why not push this down into `EvaluateAsConstantExpr()`? (Then the name you
> give the scope will also be more accurate.)
Thanks! I wrote on this level because I didn't manage to get
`SemaRef.getSourceManager()` without `SemaRef` (we pass down
`SemaRef.getASTContext()`).
Anyway I found out that we can use `SemaRef.getASTContext().getSourceManager()`
=)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136022/new/
https://reviews.llvm.org/D136022
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
