[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] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 160997. avt77 added a comment. The ability to produce debug output for 'ftiming' was added. As result now it's possible to check places where timers start/stop and for what functions it's being done (see changes in Utils.h). https://reviews.llvm.org/D47196

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added a comment. In https://reviews.llvm.org/D47196#1190377, @efriedma wrote: > I mean, which of the callers of startFrontendTimer() is calling it with a > pointer to std::declval()? F6951300: declval.log The attachment keeps debug output for ../

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added a comment. In https://reviews.llvm.org/D47196#1202805, @efriedma wrote: > > :start: means the timer was started > > In some cases, the ChildTime is already non-zero at the "start" point; what > does that mean? It means there was recursive call and this child time should be removed

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 created this revision. avt77 added reviewers: RKSimon, simon.f.whittaker, mgorny, russell.gallop, efriedma, rsmith, mzolotukhin, davezarzycki, vsk. Herald added a subscriber: mgrang. I introduce the new infrastructure supporting recursive timers in Clang. This patch is based on https://rev

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 148226. avt77 added a comment. - switched to llvm::sort - from std::sort (tnx to mgrang) - added a new counter (hope it will improve the output numbers) - slightly changed a couple of existing counters - seriously improved debug/monitor logging https://review

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 148395. avt77 added a comment. The sources were re-based to fit in LLVM_DEBUG rename. One more counter was added. Debug logging was improved again. https://reviews.llvm.org/D47196 Files: include/clang/Frontend/Utils.h lib/CodeGen/BackendUtil.cpp lib/Cod

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added a comment. Hi All, What should I do to simplify the review? I could remove all LLVM_DEBUG related stuff; I could remove all addtional counters and leave only necessary one or two of them. As result the patch will become shorter. Should I do all these truncations to simplify the revi

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added a comment. Ping! efriedma, do you have any other comments/requirements? https://reviews.llvm.org/D47196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added inline comments. Comment at: lib/Sema/SemaLambda.cpp:1447 +getFrontendFunctionTimeCtx()->startFrontendTimer( +{LSI.CallOperator, 0.0}); + } efriedma wrote: > This seems sort of late? You're starting the timer after the body has > alread

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 158221. avt77 added a comment. Herald added a subscriber: eraman. Accordingly to efriedma suggestion I removed start/stopFrontendTimer where it's possible and inserted FrontendTimeRAII in several new places. As result the patch becomes bigger and bigger. And a

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 158542. avt77 added a comment. efriedma, I removed redundant RAII objects but I still have the following: 1.0800 (271) _ZSt7declvalv (*) 1.0840 (273) _ZSt7declvalv (*) 1.0880 (269) _ZSt7declvalv (*) 1.1000 (276) _ZSt7declvalv (*) 1.1200 (282) _ZSt7declvalv

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added a comment. In https://reviews.llvm.org/D47196#1186460, @efriedma wrote: > "0.0040" is four milliseconds? You're probably crediting time incorrectly, > somehow. Can you tell which FrontendTimeRAII the time is coming from? Not sure I understand you. What do you like to know exactly

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

avt77 updated this revision to Diff 133404. avt77 added a comment. I propagated qualifiers accordingly to rjmccall's suggestion. But I changed the diagnostic: now it's more realistic from my point of view. https://reviews.llvm.org/D42530 Files: lib/Sema/SemaExpr.cpp lib/Sema/SemaExprMember

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

avt77 added a comment. Committed revision 324721. BTW, could you review https://reviews.llvm.org/D42728: it's rather similar to this one and rather small as well. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

avt77 closed this revision. avt77 added a comment. Fixed by 324721. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36492: [time-report] Add preprocessor timer

avt77 added inline comments. Comment at: include/clang/Lex/PreprocessorOptions.h:165 public: - PreprocessorOptions() : UsePredefines(true), DetailedRecord(false), + PreprocessorOptions() : PPTimer("preprocessor", "Preprocessing"), + UsePredefines(true)

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 149734. avt77 added a comment. To simplify the review I removed all LLVM_DEBUG items - now the patch is really shorter than it was before. https://reviews.llvm.org/D47196 Files: include/clang/Frontend/Utils.h lib/CodeGen/CodeGenAction.cpp lib/CodeGen/C

[PATCH] D43578: -ftime-report switch support in Clang

avt77 updated this revision to Diff 142198. avt77 added a reviewer: davezarzycki. avt77 added a comment. Herald added a subscriber: mgorny. I removed the dependence on TimePassesIsEnabled (as @davezarzycki sugested) and fixed the issue with failed test (tnx to @russell.gallop). As result the patc

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1065827, @davezarzycki wrote: > It wasn't my suggestion. @thakis wrote: "We probably should have a separate > bool in clang and key this off that and make -ftime-report set both." Sorry, but it's done now. Any comments, suggestions, req

