[PATCH] D136022: [clang] Add time profile for constant evaluation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this, having more detailed timing information for where we're spending time in the frontend will really help us to evaluate where we need to work on frontend performance.



================
Comment at: clang/lib/AST/ExprConstant.cpp:15257
+  });
+
   // FIXME: Evaluating initializers for large array and record types can cause
----------------
tbaeder wrote:
> what about `Expr::EvaluateASRvalue` and `Expr::isPotentialConstantExpression`?
Agreed; I think we should cover all the `Expr::Evaluate*` functions as well as related potentially expensive constant expression functions like `Expr::isPotentialConstantExpression` and `Expr::isIntegerConstantExpr`.


================
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;
+  });
----------------
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?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:17544
+  {
+    llvm::TimeTraceScope TimeScope("EvaluateAsConstantExpr", [&] {
+      return CE->getSourceRange().printToString(SemaRef.getSourceManager());
----------------
Why not push this down into `EvaluateAsConstantExpr()`? (Then the name you give the scope will also be more accurate.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136022/new/

https://reviews.llvm.org/D136022