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);
}
----------------
This fixes a defect that resulted in the wrong counter being instrumented for
each Case.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359
+ /// condition (i.e. no "&&" or "||").
+ static bool isLeafCondition(const Expr *C);
+
----------------
vsk wrote:
> vsk wrote:
> > It might be helpful to either require that `C` be the RHS of a logical
> > binop (e.g. by passing the binop expr in), or to document that requirement.
> Given a logical binop like `E = a && !(b || c)`,
> `isLeafCondition(E->getRHS())` is true. That seems a bit counter-intuitive,
> because `E->getRHS()` contains another leaf condition. Would it make sense to
> rename the condition (perhaps to something like 'InstrumentedCondition')?
> Have I misunderstood what a leaf condition is?
Background: isLeafCondition() is an auxiliary routine that is used during 1.)
counter allocation on binop RHS (CodeGenPGO.cpp), 2.) counter instrumentation
(CodeGenFunction.cpp), and 3.) branch region generation
(CoverageMappingGen.cpp). In the #3 case, it isn't always looking at the RHS
of a logical binop but can be used to assess whether any condition is
instrumented/evaluated.
Given your example condition:
E = a && !(b || c)
You are correct that isLeafCondition(E->getRHS()) will return true, but I think
it should actually return false here (and bypass logical-NOT), so I will adapt
this.
However, given a condition that includes an binary operator (like assign):
E = a && (x = !(b || c))
isLeafCondition(E->getRHS()) will also return true, and I think that is the
correct behavior. Given that, then I agree that maybe isLeafCondition() should
be renamed to isInstrumentedCondition() since it's not technically just leaves
that are tracked in the presence of operators.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84467/new/
https://reviews.llvm.org/D84467
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
