[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma added a comment. Somewhere around CompilerInvocation::CreateFromArgs is probably appropriate. Repository: rL LLVM https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added a comment. In https://reviews.llvm.org/D45619#1192803, @craig.topper wrote: > I'm unclear why the we would want to assign clang's FrontendTimesIsEnabled > from inside CodeGenAction. If I'm understanding the intentions here, the goal > was to add more timing infrastructure to clang.

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

craig.topper added a comment. Assignment restored in r339281. I'll file a bug to merge to 7.0 Repository: rL LLVM https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

craig.topper added a comment. Ok I'll add that back. I'm unclear why the we would want to assign clang's FrontendTimesIsEnabled from inside CodeGenAction. If I'm understanding the intentions here, the goal was to add more timing infrastructure to clang. But if the enabling is tied to

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma added a comment. Oh, oops, I should have spotted that. No, the point of this was to separate the timing infrastructure, not to change the behavior of -ftime-report. Should be a one-line change to add "llvm::TimePassesIsEnabled = TimePasses" back to BackendConsumer::BackendConsumer.

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

craig.topper added a comment. Correct me if I'm wrong, but after this change llvm no longer enables the timing of individual passes when -ftime-report is passed? Was that intentional? Repository: rL LLVM https://reviews.llvm.org/D45619 ___

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

RKSimon closed this revision. RKSimon added a comment. This was committed at https://reviews.llvm.org/rL330571 Repository: rL LLVM https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added a comment. In https://reviews.llvm.org/D45619#1075677, @bjope wrote: > buildbots have been failing all day (and were still failing). So I pushed my > suggested fix. > I hope that was OK. Thank you. It seems everything is OK now. Repository: rL LLVM

Re: [PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

On Mon, Apr 23, 2018 at 11:53 AM, Andrew V. Tischenko via Phabricator via cfe-commits wrote: > avt77 added a comment. > > In https://reviews.llvm.org/D45619#1075437, @bjope wrote: > > > I can't see that it has been reverted. > > But I guess that the table maybe is

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

bjope added a comment. buildbots have been failing all day (and were still failing). So I pushed my suggested fix. I hope that was OK. Repository: rL LLVM https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added a comment. In https://reviews.llvm.org/D45619#1075437, @bjope wrote: > I can't see that it has been reverted. > But I guess that the table maybe is sorted based on time spent in each pass? > So that is why it might be sorted differently on different buildbots (or when > using pipe

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

bjope added a comment. I can't see that it has been reverted. But I guess that the table maybe is sorted based on time spent in each pass? So that is why it might be sorted differently on different buildbots (or when using pipe etc). So I think a quick fix is to add -DAG to the checks that can

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added a comment. In https://reviews.llvm.org/D45619#1075385, @bjope wrote: > Anyway, if the order isn't deteministic, then a solution could be to use > CHECK-DAG instead of CHECK for the checks that may be reordered. For example: Thank you very much! I'll try to fix it in this way.

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added a comment. In https://reviews.llvm.org/D45619#1075260, @thakis wrote: > In any case, when you see a test failing on bots and the fix isn't obvious, > revert first to get the bots back green. I reverted it immediatly. Repository: rL LLVM https://reviews.llvm.org/D45619

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

bjope added a comment. In https://reviews.llvm.org/D45619#1075089, @avt77 wrote: > It's terrible but my new test was failed again as result of commit of this > patch! > > ///b/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Frontend/ftime-report-template-decl.cpp:155:11: > error:

Re: [PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

In any case, when you see a test failing on bots and the fix isn't obvious, revert first to get the bots back green. On Mon, Apr 23, 2018 at 8:54 AM, Andrew V. Tischenko via Phabricator via cfe-commits wrote: > avt77 reopened this revision. > avt77 added a comment. >

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 reopened this revision. avt77 added a comment. This revision is now accepted and ready to land. It's terrible but my new test was failed again as result of commit of this patch!

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

This revision was automatically updated to reflect the committed changes. Closed by commit rL330571: Use special new Clang flag FrontendTimesIsEnabled instead of llvm… (authored by avt77, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 updated this revision to Diff 143311. avt77 added a comment. I fixed issues raised by efriedma. https://reviews.llvm.org/D45619 Files: include/clang/Frontend/Utils.h lib/CodeGen/BackendUtil.cpp lib/CodeGen/CodeGenAction.cpp lib/Frontend/CMakeLists.txt

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added inline comments. Comment at: test/Frontend/ftime-report-template-decl.cpp:2 +// RUN: %clang %s -S -o - -ftime-report 2>&1 | FileCheck %s +// RUN: %clang %s -S -o - -fdelayed-template-parsing -DDELAYED_TEMPLATE_PARSING -ftime-report 2>&1 | FileCheck %s +

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma added inline comments. Comment at: lib/Frontend/FrontendTiming.cpp:14 + +#include "llvm/Support/Timer.h" + This should include clang/Frontend/Utils.h . Comment at: test/Frontend/ftime-report-template-decl.cpp:2 +// RUN: %clang %s -S

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added a comment. Hi All, Are there other issues related to this patch? If NO could anyone give me LGTM? I need this patch committed to continue with recursive time counters. https://reviews.llvm.org/D45619 ___ cfe-commits mailing list

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 updated this revision to Diff 142755. avt77 added a comment. I moved FrontendTiming.cpp from lib/Basic/ to lib/Frontend/. https://reviews.llvm.org/D45619 Files: include/clang/Frontend/Utils.h lib/CodeGen/BackendUtil.cpp lib/CodeGen/CodeGenAction.cpp lib/Frontend/CMakeLists.txt

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma added inline comments. Comment at: lib/Basic/FrontendTiming.cpp:18 + +bool FrontendTimesIsEnabled = false; + avt77 wrote: > efriedma wrote: > > Why is this in lib/Basic, when the declaration is in > > include/clang/Frontend/? > Because this library is

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 added inline comments. Comment at: lib/Basic/FrontendTiming.cpp:18 + +bool FrontendTimesIsEnabled = false; + efriedma wrote: > Why is this in lib/Basic, when the declaration is in include/clang/Frontend/? Because this library is being linked to all others

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma added a comment. This makes sense. Comment at: include/clang/Frontend/Utils.h:241 +/// then the value of this boolean will be true, otherwise false. +extern bool FrontendTimesIsEnabled; + Don't really like global variables, but I guess timers are

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

avt77 created this revision. avt77 added reviewers: mgorny, russell.gallop, efriedma, rsmith, thakis, davezarzycki, RKSimon, simon.f.whittaker. To simplify the review and commit of D43578 I decided to spilt it in several small parts. This patch is the first