[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: Thanks! https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
https://github.com/efriedma-quic closed https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: Could you please merge both of these? https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
https://github.com/efriedma-quic approved this pull request. LGTM. I'm not completely convinced by your argument here... but let's try to move forward and land this and #88898 so we can do the refactor to use isUnique(). Then we can revisit later if necessary. https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: This looks like supporting my words: https://github.com/llvm/llvm-project/blob/llvmorg-19-init/clang/lib/CodeGen/CodeGenPGO.cpp#L935-L936 https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: @efriedma-quic ping. CC @AaronBallman https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: @efriedma-quic, would it be OK to add "subexpression" visitation with a comment that I'm not sure that it is actually needed? https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: > But anyway, I think we end up doing the right thing automatically if you > ignore non-unique OpaqueValueExprs. The problem is that there is no initializing "common expression" in the AST besides non-unique `OpaqueValueExpr`, hence some code handling `ArrayInitLoopExpr` should be written so as not to lose it at all. @chapuni, @gulfemsavrun, @evodius96, @vedantk, please clarify: should or should not the implicit stuff be traversed here, particularly for array element initialization? https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
https://github.com/bolshakov-a updated
https://github.com/llvm/llvm-project/pull/1
>From a025b2a45c2a66595c111262dd43c0890f0d54b6 Mon Sep 17 00:00:00 2001
From: Bolshakov
Date: Tue, 16 Apr 2024 14:21:40 +0300
Subject: [PATCH 1/2] [Coverage] Handle array decomposition correctly
`ArrayInitLoopExpr` AST node has two occurences of its as-written
initializing expression in its subexpressions through a non-unique
`OpaqueValueExpr`. It causes double-visiting of the initializing
expression if not handled explicitly, as discussed in #85837.
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 4
clang/test/CoverageMapping/decomposition.cpp | 15 +++
2 files changed, 19 insertions(+)
create mode 100644 clang/test/CoverageMapping/decomposition.cpp
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 71215da362d3d0..569fd489dc8baa 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2171,6 +2171,10 @@ struct CounterCoverageMappingBuilder
// propagate counts into them.
}
+ void VisitArrayInitLoopExpr(const ArrayInitLoopExpr *AILE) {
+Visit(AILE->getCommonExpr()->getSourceExpr());
+ }
+
void VisitPseudoObjectExpr(const PseudoObjectExpr *POE) {
// Just visit syntatic expression as this is what users actually write.
VisitStmt(POE->getSyntacticForm());
diff --git a/clang/test/CoverageMapping/decomposition.cpp
b/clang/test/CoverageMapping/decomposition.cpp
new file mode 100644
index 00..31bd6cae2c4dec
--- /dev/null
+++ b/clang/test/CoverageMapping/decomposition.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-emit-llvm-only %s | FileCheck %s
+
+// CHECK-LABEL: _Z19array_decompositioni:
+// CHECK-NEXT: File 0, [[@LINE+6]]:32 -> {{[0-9]+}}:2 = #0
+// CHECK-NEXT: File 0, [[@LINE+8]]:20 -> [[@LINE+8]]:25 = #0
+// CHECK-NEXT: Branch,File 0, [[@LINE+7]]:20 -> [[@LINE+7]]:25 = #1,
(#0 - #1)
+// CHECK-NEXT: Gap,File 0, [[@LINE+6]]:27 -> [[@LINE+6]]:28 = #1
+// CHECK-NEXT: File 0, [[@LINE+5]]:28 -> [[@LINE+5]]:29 = #1
+// CHECK-NEXT: File 0, [[@LINE+4]]:32 -> [[@LINE+4]]:33 = (#0 - #1)
+int array_decomposition(int i) {
+ int a[] = {1, 2, 3};
+ int b[] = {4, 5, 6};
+ auto [x, y, z] = i > 0 ? a : b;
+ return x + y + z;
+}
>From 2936ef132db8908fa65293ceba6884f80c871c93 Mon Sep 17 00:00:00 2001
From: Bolshakov
Date: Wed, 17 Apr 2024 07:10:25 +0300
Subject: [PATCH 2/2] Fix up running test on Windows
---
clang/test/CoverageMapping/decomposition.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/CoverageMapping/decomposition.cpp
b/clang/test/CoverageMapping/decomposition.cpp
index 31bd6cae2c4dec..601ea630faeec9 100644
--- a/clang/test/CoverageMapping/decomposition.cpp
+++ b/clang/test/CoverageMapping/decomposition.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-emit-llvm-only %s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -triple
%itanium_abi_triple -fprofile-instrument=clang -fcoverage-mapping
-dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
// CHECK-LABEL: _Z19array_decompositioni:
// CHECK-NEXT: File 0, [[@LINE+6]]:32 -> {{[0-9]+}}:2 = #0
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: I don't see any difference on your example (with `main()` and function definitions added) with and without my patch neither in the dumped coverage mapping nor in the output of `llvm-cov show ... --show-branches=count --show-expansions -show-line-counts-or-regions` command. If the coverage mapping works with code regions, it looks quite natural that an implicit code (`foo()` call inside `f()` in the sample) has no corresponding region and hence no mapping (except macro expansions). Maybe, @hyp can clarify? https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
efriedma-quic wrote:
Say you have:
```
int foo();
struct A { A(); A(const A&, int = foo()); };
struct B { A a[10]; };
void f(const B& b) { B bb = b; }
```
We want to visit the call to foo(), I think?
https://github.com/llvm/llvm-project/pull/1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
bolshakov-a wrote: Honestly, I'm not very familiar with code coverage technique, but it seems to me that only explicitly written code is relevant for that. "Common expression" is exactly the explicitly written part. "Subexpression" is an implicitly generated per-element initializer which refers to the "common expression" (hence, without this patch, the counters for the conditional operator from the test are duplicated despite it is executed only once per function call). Which observable drawbacks do you expect from not visiting the implicit parts of the initializer? https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
efriedma-quic wrote: I don't think this works correctly? You need to evaluate both the getCommonExpr(), and the getSubExpr(). https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
https://github.com/bolshakov-a edited https://github.com/llvm/llvm-project/pull/1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Andrey Ali Khan Bolshakov (bolshakov-a)
Changes
`ArrayInitLoopExpr` AST node has two occurences of its as-written initializing
expression in its subexpressions through a non-unique `OpaqueValueExpr`. It
causes double-visiting of the initializing expression if not handled
explicitly, as discussed in #85837.
@efriedma-quic
---
Full diff: https://github.com/llvm/llvm-project/pull/1.diff
2 Files Affected:
- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+4)
- (added) clang/test/CoverageMapping/decomposition.cpp (+15)
``diff
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 71215da362d3d0..569fd489dc8baa 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2171,6 +2171,10 @@ struct CounterCoverageMappingBuilder
// propagate counts into them.
}
+ void VisitArrayInitLoopExpr(const ArrayInitLoopExpr *AILE) {
+Visit(AILE->getCommonExpr()->getSourceExpr());
+ }
+
void VisitPseudoObjectExpr(const PseudoObjectExpr *POE) {
// Just visit syntatic expression as this is what users actually write.
VisitStmt(POE->getSyntacticForm());
diff --git a/clang/test/CoverageMapping/decomposition.cpp
b/clang/test/CoverageMapping/decomposition.cpp
new file mode 100644
index 00..31bd6cae2c4dec
--- /dev/null
+++ b/clang/test/CoverageMapping/decomposition.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-emit-llvm-only %s | FileCheck %s
+
+// CHECK-LABEL: _Z19array_decompositioni:
+// CHECK-NEXT: File 0, [[@LINE+6]]:32 -> {{[0-9]+}}:2 = #0
+// CHECK-NEXT: File 0, [[@LINE+8]]:20 -> [[@LINE+8]]:25 = #0
+// CHECK-NEXT: Branch,File 0, [[@LINE+7]]:20 -> [[@LINE+7]]:25 = #1,
(#0 - #1)
+// CHECK-NEXT: Gap,File 0, [[@LINE+6]]:27 -> [[@LINE+6]]:28 = #1
+// CHECK-NEXT: File 0, [[@LINE+5]]:28 -> [[@LINE+5]]:29 = #1
+// CHECK-NEXT: File 0, [[@LINE+4]]:32 -> [[@LINE+4]]:33 = (#0 - #1)
+int array_decomposition(int i) {
+ int a[] = {1, 2, 3};
+ int b[] = {4, 5, 6};
+ auto [x, y, z] = i > 0 ? a : b;
+ return x + y + z;
+}
``
https://github.com/llvm/llvm-project/pull/1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Coverage] Handle array decomposition correctly (PR #88881)
https://github.com/bolshakov-a created
https://github.com/llvm/llvm-project/pull/1
`ArrayInitLoopExpr` AST node has two occurences of its as-written initializing
expression in its subexpressions through a non-unique `OpaqueValueExpr`. It
causes double-visiting of the initializing expression if not handled
explicitly, as discussed in #85837.
@efriedma-quic
>From a025b2a45c2a66595c111262dd43c0890f0d54b6 Mon Sep 17 00:00:00 2001
From: Bolshakov
Date: Tue, 16 Apr 2024 14:21:40 +0300
Subject: [PATCH] [Coverage] Handle array decomposition correctly
`ArrayInitLoopExpr` AST node has two occurences of its as-written
initializing expression in its subexpressions through a non-unique
`OpaqueValueExpr`. It causes double-visiting of the initializing
expression if not handled explicitly, as discussed in #85837.
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 4
clang/test/CoverageMapping/decomposition.cpp | 15 +++
2 files changed, 19 insertions(+)
create mode 100644 clang/test/CoverageMapping/decomposition.cpp
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 71215da362d3d0..569fd489dc8baa 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2171,6 +2171,10 @@ struct CounterCoverageMappingBuilder
// propagate counts into them.
}
+ void VisitArrayInitLoopExpr(const ArrayInitLoopExpr *AILE) {
+Visit(AILE->getCommonExpr()->getSourceExpr());
+ }
+
void VisitPseudoObjectExpr(const PseudoObjectExpr *POE) {
// Just visit syntatic expression as this is what users actually write.
VisitStmt(POE->getSyntacticForm());
diff --git a/clang/test/CoverageMapping/decomposition.cpp
b/clang/test/CoverageMapping/decomposition.cpp
new file mode 100644
index 00..31bd6cae2c4dec
--- /dev/null
+++ b/clang/test/CoverageMapping/decomposition.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-emit-llvm-only %s | FileCheck %s
+
+// CHECK-LABEL: _Z19array_decompositioni:
+// CHECK-NEXT: File 0, [[@LINE+6]]:32 -> {{[0-9]+}}:2 = #0
+// CHECK-NEXT: File 0, [[@LINE+8]]:20 -> [[@LINE+8]]:25 = #0
+// CHECK-NEXT: Branch,File 0, [[@LINE+7]]:20 -> [[@LINE+7]]:25 = #1,
(#0 - #1)
+// CHECK-NEXT: Gap,File 0, [[@LINE+6]]:27 -> [[@LINE+6]]:28 = #1
+// CHECK-NEXT: File 0, [[@LINE+5]]:28 -> [[@LINE+5]]:29 = #1
+// CHECK-NEXT: File 0, [[@LINE+4]]:32 -> [[@LINE+4]]:33 = (#0 - #1)
+int array_decomposition(int i) {
+ int a[] = {1, 2, 3};
+ int b[] = {4, 5, 6};
+ auto [x, y, z] = i > 0 ? a : b;
+ return x + y + z;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
