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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
