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

Fri Oct 21 10:56:03 PDT 2022

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! Please be sure to add a release note to alert folks to the time profiler becoming more awesome. :-)

================
Comment at: clang/unittests/Support/TimeProfilerTest.cpp:197-198
+
+  // NOTE: If this test is failing, run this test with
+  // `llvm::errs() << TraceGraph;` and change the assert above.
+}
----------------
Izaron wrote:
> aaron.ballman wrote:
> > This bit worries me because I suspect we'll get pretty wide variation between test bots in the build lab. Do you have an idea of how likely it is that this test will have different behavior depending on the machine?
> I'd say, the test's outcome varies if behaviour of `CompilerInstance` varies. From my (imperfect) understanding of Clang/LLVM's architecture, its interface is pretty hermetic and we won't get inconsistent behaviour depending on the current machine as soon as we ran `Compiler.ExecuteAction(Action);`.
> Since the code is machine-independents, I expect the same behaviour everywhere.
> 
> If we look on other tests, there are tests that seemingly take the machine's target triple https://github.com/llvm/llvm-project/blob/3fee9358baab54e4ed646a106297e7fb6f1b4cff/clang/unittests/CodeGen/TestCompiler.h#L40-L48 to fill some info in `CompilerInstance`.
> 
> But this is not the case in our test. Seems like the AST parsing is machine-independent 😃 
Excellent, thank you for the explanation!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136022