Re: [PATCH] D20880: [Coverage] Push a region and propagate counts through try blocks

2016-06-08 Thread Vedant Kumar via cfe-commits
vsk added a comment. Ping. http://reviews.llvm.org/D20880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-06-08 Thread Vedant Kumar via cfe-commits
vsk added a comment. Ping, any updates on this patch? http://reviews.llvm.org/D20748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r272067 - [docs] Coverage: Explain how to avoid static initializers

2016-06-07 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Tue Jun 7 17:25:29 2016 New Revision: 272067 URL: http://llvm.org/viewvc/llvm-project?rev=272067=rev Log: [docs] Coverage: Explain how to avoid static initializers Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst Modified:

Re: [PATCH] D20878: [Coverage] Do not push a new region after a CXXTryStmt

2016-06-06 Thread Vedant Kumar via cfe-commits
vsk abandoned this revision. vsk added a comment. I see your point that it isn't expensive to do a best-effort job here. I updated docs/SourceBasedCodeCoverage.rst with a limitations section as per my earlier comment. http://reviews.llvm.org/D20878

Re: [PATCH] D20997: [Coverage] Fix an assertion failure if the definition of an unused function spans multiple files.

2016-06-06 Thread Vedant Kumar via cfe-commits
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM. http://reviews.llvm.org/D20997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20997: [Coverage] Fix an assertion failure if the definition of an unused function spans multiple files.

2016-06-06 Thread Vedant Kumar via cfe-commits
vsk added a comment. Thanks, this looks good overall. I just have a few minor comments. Comment at: lib/CodeGen/CoverageMappingGen.cpp:329 @@ +328,3 @@ + FileID ParentFile = SM.getFileID(Start); + while (ParentFile != SM.getFileID(End) && !isNestedIn(End, ParentFile))

r271902 - [docs] Clarify limitations section of SourceBasedCodeCoverage.rst

2016-06-06 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Jun 6 10:44:40 2016 New Revision: 271902 URL: http://llvm.org/viewvc/llvm-project?rev=271902=rev Log: [docs] Clarify limitations section of SourceBasedCodeCoverage.rst Mention that the code coverage tool becomes less precise whenever unpredictable changes in control

Re: r271544 - [docs] Add a limitations section to SourceBasedCodeCoverage.rst

2016-06-06 Thread Vedant Kumar via cfe-commits
> On Jun 4, 2016, at 6:28 PM, Sean Silva <chisophu...@gmail.com> wrote: > > > > On Thu, Jun 2, 2016 at 10:19 AM, Vedant Kumar via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > Author: vedantk > Date: Thu Jun 2 12:19:45 2016 > New Revision: 27154

r271544 - [docs] Add a limitations section to SourceBasedCodeCoverage.rst

2016-06-02 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Thu Jun 2 12:19:45 2016 New Revision: 271544 URL: http://llvm.org/viewvc/llvm-project?rev=271544=rev Log: [docs] Add a limitations section to SourceBasedCodeCoverage.rst Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst Modified:

Re: [PATCH] D20878: [Coverage] Do not push a new region after a CXXTryStmt

2016-06-02 Thread Vedant Kumar via cfe-commits
vsk added a comment. In http://reviews.llvm.org/D20878#446690, @ikudrin wrote: > Is there a case where this patch makes things better than they were before? > Is it possible to improve handling of exceptions instead? This patch removes something which doesn't appear to serve a useful purpose.

r271472 - [docs] Fix misplaced comma

2016-06-01 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed Jun 1 21:45:59 2016 New Revision: 271472 URL: http://llvm.org/viewvc/llvm-project?rev=271472=rev Log: [docs] Fix misplaced comma Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst URL:

r271471 - [docs] Minor formatting changes and typo fixes

2016-06-01 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed Jun 1 21:25:13 2016 New Revision: 271471 URL: http://llvm.org/viewvc/llvm-project?rev=271471=rev Log: [docs] Minor formatting changes and typo fixes Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst URL:

r271461 - [docs] Use cpp code-blocks where appropriate

2016-06-01 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed Jun 1 20:15:59 2016 New Revision: 271461 URL: http://llvm.org/viewvc/llvm-project?rev=271461=rev Log: [docs] Use cpp code-blocks where appropriate Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst URL:

r271457 - [docs] Add missing newline to console section

2016-06-01 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed Jun 1 20:01:48 2016 New Revision: 271457 URL: http://llvm.org/viewvc/llvm-project?rev=271457=rev Log: [docs] Add missing newline to console section Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst URL:

r271454 - [docs] Document the source-based code coverage feature

2016-06-01 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed Jun 1 19:51:50 2016 New Revision: 271454 URL: http://llvm.org/viewvc/llvm-project?rev=271454=rev Log: [docs] Document the source-based code coverage feature Differential Revision: http://reviews.llvm.org/D20715 Added: cfe/trunk/docs/SourceBasedCodeCoverage.rst

Re: [PATCH] D20715: [docs] Document the source-based code coverage feature

2016-06-01 Thread Vedant Kumar via cfe-commits
> On Jun 1, 2016, at 11:30 AM, Justin Bogner wrote: > > Vedant Kumar writes: >> vsk created this revision. >> vsk added a reviewer: bogner. >> vsk added subscribers: kcc, cfe-commits, silvas. >> >> It would be helpful to have a user-friendly guide for

[PATCH] D20880: [Coverage] Push a region and propagate counts through try blocks

2016-06-01 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added reviewers: ikudrin, bogner, MaggieYi, phillip.power. vsk added a subscriber: cfe-commits. This lets us handle expansions in the try block properly. Here's how the final report changes: ``` Before: | 16|// CHECK: Z3fn3v: 1| 17|void fn3() TRY

[PATCH] D20878: [Coverage] Do not push a new region after a CXXTryStmt

2016-06-01 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added reviewers: bogner, MaggieYi, phillip.power. vsk added a subscriber: cfe-commits. There are three possible scenarios when entering a CXXTryStmt: 1. No exceptions thrown. 2. An exception is thrown but caught by one of the handlers. 3. An exception is

r271331 - [Coverage] Remove redundant handleFileExit() call (NFC)

2016-05-31 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Tue May 31 15:35:12 2016 New Revision: 271331 URL: http://llvm.org/viewvc/llvm-project?rev=271331=rev Log: [Coverage] Remove redundant handleFileExit() call (NFC) I added this call in r271308. It's redundant because it's dominated by a call to extendRegion(). Thanks to

r271308 - [Coverage] Fix crash on a switch partially covered by a macro (PR27948)

2016-05-31 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Tue May 31 13:06:19 2016 New Revision: 271308 URL: http://llvm.org/viewvc/llvm-project?rev=271308=rev Log: [Coverage] Fix crash on a switch partially covered by a macro (PR27948) We have to handle file exits before and after visiting regions in the switch body. Fixes

Re: [PATCH] D20715: [docs] Document the source-based code coverage feature

2016-05-31 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 59078. vsk marked an inline comment as done. vsk added a comment. - Actually link in the new document into Index.rst. http://reviews.llvm.org/D20715 Files: docs/SourceBasedCodeCoverage.rst docs/index.rst Index: docs/index.rst

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-05-27 Thread Vedant Kumar via cfe-commits
> On May 27, 2016, at 2:55 PM, Rafael Espíndola > wrote: > > On 27 May 2016 at 17:32, Vedant Kumar wrote: >> vsk added a comment. >> >> Comments on this patch -- The increment and decrement snippets seem like >> they could be pulled into helper

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-05-27 Thread Vedant Kumar via cfe-commits
vsk added a comment. Comments on this patch -- The increment and decrement snippets seem like they could be pulled into helper methods. That should make it easier to update all users of LLVMIRGeneration. I wasn't able to come up with a regression test, but I do think this patch needs one.

Re: [PATCH] D20715: [docs] Document the source-based code coverage feature

2016-05-27 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 58810. vsk marked 4 inline comments as done. vsk added a comment. Thanks for the feedback! - Addressed Sean's review comments. - Fixed the line count displayed in the summary view of `foo`. http://reviews.llvm.org/D20715 Files:

[PATCH] D20715: [docs] Document the source-based code coverage feature

2016-05-26 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added a reviewer: bogner. vsk added subscribers: kcc, cfe-commits, silvas. It would be helpful to have a user-friendly guide for code coverage. There is some overlap with [1], but this document visits issues which may affect users in more depth. Prompted by:

r270160 - [Lexer] Don't merge macro args from different macro files

2016-05-19 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Thu May 19 18:44:02 2016 New Revision: 270160 URL: http://llvm.org/viewvc/llvm-project?rev=270160=rev Log: [Lexer] Don't merge macro args from different macro files The lexer sets the end location of macro arguments incorrectly *if*, while merging consecutive args to fit

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

