[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps marked an inline comment as done. alanphipps added inline comments. Comment at: llvm/test/tools/llvm-cov/branch-noShowBranch.test:3 +// RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata +// RUN: llvm-cov show %S/Inputs/branch-c-general.o32l -i

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

dblaikie added inline comments. Comment at: llvm/test/tools/llvm-cov/branch-noShowBranch.test:3 +// RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata +// RUN: llvm-cov show %S/Inputs/branch-c-general.o32l -instr-profile %t.profdata -path-equivalence=/tm

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps marked an inline comment as done. alanphipps added inline comments. Comment at: llvm/test/tools/llvm-cov/branch-noShowBranch.test:3 +// RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata +// RUN: llvm-cov show %S/Inputs/branch-c-general.o32l -i

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

dblaikie added inline comments. Comment at: llvm/test/tools/llvm-cov/branch-noShowBranch.test:3 +// RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata +// RUN: llvm-cov show %S/Inputs/branch-c-general.o32l -instr-profile %t.profdata -path-equivalence=/tm

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps added a comment. In D84467#2449045 , @vsk wrote: > Unfortunately I don't think I'll be able to commit this on your behalf. Would > you be open to obtaining commit access from Chris and landing this directly? > I ask because, for a patch of thi

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk added a comment. In D84467#2448934 , @alanphipps wrote: > In D84467#2423636 , @vsk wrote: > >> Thank you, lgtm! > > Thank you! I also would like to ask if you could commit it for me as I don't > have access for

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps added a comment. In D84467#2423636 , @vsk wrote: > Thank you, lgtm! Thank you! I also would like to ask if you could commit it for me as I don't have access for that. Note that a handful of tests have binary files which were not uploaded wit

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thank you, lgtm! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84467/new/ https://reviews.llvm.org/D84467 ___ cfe-commits mailing list cfe-commi

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk removed a subscriber: loic-joly-sonarsource. vsk added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1169 +createBranchRegion(S->getCond(), BodyCount, + subtractCounters(CondCount, BodyCount)); } alanphipps

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps marked 2 inline comments as done. alanphipps added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1169 +createBranchRegion(S->getCond(), BodyCount, + subtractCounters(CondCount, BodyCount)); } vsk wro

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1169 +createBranchRegion(S->getCond(), BodyCount, + subtractCounters(CondCount, BodyCount)); } If `theWhileStmt->getCond()` is-a BinaryOperator, it would

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps added a comment. In D84467#2264205 , @vsk wrote: > @alanphipps thanks for bearing with me. I think this is about ready to land. > I do ask that you back out any punctuation/whitespace changes in code/tests > that aren't directly modified in yo

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk added a comment. @alanphipps thanks for bearing with me. I think this is about ready to land. I do ask that you back out any punctuation/whitespace changes in code/tests that aren't directly modified in your diff (the clang-format-diff script can help with this). It looks like some libCover

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps updated this revision to Diff 290301. alanphipps added a comment. - Rename isLeafCondition() to isInstrumentedCondition() and rephrase associated comments - Add branch regions on C++ range-based loops CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84467/new/ https://reviews.l

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps marked an inline comment as done. alanphipps added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359 + /// condition (i.e. no "&&" or "||"). + static bool isLeafCondition(const Expr *C); + vsk wrote: > alanphipps wrote: > > vsk wr

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps updated this revision to Diff 287269. alanphipps marked 2 inline comments as done. alanphipps added a comment. Updating for formatting and comments (and some test adjustments after rebase). Bypassing logical-NOT operators in CodeGenFunction::isLeafCondition(), which checks the condit

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps marked 13 inline comments as done. alanphipps added a comment. Herald added a subscriber: wenlei. In D84467#2180421 , @vsk wrote: > I haven't taken a close look at the tests yet, but plan on getting to it soon. > > I'm not sure whether this is s

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359 + /// condition (i.e. no "&&" or "||"). + static bool isLeafCondition(const Expr *C); + alanphipps wrote: > vsk wrote: > > alanphipps wrote: > > > vsk wrote: > > > > vsk wrote: > >

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps added a comment. In D84467#2180421 , @vsk wrote: > I haven't taken a close look at the tests yet, but plan on getting to it soon. > > I'm not sure whether this is something you've already tried, but for this > kind of change, it can be helpful

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps added a comment. Thank you for your comments on the review! I will respond soon; also have been swamped this week. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84467/new/ https://reviews.llvm.org/D84467 ___

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk added a comment. I haven't taken a close look at the tests yet, but plan on getting to it soon. I'm not sure whether this is something you've already tried, but for this kind of change, it can be helpful to verify that a stage2 clang self-host with assertions enabled completes successfully.

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359 + /// condition (i.e. no "&&" or "||"). + static bool isLeafCondition(const Expr *C); + alanphipps wrote: > vsk wrote: > > vsk wrote: > > > It might be helpful to either require tha

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

alanphipps marked 2 inline comments as done. alanphipps added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:1368 CaseDest = createBasicBlock("sw.bb"); - EmitBlockWithFallThrough(CaseDest, &S); + EmitBlockWithFallThrough(CaseDest, CurCase); }

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

vsk added a reviewer: zequanwu. vsk added subscribers: ikudrin, arphaman. vsk added a comment. @alanphipps thanks for the patch! I'm a bit swamped at the moment, but I hope to give a detailed review by this Wednesday. Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359 + ///