[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 par

[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 a

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1067768, @rsmith wrote: > Last time I looked at doing this, I found that LLVM's timer infrastructure > was fundamentally unsuitable for adding timers like these to Clang. Thank you for this answer. As I understand we should close both t

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1067995, @rsmith wrote: > I think https://reviews.llvm.org/D45619 is a good change, and I'd like to see > that get committed. Could you give me LGTM in this case? I'm going to publish "recursive"(ovelaped) timers but I'd like to base i

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1068417, @rsmith wrote: > I think we need to fix the overlap issue as a prerequisite to adding timers > with large amounts of overlap, especially self-overlap. Otherwise the numbers > will likely do more harm than good, as they will sign

[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 l

[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 cfe-com

[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

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 lib/Frontend/FrontendTiming

[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: https://reviews.llvm.

[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! ///b/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Frontend/ftime-report-template-decl.cpp:155:11

[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

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. Rep

[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

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 https://reviews.llvm.org/D4

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added a comment. In https://reviews.llvm.org/D47196#1134185, @efriedma wrote: > Can you give an example of what the output looks like? First of all excuse me for such delay with my answer: I was on my vacations. During bootstrap we could see something like here: [ 84%] Building CXX objec

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 154802. avt77 added a comment. I fixed all issues raised by efriedma: GlobalDecl(FD), function body, class names, etc. Many tnx for your help. https://reviews.llvm.org/D47196 Files: include/clang/Frontend/Utils.h lib/CodeGen/CodeGenAction.cpp lib/CodeG

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 added a comment. In https://reviews.llvm.org/D47196#1162321, @efriedma wrote: > Adding startFrontendTimer/stopFrontendTimer helps a little, but it's still > difficult to match a given startFrontendTimer to the corresponding > stopFrontendTimer because they're in completely different funct

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

avt77 updated this revision to Diff 156064. avt77 added a comment. I added required comments and did the required changes. https://reviews.llvm.org/D47196 Files: include/clang/Driver/CC1Options.td include/clang/Frontend/CodeGenOptions.h include/clang/Frontend/Utils.h lib/CodeGen/CodeGen

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 added a comment. >> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote: >> >>> I think we're correct not to warn here and that GCC/ICC are being noisy. >>> The existence of a temporary promotion to a wider type doesn't justify >>> warning on arithmetic between two operands that

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 updated this revision to Diff 139271. avt77 added a comment. I removed the dependence on arch and updated the tests. https://reviews.llvm.org/D44559 Files: lib/Sema/SemaChecking.cpp test/Sema/conversion.c test/SemaCXX/conversion.cpp Index: test/SemaCXX/conversion.cpp =

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 added a comment. > That's an interesting question. In general, these warnings do try to ignore > the effects of implicit promotion. We would not want -Wsign-conversion to > fire on `unsigned short x = an_unsigned_short + 1;` (or `- 1`, for that > matter), even though formally this coerc

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 added a comment. In https://reviews.llvm.org/D44559#1045758, @rjmccall wrote: > No, I still oppose this patch. OK, we have 2 possibilities here (fmpov): 1. Forget about the issue and don't do anything now - it is not a bug 2. Return the width based on real analyze of mul args: - signed

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 added a comment. > If that operation overflows, so be it — we're not going to warn about the > potential for overflow every time the user adds two ints, and by the same > token, it doesn't make any sense to warn about every time the user adds two > shorts just because the language made th

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 added a comment. In https://reviews.llvm.org/D44559#1049472, @rjmccall wrote: > You are welcome to start a thread on cfe-dev to gather support for changing > the design of -Wconversion to always warn about the potential for overflow in > sub-int multiplication. Maybe the stupid question

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 added a comment. In fact I think about mul only because of X86 nature: it always put the mul result in wider place (than its args). (Don't know about other CPUs - maybe the same?) As result we could change the current design to reflect this feature: 8bit x 8bit -> 16bit 16bit x 16bit -> 3

[PATCH] D44559: [Sema] Wrong width of result of mul operation

avt77 abandoned this revision. avt77 added a comment. It is not a bug. https://reviews.llvm.org/D44559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. Could you help me with revert of the committed patch? I'll do all necessary changes but I don't know how I can revert the patch. Repository: rL LLVM https://reviews.llvm.org/D43578 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D43578: -ftime-report switch support in Clang

avt77 reopened this revision. avt77 added a comment. This revision is now accepted and ready to land. Obviously, this patch was not ready for commit that's why I reopen the revison. But I don't know how to revert the patch from trunk: please, help me. Repository: rL LLVM https://reviews.llvm.

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1063365, @davezarzycki wrote: > A revert in practice just means undoing the changes. For example, while both > `git` and `svn` have `revert` subcommands, you can also just take the > original diff/patch, pipe it into `patch -R`, and if t

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1063462, @thakis wrote: > Reverted: Many thanks for your help! Repository: rL LLVM https://reviews.llvm.org/D43578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1062950, @thakis wrote: > @davezarzycki remarks in https://reviews.llvm.org/D45485 that this breaks the > shared build. The proposed fix there is to make several of clang's modules > depend on LLVM's IR library ("Core"). This seems weird

[PATCH] D43578: -ftime-report switch support in Clang

avt77 added a comment. In https://reviews.llvm.org/D43578#1064113, @russell.gallop wrote: > We also see an assertion failure prior to the revert. At r329738: Yes, you're right - thaks again for this test case. Repository: rL LLVM https://reviews.llvm.org/D43578 _

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

avt77 added inline comments. Comment at: lib/AST/ExprClassification.cpp:652 +if (Ctx.getCanonicalType(ASE->getBase()->getType()).isConstQualified()) + return Cl::CM_ConstQualified; + rjmccall wrote: > Is there a reason why the places that compute the typ

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

avt77 added a comment. In fact we have here another problem: it is not an attempt to assign to const variable but it is an attempt to use the field assingment inside const method: I'm working on it. https://reviews.llvm.org/D42530 ___ cfe-commits

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

avt77 added a comment. In https://reviews.llvm.org/D42530#995227, @rjmccall wrote: > That's still just const-propagation. The const qualifier on the pointee type > of this should propagate to the l-value resulting from the member access, and > the vector-specific projection logic should contin

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

avt77 updated this revision to Diff 132607. avt77 added a comment. I re-implemented the patch. Now it works properly with const methods in CXX classes. https://reviews.llvm.org/D42530 Files: lib/AST/ExprClassification.cpp lib/Sema/SemaExpr.cpp test/Sema/assign.c test/Sema/typedef-retai