[clang] [llvm] [modules] Accept equivalent module caches from different symlink (PR #90925)
https://github.com/ellishg closed https://github.com/llvm/llvm-project/pull/90925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [modules] Accept equivalent module caches from different symlink (PR #90925)
https://github.com/ellishg edited https://github.com/llvm/llvm-project/pull/90925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [modules] Accept equivalent module caches from different symlink (PR #90925)
https://github.com/ellishg updated
https://github.com/llvm/llvm-project/pull/90925
>From 4760ebce0ff7725f4bb75f5107f551d867e4db6d Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Thu, 2 May 2024 17:47:38 -0700
Subject: [PATCH 1/4] [modules] Accept equivalent module caches from different
symlink
Use `fs::equivalent()`, which follows symlinks, to check if two module cache
paths are equivalent. This prevents a PCH error when building from a different
path that is a symlink of the original.
```
error: PCH was compiled with module cache path
'/home/foo/blah/ModuleCache/2IBP1TNT8OR8D', but the path is currently
'/data/users/foo/blah/ModuleCache/2IBP1TNT8OR8D'
1 error generated.
```
---
clang/lib/Serialization/ASTReader.cpp | 20 +---
clang/test/Modules/module-symlink.m | 11 +++
2 files changed, 20 insertions(+), 11 deletions(-)
create mode 100644 clang/test/Modules/module-symlink.m
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..c20ead8b865692 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -839,17 +839,15 @@ static bool checkHeaderSearchOptions(const
HeaderSearchOptions ,
DiagnosticsEngine *Diags,
const LangOptions ,
const PreprocessorOptions ) {
- if (LangOpts.Modules) {
-if (SpecificModuleCachePath != ExistingModuleCachePath &&
-!PPOpts.AllowPCHWithDifferentModulesCachePath) {
- if (Diags)
-Diags->Report(diag::err_pch_modulecache_mismatch)
- << SpecificModuleCachePath << ExistingModuleCachePath;
- return true;
-}
- }
-
- return false;
+ if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
+ SpecificModuleCachePath == ExistingModuleCachePath ||
+ llvm::sys::fs::equivalent(SpecificModuleCachePath,
+ExistingModuleCachePath))
+return false;
+ if (Diags)
+Diags->Report(diag::err_pch_modulecache_mismatch)
+<< SpecificModuleCachePath << ExistingModuleCachePath;
+ return true;
}
bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions ,
diff --git a/clang/test/Modules/module-symlink.m
b/clang/test/Modules/module-symlink.m
new file mode 100644
index 00..be447449a0e81e
--- /dev/null
+++ b/clang/test/Modules/module-symlink.m
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules
-fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify
+
+// RUN: ln -s %t/modules %t/modules.symlink
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules.symlink -fmodules
-fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s -verify
+
+// expected-no-diagnostics
+
+@import ignored_macros;
+
+struct Point p;
>From 490eefe98e3dd020ff3e51c7f817ec2b3d3a2663 Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Fri, 3 May 2024 09:50:11 -0700
Subject: [PATCH 2/4] Require shell to fix windows test
---
clang/test/Modules/module-symlink.m | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/Modules/module-symlink.m
b/clang/test/Modules/module-symlink.m
index be447449a0e81e..9a69186c5ea28f 100644
--- a/clang/test/Modules/module-symlink.m
+++ b/clang/test/Modules/module-symlink.m
@@ -1,3 +1,5 @@
+// REQUIRES: shell
+
// RUN: rm -rf %t
// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules
-fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify
>From 6e58177107f854f42d3cdc70e796c425a1797798 Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Fri, 3 May 2024 10:34:35 -0700
Subject: [PATCH 3/4] Use VFS to check if files are equal
---
clang/lib/Serialization/ASTReader.cpp | 25 +++
clang/test/Modules/module-symlink.m | 1 +
llvm/include/llvm/Support/VirtualFileSystem.h | 4 +++
llvm/lib/Support/VirtualFileSystem.cpp| 10
4 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index c20ead8b865692..d35c870926f96e 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -833,16 +833,18 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions(
/// against the header search options in an existing preprocessor.
///
/// \param Diags If non-null, produce diagnostics for any mismatches incurred.
-static bool checkHeaderSearchOptions(const HeaderSearchOptions ,
+static bool checkHeaderSearchOptions(llvm::vfs::FileSystem ,
StringRef SpecificModuleCachePath,
StringRef ExistingModuleCachePath,
DiagnosticsEngine *Diags,
const LangOptions ,
const PreprocessorOptions ) {
if
[clang] [llvm] [modules] Accept equivalent module caches from different symlink (PR #90925)
ellishg wrote: @benlangmuir do you have other concerns? Can I get a stamp? https://github.com/llvm/llvm-project/pull/90925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [modules] Accept equivalent module caches from different symlink (PR #90925)
@@ -839,17 +839,15 @@ static bool checkHeaderSearchOptions(const
HeaderSearchOptions ,
DiagnosticsEngine *Diags,
const LangOptions ,
const PreprocessorOptions ) {
- if (LangOpts.Modules) {
-if (SpecificModuleCachePath != ExistingModuleCachePath &&
-!PPOpts.AllowPCHWithDifferentModulesCachePath) {
- if (Diags)
-Diags->Report(diag::err_pch_modulecache_mismatch)
- << SpecificModuleCachePath << ExistingModuleCachePath;
- return true;
-}
- }
-
- return false;
+ if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
+ SpecificModuleCachePath == ExistingModuleCachePath ||
+ llvm::sys::fs::equivalent(SpecificModuleCachePath,
ellishg wrote:
Thanks for the advice. I realized that this does not handle the case where both
paths are equal, but do not exist. In that case, no error will be thrown. Is
that acceptable? I guess this was the case before this pr.
https://github.com/llvm/llvm-project/pull/90925
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [modules] Accept equivalent module caches from different symlink (PR #90925)
https://github.com/ellishg updated
https://github.com/llvm/llvm-project/pull/90925
>From 4760ebce0ff7725f4bb75f5107f551d867e4db6d Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Thu, 2 May 2024 17:47:38 -0700
Subject: [PATCH 1/3] [modules] Accept equivalent module caches from different
symlink
Use `fs::equivalent()`, which follows symlinks, to check if two module cache
paths are equivalent. This prevents a PCH error when building from a different
path that is a symlink of the original.
```
error: PCH was compiled with module cache path
'/home/foo/blah/ModuleCache/2IBP1TNT8OR8D', but the path is currently
'/data/users/foo/blah/ModuleCache/2IBP1TNT8OR8D'
1 error generated.
```
---
clang/lib/Serialization/ASTReader.cpp | 20 +---
clang/test/Modules/module-symlink.m | 11 +++
2 files changed, 20 insertions(+), 11 deletions(-)
create mode 100644 clang/test/Modules/module-symlink.m
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..c20ead8b865692 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -839,17 +839,15 @@ static bool checkHeaderSearchOptions(const
HeaderSearchOptions ,
DiagnosticsEngine *Diags,
const LangOptions ,
const PreprocessorOptions ) {
- if (LangOpts.Modules) {
-if (SpecificModuleCachePath != ExistingModuleCachePath &&
-!PPOpts.AllowPCHWithDifferentModulesCachePath) {
- if (Diags)
-Diags->Report(diag::err_pch_modulecache_mismatch)
- << SpecificModuleCachePath << ExistingModuleCachePath;
- return true;
-}
- }
-
- return false;
+ if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
+ SpecificModuleCachePath == ExistingModuleCachePath ||
+ llvm::sys::fs::equivalent(SpecificModuleCachePath,
+ExistingModuleCachePath))
+return false;
+ if (Diags)
+Diags->Report(diag::err_pch_modulecache_mismatch)
+<< SpecificModuleCachePath << ExistingModuleCachePath;
+ return true;
}
bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions ,
diff --git a/clang/test/Modules/module-symlink.m
b/clang/test/Modules/module-symlink.m
new file mode 100644
index 00..be447449a0e81e
--- /dev/null
+++ b/clang/test/Modules/module-symlink.m
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules
-fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify
+
+// RUN: ln -s %t/modules %t/modules.symlink
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules.symlink -fmodules
-fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s -verify
+
+// expected-no-diagnostics
+
+@import ignored_macros;
+
+struct Point p;
>From 490eefe98e3dd020ff3e51c7f817ec2b3d3a2663 Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Fri, 3 May 2024 09:50:11 -0700
Subject: [PATCH 2/3] Require shell to fix windows test
---
clang/test/Modules/module-symlink.m | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/Modules/module-symlink.m
b/clang/test/Modules/module-symlink.m
index be447449a0e81e..9a69186c5ea28f 100644
--- a/clang/test/Modules/module-symlink.m
+++ b/clang/test/Modules/module-symlink.m
@@ -1,3 +1,5 @@
+// REQUIRES: shell
+
// RUN: rm -rf %t
// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules
-fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify
>From 6e58177107f854f42d3cdc70e796c425a1797798 Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Fri, 3 May 2024 10:34:35 -0700
Subject: [PATCH 3/3] Use VFS to check if files are equal
---
clang/lib/Serialization/ASTReader.cpp | 25 +++
clang/test/Modules/module-symlink.m | 1 +
llvm/include/llvm/Support/VirtualFileSystem.h | 4 +++
llvm/lib/Support/VirtualFileSystem.cpp| 10
4 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index c20ead8b865692..d35c870926f96e 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -833,16 +833,18 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions(
/// against the header search options in an existing preprocessor.
///
/// \param Diags If non-null, produce diagnostics for any mismatches incurred.
-static bool checkHeaderSearchOptions(const HeaderSearchOptions ,
+static bool checkHeaderSearchOptions(llvm::vfs::FileSystem ,
StringRef SpecificModuleCachePath,
StringRef ExistingModuleCachePath,
DiagnosticsEngine *Diags,
const LangOptions ,
const PreprocessorOptions ) {
if
[clang] [modules] Accept equivalent module caches from different symlink (PR #90925)
https://github.com/ellishg updated
https://github.com/llvm/llvm-project/pull/90925
>From 4760ebce0ff7725f4bb75f5107f551d867e4db6d Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Thu, 2 May 2024 17:47:38 -0700
Subject: [PATCH 1/2] [modules] Accept equivalent module caches from different
symlink
Use `fs::equivalent()`, which follows symlinks, to check if two module cache
paths are equivalent. This prevents a PCH error when building from a different
path that is a symlink of the original.
```
error: PCH was compiled with module cache path
'/home/foo/blah/ModuleCache/2IBP1TNT8OR8D', but the path is currently
'/data/users/foo/blah/ModuleCache/2IBP1TNT8OR8D'
1 error generated.
```
---
clang/lib/Serialization/ASTReader.cpp | 20 +---
clang/test/Modules/module-symlink.m | 11 +++
2 files changed, 20 insertions(+), 11 deletions(-)
create mode 100644 clang/test/Modules/module-symlink.m
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..c20ead8b865692 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -839,17 +839,15 @@ static bool checkHeaderSearchOptions(const
HeaderSearchOptions ,
DiagnosticsEngine *Diags,
const LangOptions ,
const PreprocessorOptions ) {
- if (LangOpts.Modules) {
-if (SpecificModuleCachePath != ExistingModuleCachePath &&
-!PPOpts.AllowPCHWithDifferentModulesCachePath) {
- if (Diags)
-Diags->Report(diag::err_pch_modulecache_mismatch)
- << SpecificModuleCachePath << ExistingModuleCachePath;
- return true;
-}
- }
-
- return false;
+ if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
+ SpecificModuleCachePath == ExistingModuleCachePath ||
+ llvm::sys::fs::equivalent(SpecificModuleCachePath,
+ExistingModuleCachePath))
+return false;
+ if (Diags)
+Diags->Report(diag::err_pch_modulecache_mismatch)
+<< SpecificModuleCachePath << ExistingModuleCachePath;
+ return true;
}
bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions ,
diff --git a/clang/test/Modules/module-symlink.m
b/clang/test/Modules/module-symlink.m
new file mode 100644
index 00..be447449a0e81e
--- /dev/null
+++ b/clang/test/Modules/module-symlink.m
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules
-fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify
+
+// RUN: ln -s %t/modules %t/modules.symlink
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules.symlink -fmodules
-fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s -verify
+
+// expected-no-diagnostics
+
+@import ignored_macros;
+
+struct Point p;
>From 490eefe98e3dd020ff3e51c7f817ec2b3d3a2663 Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Fri, 3 May 2024 09:50:11 -0700
Subject: [PATCH 2/2] Require shell to fix windows test
---
clang/test/Modules/module-symlink.m | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/Modules/module-symlink.m
b/clang/test/Modules/module-symlink.m
index be447449a0e81e..9a69186c5ea28f 100644
--- a/clang/test/Modules/module-symlink.m
+++ b/clang/test/Modules/module-symlink.m
@@ -1,3 +1,5 @@
+// REQUIRES: shell
+
// RUN: rm -rf %t
// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules
-fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [modules] Accept equivalent module caches from different symlink (PR #90925)
https://github.com/ellishg created
https://github.com/llvm/llvm-project/pull/90925
Use `fs::equivalent()`, which follows symlinks, to check if two module cache
paths are equivalent. This prevents a PCH error when building from a different
path that is a symlink of the original.
```
error: PCH was compiled with module cache path
'/home/foo/blah/ModuleCache/2IBP1TNT8OR8D', but the path is currently
'/data/users/foo/blah/ModuleCache/2IBP1TNT8OR8D'
1 error generated.
```
>From 4760ebce0ff7725f4bb75f5107f551d867e4db6d Mon Sep 17 00:00:00 2001
From: Ellis Hoag
Date: Thu, 2 May 2024 17:47:38 -0700
Subject: [PATCH] [modules] Accept equivalent module caches from different
symlink
Use `fs::equivalent()`, which follows symlinks, to check if two module cache
paths are equivalent. This prevents a PCH error when building from a different
path that is a symlink of the original.
```
error: PCH was compiled with module cache path
'/home/foo/blah/ModuleCache/2IBP1TNT8OR8D', but the path is currently
'/data/users/foo/blah/ModuleCache/2IBP1TNT8OR8D'
1 error generated.
```
---
clang/lib/Serialization/ASTReader.cpp | 20 +---
clang/test/Modules/module-symlink.m | 11 +++
2 files changed, 20 insertions(+), 11 deletions(-)
create mode 100644 clang/test/Modules/module-symlink.m
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..c20ead8b865692 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -839,17 +839,15 @@ static bool checkHeaderSearchOptions(const
HeaderSearchOptions ,
DiagnosticsEngine *Diags,
const LangOptions ,
const PreprocessorOptions ) {
- if (LangOpts.Modules) {
-if (SpecificModuleCachePath != ExistingModuleCachePath &&
-!PPOpts.AllowPCHWithDifferentModulesCachePath) {
- if (Diags)
-Diags->Report(diag::err_pch_modulecache_mismatch)
- << SpecificModuleCachePath << ExistingModuleCachePath;
- return true;
-}
- }
-
- return false;
+ if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
+ SpecificModuleCachePath == ExistingModuleCachePath ||
+ llvm::sys::fs::equivalent(SpecificModuleCachePath,
+ExistingModuleCachePath))
+return false;
+ if (Diags)
+Diags->Report(diag::err_pch_modulecache_mismatch)
+<< SpecificModuleCachePath << ExistingModuleCachePath;
+ return true;
}
bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions ,
diff --git a/clang/test/Modules/module-symlink.m
b/clang/test/Modules/module-symlink.m
new file mode 100644
index 00..be447449a0e81e
--- /dev/null
+++ b/clang/test/Modules/module-symlink.m
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules
-fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify
+
+// RUN: ln -s %t/modules %t/modules.symlink
+// RUN: %clang_cc1 -fmodules-cache-path=%t/modules.symlink -fmodules
-fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s -verify
+
+// expected-no-diagnostics
+
+@import ignored_macros;
+
+struct Point p;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] Fix SyntaxWarning messages from python 3.12 (PR #86806)
@@ -101,7 +101,7 @@ def extract_result_types(comment): def strip_doxygen(comment): -"""Returns the given comment without \-escaped words.""" +"""Returns the given comment without \\-escaped words.""" ellishg wrote: I see lots of changes to comment blocks in this patch. Can we use `'''` instead to fix those warnings? https://github.com/llvm/llvm-project/pull/86806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SpecialCaseList] Use glob by default (PR #74809)
ellishg wrote: > A report from the field: we had an ignorelist that contained > `[cfi-vcall|cfi-nvcall|cfi-icall]`. This was recommended syntax from the > documentation > (https://releases.llvm.org/17.0.1/tools/clang/docs/SanitizerSpecialCaseList.html)... > but it broke with the transition. This took a significant effort to track > down. > > > > I see the following issues with the transition: > > > > - There was no release note, or any sort of announcement of this breaking > change. > > - No version of clang produces a diagnostic if you use the affected syntax; > the behavior just silently changes. > > - The proposed transition period is extremely short; it's impossible to write > an ignorelist that works on both clang 17 and clang 19 if you use any > regex/glob features other than "*". I [announced this change on discourse](https://discourse.llvm.org/t/use-glob-instead-of-regex-for-specialcaselists/71666?u=ellishg) last year. I think others have seen this specific bug before, so maybe I should call out this case in that post. https://github.com/llvm/llvm-project/pull/74809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [InstrProf] Single byte counters in coverage (PR #75425)
ellishg wrote: LGTM, but I'm less familiar with the clang coverage code. So I'll give others some time to accept. https://github.com/llvm/llvm-project/pull/75425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [InstrProf] Single byte counters in coverage (PR #75425)
@@ -821,15 +822,23 @@ void InstrProfRecord::merge(InstrProfRecord ,
uint64_t Weight,
for (size_t I = 0, E = Other.Counts.size(); I < E; ++I) {
bool Overflowed;
-uint64_t Value =
-SaturatingMultiplyAdd(Other.Counts[I], Weight, Counts[I], );
-if (Value > getInstrMaxCountValue()) {
- Value = getInstrMaxCountValue();
- Overflowed = true;
+uint64_t Value;
+// When a profile has single byte coverage, use || to merge counters.
+if (HasSingleByteCoverage)
+ Value = Other.Counts[I] || Counts[I];
ellishg wrote:
I'm actually surprised this doesn't break any tests. If not, then I might need
to add some more.
https://github.com/llvm/llvm-project/pull/75425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [InstrProf] Single byte counters in coverage (PR #75425)
@@ -821,15 +822,23 @@ void InstrProfRecord::merge(InstrProfRecord ,
uint64_t Weight,
for (size_t I = 0, E = Other.Counts.size(); I < E; ++I) {
bool Overflowed;
-uint64_t Value =
-SaturatingMultiplyAdd(Other.Counts[I], Weight, Counts[I], );
-if (Value > getInstrMaxCountValue()) {
- Value = getInstrMaxCountValue();
- Overflowed = true;
+uint64_t Value;
+// When a profile has single byte coverage, use || to merge counters.
+if (HasSingleByteCoverage)
+ Value = Other.Counts[I] || Counts[I];
ellishg wrote:
Yes, we actually do use block coverage to collect many raw profiles and
aggregate them into an indexed profile. Since the counts can be larger than
one, it helps differentiate between some hot and cold blocks during
optimization.
https://github.com/llvm/llvm-project/pull/75425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [compiler-rt] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -118,18 +118,18 @@ cl::opt ProfiledBinary(
"profiled-binary", cl::init(""),
cl::desc("Path to binary from which the profile was collected."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt DebugInfoFilename(
-"debug-info", cl::init(""),
+cl::list DebugInfoFilenames(
+"debug-info",
cl::desc(
"For show, read and extract profile metadata from debug info and show "
"the functions it found. For merge, use the provided debug info to "
"correlate the raw profile."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt
-BinaryFilename("binary-file", cl::init(""),
- cl::desc("For merge, use the provided unstripped bianry to "
-"correlate the raw profile."),
- cl::sub(MergeSubcommand));
+cl::list
+BinaryFilenames("binary-file",
+cl::desc("For merge, use the provided unstripped bianry to
"
+ "correlate the raw profile."),
+cl::sub(MergeSubcommand));
ellishg wrote:
Sounds good, let me know how you're experience goes
https://github.com/llvm/llvm-project/pull/75957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang] [clang-tools-extra] [llvm] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -118,18 +118,18 @@ cl::opt ProfiledBinary(
"profiled-binary", cl::init(""),
cl::desc("Path to binary from which the profile was collected."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt DebugInfoFilename(
-"debug-info", cl::init(""),
+cl::list DebugInfoFilenames(
+"debug-info",
cl::desc(
"For show, read and extract profile metadata from debug info and show "
"the functions it found. For merge, use the provided debug info to "
"correlate the raw profile."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt
-BinaryFilename("binary-file", cl::init(""),
- cl::desc("For merge, use the provided unstripped bianry to "
-"correlate the raw profile."),
- cl::sub(MergeSubcommand));
+cl::list
+BinaryFilenames("binary-file",
+cl::desc("For merge, use the provided unstripped bianry to
"
+ "correlate the raw profile."),
+cl::sub(MergeSubcommand));
ellishg wrote:
> > Another option would be to extend the pattern strings to support %b to
> > expand to the binary id or the binary name. Do you think that would work?
>
> If the binary is built with binary id embedded, the raw profiles will also
> have binary id embedded. So there's no need to make `%b` expand to the binary
> id. The point for accepting multiple correlation files is to avoid some
> customizing scripts which associate raw profiles with different binaries.
>
> The output indexed profile file will contain profile information for multiple
> binaries. I'm not sure if this will cause problems/inconvenience for
> processing. What's your thoughts on this @gulfemsavrun @petrhosek?
In my experience, automated profile generation systems already need customized
scripts to gather raw profile data for merging. I found that it isn't hard to
group raw profiles by the binaries they come from and merge profiles for each
binary individually. I would prefer this over making the profile parsing logic
more complicated, which is already difficult to understand.
https://github.com/llvm/llvm-project/pull/75957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SpecialCaseList] Use glob by default (PR #74809)
ellishg wrote: > This caused some ignorelist changes, e.g. > > > > `src:*third_party/vulkan_memory_allocator/include/vk_mem_alloc.h` > > > > didn't work anymore and the opt-out made it work again. Still investigating > why. Not sure if it's the reason, but the `.` in `vk_mem_alloc.h` matches any character for regex, but only the literal for glob (use `?` for this). I think everything else is the same. https://github.com/llvm/llvm-project/pull/74809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -118,18 +118,18 @@ cl::opt ProfiledBinary(
"profiled-binary", cl::init(""),
cl::desc("Path to binary from which the profile was collected."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt DebugInfoFilename(
-"debug-info", cl::init(""),
+cl::list DebugInfoFilenames(
+"debug-info",
cl::desc(
"For show, read and extract profile metadata from debug info and show "
"the functions it found. For merge, use the provided debug info to "
"correlate the raw profile."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt
-BinaryFilename("binary-file", cl::init(""),
- cl::desc("For merge, use the provided unstripped bianry to "
-"correlate the raw profile."),
- cl::sub(MergeSubcommand));
+cl::list
+BinaryFilenames("binary-file",
+cl::desc("For merge, use the provided unstripped bianry to
"
+ "correlate the raw profile."),
+cl::sub(MergeSubcommand));
ellishg wrote:
For convenience, here are the docs:
https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#running-the-instrumented-program
https://github.com/llvm/llvm-project/pull/75957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -118,18 +118,18 @@ cl::opt ProfiledBinary(
"profiled-binary", cl::init(""),
cl::desc("Path to binary from which the profile was collected."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt DebugInfoFilename(
-"debug-info", cl::init(""),
+cl::list DebugInfoFilenames(
+"debug-info",
cl::desc(
"For show, read and extract profile metadata from debug info and show "
"the functions it found. For merge, use the provided debug info to "
"correlate the raw profile."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt
-BinaryFilename("binary-file", cl::init(""),
- cl::desc("For merge, use the provided unstripped bianry to "
-"correlate the raw profile."),
- cl::sub(MergeSubcommand));
+cl::list
+BinaryFilenames("binary-file",
+cl::desc("For merge, use the provided unstripped bianry to
"
+ "correlate the raw profile."),
+cl::sub(MergeSubcommand));
ellishg wrote:
Another option would be to extend the pattern strings to support `%b` to expand
to the binary id or the binary name. Do you think that would work?
https://github.com/llvm/llvm-project/pull/75957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [compiler-rt] [clang] [llvm] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -481,3 +509,49 @@ Error
BinaryInstrProfCorrelator::correlateProfileNameImpl() {
this->Names.append(this->Ctx->NameStart, this->Ctx->NameSize);
return Error::success();
}
+
+llvm::Expected>
InstrProfCorrelators::get(
+ArrayRef>
+CorrelateInputs,
+uint32_t MaxWarnings) {
+ StringMap> CorrelatorMap;
+ StringMap FileMap;
+ auto WarnCounter =
+ std::make_unique(MaxWarnings);
+ std::unique_ptr CorrelateLock = std::make_unique();
+ std::unique_ptr WarnLock = std::make_unique();
+ for (const auto : CorrelateInputs) {
+std::unique_ptr Correlator;
+if (auto Err = InstrProfCorrelator::get(Input.first, Input.second,
+*CorrelateLock.get(),
+*WarnLock.get(), WarnCounter.get())
+ .moveInto(Correlator))
+ return Err;
+std::string BuildID = toHex(Correlator->getBuildID());
+FileMap.try_emplace(BuildID, Input.first);
+bool Inserted =
+CorrelatorMap.try_emplace(BuildID, std::move(Correlator)).second;
+if (!Inserted && WarnCounter->shouldEmitWarning()) {
+ std::lock_guard Guard(*WarnLock);
+ WithColor::warning() << format(
+ "Duplicate build id (%s) found for %s and %s\n", BuildID.c_str(),
+ FileMap[BuildID].str().c_str(), Input.first.str().c_str());
+}
+ }
+ return std::make_unique(
+ std::move(CorrelatorMap), std::move(CorrelateLock), std::move(WarnLock),
+ std::move(WarnCounter));
+}
+
+llvm::Expected
+InstrProfCorrelators::getCorrelator(object::BuildIDRef BuildID) const {
+ std::string BuildIDStr = toHex(BuildID);
+ auto I = CorrelatorMap.find(BuildIDStr);
+ if (I == CorrelatorMap.end())
+return make_error(
+instrprof_error::unable_to_correlate_profile,
+"missing correlator file with build id " + BuildIDStr + "\n");
+ if (auto Err = I->getValue()->correlateProfileData())
ellishg wrote:
We can still process correlation files in parallel without processing them
lazily. In `llvm-profdata.cpp` we would just process all the correlation files
before processing any raw profiles. I guess we would still need an atomic
counter to make sure we don't emit too many warnings.
https://github.com/llvm/llvm-project/pull/75957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [llvm] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -0,0 +1,33 @@ +// REQUIRES: lld-available ellishg wrote: Can we also add a test to `Darwin/instrprof-correlation-mixed.test`? https://github.com/llvm/llvm-project/pull/75957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang-tools-extra] [clang] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -481,3 +509,49 @@ Error
BinaryInstrProfCorrelator::correlateProfileNameImpl() {
this->Names.append(this->Ctx->NameStart, this->Ctx->NameSize);
return Error::success();
}
+
+llvm::Expected>
InstrProfCorrelators::get(
+ArrayRef>
+CorrelateInputs,
+uint32_t MaxWarnings) {
+ StringMap> CorrelatorMap;
+ StringMap FileMap;
+ auto WarnCounter =
+ std::make_unique(MaxWarnings);
+ std::unique_ptr CorrelateLock = std::make_unique();
+ std::unique_ptr WarnLock = std::make_unique();
+ for (const auto : CorrelateInputs) {
+std::unique_ptr Correlator;
+if (auto Err = InstrProfCorrelator::get(Input.first, Input.second,
+*CorrelateLock.get(),
+*WarnLock.get(), WarnCounter.get())
+ .moveInto(Correlator))
+ return Err;
+std::string BuildID = toHex(Correlator->getBuildID());
+FileMap.try_emplace(BuildID, Input.first);
+bool Inserted =
+CorrelatorMap.try_emplace(BuildID, std::move(Correlator)).second;
+if (!Inserted && WarnCounter->shouldEmitWarning()) {
+ std::lock_guard Guard(*WarnLock);
+ WithColor::warning() << format(
+ "Duplicate build id (%s) found for %s and %s\n", BuildID.c_str(),
+ FileMap[BuildID].str().c_str(), Input.first.str().c_str());
+}
+ }
+ return std::make_unique(
+ std::move(CorrelatorMap), std::move(CorrelateLock), std::move(WarnLock),
+ std::move(WarnCounter));
+}
+
+llvm::Expected
+InstrProfCorrelators::getCorrelator(object::BuildIDRef BuildID) const {
+ std::string BuildIDStr = toHex(BuildID);
+ auto I = CorrelatorMap.find(BuildIDStr);
+ if (I == CorrelatorMap.end())
+return make_error(
+instrprof_error::unable_to_correlate_profile,
+"missing correlator file with build id " + BuildIDStr + "\n");
+ if (auto Err = I->getValue()->correlateProfileData())
ellishg wrote:
It seems that we are doing a "lazy correlation" by only calling
`correlateProfileData()` when needed by the `InstrProfReader`. As I understand,
this is the reason that this patch adds atomics and mutexes, which makes this
patch significantly more complicated. Could we avoid this by calling
`correlateProfileData()` once for each debug info/binary file before reading
any raw profiles? I guess this would be wasted work if it turns out we don't
need some debug info/binary file, but the user could easily avoid that wasted
work by not passing in the file in the first place.
https://github.com/llvm/llvm-project/pull/75957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [clang-tools-extra] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -0,0 +1,33 @@ +// REQUIRES: lld-available +// Test llvm-profdata merging with multiple correlation files mixing different correlation modes. + +// RUN: %clang_pgogen -o %t.normal -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp +// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t.normal +// RUN: llvm-profdata merge -o %t.normal.profdata %t.profraw + +// Compiling with differnt configs. ellishg wrote: ```suggestion // Compiling with different configs. ``` https://github.com/llvm/llvm-project/pull/75957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] [compiler-rt] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -118,18 +118,18 @@ cl::opt ProfiledBinary(
"profiled-binary", cl::init(""),
cl::desc("Path to binary from which the profile was collected."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt DebugInfoFilename(
-"debug-info", cl::init(""),
+cl::list DebugInfoFilenames(
+"debug-info",
cl::desc(
"For show, read and extract profile metadata from debug info and show "
"the functions it found. For merge, use the provided debug info to "
"correlate the raw profile."),
cl::sub(ShowSubcommand), cl::sub(MergeSubcommand));
-cl::opt
-BinaryFilename("binary-file", cl::init(""),
- cl::desc("For merge, use the provided unstripped bianry to "
-"correlate the raw profile."),
- cl::sub(MergeSubcommand));
+cl::list
+BinaryFilenames("binary-file",
+cl::desc("For merge, use the provided unstripped bianry to
"
+ "correlate the raw profile."),
+cl::sub(MergeSubcommand));
ellishg wrote:
It looks like we are switching from passing a single correlation file to a list
of correlation files. Is that so that we can pass all raw profiles into a
single `llvm-profdata merge` command? Presumably we should know which binaries
produce which raw profiles. Then we could run an `llvm-profdata merge` command
for each correlation file and its corresponding raw profiles. Ad we can either
merge together all the intermediate indexed profiles, or simply pass the final
indexed profile into all the merge commands. If I remember correctly, the first
merge command can accept an empty indexed profile. Does this patch provide any
advantages over the process I've described?
https://github.com/llvm/llvm-project/pull/75957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [llvm] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -0,0 +1,33 @@ +// REQUIRES: lld-available +// Test llvm-profdata merging with multiple correlation files mixing different correlation modes. + +// RUN: %clang_pgogen -o %t.normal -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp +// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t.normal +// RUN: llvm-profdata merge -o %t.normal.profdata %t.profraw + +// Compiling with differnt configs. +// RUN: %clang_pgogen -o %t -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp -c -o %t-main.o +// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp -c -o %t-main.debug.o +// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-foo.cpp -fpic -shared -Wl,--build-id -o %t-libfoo.debug.so +// RUN: %clang_pgogen -o %t -mllvm -profile-correlate=binary -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-foo.cpp -fpic -shared -Wl,--build-id -o %t-libfoo.binary.so + +// Test mixing default raw profile and lightweight raw profile generated with debug info correlate. +// The raw profiles are mixed in %t.proflite. +// RUN: %clang_pgogen -o %t %t-main.o %t-libfoo.debug.so -Wl,--build-id -o %t +// RUN: env LLVM_PROFILE_FILE=%t.proflite %run %t +// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t-libfoo.debug.so %t.proflite +// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata) +// Two separate raw profiles. +// RUN: rm -rf %t.dir && mkdir %t.dir +// RUN: env LLVM_PROFILE_FILE=%t.dir/raw%m.proflite %run %t +// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t-libfoo.debug.so %t.dir +// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata) + +// Test lightweight raw profiles generated with debug info correlate and binary correlate. +// Note we can not mix different correlation modes in static linking because when merging, the same correlate file can not be used for more than one correaltion mode. ellishg wrote: ```suggestion // Note we can not mix different correlation modes in static linking because when merging, the same correlate file can not be used for more than one correlation mode. ``` https://github.com/llvm/llvm-project/pull/75957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang] [clang-tools-extra] [llvm] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -0,0 +1,33 @@ +// REQUIRES: lld-available ellishg wrote: Why is lld required? https://github.com/llvm/llvm-project/pull/75957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [llvm] [Profile] Allow profile merging with multiple correlate files. (PR #75957)
@@ -25,6 +25,25 @@ // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata) +// Test debug info correlate with build id. ellishg wrote: Can you add a test to `Darwin/instrprof-debug-info-correlate.c` too? https://github.com/llvm/llvm-project/pull/75957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang] [InstrProf] Single byte counters in coverage (PR #75425)
@@ -821,15 +822,23 @@ void InstrProfRecord::merge(InstrProfRecord ,
uint64_t Weight,
for (size_t I = 0, E = Other.Counts.size(); I < E; ++I) {
bool Overflowed;
-uint64_t Value =
-SaturatingMultiplyAdd(Other.Counts[I], Weight, Counts[I], );
-if (Value > getInstrMaxCountValue()) {
- Value = getInstrMaxCountValue();
- Overflowed = true;
+uint64_t Value;
+// When a profile has single byte coverage, use || to merge counters.
+if (HasSingleByteCoverage)
+ Value = Other.Counts[I] || Counts[I];
ellishg wrote:
As I understand, this is needed so that when we merge multiple raw coverage
profiles we get an indexed profile with counters that are either 0 or 1.
If we instead merge by adding counters, we would get technically inaccurate
counts. However, they would be accurate if interpreted for coverage, i.e., 0 is
not covered and >0 is covered. In addition, indexed profiles would have more
information because if a counter is large, we know it is commonly executed
because it was covered in many raw profiles.
Do you think it makes sense to merge these as counters for the indexed profile
and simply interpret them as coverage?
https://github.com/llvm/llvm-project/pull/75425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [compiler-rt] [clang-tools-extra] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh ellishg wrote: I don't think there should be any sharing between `llvm/test` and `compiler-rt/test`. As you can see in `instrprof-basic.c`, it generates raw profiles and then generates and checks the IR to make sure profile data is consumed. `compiler-rt/test` is useful if we need to run code, but it doesn't run on all platforms. If you only need to check IR, it is better to put the test in `llvm/test`. That being said, I wonder if it makes sense to split this into two tests. One in `compiler-rt/test` to make sure raw profiles are generated and consumed correctly, and verifying some IR. The other in `llvm/test` to make sure that IR is correct using a `.proftext` file, which can be tested quickly. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)
ellishg wrote: > Probably would be good to introduce the `-v1` version and require it first, > then eventually change the default - so people don't get a silent behavior > change? Even the existing users only using `*` and `.` need to change their > syntax to migrate to v2, right? They'll need to change `.*` to `*` and `.` to > `?` and if they don't, their matches will start behaving strangely without a > specific/actionable error message? Since `#!special-case-list-v1` is just a comment and `v1` is the default before this change, users can actually add it to the first line today and the behavior won't change until Clang 19. https://github.com/llvm/llvm-project/pull/74809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SpecialCaseList] Use glob by default (PR #74809)
https://github.com/ellishg approved this pull request. Looks good to me. Thanks for following up! https://github.com/llvm/llvm-project/pull/74809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [compiler-rt] [llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh ellishg wrote: Ok so generating the `.profraw` profile is important. I see that currently `update_thinlto_indirect_call_promotion_inputs.sh` is generating the raw profile, which needs to be invoked manually. Instead, can we move this test to `compiler-rt` where test code is actually run? Checkout `instrprof-basic.c` as an example. https://github.com/llvm/llvm-project/blob/49b27b150b97c190dedf8b45bf991c4b811ed953/compiler-rt/test/profile/instrprof-basic.c#L1-L3 https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [compiler-rt] [llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh + +; Do setup work for all below tests: annotate value profiles, generate bitcode and combined index. +; Explicitly turn off ICP pass in Inputs/thinlto_indirect_call_promotion.ll. +; This way ICP happens in %t.bc after _Z11global_funcv and two indirect callees are imported. +; RUN: opt -passes=pgo-instr-use -pgo-test-profile-file=%p/Inputs/thinlto_icall_prom.profdata -module-summary %s -o %t.bc ellishg wrote: I think it's a bit cleaner to use [`split-file`](https://llvm.org/docs/TestingGuide.html#extra-files) and have all the `.ll` code in this file instead of putting different files in `Inputs/`. You can even have the `.proftext` file here if that works. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [compiler-rt] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
ellishg wrote: Please remember to remove the binaries that are not used. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [compiler-rt] [llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh ellishg wrote: Can we create a `.proftext` file and use it for this test instead of generating it from the script above? https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject ,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
-Name.append(FileName.empty() ? "" : FileName);
-Name.append(";");
- }
Mangler().getNameWithPrefix(Name, , /*CannotUsePrivateLabel=*/true);
ellishg wrote:
I think it makes more sense to use linkage-names for IRPGO, `-order_file`, and
ThinLTO. `-order_file` is used in the linker when it only knows linkage-names,
so I don't think it makes sense to feed it mangled names.
https://github.com/llvm/llvm-project/pull/74008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject ,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
-Name.append(FileName.empty() ? "" : FileName);
-Name.append(";");
- }
Mangler().getNameWithPrefix(Name, , /*CannotUsePrivateLabel=*/true);
ellishg wrote:
> Was PGO broken in general before D156569? Or is this specific to
> -symbol-ordering-file and -order_file?
It wasn't broken in general, but it's needed to get `-order_file` working
correctly.
> Also, it looks like some of the main changes here by the mangler for
> objective C are to strip the "\01" prefix. Is this different than the support
> to remove the '\1' mangling escape in getGlobalIdentifier()?
I think both remove the `\01` prefix. But the mangler might also prepend
`_`/`l_` depending on the linkage, and `getGlobalIdentifier()` does not do this.
https://github.com/llvm/llvm-project/pull/74008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject ,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
-Name.append(FileName.empty() ? "" : FileName);
-Name.append(";");
- }
Mangler().getNameWithPrefix(Name, , /*CannotUsePrivateLabel=*/true);
ellishg wrote:
Here is the unittest for these functions which shows some examples. Even for
c++ names there are differences
https://github.com/llvm/llvm-project/blob/70187ebadf20f014a7821cf322eae60192dbe4cc/llvm/unittests/ProfileData/InstrProfTest.cpp#L542-L609
https://github.com/llvm/llvm-project/pull/74008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject ,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
-Name.append(FileName.empty() ? "" : FileName);
-Name.append(";");
- }
Mangler().getNameWithPrefix(Name, , /*CannotUsePrivateLabel=*/true);
ellishg wrote:
I think we can only generate the linkage name at compile time because we need
to know the linkage, which `llvm-profdata` doesn't have.
https://github.com/llvm/llvm-project/pull/74008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject ,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
-Name.append(FileName.empty() ? "" : FileName);
-Name.append(";");
- }
Mangler().getNameWithPrefix(Name, , /*CannotUsePrivateLabel=*/true);
ellishg wrote:
Well, I guess the name is still mangled, but I used
`Mangler().getNameWithPrefix()` to get the linkage name.
https://github.com/llvm/llvm-project/pull/74008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -246,11 +246,27 @@ std::string InstrProfError::message() const {
char InstrProfError::ID = 0;
-std::string getPGOFuncName(StringRef RawFuncName,
- GlobalValue::LinkageTypes Linkage,
+std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage,
ellishg wrote:
I didn't rename `getPGOFuncName()` in https://reviews.llvm.org/D156569 because
Swift and Clang FE-PGO relied on it. Is that still the case?
https://github.com/llvm/llvm-project/pull/74008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject ,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
-Name.append(FileName.empty() ? "" : FileName);
-Name.append(";");
- }
Mangler().getNameWithPrefix(Name, , /*CannotUsePrivateLabel=*/true);
ellishg wrote:
D156569 changes the format from `[:]` to
`[;]`. Note the change from **function-name** to
**linkage-name**. Having the mangled name is required so that we can pass a
symbol order via `-symbol-ordering-file` or `-order_file`.
https://github.com/llvm/llvm-project/pull/74008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC] Fix CSPGO clang pass manager test (PR #72681)
https://github.com/ellishg closed https://github.com/llvm/llvm-project/pull/72681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC] Fix CSPGO clang pass manager test (PR #72681)
https://github.com/ellishg created https://github.com/llvm/llvm-project/pull/72681 Fix a `CHECK-NOT` line in a cspgo clang test >From f548c66f5084e9ce57565b6acc6f6caae0f89a8a Mon Sep 17 00:00:00 2001 From: Ellis Hoag Date: Fri, 17 Nov 2023 09:40:34 -0800 Subject: [PATCH] [NFC] Fix CSPGO clang pass manager test --- clang/test/CodeGen/cspgo-instrumentation_thinlto.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/CodeGen/cspgo-instrumentation_thinlto.c b/clang/test/CodeGen/cspgo-instrumentation_thinlto.c index bf113ae22ca..f79433827ce1745 100644 --- a/clang/test/CodeGen/cspgo-instrumentation_thinlto.c +++ b/clang/test/CodeGen/cspgo-instrumentation_thinlto.c @@ -21,11 +21,11 @@ // Ensure Pass PGOInstrumentationUsePass is invoked Once in PreLink. // RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/cs.profdata %s -flto=thin -fdebug-pass-manager -emit-llvm-bc -o %t/foo_fe_pm.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE // CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE: Running pass: PGOInstrumentationUse -// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-Running pass: PGOInstrumentationUse +// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NOT: Running pass: PGOInstrumentationUse // // RUN: llvm-lto -thinlto -o %t/foo_pm %t/foo_fe_pm.bc // Ensure Pass PGOInstrumentationUSEPass is invoked in PostLink. -// RUN: %clang_cc1 -O2 -x ir %t/foo_fe_pm.bc -fthinlto-index=%t/foo_pm.thinlto.bc -fdebug-pass-manager -fprofile-instrument-use-path=%t/cs.profdata -flto=thin -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST -dump-input=always +// RUN: %clang_cc1 -O2 -x ir %t/foo_fe_pm.bc -fthinlto-index=%t/foo_pm.thinlto.bc -fdebug-pass-manager -fprofile-instrument-use-path=%t/cs.profdata -flto=thin -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST // CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST: Running pass: PGOInstrumentationUse // CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST-NOT: Running pass: PGOInstrumentationUse // ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang] [Profile] Refactor profile correlation. (PR #70856)
https://github.com/ellishg approved this pull request. Sounds fine to me, but I guess I don't understand why `-profile-correlate=` doesn't work. Do you still plan to add the flag later? https://github.com/llvm/llvm-project/pull/70856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Profile] Refactor profile correlation. (PR #69656)
https://github.com/ellishg approved this pull request. https://github.com/llvm/llvm-project/pull/69656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Profile] Refactor profile correlation. (PR #69656)
@@ -24,15 +24,38 @@
using namespace llvm;
-/// Get the __llvm_prf_cnts section.
-Expected getCountersSection(const object::ObjectFile )
{
+namespace llvm {
+// Deprecated. Use -profile-correlate=debug-info.
+cl::opt DebugInfoCorrelate(
+"debug-info-correlate",
+cl::desc("Use debug info to correlate profiles (Deprecated). Use "
+ "-profile-correlate=debug-info instead."),
+cl::init(false));
+
+cl::opt ProfileCorrelate(
+"profile-correlate",
+cl::desc("Use debug info or binary file to correlate profiles."),
+cl::init(InstrProfCorrelator::NONE),
+cl::values(clEnumValN(InstrProfCorrelator::NONE, "",
+ "No profile correlation"),
+ clEnumValN(InstrProfCorrelator::DEBUG_INFO, "debug-info",
+ "Use debug info to correlate")));
ellishg wrote:
I guess the root of the problem is that `Instrumentation` depends on
`ProfileData`, so we can't have `InstrProfCorrelator.cpp` use a symbol defined
in `Instrumentation`. That is unfortunate because `-debug-info-correlate`
(`-profile-correlate=debug-info`) is a codegen flag while
`InstrProfCorrelator.cpp` is primarily used by `llvm-prodata` which will not
use `-debug-info-correlate`.
I really think `-debug-info-correlate` does not belong in
`InstrProfCorrelator.cpp`. I'm fine with keeping
`InstrProfCorrelator::ProfCorrelatorKind` in this file, but the the flags
should stay in `InstrProfiling.cpp` and if needed we can pass
`ProfCorrelatorKind` as a parameter.
https://github.com/llvm/llvm-project/pull/69656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Profile] Refactor profile correlation. (PR #69656)
@@ -55,6 +56,7 @@ #include "llvm/Target/TargetOptions.h" #include "llvm/TargetParser/SubtargetFeature.h" #include "llvm/TargetParser/Triple.h" +#include "llvm/Transforms/HipStdPar/HipStdPar.h" ellishg wrote: Is this include used? https://github.com/llvm/llvm-project/pull/69656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Profile] Refactor profile correlation. (PR #69656)
@@ -24,15 +24,38 @@
using namespace llvm;
-/// Get the __llvm_prf_cnts section.
-Expected getCountersSection(const object::ObjectFile )
{
+namespace llvm {
+// Deprecated. Use -profile-correlate=debug-info.
+cl::opt DebugInfoCorrelate(
+"debug-info-correlate",
+cl::desc("Use debug info to correlate profiles (Deprecated). Use "
+ "-profile-correlate=debug-info instead."),
+cl::init(false));
+
+cl::opt ProfileCorrelate(
+"profile-correlate",
+cl::desc("Use debug info or binary file to correlate profiles."),
+cl::init(InstrProfCorrelator::NONE),
+cl::values(clEnumValN(InstrProfCorrelator::NONE, "",
+ "No profile correlation"),
+ clEnumValN(InstrProfCorrelator::DEBUG_INFO, "debug-info",
+ "Use debug info to correlate")));
ellishg wrote:
This patch moves these options from `InstrProfiling.cpp` to
`InstrProfCorrelator.cpp`, but this file doesn't actually use these flags. Is
there any reason why we can't keep the flags in `InstrProfiling.cpp`?
https://github.com/llvm/llvm-project/pull/69656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Profile] Refactor profile correlation. (PR #69656)
@@ -24,15 +24,37 @@
using namespace llvm;
-/// Get the __llvm_prf_cnts section.
-Expected getCountersSection(const object::ObjectFile )
{
+namespace llvm {
+// Deprecated. Use -profile-correlate=debug-info.
+cl::opt
+DebugInfoCorrelate("profile-correlate=debug-info",
ellishg wrote:
```suggestion
DebugInfoCorrelate("debug-info-correlate",
```
I think you meant to keep `-debug-info-correlate` unchanged. Also, please add
the deprecated comment to the help message.
Maybe after the next LLVM release we can remove this flag. This will allow us
and others to easily migrate to the new flag.
https://github.com/llvm/llvm-project/pull/69656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][InstrProf] Allow absolute path in fun.list of -fprofile-list= (PR #67519)
@@ -139,9 +139,23 @@ std::optional
ProfileList::isFileExcluded(StringRef FileName,
CodeGenOptions::ProfileInstrKind Kind) const {
StringRef Section = getSectionName(Kind);
- // Check for "source:="
+
+ // Convert the input file path to its canonical (absolute) form
+ llvm::SmallString<128> CanonicalFileName(FileName);
+ llvm::sys::fs::make_absolute(CanonicalFileName);
ellishg wrote:
I consider `src`/`!src` to be a deprecated format and users would probably only
use one of the two formats. So I think either case is fine, but this change
seems to make more sense to me.
https://clang.llvm.org/docs/UsersManual.html#older-prefixes
https://github.com/llvm/llvm-project/pull/67519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -60,10 +60,7 @@ using namespace llvm;
#define DEBUG_TYPE "instrprof"
namespace llvm {
-cl::opt
-DebugInfoCorrelate("debug-info-correlate",
- cl::desc("Use debug info to correlate profiles."),
- cl::init(false));
ellishg wrote:
Is it possible to make this an alias to `-profile-correlate=debug-info`? I do
like the new flag better, but I want to make sure we don't break existing code
that relies on this flag. We should give users a chance to migrate.
https://github.com/llvm/llvm-project/pull/69493
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
@@ -0,0 +1,28 @@
+//===- MarkColdFunctions.h - *- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H
+
+#include "llvm/IR/PassManager.h"
+#include "llvm/Support/PGOOptions.h"
+
+namespace llvm {
+
+struct MarkColdFunctionsPass : public PassInfoMixin {
ellishg wrote:
I think this pass could be generalized a bit. It seems that the goal for this
pr is to reduce the size overhead of `-O2` by forcing cold functions to be
`optsize`/`minsize`. Another goal could be to improve the performance of `-Oz`
by forcing very hot functions to be `optsize` (as opposed to `minsize`) or
remove `optsize`/`minsize` from them entirely. This is actually something we
explored in [this paper](https://dl.acm.org/doi/10.1145/3497776.3517764) a
while back and I would eventually like to upstream something similar. I believe
this could be incorporated into this pass, but the name seems to be too vague.
I think "PGO" should to be in the name so users know that it is involved. Maybe
`PGOForceFuncAttrsPass` or `ProfileGuidedFuncAttrsPass`?
CC @kyulee-com
https://github.com/llvm/llvm-project/pull/69030
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
@@ -1127,6 +1134,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, if (EnableSyntheticCounts && !PGOOpt) MPM.addPass(SyntheticCountsPropagation()); + if (EnableMarkColdFunctions && PGOOpt && + (PGOOpt->Action == PGOOptions::SampleUse || + PGOOpt->Action == PGOOptions::IRUse)) ellishg wrote: It seems that this pass isn't enabled for CS profiles. Can they benefit from this pass? https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 8eb3470 - [SpecialCaseList] Add option to use Globs instead of Regex to match patterns
Author: Ellis Hoag
Date: 2023-09-01T09:06:11-07:00
New Revision: 8eb34700c2b1847ec6dfb8f92b305b65278d2ec0
URL:
https://github.com/llvm/llvm-project/commit/8eb34700c2b1847ec6dfb8f92b305b65278d2ec0
DIFF:
https://github.com/llvm/llvm-project/commit/8eb34700c2b1847ec6dfb8f92b305b65278d2ec0.diff
LOG: [SpecialCaseList] Add option to use Globs instead of Regex to match
patterns
Add an option in `SpecialCaseList` to use Globs instead of Regex to match
patterns. `GlobPattern` was extended in https://reviews.llvm.org/D153587 to
support brace expansions which allows us to use patterns like
`*/src/foo.{c,cpp}`. It turns out that most patterns only take advantage of `*`
so using Regex was overkill and required lots of escaping in practice. This
often led to bugs due to forgetting to escape special characters.
Since this would be a breaking change, we temporarily support Regex by default
and use Globs when `#!special-case-list-v2` is the first line in the file.
Users should switch to the glob format described in
https://llvm.org/doxygen/classllvm_1_1GlobPattern.html. For example,
`(abc|def)` should become `{abc,def}`.
See discussion in https://reviews.llvm.org/D152762 and
https://discourse.llvm.org/t/use-glob-instead-of-regex-for-specialcaselists/71666.
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D154014
Added:
Modified:
clang/docs/SanitizerSpecialCaseList.rst
clang/lib/Basic/ProfileList.cpp
clang/lib/Basic/SanitizerSpecialCaseList.cpp
llvm/include/llvm/Support/SpecialCaseList.h
llvm/lib/Support/SpecialCaseList.cpp
llvm/unittests/Support/SpecialCaseListTest.cpp
Removed:
diff --git a/clang/docs/SanitizerSpecialCaseList.rst
b/clang/docs/SanitizerSpecialCaseList.rst
index 15e19b9c129ca46..ab39276b0439577 100644
--- a/clang/docs/SanitizerSpecialCaseList.rst
+++ b/clang/docs/SanitizerSpecialCaseList.rst
@@ -15,7 +15,7 @@ file at compile-time.
Goal and usage
==
-User of sanitizer tools, such as :doc:`AddressSanitizer`,
:doc:`ThreadSanitizer`
+Users of sanitizer tools, such as :doc:`AddressSanitizer`,
:doc:`ThreadSanitizer`
or :doc:`MemorySanitizer` may want to disable or alter some checks for
certain source-level entities to:
@@ -54,37 +54,48 @@ Format
Ignorelists consist of entries, optionally grouped into sections. Empty lines
and lines starting with "#" are ignored.
-Section names are regular expressions written in square brackets that denote
+.. note::
+
+ In `D154014 `_ we transitioned to using
globs instead
+ of regexes to match patterns in special case lists. Since this was a
+ breaking change, we will temporarily support the original behavior using
+ regexes. If ``#!special-case-list-v2`` is the first line of the file, then
+ we will use the new behavior using globs. For more details, see
+ `this discourse post
`_.
+
+
+Section names are globs written in square brackets that denote
which sanitizer the following entries apply to. For example, ``[address]``
-specifies AddressSanitizer while ``[cfi-vcall|cfi-icall]`` specifies Control
+specifies AddressSanitizer while ``[{cfi-vcall,cfi-icall}]`` specifies Control
Flow Integrity virtual and indirect call checking. Entries without a section
will be placed under the ``[*]`` section applying to all enabled sanitizers.
-Entries contain an entity type, followed by a colon and a regular expression,
+Entries contain an entity type, followed by a colon and a glob,
specifying the names of the entities, optionally followed by an equals sign and
-a tool-specific category, e.g. ``fun:*ExampleFunc=example_category``. The
-meaning of ``*`` in regular expression for entity names is
diff erent - it is
-treated as in shell wildcarding. Two generic entity types are ``src`` and
+a tool-specific category, e.g. ``fun:*ExampleFunc=example_category``.
+Two generic entity types are ``src`` and
``fun``, which allow users to specify source files and functions, respectively.
Some sanitizer tools may introduce custom entity types and categories - refer
to
tool-specific docs.
.. code-block:: bash
+#!special-case-list-v2
+# The line above is explained in the note above
# Lines starting with # are ignored.
-# Turn off checks for the source file (use absolute path or path relative
-# to the current working directory):
-src:/path/to/source/file.c
+# Turn off checks for the source file
+# Entries without sections are placed into [*] and apply to all sanitizers
+src:path/to/source/file.c
+src:*/source/file.c
# Turn off checks for this main file, including files included by it.
# Useful when the main file instead of an included file should be ignored.
mainfile:file.c
# Turn off checks for a particular functions
[clang] 9e11d68 - Improve reliability of CompilationDatabaseTest
Author: Ellis Hoag
Date: 2023-08-14T13:46:21-07:00
New Revision: 9e11d6850a5a5a3518f300769724a5c13d2e6ec6
URL:
https://github.com/llvm/llvm-project/commit/9e11d6850a5a5a3518f300769724a5c13d2e6ec6
DIFF:
https://github.com/llvm/llvm-project/commit/9e11d6850a5a5a3518f300769724a5c13d2e6ec6.diff
LOG: Improve reliability of CompilationDatabaseTest
We've seen `CompilationDatabaseTest.cpp` fail because the order of the files
returned by `getAllFiles()` was in a different order than expected. Use
`UnorderedElementsAreArray()` to handle different file orders.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D157904
Added:
Modified:
clang/unittests/Tooling/CompilationDatabaseTest.cpp
Removed:
diff --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index ee91af7a631fe0..5173d472486bfc 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -6,9 +6,6 @@
//
//===--===//
-#include "clang/AST/DeclCXX.h"
-#include "clang/AST/DeclGroup.h"
-#include "clang/Frontend/FrontendAction.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/FileMatchTrie.h"
#include "clang/Tooling/JSONCompilationDatabase.h"
@@ -24,6 +21,8 @@ namespace tooling {
using testing::ElementsAre;
using testing::EndsWith;
+using testing::IsEmpty;
+using testing::UnorderedElementsAreArray;
static void expectFailure(StringRef JSONDatabase, StringRef Explanation) {
std::string ErrorMessage;
@@ -83,8 +82,8 @@ getAllCompileCommands(JSONCommandLineSyntax Syntax, StringRef
JSONDatabase,
TEST(JSONCompilationDatabase, GetAllFiles) {
std::string ErrorMessage;
- EXPECT_EQ(std::vector(),
-getAllFiles("[]", ErrorMessage, JSONCommandLineSyntax::Gnu))
+ EXPECT_THAT(getAllFiles("[]", ErrorMessage, JSONCommandLineSyntax::Gnu),
+ IsEmpty())
<< ErrorMessage;
std::vector expected_files;
@@ -97,8 +96,7 @@ TEST(JSONCompilationDatabase, GetAllFiles) {
expected_files.push_back(std::string(PathStorage.str()));
llvm::sys::path::native("//net/file1", PathStorage);
expected_files.push_back(std::string(PathStorage.str()));
- EXPECT_EQ(expected_files,
-getAllFiles(R"json(
+ EXPECT_THAT(getAllFiles(R"json(
[
{
"directory": "//net/dir",
@@ -121,7 +119,8 @@ TEST(JSONCompilationDatabase, GetAllFiles) {
"file": "//net/dir/foo/../file3"
}
])json",
-ErrorMessage, JSONCommandLineSyntax::Gnu))
+ ErrorMessage, JSONCommandLineSyntax::Gnu),
+ UnorderedElementsAreArray(expected_files))
<< ErrorMessage;
}
@@ -550,7 +549,7 @@ TEST(FixedCompilationDatabase, GetAllFiles) {
CommandLine.push_back("two");
FixedCompilationDatabase Database(".", CommandLine);
- EXPECT_EQ(0ul, Database.getAllFiles().size());
+ EXPECT_THAT(Database.getAllFiles(), IsEmpty());
}
TEST(FixedCompilationDatabase, GetAllCompileCommands) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] bf8fe1c - Fix clang driver tests for cspgo in lld
Author: Ellis Hoag Date: 2023-05-31T18:21:41-07:00 New Revision: bf8fe1c38f1031c88c80e0e86ffea4375e7693ff URL: https://github.com/llvm/llvm-project/commit/bf8fe1c38f1031c88c80e0e86ffea4375e7693ff DIFF: https://github.com/llvm/llvm-project/commit/bf8fe1c38f1031c88c80e0e86ffea4375e7693ff.diff LOG: Fix clang driver tests for cspgo in lld The tests introduced by https://reviews.llvm.org/D151589 were failing because I guess some test platforms don't have `lld`. Similar tests add `-B%S/Inputs/lld` to the clang commands so lets try this here to fix the tests. ``` clang: error: invalid linker name in argument '-fuse-ld=lld' ``` Added: Modified: clang/test/Driver/cspgo-lto.c Removed: diff --git a/clang/test/Driver/cspgo-lto.c b/clang/test/Driver/cspgo-lto.c index a22b2f83c4aa..232d21bbf948 100644 --- a/clang/test/Driver/cspgo-lto.c +++ b/clang/test/Driver/cspgo-lto.c @@ -5,14 +5,14 @@ // CHECK: -plugin-opt=cs-profile-path=default.profdata -// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -fprofile-use 2>&1 | FileCheck %s --check-prefix=DARWIN-USE1 -// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -fprofile-use=a.profdata 2>&1 | FileCheck %s --check-prefix=DARWIN-USE2 +// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -B%S/Inputs/lld -fprofile-use 2>&1 | FileCheck %s --check-prefix=DARWIN-USE1 +// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -B%S/Inputs/lld -fprofile-use=a.profdata 2>&1 | FileCheck %s --check-prefix=DARWIN-USE2 // DARWIN-USE1: "--cs-profile-path=default.profdata" // DARWIN-USE2: "--cs-profile-path=a.profdata" -// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -fcs-profile-generate 2>&1 | FileCheck %s --check-prefix=DARWIN-GEN1 -// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -fcs-profile-generate=directory 2>&1 | FileCheck %s --check-prefix=DARWIN-GEN2 +// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -B%S/Inputs/lld -fcs-profile-generate 2>&1 | FileCheck %s --check-prefix=DARWIN-GEN1 +// RUN: %clang --target=apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -B%S/Inputs/lld -fcs-profile-generate=directory 2>&1 | FileCheck %s --check-prefix=DARWIN-GEN2 // DARWIN-GEN1: "--cs-profile-generate" // DARWIN-GEN1-SAME: "--cs-profile-path=default_%m.profraw" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 85af42d - [lld] add context-sensitive PGO options for MachO
Author: Ellis Hoag
Date: 2023-05-31T17:53:46-07:00
New Revision: 85af42df5dbb964d767feb16a5551dddb36fd4f1
URL:
https://github.com/llvm/llvm-project/commit/85af42df5dbb964d767feb16a5551dddb36fd4f1
DIFF:
https://github.com/llvm/llvm-project/commit/85af42df5dbb964d767feb16a5551dddb36fd4f1.diff
LOG: [lld] add context-sensitive PGO options for MachO
Enable support for CSPGO for lld MachO targets.
Since lld MachO does not support `-plugin-opt=`, we need to create the
`--cs-profile-generate` and `--cs-profile-path=` options and propagate them in
`Darwin.cpp`. These flags are not supported by ld64.
Also outline code into `getLastCSProfileGenerateArg()` to share between
`CommonArgs.cpp` and `Darwin.cpp`.
CSPGO is already implemented for ELF (https://reviews.llvm.org/D56675) and COFF
(https://reviews.llvm.org/D98763).
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D151589
Added:
lld/test/MachO/cspgo-gen.ll
lld/test/MachO/cspgo-use.ll
Modified:
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/CommonArgs.h
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/cspgo-lto.c
lld/MachO/Config.h
lld/MachO/Driver.cpp
lld/MachO/LTO.cpp
lld/MachO/Options.td
lld/test/lit.cfg.py
Removed:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp
b/clang/lib/Driver/ToolChains/Clang.cpp
index e22c2ce7f2ede..02eec4d59b620 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -716,12 +716,7 @@ static void addPGOAndCoverageFlags(const ToolChain ,
Compilation ,
PGOGenerateArg->getOption().matches(options::OPT_fno_profile_generate))
PGOGenerateArg = nullptr;
- auto *CSPGOGenerateArg = Args.getLastArg(options::OPT_fcs_profile_generate,
-
options::OPT_fcs_profile_generate_EQ,
- options::OPT_fno_profile_generate);
- if (CSPGOGenerateArg &&
- CSPGOGenerateArg->getOption().matches(options::OPT_fno_profile_generate))
-CSPGOGenerateArg = nullptr;
+ auto *CSPGOGenerateArg = getLastCSProfileGenerateArg(Args);
auto *ProfileGenerateArg = Args.getLastArg(
options::OPT_fprofile_instr_generate,
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 57bf345f1708e..a952fdbacb386 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -772,16 +772,7 @@ void tools::addLTOOptions(const ToolChain ,
const ArgList ,
"sample-profile=" + FName));
}
- auto *CSPGOGenerateArg = Args.getLastArg(options::OPT_fcs_profile_generate,
-
options::OPT_fcs_profile_generate_EQ,
- options::OPT_fno_profile_generate);
- if (CSPGOGenerateArg &&
- CSPGOGenerateArg->getOption().matches(options::OPT_fno_profile_generate))
-CSPGOGenerateArg = nullptr;
-
- auto *ProfileUseArg = getLastProfileUseArg(Args);
-
- if (CSPGOGenerateArg) {
+ if (auto *CSPGOGenerateArg = getLastCSProfileGenerateArg(Args)) {
CmdArgs.push_back(Args.MakeArgString(Twine(PluginOptPrefix) + ExtraDash +
"cs-profile-generate"));
if (CSPGOGenerateArg->getOption().matches(
@@ -794,7 +785,7 @@ void tools::addLTOOptions(const ToolChain , const
ArgList ,
CmdArgs.push_back(
Args.MakeArgString(Twine(PluginOptPrefix) + ExtraDash +
"cs-profile-path=default_%m.profraw"));
- } else if (ProfileUseArg) {
+ } else if (auto *ProfileUseArg = getLastProfileUseArg(Args)) {
SmallString<128> Path(
ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
if (Path.empty() || llvm::sys::fs::is_directory(Path))
@@ -1348,6 +1339,17 @@ void tools::claimNoWarnArgs(const ArgList ) {
Args.ClaimAllArgs(options::OPT_fno_lto);
}
+Arg *tools::getLastCSProfileGenerateArg(const ArgList ) {
+ auto *CSPGOGenerateArg = Args.getLastArg(options::OPT_fcs_profile_generate,
+
options::OPT_fcs_profile_generate_EQ,
+ options::OPT_fno_profile_generate);
+ if (CSPGOGenerateArg &&
+ CSPGOGenerateArg->getOption().matches(options::OPT_fno_profile_generate))
+CSPGOGenerateArg = nullptr;
+
+ return CSPGOGenerateArg;
+}
+
Arg *tools::getLastProfileUseArg(const ArgList ) {
auto *ProfileUseArg = Args.getLastArg(
options::OPT_fprofile_instr_use, options::OPT_fprofile_instr_use_EQ,
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.h
b/clang/lib/Driver/ToolChains/CommonArgs.h
index c196cbc28e218..66948f3f586ba 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.h
+++
[clang] 970e1ea - [clang] Fix crash with -funique-internal-linkage-names
Author: Ellis Hoag
Date: 2022-10-17T08:57:23-07:00
New Revision: 970e1ea01aa0dce4f606eee1610c92b8b838f303
URL:
https://github.com/llvm/llvm-project/commit/970e1ea01aa0dce4f606eee1610c92b8b838f303
DIFF:
https://github.com/llvm/llvm-project/commit/970e1ea01aa0dce4f606eee1610c92b8b838f303.diff
LOG: [clang] Fix crash with -funique-internal-linkage-names
Calling `getFunctionLinkage(CalleeInfo.getCalleeDecl())` will crash when the
declaration does not have a body, e.g., `extern void foo();`. Instead, we can
use `isExternallyVisible()` to see if the delcaration has internal linkage.
I believe using `!isExternallyVisible()` is correct because the clang linkage
must be `InternalLinkage` or `UniqueExternalLinkage`, both of which are
"internal linkage" in llvm.
https://github.com/llvm/llvm-project/blob/9c26f51f5e178ac0fda98419e3a61d205d3b58b1/clang/include/clang/Basic/Linkage.h#L28-L40
Fixes https://github.com/llvm/llvm-project/issues/54139
Reviewed By: tmsriram
Differential Revision: https://reviews.llvm.org/D135926
Added:
clang/test/CodeGen/unique-internal-linkage-names.c
Modified:
clang/lib/CodeGen/CGCall.cpp
Removed:
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index a59d702faaa2..8cea9f3397b6 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2264,9 +2264,8 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
// Add "sample-profile-suffix-elision-policy" attribute for internal linkage
// functions with -funique-internal-linkage-names.
if (TargetDecl && CodeGenOpts.UniqueInternalLinkageNames) {
-if (isa(TargetDecl)) {
- if (this->getFunctionLinkage(CalleeInfo.getCalleeDecl()) ==
- llvm::GlobalValue::InternalLinkage)
+if (const auto *FD = dyn_cast_or_null(TargetDecl)) {
+ if (!FD->isExternallyVisible())
FuncAttrs.addAttribute("sample-profile-suffix-elision-policy",
"selected");
}
diff --git a/clang/test/CodeGen/unique-internal-linkage-names.c
b/clang/test/CodeGen/unique-internal-linkage-names.c
new file mode 100644
index ..6f8f243ef14a
--- /dev/null
+++ b/clang/test/CodeGen/unique-internal-linkage-names.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -S -emit-llvm -funique-internal-linkage-names -o - |
FileCheck %s
+
+// Check that we do not crash when overloading extern functions.
+
+inline void overloaded_external() {}
+extern void overloaded_external();
+
+// CHECK: define internal void @overloaded_internal() [[ATTR:#[0-9]+]] {
+static void overloaded_internal() {}
+extern void overloaded_internal();
+
+void markUsed() {
+ overloaded_external();
+ overloaded_internal();
+}
+
+// CHECK: attributes [[ATTR]] =
+// CHECK-SAME: "sample-profile-suffix-elision-policy"="selected"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6f4c3c0 - [InstrProf][attempt 2] Add new format for -fprofile-list=
Author: Ellis Hoag Date: 2022-08-04T17:12:56-07:00 New Revision: 6f4c3c0f6463880b685bfbca1932c06fd0c1f015 URL: https://github.com/llvm/llvm-project/commit/6f4c3c0f6463880b685bfbca1932c06fd0c1f015 DIFF: https://github.com/llvm/llvm-project/commit/6f4c3c0f6463880b685bfbca1932c06fd0c1f015.diff LOG: [InstrProf][attempt 2] Add new format for -fprofile-list= In D130807 we added the `skipprofile` attribute. This commit changes the format so we can either `forbid` or `skip` profiling functions by adding the `noprofile` or `skipprofile` attributes, respectively. The behavior of the original format remains unchanged. Also, add the `skipprofile` attribute when using `-fprofile-function-groups`. This was originally landed as https://reviews.llvm.org/D130808 but was reverted due to a Windows test failure. Differential Revision: https://reviews.llvm.org/D131195 Added: clang/test/CodeGen/profile-filter-new.c Modified: clang/docs/UsersManual.rst clang/include/clang/Basic/ProfileList.h clang/lib/Basic/ProfileList.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h clang/test/CodeGen/profile-function-groups.c Removed: diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index 15df488e802de..f9ccca65f3889 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -2500,43 +2500,66 @@ This can be done using the ``-fprofile-list`` option. .. code-block:: console -$ echo "fun:test" > fun.list $ clang++ -O2 -fprofile-instr-generate -fprofile-list=fun.list code.cc -o code -The option can be specified multiple times to pass multiple files. + The option can be specified multiple times to pass multiple files. -.. code-block:: console + .. code-block:: console + +$ clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code + +Supported sections are ``[clang]``, ``[llvm]``, and ``[csllvm]`` representing +clang PGO, IRPGO, and CSIRPGO, respectively. Supported prefixes are ``function`` +and ``source``. Supported categories are ``allow``, ``skip``, and ``forbid``. +``skip`` adds the ``skipprofile`` attribute while ``forbid`` adds the +``noprofile`` attribute to the appropriate function. Use +``default:`` to specify the default category. + + .. code-block:: console + +$ cat fun.list +# The following cases are for clang instrumentation. +[clang] + +# We might not want to profile functions that are inlined in many places. +function:inlinedLots=skip + +# We want to forbid profiling where it might be dangerous. +source:lib/unsafe/*.cc=forbid -$ echo "!fun:*test*" > fun.list -$ echo "src:code.cc" > src.list -% clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code +# Otherwise we allow profiling. +default:allow -To filter individual functions or entire source files using ``fun:`` or -``src:`` respectively. To exclude a function or a source file, use -``!fun:`` or ``!src:`` respectively. The format also supports -wildcard expansion. The compiler generated functions are assumed to be located -in the main source file. It is also possible to restrict the filter to a -particular instrumentation type by using a named section. +Older Prefixes +"" + An older format is also supported, but it is only able to add the + ``noprofile`` attribute. + To filter individual functions or entire source files use ``fun:`` or + ``src:`` respectively. To exclude a function or a source file, use + ``!fun:`` or ``!src:`` respectively. The format also supports + wildcard expansion. The compiler generated functions are assumed to be located + in the main source file. It is also possible to restrict the filter to a + particular instrumentation type by using a named section. -.. code-block:: none + .. code-block:: none - # all functions whose name starts with foo will be instrumented. - fun:foo* +# all functions whose name starts with foo will be instrumented. +fun:foo* - # except for foo1 which will be excluded from instrumentation. - !fun:foo1 +# except for foo1 which will be excluded from instrumentation. +!fun:foo1 - # every function in path/to/foo.cc will be instrumented. - src:path/to/foo.cc +# every function in path/to/foo.cc will be instrumented. +src:path/to/foo.cc - # bar will be instrumented only when using backend instrumentation. - # Recognized section names are clang, llvm and csllvm. - [llvm] - fun:bar +# bar will be instrumented only when using backend instrumentation. +# Recognized section names are clang, llvm and csllvm. +[llvm] +fun:bar -When the file contains only excludes, all files and functions except for the -excluded ones will be
[clang] b692312 - [InstrProf] Add new format for -fprofile-list=
Author: Ellis Hoag Date: 2022-08-04T08:49:43-07:00 New Revision: b692312ca432d9a379f67a8d83177a6f1722baaa URL: https://github.com/llvm/llvm-project/commit/b692312ca432d9a379f67a8d83177a6f1722baaa DIFF: https://github.com/llvm/llvm-project/commit/b692312ca432d9a379f67a8d83177a6f1722baaa.diff LOG: [InstrProf] Add new format for -fprofile-list= In D130807 we added the `skipprofile` attribute. This commit changes the format so we can either `forbid` or `skip` profiling functions by adding the `noprofile` or `skipprofile` attributes, respectively. The behavior of the original format remains unchanged. Also, add the `skipprofile` attribute when using `-fprofile-function-groups`. Reviewed By: phosek Differential Revision: https://reviews.llvm.org/D130808 Added: clang/test/CodeGen/profile-filter-new.c Modified: clang/docs/UsersManual.rst clang/include/clang/Basic/ProfileList.h clang/lib/Basic/ProfileList.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h clang/test/CodeGen/profile-function-groups.c Removed: diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index 15df488e802de..f9ccca65f3889 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -2500,43 +2500,66 @@ This can be done using the ``-fprofile-list`` option. .. code-block:: console -$ echo "fun:test" > fun.list $ clang++ -O2 -fprofile-instr-generate -fprofile-list=fun.list code.cc -o code -The option can be specified multiple times to pass multiple files. + The option can be specified multiple times to pass multiple files. -.. code-block:: console + .. code-block:: console + +$ clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code + +Supported sections are ``[clang]``, ``[llvm]``, and ``[csllvm]`` representing +clang PGO, IRPGO, and CSIRPGO, respectively. Supported prefixes are ``function`` +and ``source``. Supported categories are ``allow``, ``skip``, and ``forbid``. +``skip`` adds the ``skipprofile`` attribute while ``forbid`` adds the +``noprofile`` attribute to the appropriate function. Use +``default:`` to specify the default category. + + .. code-block:: console + +$ cat fun.list +# The following cases are for clang instrumentation. +[clang] + +# We might not want to profile functions that are inlined in many places. +function:inlinedLots=skip + +# We want to forbid profiling where it might be dangerous. +source:lib/unsafe/*.cc=forbid -$ echo "!fun:*test*" > fun.list -$ echo "src:code.cc" > src.list -% clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code +# Otherwise we allow profiling. +default:allow -To filter individual functions or entire source files using ``fun:`` or -``src:`` respectively. To exclude a function or a source file, use -``!fun:`` or ``!src:`` respectively. The format also supports -wildcard expansion. The compiler generated functions are assumed to be located -in the main source file. It is also possible to restrict the filter to a -particular instrumentation type by using a named section. +Older Prefixes +"" + An older format is also supported, but it is only able to add the + ``noprofile`` attribute. + To filter individual functions or entire source files use ``fun:`` or + ``src:`` respectively. To exclude a function or a source file, use + ``!fun:`` or ``!src:`` respectively. The format also supports + wildcard expansion. The compiler generated functions are assumed to be located + in the main source file. It is also possible to restrict the filter to a + particular instrumentation type by using a named section. -.. code-block:: none + .. code-block:: none - # all functions whose name starts with foo will be instrumented. - fun:foo* +# all functions whose name starts with foo will be instrumented. +fun:foo* - # except for foo1 which will be excluded from instrumentation. - !fun:foo1 +# except for foo1 which will be excluded from instrumentation. +!fun:foo1 - # every function in path/to/foo.cc will be instrumented. - src:path/to/foo.cc +# every function in path/to/foo.cc will be instrumented. +src:path/to/foo.cc - # bar will be instrumented only when using backend instrumentation. - # Recognized section names are clang, llvm and csllvm. - [llvm] - fun:bar +# bar will be instrumented only when using backend instrumentation. +# Recognized section names are clang, llvm and csllvm. +[llvm] +fun:bar -When the file contains only excludes, all files and functions except for the -excluded ones will be instrumented. Otherwise, only the files and functions -specified will be instrumented. + When the
[clang] 12e78ff - [InstrProf] Add the skipprofile attribute
Author: Ellis Hoag
Date: 2022-08-04T08:45:27-07:00
New Revision: 12e78ff88105f2dc6cb1449d6fcd5d8f69e0512f
URL:
https://github.com/llvm/llvm-project/commit/12e78ff88105f2dc6cb1449d6fcd5d8f69e0512f
DIFF:
https://github.com/llvm/llvm-project/commit/12e78ff88105f2dc6cb1449d6fcd5d8f69e0512f.diff
LOG: [InstrProf] Add the skipprofile attribute
As discussed in [0], this diff adds the `skipprofile` attribute to
prevent the function from being profiled while allowing profiled
functions to be inlined into it. The `noprofile` attribute remains
unchanged.
The `noprofile` attribute is used for functions where it is
dangerous to add instrumentation to while the `skipprofile` attribute is
used to reduce code size or performance overhead.
[0]
https://discourse.llvm.org/t/why-does-the-noprofile-attribute-restrict-inlining/64108
Reviewed By: phosek
Differential Revision: https://reviews.llvm.org/D130807
Added:
Modified:
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenPGO.cpp
clang/test/CodeGen/profile-function-groups.c
llvm/docs/LangRef.rst
llvm/include/llvm/Bitcode/LLVMBitCodes.h
llvm/include/llvm/IR/Attributes.td
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/test/Bitcode/attributes.ll
llvm/test/Transforms/GCOVProfiling/noprofile.ll
llvm/test/Transforms/PGOProfile/noprofile.ll
Removed:
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index b61a2a68687d9..30cf162e36206 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1522,7 +1522,8 @@ class CodeGenFunction : public CodeGenTypeCache {
/// If \p StepV is null, the default increment is 1.
void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) {
if (CGM.getCodeGenOpts().hasProfileClangInstr() &&
-!CurFn->hasFnAttribute(llvm::Attribute::NoProfile))
+!CurFn->hasFnAttribute(llvm::Attribute::NoProfile) &&
+!CurFn->hasFnAttribute(llvm::Attribute::SkipProfile))
PGO.emitCounterIncrement(Builder, S, StepV);
PGO.setCurrentStmt(S);
}
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp
b/clang/lib/CodeGen/CodeGenPGO.cpp
index 587bcef78ee51..03531e32f1267 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -822,6 +822,8 @@ void CodeGenPGO::assignRegionCounters(GlobalDecl GD,
llvm::Function *Fn) {
CGM.ClearUnusedCoverageMapping(D);
if (Fn->hasFnAttribute(llvm::Attribute::NoProfile))
return;
+ if (Fn->hasFnAttribute(llvm::Attribute::SkipProfile))
+return;
setFuncName(Fn);
diff --git a/clang/test/CodeGen/profile-function-groups.c
b/clang/test/CodeGen/profile-function-groups.c
index 9e04c9060df48..232abd767a8fd 100644
--- a/clang/test/CodeGen/profile-function-groups.c
+++ b/clang/test/CodeGen/profile-function-groups.c
@@ -1,9 +1,9 @@
-// RUN: %clang -fprofile-generate -fprofile-function-groups=3
-fprofile-selected-function-group=0 -emit-llvm -S %s -o - | FileCheck %s
--check-prefixes=CHECK,SELECT0
-// RUN: %clang -fprofile-generate -fprofile-function-groups=3
-fprofile-selected-function-group=1 -emit-llvm -S %s -o - | FileCheck %s
--check-prefixes=CHECK,SELECT1
-// RUN: %clang -fprofile-generate -fprofile-function-groups=3
-fprofile-selected-function-group=2 -emit-llvm -S %s -o - | FileCheck %s
--check-prefixes=CHECK,SELECT2
+// RUN: %clang -fprofile-generate -fprofile-function-groups=3
-fprofile-selected-function-group=0 -emit-llvm -S %s -o - | FileCheck %s
--implicit-check-not="; {{.* (noprofile|skipprofile)}}"
--check-prefixes=CHECK,SELECT0
+// RUN: %clang -fprofile-generate -fprofile-function-groups=3
-fprofile-selected-function-group=1 -emit-llvm -S %s -o - | FileCheck %s
--implicit-check-not="; {{.* (noprofile|skipprofile)}}"
--check-prefixes=CHECK,SELECT1
+// RUN: %clang -fprofile-generate -fprofile-function-groups=3
-fprofile-selected-function-group=2 -emit-llvm -S %s -o - | FileCheck %s
--implicit-check-not="; {{.* (noprofile|skipprofile)}}"
--check-prefixes=CHECK,SELECT2
// Group 0
-// SELECT0-NOT: noprofile
+
// SELECT1: noprofile
// SELECT2: noprofile
// CHECK: define {{.*}} @hoo()
@@ -11,7 +11,7 @@ void hoo() {}
// Group 1
// SELECT0: noprofile
-// SELECT1-NOT: noprofile
+
// SELECT2: noprofile
// CHECK: define {{.*}} @goo()
void goo() {}
@@ -19,6 +19,6 @@ void goo() {}
// Group 2
// SELECT0: noprofile
// SELECT1: noprofile
-// SELECT2-NOT: noprofile
+
// CHECK: define {{.*}} @boo()
void boo() {}
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 2abe628cee8f2..a3c0824f16c6c 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1803,8
[clang] af58684 - [InstrProf] Add options to profile function groups
Author: Ellis Hoag Date: 2022-07-14T11:41:30-07:00 New Revision: af58684f272046f293a9f469f03d23bd2b138349 URL: https://github.com/llvm/llvm-project/commit/af58684f272046f293a9f469f03d23bd2b138349 DIFF: https://github.com/llvm/llvm-project/commit/af58684f272046f293a9f469f03d23bd2b138349.diff LOG: [InstrProf] Add options to profile function groups Add two options, `-fprofile-function-groups=N` and `-fprofile-selected-function-group=i` used to partition functions into `N` groups and only instrument the functions in group `i`. Similar options were added to xray in https://reviews.llvm.org/D87953 and the goal is the same; to reduce instrumented size overhead by spreading the overhead across multiple builds. Raw profiles from different groups can be added like normal using the `llvm-profdata merge` command. Reviewed By: ianlevesque Differential Revision: https://reviews.llvm.org/D129594 Added: clang/test/CodeGen/profile-function-groups.c compiler-rt/test/profile/instrprof-groups.c Modified: clang/docs/ClangCommandLineReference.rst clang/docs/UsersManual.rst clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/Options.td clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h clang/lib/Driver/ToolChains/Clang.cpp Removed: diff --git a/clang/docs/ClangCommandLineReference.rst b/clang/docs/ClangCommandLineReference.rst index 776b84da96572..216872b60cdc8 100644 --- a/clang/docs/ClangCommandLineReference.rst +++ b/clang/docs/ClangCommandLineReference.rst @@ -2329,6 +2329,10 @@ Use instrumentation data for profile-guided optimization Filename defining the list of functions/files to instrument +.. option:: -fprofile-function-groups=, -fprofile-selected-function-group= + +Partition functions into groups and select only functions in group to be instrumented + .. option:: -fprofile-remapping-file= Use the remappings described in to match the profile data against names in the program diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index e12dc72407b13..c2767d65adbf0 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -2513,6 +2513,32 @@ When the file contains only excludes, all files and functions except for the excluded ones will be instrumented. Otherwise, only the files and functions specified will be instrumented. +Instrument function groups +^^ + +Sometimes it is desirable to minimize the size overhead of instrumented +binaries. One way to do this is to partition functions into groups and only +instrument functions in a specified group. This can be done using the +`-fprofile-function-groups` and `-fprofile-selected-function-group` options. + +.. option:: -fprofile-function-groups=, -fprofile-selected-function-group= + + The following uses 3 groups + + .. code-block:: console + +$ clang++ -Oz -fprofile-generate=group_0/ -fprofile-function-groups=3 -fprofile-selected-function-group=0 code.cc -o code.0 +$ clang++ -Oz -fprofile-generate=group_1/ -fprofile-function-groups=3 -fprofile-selected-function-group=1 code.cc -o code.1 +$ clang++ -Oz -fprofile-generate=group_2/ -fprofile-function-groups=3 -fprofile-selected-function-group=2 code.cc -o code.2 + + After collecting raw profiles from the three binaries, they can be merged into + a single profile like normal. + + .. code-block:: console + +$ llvm-profdata merge -output=code.profdata group_*/*.profraw + + Profile remapping ^ diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index b1d394edd04ab..ef7957979dccd 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -213,6 +213,10 @@ CODEGENOPT(AtomicProfileUpdate , 1, 0) ///< Set -fprofile-update=atomic ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 2, ProfileNone) /// Choose profile kind for PGO use compilation. ENUM_CODEGENOPT(ProfileUse, ProfileInstrKind, 2, ProfileNone) +/// Partition functions into N groups and select only functions in group i to be +/// instrumented. Selected group numbers can be 0 to N-1 inclusive. +VALUE_CODEGENOPT(ProfileTotalFunctionGroups, 32, 1) +VALUE_CODEGENOPT(ProfileSelectedFunctionGroup, 32, 0) CODEGENOPT(CoverageMapping , 1, 0) ///< Generate coverage mapping regions to ///< enable code coverage analysis. CODEGENOPT(DumpCoverageMapping , 1, 0) ///< Dump the generated coverage mapping diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 532d7780c529b..404effb4e1de4 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1333,6 +1333,15 @@ def fprofile_list_EQ : Joined<["-"], "fprofile-list=">,
[clang] 4f61749 - [clang] support relative roots to vfs overlays
Author: Richard Howell
Date: 2022-01-19T10:13:06-08:00
New Revision: 4f61749e16f63b0c9ebd9bdd1f8bf4f570a31692
URL:
https://github.com/llvm/llvm-project/commit/4f61749e16f63b0c9ebd9bdd1f8bf4f570a31692
DIFF:
https://github.com/llvm/llvm-project/commit/4f61749e16f63b0c9ebd9bdd1f8bf4f570a31692.diff
LOG: [clang] support relative roots to vfs overlays
This diff adds support for relative roots to VFS overlays. The directory root
will be made absolute from the current working directory and will be used to
determine the path style to use. This supports the use of VFS overlays with
remote build systems that might use a different working directory for each
compilation.
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D116174
Added:
clang/test/VFS/Inputs/vfsoverlay-root-relative.yaml
clang/test/VFS/vfsoverlay-relative-root.c
Modified:
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Removed:
diff --git a/clang/test/VFS/Inputs/vfsoverlay-root-relative.yaml
b/clang/test/VFS/Inputs/vfsoverlay-root-relative.yaml
new file mode 100644
index ..9ed7ec59d11e
--- /dev/null
+++ b/clang/test/VFS/Inputs/vfsoverlay-root-relative.yaml
@@ -0,0 +1,17 @@
+{
+ 'version': 0,
+ 'fallthrough': true,
+ 'overlay-relative': true,
+ 'roots': [
+{ 'name': 'virtual',
+ 'type': 'directory',
+ 'contents': [
+{
+ 'external-contents': 'actual_header.h',
+ 'type': 'file',
+ 'name': 'virtual_header.h',
+}
+ ]
+}
+ ]
+}
diff --git a/clang/test/VFS/vfsoverlay-relative-root.c
b/clang/test/VFS/vfsoverlay-relative-root.c
new file mode 100644
index ..30a7a79eceae
--- /dev/null
+++ b/clang/test/VFS/vfsoverlay-relative-root.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -Werror -ivfsoverlay
%S/Inputs/vfsoverlay-root-relative.yaml -I virtual -fsyntax-only %s
+
+#include "virtual_header.h"
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h
b/llvm/include/llvm/Support/VirtualFileSystem.h
index 78bcdbf3e932..9b5ff8f20ae2 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -547,6 +547,9 @@ class RedirectingFileSystemParser;
/// }
/// \endverbatim
///
+/// The roots may be absolute or relative. If relative they will be made
+/// absolute against the current working directory.
+///
/// All configuration options are optional.
/// 'case-sensitive':
/// 'use-external-names':
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp
b/llvm/lib/Support/VirtualFileSystem.cpp
index bec4e8dbe06c..7b752b557f8e 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -1649,10 +1649,19 @@ class llvm::vfs::RedirectingFileSystemParser {
sys::path::Style::windows_backslash)) {
path_style = sys::path::Style::windows_backslash;
} else {
-assert(NameValueNode && "Name presence should be checked earlier");
-error(NameValueNode,
+// Relative VFS root entries are made absolute to the current working
+// directory, then we can determine the path style from that.
+auto EC = sys::fs::make_absolute(Name);
+if (EC) {
+ assert(NameValueNode && "Name presence should be checked earlier");
+ error(
+ NameValueNode,
"entry with relative path at the root level is not
discoverable");
-return nullptr;
+ return nullptr;
+}
+path_style = sys::path::is_absolute(Name, sys::path::Style::posix)
+ ? sys::path::Style::posix
+ : sys::path::Style::windows_backslash;
}
}
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp
b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index be32971908ea..caae58f74f63 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -2164,6 +2164,11 @@ TEST_F(VFSFromYAMLTest,
RecursiveDirectoryIterationLevel) {
TEST_F(VFSFromYAMLTest, RelativePaths) {
IntrusiveRefCntPtr Lower(new DummyFileSystem());
+ std::error_code EC;
+ SmallString<128> CWD;
+ EC = llvm::sys::fs::current_path(CWD);
+ ASSERT_FALSE(EC);
+
// Filename at root level without a parent directory.
IntrusiveRefCntPtr FS = getFromYAMLString(
"{ 'roots': [\n"
@@ -2172,16 +2177,26 @@ TEST_F(VFSFromYAMLTest, RelativePaths) {
" }\n"
"] }",
Lower);
- EXPECT_EQ(nullptr, FS.get());
+ ASSERT_TRUE(FS.get() != nullptr);
+ SmallString<128> ExpectedPathNotInDir("file-not-in-directory.h");
+ llvm::sys::fs::make_absolute(ExpectedPathNotInDir);
+ checkContents(FS->dir_begin(CWD, EC), {ExpectedPathNotInDir});
// Relative file
[clang] e27b5f9 - [clang][AST] Fix crash when printing error
Author: Ellis Hoag
Date: 2022-01-02T18:03:42-08:00
New Revision: e27b5f9371382952eb5482ad151bb6fcb4cd0d7c
URL:
https://github.com/llvm/llvm-project/commit/e27b5f9371382952eb5482ad151bb6fcb4cd0d7c
DIFF:
https://github.com/llvm/llvm-project/commit/e27b5f9371382952eb5482ad151bb6fcb4cd0d7c.diff
LOG: [clang][AST] Fix crash when printing error
Clang will crash if it tries to compile the following code. This commit
fixes it.
```
$ cat foo.c
void foo(_Nullable int *ptr) {
__auto_type _Nonnull a = ptr;
};
$ clang foo.c -c -Wnullable-to-nonnull-conversion
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D116342
Added:
Modified:
clang/lib/AST/TypePrinter.cpp
clang/test/Sema/nullability.c
Removed:
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 2a33a69f288d4..cf520fcb037ed 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -280,7 +280,7 @@ bool TypePrinter::canPrefixQualifiers(const Type *T,
case Type::Attributed: {
// We still want to print the address_space before the type if it is an
// address_space attribute.
- const auto *AttrTy = cast(T);
+ const auto *AttrTy = cast(UnderlyingType);
CanPrefixQualifiers = AttrTy->getAttrKind() == attr::AddressSpace;
}
}
diff --git a/clang/test/Sema/nullability.c b/clang/test/Sema/nullability.c
index d462886de0436..977b29e9bf9dd 100644
--- a/clang/test/Sema/nullability.c
+++ b/clang/test/Sema/nullability.c
@@ -125,6 +125,7 @@ void nullable_to_nonnull(_Nullable int *ptr) {
int *a = ptr; // okay
_Nonnull int *b = ptr; // expected-warning{{implicit conversion from
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int *
_Nonnull'}}
b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int
* _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+ __auto_type _Nonnull c = ptr; // expected-warning{{implicit conversion from
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int *
_Nullable _Nonnull'}}
accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int *
_Nonnull'}}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ac719d7 - [InstrProf] Don't profile merge by default in lightweight mode
Author: Ellis Hoag
Date: 2021-12-20T09:51:49-08:00
New Revision: ac719d7c9ae6a20a6ae530e308331b3d51b3d70e
URL:
https://github.com/llvm/llvm-project/commit/ac719d7c9ae6a20a6ae530e308331b3d51b3d70e
DIFF:
https://github.com/llvm/llvm-project/commit/ac719d7c9ae6a20a6ae530e308331b3d51b3d70e.diff
LOG: [InstrProf] Don't profile merge by default in lightweight mode
Profile merging is not supported when using debug info profile
correlation because the data section won't be in the binary at runtime.
Change the default profile name in this mode to `default_%p.proflite` so
we don't use profile merging.
Reviewed By: kyulee
Differential Revision: https://reviews.llvm.org/D115979
Added:
Modified:
clang/lib/CodeGen/BackendUtil.cpp
compiler-rt/lib/profile/InstrProfilingMerge.c
Removed:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index 49a278f1a09fc..3195615ae561c 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -102,7 +102,7 @@ namespace {
// Default filename used for profile generation.
std::string getDefaultProfileGenName() {
- return DebugInfoCorrelate ? "default_%m.proflite" : "default_%m.profraw";
+ return DebugInfoCorrelate ? "default_%p.proflite" : "default_%m.profraw";
}
class EmitAssemblyHelper {
diff --git a/compiler-rt/lib/profile/InstrProfilingMerge.c
b/compiler-rt/lib/profile/InstrProfilingMerge.c
index 6262762de0f1d..bf99521d4da7e 100644
--- a/compiler-rt/lib/profile/InstrProfilingMerge.c
+++ b/compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -95,8 +95,13 @@ static uintptr_t signextIfWin64(void *V) {
COMPILER_RT_VISIBILITY
int __llvm_profile_merge_from_buffer(const char *ProfileData,
uint64_t ProfileSize) {
- if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)
+ if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) {
+PROF_ERR(
+"%s\n",
+"Debug info correlation does not support profile merging at runtime. "
+"Instead, merge raw profiles using the llvm-profdata tool.");
return 1;
+ }
__llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
__llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 58d9c1a - [Try2][InstrProf] Attach debug info to counters
Author: Ellis Hoag
Date: 2021-12-16T14:20:30-08:00
New Revision: 58d9c1aec88d5d4c783643df057d87f6b0c9f693
URL:
https://github.com/llvm/llvm-project/commit/58d9c1aec88d5d4c783643df057d87f6b0c9f693
DIFF:
https://github.com/llvm/llvm-project/commit/58d9c1aec88d5d4c783643df057d87f6b0c9f693.diff
LOG: [Try2][InstrProf] Attach debug info to counters
Add the llvm flag `-debug-info-correlate` to attach debug info to
instrumentation counters so we can correlate raw profile data to their
functions. Raw profiles are dumped as `.proflite` files. The next diff enables
`llvm-profdata` to consume `.proflite` and debug info files to produce a normal
`.profdata` profile.
Part of the "lightweight instrumentation" work:
https://groups.google.com/g/llvm-dev/c/r03Z6JoN7d4
The original diff https://reviews.llvm.org/D114565 was reverted because of the
`Instrumentation/InstrProfiling/debug-info-correlate.ll` test, which is fixed
in this commit.
Reviewed By: kyulee
Differential Revision: https://reviews.llvm.org/D115693
Added:
llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
Modified:
clang/lib/CodeGen/BackendUtil.cpp
compiler-rt/include/profile/InstrProfData.inc
compiler-rt/lib/profile/InstrProfiling.c
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/lib/profile/InstrProfilingWriter.c
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfData.inc
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/InstrProfWriter.cpp
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
Removed:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index 510f3911939cf..49a278f1a09fc 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -94,10 +94,16 @@ using namespace llvm;
llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
#include "llvm/Support/Extension.def"
+namespace llvm {
+extern cl::opt DebugInfoCorrelate;
+}
+
namespace {
// Default filename used for profile generation.
-static constexpr StringLiteral DefaultProfileGenName = "default_%m.profraw";
+std::string getDefaultProfileGenName() {
+ return DebugInfoCorrelate ? "default_%m.proflite" : "default_%m.profraw";
+}
class EmitAssemblyHelper {
DiagnosticsEngine
@@ -886,7 +892,7 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager
,
if (!CodeGenOpts.InstrProfileOutput.empty())
PMBuilder.PGOInstrGen = CodeGenOpts.InstrProfileOutput;
else
- PMBuilder.PGOInstrGen = std::string(DefaultProfileGenName);
+ PMBuilder.PGOInstrGen = getDefaultProfileGenName();
}
if (CodeGenOpts.hasProfileIRUse()) {
PMBuilder.PGOInstrUse = CodeGenOpts.ProfileInstrumentUsePath;
@@ -1231,7 +1237,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
if (CodeGenOpts.hasProfileIRInstr())
// -fprofile-generate.
PGOOpt = PGOOptions(CodeGenOpts.InstrProfileOutput.empty()
-? std::string(DefaultProfileGenName)
+? getDefaultProfileGenName()
: CodeGenOpts.InstrProfileOutput,
"", "", PGOOptions::IRInstr, PGOOptions::NoCSAction,
CodeGenOpts.DebugInfoForProfiling);
@@ -1269,13 +1275,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
"Cannot run CSProfileGen pass with ProfileGen or SampleUse "
" pass");
PGOOpt->CSProfileGenFile = CodeGenOpts.InstrProfileOutput.empty()
- ? std::string(DefaultProfileGenName)
+ ? getDefaultProfileGenName()
: CodeGenOpts.InstrProfileOutput;
PGOOpt->CSAction = PGOOptions::CSIRInstr;
} else
PGOOpt = PGOOptions("",
CodeGenOpts.InstrProfileOutput.empty()
- ? std::string(DefaultProfileGenName)
+ ? getDefaultProfileGenName()
: CodeGenOpts.InstrProfileOutput,
"", PGOOptions::NoAction, PGOOptions::CSIRInstr,
CodeGenOpts.DebugInfoForProfiling);
diff --git a/compiler-rt/include/profile/InstrProfData.inc
b/compiler-rt/include/profile/InstrProfData.inc
index 008b8dde5820a..44719126b5965 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -653,15 +653,17 @@ serializeValueProfDataFrom(ValueProfRecordClosure
*Closure,
/* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
* version for other variants of profile. We set the lowest bit of the upper 8
- * bits (i.e. bit 56) to 1 to indicate if this is an
[clang] c809da7 - Revert "[InstrProf] Attach debug info to counters"
Author: Ellis Hoag
Date: 2021-12-13T18:15:17-08:00
New Revision: c809da7d9ce78a463f9c5e38a9bf7b4c22b232de
URL:
https://github.com/llvm/llvm-project/commit/c809da7d9ce78a463f9c5e38a9bf7b4c22b232de
DIFF:
https://github.com/llvm/llvm-project/commit/c809da7d9ce78a463f9c5e38a9bf7b4c22b232de.diff
LOG: Revert "[InstrProf] Attach debug info to counters"
This reverts commit 800bf8ed29fbcaa9436540e83bc119ec92e7d40f.
The `Instrumentation/InstrProfiling/debug-info-correlate.ll` test was
failing because I forgot the `llc` commands are architecture specific.
I'll follow up with a fix.
Differential Revision: https://reviews.llvm.org/D115689
Added:
Modified:
clang/lib/CodeGen/BackendUtil.cpp
compiler-rt/include/profile/InstrProfData.inc
compiler-rt/lib/profile/InstrProfiling.c
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/lib/profile/InstrProfilingWriter.c
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfData.inc
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/InstrProfWriter.cpp
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
Removed:
llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index e5fcf2b2c968f..510f3911939cf 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -94,17 +94,10 @@ using namespace llvm;
llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
#include "llvm/Support/Extension.def"
-namespace llvm {
-extern cl::opt DebugInfoCorrelate;
-}
-
namespace {
// Default filename used for profile generation.
-Twine getDefaultProfileGenName() {
- const Twine Extension = DebugInfoCorrelate ? "proflite" : "profraw";
- return "default_%m." + Extension;
-}
+static constexpr StringLiteral DefaultProfileGenName = "default_%m.profraw";
class EmitAssemblyHelper {
DiagnosticsEngine
@@ -893,7 +886,7 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager
,
if (!CodeGenOpts.InstrProfileOutput.empty())
PMBuilder.PGOInstrGen = CodeGenOpts.InstrProfileOutput;
else
- PMBuilder.PGOInstrGen = getDefaultProfileGenName().str();
+ PMBuilder.PGOInstrGen = std::string(DefaultProfileGenName);
}
if (CodeGenOpts.hasProfileIRUse()) {
PMBuilder.PGOInstrUse = CodeGenOpts.ProfileInstrumentUsePath;
@@ -1238,7 +1231,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
if (CodeGenOpts.hasProfileIRInstr())
// -fprofile-generate.
PGOOpt = PGOOptions(CodeGenOpts.InstrProfileOutput.empty()
-? getDefaultProfileGenName().str()
+? std::string(DefaultProfileGenName)
: CodeGenOpts.InstrProfileOutput,
"", "", PGOOptions::IRInstr, PGOOptions::NoCSAction,
CodeGenOpts.DebugInfoForProfiling);
@@ -1276,13 +1269,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
"Cannot run CSProfileGen pass with ProfileGen or SampleUse "
" pass");
PGOOpt->CSProfileGenFile = CodeGenOpts.InstrProfileOutput.empty()
- ? getDefaultProfileGenName().str()
+ ? std::string(DefaultProfileGenName)
: CodeGenOpts.InstrProfileOutput;
PGOOpt->CSAction = PGOOptions::CSIRInstr;
} else
PGOOpt = PGOOptions("",
CodeGenOpts.InstrProfileOutput.empty()
- ? getDefaultProfileGenName().str()
+ ? std::string(DefaultProfileGenName)
: CodeGenOpts.InstrProfileOutput,
"", PGOOptions::NoAction, PGOOptions::CSIRInstr,
CodeGenOpts.DebugInfoForProfiling);
diff --git a/compiler-rt/include/profile/InstrProfData.inc
b/compiler-rt/include/profile/InstrProfData.inc
index 44719126b5965..008b8dde5820a 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -653,17 +653,15 @@ serializeValueProfDataFrom(ValueProfRecordClosure
*Closure,
/* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
* version for other variants of profile. We set the lowest bit of the upper 8
- * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
+ * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentaiton
* generated profile, and 0 if this is a Clang FE generated profile.
* 1 in bit 57 indicates there are context-sensitive records in the profile.
- * The 59th bit indicates whether to use debug info to correlate profiles.
*/
[clang] 800bf8e - [InstrProf] Attach debug info to counters
Author: Ellis Hoag
Date: 2021-12-13T17:51:22-08:00
New Revision: 800bf8ed29fbcaa9436540e83bc119ec92e7d40f
URL:
https://github.com/llvm/llvm-project/commit/800bf8ed29fbcaa9436540e83bc119ec92e7d40f
DIFF:
https://github.com/llvm/llvm-project/commit/800bf8ed29fbcaa9436540e83bc119ec92e7d40f.diff
LOG: [InstrProf] Attach debug info to counters
Add the llvm flag `-debug-info-correlate` to attach debug info to
instrumentation counters so we can correlate raw profile data to their
functions. Raw profiles are dumped as `.proflite` files. The next diff enables
`llvm-profdata` to consume `.proflite` and debug info files to produce a normal
`.profdata` profile.
Part of the "lightweight instrumentation" work:
https://groups.google.com/g/llvm-dev/c/r03Z6JoN7d4
Reviewed By: kyulee
Differential Revision: https://reviews.llvm.org/D114565
Added:
llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
Modified:
clang/lib/CodeGen/BackendUtil.cpp
compiler-rt/include/profile/InstrProfData.inc
compiler-rt/lib/profile/InstrProfiling.c
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/lib/profile/InstrProfilingWriter.c
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfData.inc
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/InstrProfWriter.cpp
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
Removed:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index 510f3911939cf..e5fcf2b2c968f 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -94,10 +94,17 @@ using namespace llvm;
llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
#include "llvm/Support/Extension.def"
+namespace llvm {
+extern cl::opt DebugInfoCorrelate;
+}
+
namespace {
// Default filename used for profile generation.
-static constexpr StringLiteral DefaultProfileGenName = "default_%m.profraw";
+Twine getDefaultProfileGenName() {
+ const Twine Extension = DebugInfoCorrelate ? "proflite" : "profraw";
+ return "default_%m." + Extension;
+}
class EmitAssemblyHelper {
DiagnosticsEngine
@@ -886,7 +893,7 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager
,
if (!CodeGenOpts.InstrProfileOutput.empty())
PMBuilder.PGOInstrGen = CodeGenOpts.InstrProfileOutput;
else
- PMBuilder.PGOInstrGen = std::string(DefaultProfileGenName);
+ PMBuilder.PGOInstrGen = getDefaultProfileGenName().str();
}
if (CodeGenOpts.hasProfileIRUse()) {
PMBuilder.PGOInstrUse = CodeGenOpts.ProfileInstrumentUsePath;
@@ -1231,7 +1238,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
if (CodeGenOpts.hasProfileIRInstr())
// -fprofile-generate.
PGOOpt = PGOOptions(CodeGenOpts.InstrProfileOutput.empty()
-? std::string(DefaultProfileGenName)
+? getDefaultProfileGenName().str()
: CodeGenOpts.InstrProfileOutput,
"", "", PGOOptions::IRInstr, PGOOptions::NoCSAction,
CodeGenOpts.DebugInfoForProfiling);
@@ -1269,13 +1276,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
"Cannot run CSProfileGen pass with ProfileGen or SampleUse "
" pass");
PGOOpt->CSProfileGenFile = CodeGenOpts.InstrProfileOutput.empty()
- ? std::string(DefaultProfileGenName)
+ ? getDefaultProfileGenName().str()
: CodeGenOpts.InstrProfileOutput;
PGOOpt->CSAction = PGOOptions::CSIRInstr;
} else
PGOOpt = PGOOptions("",
CodeGenOpts.InstrProfileOutput.empty()
- ? std::string(DefaultProfileGenName)
+ ? getDefaultProfileGenName().str()
: CodeGenOpts.InstrProfileOutput,
"", PGOOptions::NoAction, PGOOptions::CSIRInstr,
CodeGenOpts.DebugInfoForProfiling);
diff --git a/compiler-rt/include/profile/InstrProfData.inc
b/compiler-rt/include/profile/InstrProfData.inc
index 008b8dde5820a..44719126b5965 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -653,15 +653,17 @@ serializeValueProfDataFrom(ValueProfRecordClosure
*Closure,
/* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
* version for other variants of profile. We set the lowest bit of the upper 8
- * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentaiton
+ * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
* generated profile, and 0
