@@ -217,6 +217,9 @@ class CounterExpressionBuilder {
using LineColPair = std::pair;
+using MCDCConditionID = unsigned int;
+using MCDCConditionIDs = std::array;
evodius96 wrote:
is `std::array<>` better than `std::pair<>` when it only has two elements?
http
@@ -217,6 +217,9 @@ class CounterExpressionBuilder {
using LineColPair = std::pair;
+using MCDCConditionID = unsigned int;
+using MCDCConditionIDs = std::array;
evodius96 wrote:
Ah, well it's iterable.
https://github.com/llvm/llvm-project/pull/81221
___
https://github.com/evodius96 approved this pull request.
I don't know if anyone else has any preference around the additional
mcdc-specific header files, but generally it looks good to me.
https://github.com/llvm/llvm-project/pull/81497
___
cfe-commit
https://github.com/evodius96 approved this pull request.
https://github.com/llvm/llvm-project/pull/81221
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1201,19 +1197,22 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy
&Builder, const Expr *S,
// Extract the ID of the condition we are setting in the bitmap.
const auto &Branch = BranchStateIter->second;
assert(Branch.ID >= 0 && "Condition has no ID!");
+ asse
@@ -983,7 +979,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) {
// for most embedded applications. Setting a maximum value prevents the
// bitmap footprint from growing too large without the user's knowledge. In
// the future, this value could be adjusted with a c
@@ -1201,19 +1197,22 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy
&Builder, const Expr *S,
// Extract the ID of the condition we are setting in the bitmap.
const auto &Branch = BranchStateIter->second;
assert(Branch.ID >= 0 && "Condition has no ID!");
+ asse
evodius96 wrote:
Hi! I think this may be the same issue as
https://github.com/llvm/llvm-project/issues/77871 and a fix is under review by
@chapuni Can you verify?
https://github.com/llvm/llvm-project/pull/80098
___
cfe-commits mailing list
cfe-commi
evodius96 wrote:
Added a couple of comments, but otherwise LGTM. Thank you for optimizing this.
https://github.com/llvm/llvm-project/pull/79727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
https://github.com/evodius96 approved this pull request.
https://github.com/llvm/llvm-project/pull/79727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/evodius96 created
https://github.com/llvm/llvm-project/pull/78202
Clean-up of the algorithm that assigns MC/DC True/False control-flow condition
IDs when constructing an MC/DC decision region. This patch creates a common
API for setting/getting the condition IDs, making the
evodius96 wrote:
This fixes issue reported here:
https://github.com/llvm/llvm-project/issues/77873
https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/
evodius96 wrote:
I'll address the clang-format issues
https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/evodius96 updated
https://github.com/llvm/llvm-project/pull/78202
>From 8751417f4889a120193a604e2ea91627f3b064e8 Mon Sep 17 00:00:00 2001
From: Alan Phipps
Date: Mon, 15 Jan 2024 12:24:36 -0600
Subject: [PATCH 1/2] [clang][CoverageMapping] Refactor when setting MC/DC
True/Fa
@@ -746,21 +781,52 @@ struct MCDCCoverageBuilder {
// assign that ID to its LHS node. Its RHS will receive a new ID.
if (CondIDs.contains(CodeGenFunction::stripCond(E))) {
// If Stmt has an ID, assign its ID to LHS
- CondIDs[CodeGenFunction::stripCond(E->get
@@ -1847,30 +1916,13 @@ struct CounterCoverageMappingBuilder
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
-// Extract the MCDC condition IDs (returns 0 if not needed).
evodius96 wrote:
Since the APIs are all
@@ -298,7 +298,7 @@ struct CounterMappingRegion {
unsigned ExpandedFileID, unsigned LineStart,
unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd, RegionKind Kind)
- : MCDCParams(MCDCParams), Expa
@@ -668,6 +668,8 @@ struct MCDCCoverageBuilder {
llvm::SmallVector NestLevel;
llvm::DenseMap &CondIDs;
llvm::DenseMap &MCDCBitmapMap;
+ llvm::DenseMap TrueCondIDs;
evodius96 wrote:
We can use these to track a Condition's True/False IDs and make all of t
@@ -298,7 +298,7 @@ struct CounterMappingRegion {
unsigned ExpandedFileID, unsigned LineStart,
unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd, RegionKind Kind)
- : MCDCParams(MCDCParams), Expa
@@ -298,7 +298,7 @@ struct CounterMappingRegion {
unsigned ExpandedFileID, unsigned LineStart,
unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd, RegionKind Kind)
- : MCDCParams(MCDCParams), Expa
evodius96 wrote:
> I've found this change fails with the expression; `((a && (b || c) || (d &&
> e)) && f)`.
Thank you for pointing this out. I think you figured out the point I was
struggling to get right -- when to actually "pop" the child Decision from the
stack, and also to separate the
https://github.com/evodius96 updated
https://github.com/llvm/llvm-project/pull/78202
>From 8751417f4889a120193a604e2ea91627f3b064e8 Mon Sep 17 00:00:00 2001
From: Alan Phipps
Date: Mon, 15 Jan 2024 12:24:36 -0600
Subject: [PATCH 1/3] [clang][CoverageMapping] Refactor when setting MC/DC
True/Fa
@@ -676,41 +679,25 @@ struct MCDCCoverageBuilder {
return E->getOpcode() == BO_LAnd;
}
- /// Push an ID onto the corresponding RHS stack.
- void pushRHS(const BinaryOperator *E) {
-llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS;
-rhs.push_back(CondIDs[Code
https://github.com/evodius96 updated
https://github.com/llvm/llvm-project/pull/78202
>From 8751417f4889a120193a604e2ea91627f3b064e8 Mon Sep 17 00:00:00 2001
From: Alan Phipps
Date: Mon, 15 Jan 2024 12:24:36 -0600
Subject: [PATCH 1/4] [clang][CoverageMapping] Refactor when setting MC/DC
True/Fa
evodius96 wrote:
> Could you mention the issue #77873 in the description or in the subject?
Yes, I will change this!
https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin
@@ -1018,13 +1011,15 @@ struct CounterCoverageMappingBuilder
return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext()));
}
+ using MCDCDecisionIDPair = MCDCCoverageBuilder::DecisionIDPair;
+
/// Create a Branch Region around an instrumentable condition
@@ -663,54 +668,40 @@ struct MCDCCoverageBuilder {
private:
CodeGenModule &CGM;
- llvm::SmallVector AndRHS;
- llvm::SmallVector OrRHS;
- llvm::SmallVector NestLevel;
+ llvm::SmallVector DecisionStack;
llvm::DenseMap &CondIDs;
llvm::DenseMap &MCDCBitmapMap;
MCDCC
@@ -722,6 +713,9 @@ struct MCDCCoverageBuilder {
return I->second;
}
+ /// Return the LHS Decision ([0,0] if not set).
+ const DecisionIDPair back() const { return DecisionStack.back(); }
evodius96 wrote:
Ah, you're right.
https://github.com/llvm/l
@@ -722,6 +713,9 @@ struct MCDCCoverageBuilder {
return I->second;
}
+ /// Return the LHS Decision ([0,0] if not set).
+ const DecisionIDPair back() const { return DecisionStack.back(); }
evodius96 wrote:
On second thought, could you elaborate on yo
https://github.com/evodius96 edited
https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -722,6 +713,9 @@ struct MCDCCoverageBuilder {
return I->second;
}
+ /// Return the LHS Decision ([0,0] if not set).
+ const DecisionIDPair back() const { return DecisionStack.back(); }
evodius96 wrote:
Ah, ok I think I misunderstood. You're sugg
https://github.com/evodius96 updated
https://github.com/llvm/llvm-project/pull/78202
>From 8751417f4889a120193a604e2ea91627f3b064e8 Mon Sep 17 00:00:00 2001
From: Alan Phipps
Date: Mon, 15 Jan 2024 12:24:36 -0600
Subject: [PATCH 1/5] [clang][CoverageMapping] Refactor when setting MC/DC
True/Fa
https://github.com/evodius96 edited
https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/evodius96 closed
https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/evodius96 created
https://github.com/llvm/llvm-project/pull/78814
This is a fix for MC/DC issue https://github.com/llvm/llvm-project/issues/78453
in which a ConditionalOperator that evaluates a complex condition was
incorrectly updating its global bitmap after visiting its L
Author: Alan Phipps
Date: 2020-12-23T12:57:27-06:00
New Revision: b920adf3b4f16bef8dde937b67874d8e8ac1030e
URL:
https://github.com/llvm/llvm-project/commit/b920adf3b4f16bef8dde937b67874d8e8ac1030e
DIFF:
https://github.com/llvm/llvm-project/commit/b920adf3b4f16bef8dde937b67874d8e8ac1030e.diff
L
Author: Alan Phipps
Date: 2020-12-23T13:04:37-06:00
New Revision: bbd758a7913b1c374ca26e5a734a01200754fe0e
URL:
https://github.com/llvm/llvm-project/commit/bbd758a7913b1c374ca26e5a734a01200754fe0e
DIFF:
https://github.com/llvm/llvm-project/commit/bbd758a7913b1c374ca26e5a734a01200754fe0e.diff
L
evodius96 wrote:
Just a warning that I am planning to push a significant change to enable MC/DC
within the next few hours that will cause some conflicts with what you've done
here. I apologize for the inconvenience!
https://github.com/llvm/llvm-project/pull/66825
__
@@ -1519,38 +1543,53 @@ struct CounterCoverageMappingBuilder
}
// Create Branch Region around condition.
-createBranchRegion(S->getCond(), BodyCount,
- subtractCounters(CondCount, BodyCount));
+if (!llvm::EnableSingleByteCoverage)
+ c
https://github.com/evodius96 approved this pull request.
LGTM. I would like to see this broadened to include branch coverage as well!
https://github.com/llvm/llvm-project/pull/75425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.
@@ -239,9 +239,12 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions.
-if (LogOpStack.empty())
+/// At the top of the logical operato
@@ -292,7 +295,7 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
"contains an operation with a nested boolean expression. "
"Expression will not be covered");
Diag.Report(S->getBeginLoc(), DiagID);
-return fals
@@ -239,9 +239,12 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions.
-if (LogOpStack.empty())
+/// At the top of the logical operato
https://github.com/evodius96 approved this pull request.
LGTM. thanks for catching and fixing this. Would also be good to get this onto
18.x if possible.
https://github.com/llvm/llvm-project/pull/82464
___
cfe-commits mailing list
cfe-commits@lists.l
@@ -292,7 +295,7 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
"contains an operation with a nested boolean expression. "
"Expression will not be covered");
Diag.Report(S->getBeginLoc(), DiagID);
-return fals
@@ -484,10 +484,31 @@ MC/DC Instrumentation
-
When instrumenting for Modified Condition/Decision Coverage (MC/DC) using the
-clang option ``-fcoverage-mcdc``, users are limited to at most **six**
leaf-level
-conditions in a boolean expression. A warning w
evodius96 wrote:
> Could I update release notes later? I am afraid since I have been always
> missing release schedules.
I think I'm OK with this; I think the changes make sense. I'd like to see how
others feel.
https://github.com/llvm/llvm-project/pull/82448
https://github.com/evodius96 approved this pull request.
LGTM but I'd like to see if @ornata or others can definitively approve.
Thanks for this work! Comprehensively I think LLVM MC/DC is really good, and
there's still more we can do.
https://github.com/llvm/llvm-project/pull/82448
__
evodius96 wrote:
Somewhat related to this -- we (TI) are presently in the process of upstreaming
our mod to save/restore VFP registers with the interrupt_save_fp attribute. See
#89654. I think the patch needs to be updated in light of upstream changes to
the Arm frame lowering code.
https://
https://github.com/evodius96 approved this pull request.
LGTM thanks
https://github.com/llvm/llvm-project/pull/95496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/evodius96 approved this pull request.
https://github.com/llvm/llvm-project/pull/95887
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
evodius96 wrote:
I think this is a useful option, thanks. So the default preserves existing
behavior? I think that's perfect and amenable to change in the future of needed.
https://github.com/llvm/llvm-project/pull/94137
___
cfe-commits mailing list
c
https://github.com/evodius96 approved this pull request.
https://github.com/llvm/llvm-project/pull/89572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: Alan Phipps
Date: 2021-01-05T13:35:52-06:00
New Revision: 16f3401eae4310c95163269c41d9b45261f0c7c3
URL:
https://github.com/llvm/llvm-project/commit/16f3401eae4310c95163269c41d9b45261f0c7c3
DIFF:
https://github.com/llvm/llvm-project/commit/16f3401eae4310c95163269c41d9b45261f0c7c3.diff
L
Author: Alan Phipps
Date: 2021-01-05T14:53:07-06:00
New Revision: 216894211713bbb1e8beb249f2b008c11a9d8c2c
URL:
https://github.com/llvm/llvm-project/commit/216894211713bbb1e8beb249f2b008c11a9d8c2c
DIFF:
https://github.com/llvm/llvm-project/commit/216894211713bbb1e8beb249f2b008c11a9d8c2c.diff
L
@@ -1128,15 +1132,22 @@ struct CounterCoverageMappingBuilder
BranchParams = mcdc::BranchParameters{ID, Conds};
// If a condition can fold to true or false, the corresponding branch
- // will be removed. Create a region with both counters hard-coded to
-
@@ -47,7 +47,7 @@
// CHECK: Branch (103:9): [True: 9, False: 1]
// CHECK: switches()
-// CHECK: Branch (113:3): [True: 1, False: 0]
+// CHECK: Branch (113:3): [Folded - Ignored]
evodius96 wrote:
I'm not sure I see why the evaluation of this case (and line 60)
evodius96 wrote:
> I have no authority to request reviewers or commit this patch. So let's @
> someone, @chapuni @ornata @hanickadot
I had some comments in the pending state that I just now noticed :( sorry about
that.
https://github.com/llvm/llvm-project/pull/94137
__
https://github.com/evodius96 approved this pull request.
https://github.com/llvm/llvm-project/pull/112694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
evodius96 wrote:
I think this is a meaningful enhancement to branch coverage. I don't have much
to add to what's been said. LGTM. Thanks!
https://github.com/llvm/llvm-project/pull/112694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:/
https://github.com/evodius96 approved this pull request.
https://github.com/llvm/llvm-project/pull/120418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
evodius96 wrote:
I agree; I apologize for my silence as I have had several distractions. I plan
to start looking at more of these next week.
https://github.com/llvm/llvm-project/pull/121197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https
62 matches
Mail list logo