2016-05-19 Thread Vedant Kumar via cfe-commits
Yes, here are the results from the unpatched compiler: Avg. wall time (s): 0.73241 Std. deviation: 0.05850 And here are the results from the patched compiler: Avg. wall time (s): 0.75554 Std. deviation: 0.07492 The testing methodology was the same (100 trials), except I used `clang -x

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

2016-05-19 Thread Vedant Kumar via cfe-commits
vsk added a comment. I discussed this bug with Argyrios off-list, who lgtm'd on the condition that it doesn't introduce a performance regression. He suggested preprocessing Cocoa.h to stress the patch. After running a stabilization script, I used this command to stress RelNoAsserts builds of

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

2016-05-19 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 57810. vsk added a comment. - Fix explanation of the test case in test/CoverageMapping. http://reviews.llvm.org/D20401 Files: lib/Lex/TokenLexer.cpp test/CoverageMapping/Inputs/macros.h test/CoverageMapping/include-macros.c unittests/Lex/LexerTest.cpp

r270021 - Reapply^3 "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC"

2016-05-18 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed May 18 22:54:54 2016 New Revision: 270021 URL: http://llvm.org/viewvc/llvm-project?rev=270021=rev Log: Reapply^3 "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC" Sync up with "(llvm) Use Error in InstrProf and Coverage". Modified:

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

2016-05-18 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 57711. vsk added a comment. - Add some comments to the unit test. http://reviews.llvm.org/D20401 Files: lib/Lex/TokenLexer.cpp test/CoverageMapping/Inputs/macros.h test/CoverageMapping/include-macros.c unittests/Lex/LexerTest.cpp Index:

Re: Patch submission for bug 27400

2016-05-17 Thread Vedant Kumar via cfe-commits
> On May 17, 2016, at 2:59 AM, Mads Ravn wrote: > > Hi guys, > > I just wanted to check up on this patch. I heard I could just reply to this > mail and see if I could 'ping' anyone in this regard. Hope it's OK. Sorry for the delay! This looks good. Committed as r269786.

[clang-tools-extra] r269786 - [clang-tidy] Skip misc-macro-parentheses for namespaces (Fix PR27400)

2016-05-17 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Tue May 17 12:26:02 2016 New Revision: 269786 URL: http://llvm.org/viewvc/llvm-project?rev=269786=rev Log: [clang-tidy] Skip misc-macro-parentheses for namespaces (Fix PR27400) If a use of a macro argument is preceded by the `namespace` keyword, do not warn that the use

r269701 - Revert "Reapply^2 "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC""

2016-05-16 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon May 16 16:04:19 2016 New Revision: 269701 URL: http://llvm.org/viewvc/llvm-project?rev=269701=rev Log: Revert "Reapply^2 "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC"" This reverts commit r269695. The llvm commit does not pass the MSVC bot.

r269695 - Reapply^2 "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC"

2016-05-16 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon May 16 15:50:13 2016 New Revision: 269695 URL: http://llvm.org/viewvc/llvm-project?rev=269695=rev Log: Reapply^2 "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC" Sync up with "(llvm) Use Error in InstrProf and Coverage". Differential Revision:

r269492 - Reapply "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC"

2016-05-13 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Fri May 13 16:51:02 2016 New Revision: 269492 URL: http://llvm.org/viewvc/llvm-project?rev=269492=rev Log: Reapply "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC" Sync up with "(llvm) Use Error in InstrProf and Coverage". Differential Revision:

r269468 - Revert "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC"

2016-05-13 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Fri May 13 15:10:22 2016 New Revision: 269468 URL: http://llvm.org/viewvc/llvm-project?rev=269468=rev Log: Revert "[ProfileData] (clang) Use Error in InstrProf and Coverage, NFC" This reverts commit r269463. It fails two llvm-profdata tests. Modified:

r269463 - [ProfileData] (clang) Use Error in InstrProf and Coverage, NFC

2016-05-13 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Fri May 13 15:01:34 2016 New Revision: 269463 URL: http://llvm.org/viewvc/llvm-project?rev=269463=rev Log: [ProfileData] (clang) Use Error in InstrProf and Coverage, NFC Sync up with "(llvm) Use Error in InstrProf and Coverage". Modified:

Re: Patch submission for bug 27400

2016-05-11 Thread Vedant Kumar via cfe-commits
Hi, Thanks for the patch! This patch is missing a small, lit-style test case. You can find examples of test cases here: extra/test/clang-tidy/ Apart from that, my only other nit-pick is that llvm uses 2-space indents, and spaces between "if" and "(". If you reply to this list with an

Re: [PATCH] D15674: [CodeGen] Fix assignments of inline layouts into the byref structure

2016-05-10 Thread Vedant Kumar via cfe-commits
vsk accepted this revision. vsk added a reviewer: vsk. vsk added a comment. This revision is now accepted and ready to land. Closing old review. http://reviews.llvm.org/D15674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D17299: [ASTReader] Report error when accessing corrupt record data

2016-05-10 Thread Vedant Kumar via cfe-commits
vsk abandoned this revision. vsk added a comment. I haven't measured the performance overhead of this patch. The failure case it's supposed to protect against is exceptionally rare: we haven't seen it happen again. In light of that, and considering that it clutters up the code, I think it'd be

r268947 - [cmake] Enable zlib support for Apple stage2 builds

2016-05-09 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon May 9 13:40:09 2016 New Revision: 268947 URL: http://llvm.org/viewvc/llvm-project?rev=268947=rev Log: [cmake] Enable zlib support for Apple stage2 builds This allows llvm-profdata to interact with profiles containing compressed name data. rdar://problem/26122944

Re: [PATCH] D19902: [ProfileData] (clang) Use Error in InstrProf and Coverage

2016-05-05 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 56346. vsk added a comment. - Handle an ECError we can see come out of opening a memory buffer. http://reviews.llvm.org/D19902 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenPGO.cpp lib/Frontend/CompilerInvocation.cpp Index:

Re: [PATCH] D19243: [CodeGen] Move some CodeGenPGO stuff out of CodeGenFunction.h

2016-05-01 Thread Vedant Kumar via cfe-commits
vsk abandoned this revision. vsk added a comment. It is a bit messy. (Aside: I don't think the size of CodeGenPGO is visible with this change, so unique_ptr/default_delete wouldn't work?) http://reviews.llvm.org/D19243 ___ cfe-commits mailing

Re: [PATCH] D19725: [Coverage] Fix an issue where a coverage region might not be created for a macro containing for or while statements.

2016-04-29 Thread Vedant Kumar via cfe-commits
vsk added a comment. Thanks, lgtm with a nit. Comment at: lib/CodeGen/CoverageMappingGen.cpp:452 @@ +451,3 @@ +[=](const SourceMappingRegion ) { + assert(Region.hasStartLoc() && "incomplete region"); +

Re: r267040 - clang-cl: Don't assert on using /Yc with non-source files, PR27450

2016-04-21 Thread Vedant Kumar via cfe-commits
ine looks like this: > > clang-3.9: error: argument unused during compilation: '-Werror' > clang-3.9: error: argument unused during compilation: '-include pchfile.h' > clang-3.9: error: argument unused during compilation: '-U > sers/buildslave/jenkins/sharedspace/...' << !

Re: [PATCH] D17392: Embed bitcode in object file (clang cc1 part)

2016-04-21 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. Just a few nitpicks. Comment at: lib/CodeGen/BackendUtil.cpp:785 @@ +784,3 @@ + llvm::WriteBitcodeToFile(M, OS, /* ShouldPreserveUseListOrder */ true); + ModuleData = ArrayRef((uint8_t*)OS.str().data(), +

Re: r267040 - clang-cl: Don't assert on using /Yc with non-source files, PR27450

2016-04-21 Thread Vedant Kumar via cfe-commits
016, at 2:42 PM, Vedant Kumar via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > No, removing -Werror doesn't help. > > What's strange is that this doesn't reproduce on my machine -- just on the > bots. > > I'll poke around a bit more. > > vedant >

Re: r267040 - clang-cl: Don't assert on using /Yc with non-source files, PR27450

2016-04-21 Thread Vedant Kumar via cfe-commits
://lab.llvm.org:8011/console > look happy. > > On Thu, Apr 21, 2016 at 4:58 PM, Vedant Kumar via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > Hi Nico, > > This seems to be causing a test failure on one of our bots: > > http://lab.llvm.org:8080/green/job/

[PATCH] D19295: [profile] Clang support for memory-mapping profile counters

2016-04-19 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added a reviewer: davidxl. vsk added a subscriber: cfe-commits. **Summary** Using memory-mapped profile counters makes it possible to take snapshots of a running process's profiling information without changing the program. This is useful if the process exits

Re: [PATCH] D19243: [CodeGen] Move some CodeGenPGO stuff out of CodeGenFunction.h

2016-04-18 Thread Vedant Kumar via cfe-commits
vsk updated the summary for this revision. vsk updated this revision to Diff 54128. vsk added a comment. - Remove extraneous whitespace changes. http://reviews.llvm.org/D19243 Files: lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGCall.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGObjC.cpp

[PATCH] D19243: [CodeGen] Move some CodeGenPGO stuff out of CodeGenFunction.h

2016-04-18 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added a reviewer: bogner. vsk added a subscriber: cfe-commits. Cons: 1 extra malloc per CodeGenFunction instantiation. Pros: This makes incremental builds noticeably faster, especially on my laptop. E.g with this patch there's no need to re-compile

Re: [PATCH] D19184: Remove MaxFunctionCount module flag annotation

2016-04-18 Thread Vedant Kumar via cfe-commits
vsk added a comment. Lgtm http://reviews.llvm.org/D19184 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r265252 - [c-index-test] Fix leak in print_completion_result, NFC

2016-04-02 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Sat Apr 2 19:54:46 2016 New Revision: 265252 URL: http://llvm.org/viewvc/llvm-project?rev=265252=rev Log: [c-index-test] Fix leak in print_completion_result, NFC Modified: cfe/trunk/tools/c-index-test/c-index-test.c Modified:

r264874 - [c-index-test] Delete dead function, NFC

2016-03-30 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed Mar 30 11:03:02 2016 New Revision: 264874 URL: http://llvm.org/viewvc/llvm-project?rev=264874=rev Log: [c-index-test] Delete dead function, NFC Modified: cfe/trunk/tools/c-index-test/c-index-test.c Modified: cfe/trunk/tools/c-index-test/c-index-test.c URL:

Re: [PATCH] D18289: Attach profile summary information to module

2016-03-24 Thread Vedant Kumar via cfe-commits
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. This lgtm. IMO we'd get the same coverage if the C file contained `void foo() {}`, but that's just a nit :). It'd be good to follow up with David about what he thinks is lacking in

Re: [PATCH] D18289: Attach profile summary information to module

2016-03-22 Thread Vedant Kumar via cfe-commits
vsk added inline comments. Comment at: test/Profile/profile-summary.c:5 @@ +4,3 @@ +// RUN: %clang %s -o - -mllvm -disable-llvm-optzns -emit-llvm -S -fprofile-instr-use=%t.profdata | FileCheck %s +// +int begin(int i) { davidxl wrote: > vsk wrote: > > ISTM that

Re: r264021 - [Perf-training] Fixing an issue with multi-threading PGO generation

2016-03-22 Thread Vedant Kumar via cfe-commits
I think setting `cc1_env["LLVM_PROFILE_FILE"] = os.devnull` would be simpler. vedant > On Mar 21, 2016, at 7:55 PM, Chris Bieneman via cfe-commits > wrote: > > Author: cbieneman > Date: Mon Mar 21 21:55:40 2016 > New Revision: 264021 > > URL:

Re: [PATCH] D18289: Attach profile summary information to module

2016-03-19 Thread Vedant Kumar via cfe-commits
vsk added a comment. Thanks! Some inline comments -- Comment at: lib/CodeGen/CodeGenModule.cpp:399 @@ -398,1 +398,3 @@ getModule().setMaximumFunctionCount(PGOReader->getMaximumFunctionCount()); +auto *SummaryMD = PGOReader->getSummary().getMD(getModule().getContext());

r262697 - [Coverage] Fix the start/end locations of switch statements

2016-03-04 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Fri Mar 4 02:07:15 2016 New Revision: 262697 URL: http://llvm.org/viewvc/llvm-project?rev=262697=rev Log: [Coverage] Fix the start/end locations of switch statements While pushing switch statements onto the region stack we neglected to specify their start/end locations.

Re: [PATCH] D17299: [ASTReader] Report error when accessing corrupt record data

2016-03-02 Thread Vedant Kumar via cfe-commits
vsk added a comment. Gentle ping. For context, we've had a few situations where corrupt pch files trigger surprising assertion failures in our world builds. http://reviews.llvm.org/D17299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D17299: [ASTReader] Report error when accessing corrupt record data

2016-02-16 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added reviewers: rsmith, benlangmuir. vsk added a subscriber: cfe-commits. If we try to read a corrupt pch, we can easily assert-fail or trigger invalid memory accesses when manipulating ASTReader::RecordData objects. This problem is easy enough to diagnose when

r260927 - Simplify users of StringRef::{l,r}trim (clang) (NFC)

2016-02-15 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Feb 15 20:14:44 2016 New Revision: 260927 URL: http://llvm.org/viewvc/llvm-project?rev=260927=rev Log: Simplify users of StringRef::{l,r}trim (clang) (NFC) r260925 introduced a version of the *trim methods which is preferable when trimming a single kind of character.

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Vedant Kumar via cfe-commits
vsk added a comment. Lgtm. http://reviews.llvm.org/D16947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16934: [Coverage] Fix crash in VisitIfStmt

2016-02-08 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 47230. vsk added a comment. - Check that we don't crash on a nested 'STMT' test case. http://reviews.llvm.org/D16934 Files: lib/CodeGen/CoverageMappingGen.cpp test/CoverageMapping/macro-expressions.cpp Index: test/CoverageMapping/macro-expressions.cpp

r260129 - [Coverage] Fix crash when handling certain macro expansions

2016-02-08 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Feb 8 13:25:45 2016 New Revision: 260129 URL: http://llvm.org/viewvc/llvm-project?rev=260129=rev Log: [Coverage] Fix crash when handling certain macro expansions When handling 'if' statements, we crash if the condition and the consequent branch are spanned by a single

Re: [PATCH] D16934: [Coverage] Fix crash in VisitIfStmt

2016-02-08 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 47229. vsk added a comment. - Made the check a bit more general by moving it to `propagateCounts`. - Added 3 more tests. http://reviews.llvm.org/D16934 Files: lib/CodeGen/CoverageMappingGen.cpp test/CoverageMapping/macro-expressions.cpp Index:

[PATCH] D16934: [Coverage] Fix crash in VisitIfStmt

2016-02-05 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added a reviewer: bogner. vsk added a subscriber: cfe-commits. When handling 'if' statements, we crash if the condition and the consequent branch are spanned by a single macro expansion. The crash occurs because of a sanity 'reset' in `popRegions()`: if an

r259061 - [Coverage] Use a set to track visited FileIDs (NFC)

2016-01-28 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Thu Jan 28 11:52:18 2016 New Revision: 259061 URL: http://llvm.org/viewvc/llvm-project?rev=259061=rev Log: [Coverage] Use a set to track visited FileIDs (NFC) Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp

[PATCH] D16395: [Coverage] Reduce complexity of adding function mapping records

2016-01-20 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added a reviewer: davidxl. vsk added a subscriber: cfe-commits. Replace a string append operation in addFunctionMappingRecord with a vector append. The existing behavior is quadratic in the worst case: this patch makes it linear. http://reviews.llvm.org/D16395

Re: [PATCH] D15097: [Sema] Issue a warning for integer overflow in struct initializer

2016-01-08 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. Looks good to me. Caveat: This isn't quite my area. This seems reasonable given what I've read in `EvaluateForOverflow` and `EvaluateAsRValue`. I think you should be fine to commit if no one objects soon. http://reviews.llvm.org/D15097

Re: [PATCH] D8940: Clang changes for indirect call target profiling

2016-01-08 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. Thanks betul! Just a minor nit. Comment at: lib/CodeGen/CGCall.cpp:3550 @@ +3549,3 @@ +DirectCallee = dyn_cast (Callee); + if (!DirectCallee) +PGO.valueProfile(Builder, llvm::IPVK_IndirectCallTarget,

Re: [PATCH] D15674: [CodeGen] Fix assignments of inline layouts into the byref structure

2015-12-21 Thread Vedant Kumar via cfe-commits
vsk updated this revision to Diff 43384. vsk added a comment. - Update patch according to John's comments. http://reviews.llvm.org/D15674 Files: lib/CodeGen/CGObjCMac.cpp lib/CodeGen/CGObjCRuntime.h test/CodeGenObjCXX/blocks.mm Index: test/CodeGenObjCXX/blocks.mm

r256186 - Revert "[CodeGen] Fix assignments of inline layouts into the byref structure"

2015-12-21 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Dec 21 13:43:25 2015 New Revision: 256186 URL: http://llvm.org/viewvc/llvm-project?rev=256186=rev Log: Revert "[CodeGen] Fix assignments of inline layouts into the byref structure" This reverts commit r256185. It breaks CodeGenObjC/fragile-arc.m. Modified:

Re: [PATCH] D15674: [CodeGen] Fix assignments of inline layouts into the byref structure

2015-12-21 Thread Vedant Kumar via cfe-commits
vsk added a comment. Thanks for the review. Committed r256185 http://reviews.llvm.org/D15674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r256190 - Reapply "[CodeGen] Fix assignments of inline layouts into the byref structure"

2015-12-21 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Dec 21 14:21:15 2015 New Revision: 256190 URL: http://llvm.org/viewvc/llvm-project?rev=256190=rev Log: Reapply "[CodeGen] Fix assignments of inline layouts into the byref structure" When using blocks, a byref structure is created to represent the closure. The

r256185 - [CodeGen] Fix assignments of inline layouts into the byref structure

2015-12-21 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Dec 21 13:30:37 2015 New Revision: 256185 URL: http://llvm.org/viewvc/llvm-project?rev=256185=rev Log: [CodeGen] Fix assignments of inline layouts into the byref structure When using blocks, a byref structure is created to represent the closure. The "byref.layout" field

[PATCH] D15674: [CodeGen] Fix assignments of inline layouts into the byref structure

2015-12-19 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added a reviewer: rjmccall. vsk added a subscriber: cfe-commits. [CodeGen] Fix assignments of inline layouts into the byref structure When using blocks, a byref structure is created to represent the closure. The "byref.layout" field of this structure is an i8*.

Re: [PATCH] D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4

2015-12-15 Thread Vedant Kumar via cfe-commits
vsk accepted this revision. vsk added a reviewer: vsk. vsk added a comment. This revision is now accepted and ready to land. Lgtm http://reviews.llvm.org/D15222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4

2015-12-15 Thread Vedant Kumar via cfe-commits
vsk added a comment. Sure! Thanks. http://reviews.llvm.org/D15222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4

2015-12-14 Thread Vedant Kumar via cfe-commits
vsk added inline comments. Comment at: lib/Driver/Tools.cpp:4067 @@ -4049,1 +4066,3 @@ + // Add runtime flag for PS4 when PGO or Coverage are enabled. + if (getToolChain().getTriple().isPS4CPU()) Sorry, I don't know why I thought this was in ParsePIC. This

Re: [PATCH] D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4

2015-12-11 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. Thanks, comments inline -- Comment at: lib/Driver/Tools.cpp:4064 @@ -4046,1 +4063,3 @@ + // Add runtime flag for PS4 when PGO or Coverage are enabled. + if (getToolChain().getTriple().isPS4CPU()) Profiling

Re: [PATCH] D15462: [CMake] Add support for generating profdata for clang from training files

2015-12-11 Thread Vedant Kumar via cfe-commits
vsk added a comment. Thanks! Interesting approach. I think @cmatthews would appreciate a `generate-profdata-from-lnt` target if you're up for it :). Comments inline -- Comment at: utils/perf-training/CMakeLists.txt:2 @@ +1,3 @@ +if(LLVM_BUILD_INSTRUMENTED AND NOT WIN32) + if

r255091 - [Basic] Rangify two for loops. NFC.

2015-12-08 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Tue Dec 8 19:44:02 2015 New Revision: 255091 URL: http://llvm.org/viewvc/llvm-project?rev=255091=rev Log: [Basic] Rangify two for loops. NFC. Modified: cfe/trunk/lib/Basic/FileManager.cpp Modified: cfe/trunk/lib/Basic/FileManager.cpp URL:

r253853 - [Driver] Mark isForDiagnostics as const. NFC.

2015-11-22 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Nov 23 00:40:49 2015 New Revision: 253853 URL: http://llvm.org/viewvc/llvm-project?rev=253853=rev Log: [Driver] Mark isForDiagnostics as const. NFC. Modified: cfe/trunk/include/clang/Driver/Compilation.h Modified: cfe/trunk/include/clang/Driver/Compilation.h URL:

r253177 - [Basic] Use a bitfield in SLocEntry for clarity. NFC.

2015-11-15 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Sun Nov 15 18:11:58 2015 New Revision: 253177 URL: http://llvm.org/viewvc/llvm-project?rev=253177=rev Log: [Basic] Use a bitfield in SLocEntry for clarity. NFC. Modified: cfe/trunk/include/clang/Basic/SourceManager.h Modified:

r253178 - [Frontend] Rangify for loop. NFC.

2015-11-15 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Sun Nov 15 18:59:34 2015 New Revision: 253178 URL: http://llvm.org/viewvc/llvm-project?rev=253178=rev Log: [Frontend] Rangify for loop. NFC. Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL:

r253181 - [Basic] Replace vector with BitVector in SourceManager. NFC.

2015-11-15 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Sun Nov 15 22:39:22 2015 New Revision: 253181 URL: http://llvm.org/viewvc/llvm-project?rev=253181=rev Log: [Basic] Replace vector with BitVector in SourceManager. NFC. Modified: cfe/trunk/include/clang/Basic/SourceManager.h Modified:

r252828 - [Basic] Fix DRY violation, just call getLineTable() (NFC)

2015-11-11 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Wed Nov 11 18:11:19 2015 New Revision: 252828 URL: http://llvm.org/viewvc/llvm-project?rev=252828=rev Log: [Basic] Fix DRY violation, just call getLineTable() (NFC) Modified: cfe/trunk/lib/Basic/SourceManager.cpp Modified: cfe/trunk/lib/Basic/SourceManager.cpp URL:

[PATCH] D14521: [Driver] Use platform-appropriate profiling libraries for WatchOS, TVOS

2015-11-09 Thread Vedant Kumar via cfe-commits
vsk created this revision. vsk added a reviewer: t.p.northover. vsk added a subscriber: cfe-commits. When adding profiling instrumentation, use `libclang_rt.profile_tvos.a` for TVOS targets and `libclang_rt.profile_watchos.a` for WatchOS targets. I've also fixed up a comment and added an

r252558 - [Driver] Use platform-appropriate profiling libraries for WatchOS, TVOS

2015-11-09 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Mon Nov 9 18:20:34 2015 New Revision: 252558 URL: http://llvm.org/viewvc/llvm-project?rev=252558=rev Log: [Driver] Use platform-appropriate profiling libraries for WatchOS, TVOS When adding profiling instrumentation, use libclang_rt.profile_tvos.a for TVOS targets and

Re: r251385 - Create undef reference to profile hook symbol

2015-10-28 Thread Vedant Kumar via cfe-commits
> On Oct 28, 2015, at 11:03 AM, Justin Bogner via cfe-commits > wrote: > > Xinliang David Li via cfe-commits writes: >> Author: davidxl >> Date: Tue Oct 27 00:15:35 2015 >> New Revision: 251385 >> >> URL:

Re: [PATCH] D14030: Use linker option to reference profile runtime hook (Linux)

2015-10-26 Thread Vedant Kumar via cfe-commits
vsk added a comment. This looks good to me. Does this mean we will get rid of `getInstrProfRuntimeHookVarUseFuncName`? http://reviews.llvm.org/D14030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D14014: Checker of proper vfork usage

2015-10-23 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. Thanks for the patch. I didn't know vfork() is in regular use. Afaict copy-on-write fork() and threading both solve a similar (the same?) problem. I'm also not convinced that flagging all stores in the vfork child process is the right thing to

Re: [PATCH] D12686: Add support for GCC's '__auto_type' extension.

2015-10-12 Thread Vedant Kumar via cfe-commits
vsk added a comment. This lgtm. If you don't have commit rights, I can land this for you if you'd like. http://reviews.llvm.org/D12686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

r249801 - [Sema] Add "Ty" suffix to QualType variables for clarity (NFC)

2015-10-08 Thread Vedant Kumar via cfe-commits
Author: vedantk Date: Thu Oct 8 20:47:26 2015 New Revision: 249801 URL: http://llvm.org/viewvc/llvm-project?rev=249801=rev Log: [Sema] Add "Ty" suffix to QualType variables for clarity (NFC) Modified: cfe/trunk/lib/Sema/SemaExpr.cpp Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL:

Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast

2015-09-30 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. The patch lgtm. Has there been a discussion on cfe-dev about adopting these guidelines? I count well over 500 uses of reinterpret_cast in llvm+clang, so we might not all be on the same page on this. http://reviews.llvm.org/D13313

Re: [PATCH] D12774: createUniqueFile() is documented to create the file in the temporary directory unless it's supplied an absolute path.Make sure the output filepath supplied to createUniqueFile() in

2015-09-30 Thread Vedant Kumar via cfe-commits
vsk added a comment. LGTM, thanks. http://reviews.llvm.org/D12774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12774: createUniqueFile() is documented to create the file in the temporary directory unless it's supplied an absolute path.Make sure the output filepath supplied to createUniqueFile() in

2015-09-22 Thread Vedant Kumar via cfe-commits
vsk added a comment. Yes, that looks good. http://reviews.llvm.org/D12774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13004: Create a new attribute set when the definition is parsed after a declaration of a function

2015-09-21 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. Thanks, LGTM. http://reviews.llvm.org/D13004 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

<    1   2   3   4   5   6   >