[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Hi @nathanchance, @mgehre, fixed here: https://reviews.llvm.org/D66873, please, look at it to commit. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. In D63325#1645892 , @mgehre wrote: > I'm seeing the same issue `Not all CodeGen sections are inside any Frontend > section!` with python 3.7.1. Json: F9863382: check-time-trace-sections.json >

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

mgehre added a comment. I'm seeing the same issue `Not all CodeGen sections are inside any Frontend section!` with python 3.7.1. Json: F9863382: check-time-trace-sections.json Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

nathanchance added a comment. > Could it be an issue with python? What is the version you are using? I would assume so... $ /usr/bin/python --version Python 3.7.4 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 ___

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. In D63325#1643023 , @nathanchance wrote: > Done, thanks for looking into this! > > F9847579: check-time-trace-sections.json Hmm, I still didn't manage to reproduce the issue using your

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

nathanchance added a comment. Done, thanks for looking into this! F9847579: check-time-trace-sections.json Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Hi @nathanchance , I could not reproduce your failure using the same revision and `build-llvm` script. Could you please attach your json report located at `/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections.js

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

nathanchance added a comment. This test case has never passed for me with `check-clang`, is there something environment related that is needed with this or are others experiencing this too? [147/148] Running the Clang regression tests llvm-lit: /home/nathan/cbl/git/tc-build/llvm-project/ll

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

This revision was automatically updated to reflect the committed changes. Closed by commit rL369308: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks (authored by anton-afanasyev, committed by ). Herald added a subscriber: kristina. Changed prior to commit: https://r

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

russell.gallop accepted this revision. russell.gallop added a comment. This revision is now accepted and ready to land. I don't know a lot about the structure of clang but it LGTM from the point of view of the code and traces coming out. I'm not very keen on having two "Frontend" sections, but I

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Hi @lebedev.ri, could you please lgtm this or elaborate on possible issue with this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 _

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Hi all! Could it be accepted or reviewed more? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 ___ cfe-commits mailing list cfe

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. F9459646: spirit-c5ceba.json F9459650: Снимок экрана от 2019-07-04 18-54-58.png I've upload json-file and printscreen of its visualization (used https://speedscope.app). This is an example o

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev marked an inline comment as done. anton-afanasyev added a comment. Is it ok now? I doubt that main code refactoring is a good way when adding support timer code. So ended with more robust solution, though it leads to two `Frontend` sections. Repository: rG LLVM Github Monorep

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev marked 2 inline comments as done. anton-afanasyev added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:67 // Only include sections longer than TimeTraceGranularity msec. -if (duration_cast(E.Duration).count() > TimeTraceGranularity)

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev updated this revision to Diff 206187. anton-afanasyev added a comment. Changed comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 Files: clang/lib/CodeGen/CodeGenAction.cpp clang/te

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

russell.gallop added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:67 // Only include sections longer than TimeTraceGranularity msec. -if (duration_cast(E.Duration).count() > TimeTraceGranularity) This comment looks wrong since this ch

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. Also, first `"Frontend"` section contains `"ParseTemplate"` and `"PerformPendingInstantiations"` sections. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 ___

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. In D63325#1553458 , @lebedev.ri wrote: > Hm, i started writing previous comment before you posted your last comment, > so i didn't see the last update. > This looks less intrusive, yes, but two observations: > > 1. You no

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

lebedev.ri added a comment. Hm, i started writing previous comment before you posted your last comment, so i didn't see the last update. This looks less intrusive, yes, but two observations: 1. You now have two `"Frontend"` sections - first one being for lexing time 2. That lexing section is not

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

lebedev.ri resigned from this revision. lebedev.ri added a comment. I'm sorry, i should have noticed that earlier, the handling of `"Frontend"` section still looks way too intrusive to me, it breaks abstractions. The only simple solution i see is to 1. Insert a timer for `PrettyStackTraceStrin

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev updated this revision to Diff 205978. anton-afanasyev added a comment. Hi @lebedev.ri, I've turned back to the solution with one `TimeTraceScope` for "Frontend" inside `BackendConsumer::HandleTranslationUnit()`, since it looks more robust. This admits several (two for now) "Front

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev updated this revision to Diff 205673. anton-afanasyev added a comment. Updated, changed test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 Files: clang/lib/CodeGen/CodeGenAction.cpp cl

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev marked an inline comment as done. anton-afanasyev added inline comments. Comment at: clang/lib/Parse/ParseAST.cpp:167 if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get())) return; } @lebedev.ri This `return` should follow a

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added a comment. In D63325#1549316 , @lebedev.ri wrote: > Hmm, i'm only now noticing how fraglie this looks. > How do we know that `clang::ParseAST` will only ever be called with > `BackendConsumer` `ASTConsumer`? > How do we know that `

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

lebedev.ri added a comment. Hmm, i'm only now noticing how fraglie this looks. How do we know that `clang::ParseAST` will only ever be called with `BackendConsumer` `ASTConsumer`? How do we know that `BackendConsumer` will only ever be `ASTConsumer` from `clang::ParseAST`? Repository: rG LLV

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev marked 2 inline comments as done. anton-afanasyev added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252 + // Finish "Frontend" section starting inside clang::ParseAST() + if (llvm::timeTraceProfilerEnabled()) +llvm::timeT

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev updated this revision to Diff 205451. anton-afanasyev marked an inline comment as done. anton-afanasyev added a comment. Updated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 Files: clan

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252 + // Finish "Frontend" section starting inside clang::ParseAST() + if (llvm::timeTraceProfilerEnabled()) +llvm::timeTraceProfil

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev updated this revision to Diff 205446. anton-afanasyev marked 4 inline comments as done. anton-afanasyev added a comment. Updated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 Files: clan

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev marked 10 inline comments as done. anton-afanasyev added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252 + // Finish "Frontend" section starting inside clang::ParseAST() + if (llvm::timeTraceProfilerEnabled()) +llvm::time

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252 + // Finish "Frontend" section starting inside clang::ParseAST() + if (llvm::timeTraceProfilerEnabled()) +llvm::timeTraceProfilerEnd(); I think i'm missing

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:232 { +llvm::TimeTraceScope TimeScope("Frontend", StringRef("")); PrettyStackTraceString CrashInfo("Per-file LLVM IR generation"); lebedev.ri wrote: > T

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev updated this revision to Diff 205297. anton-afanasyev marked 2 inline comments as done. anton-afanasyev added a comment. Updated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 Files: clan

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:232 { +llvm::TimeTraceScope TimeScope("Frontend", StringRef("")); PrettyStackTraceString CrashInfo("Per-file LLVM IR generation"); This looks more like `Fron

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev updated this revision to Diff 204738. anton-afanasyev added a comment. Small fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63325/new/ https://reviews.llvm.org/D63325 Files: clang/lib/CodeGen/CodeGenAction.cpp clang/test/Dri

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

anton-afanasyev created this revision. anton-afanasyev added reviewers: russell.gallop, lebedev.ri, thakis. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. Add `Frontend` time trace entry to `HandleTranslationUnit()` function. Add test to check a