Successfully identified regression in *llvm* in CI configuration
tcwg_bmk_llvm_apm/llvm-master-aarch64-spec2k6-Oz_LTO. So far, this commit has
regressed CI configurations:
- tcwg_bmk_llvm_apm/llvm-master-aarch64-spec2k6-Oz_LTO
Culprit:
<cut>
commit 4aafd5f00c2a772337ec065d4542ef158453a343
Author: Jan Svoboda <jan_svob...@apple.com>
Date: Fri Aug 6 14:46:41 2021 +0200
[clang] Remove misleading assertion in FullSourceLoc
D31709 added an assertion was added to `FullSourceLoc::hasManager()` that
ensured a valid `SourceLocation` is always paired with a `SourceManager`, and
missing `SourceManager` is always paired with an invalid `SourceLocation`.
This appears to be incorrect, since clients never cared about constructing
`FullSourceLoc` to uphold that invariant, or always checking `isValid()` before
calling `hasManager()`.
The assertion started failing when serializing diagnostics pointing into an
explicit module. Explicit modules don't have valid `SourceLocation` for the
`import` statement, since they are "imported" from the command-line argument
`-fmodule-name=x.pcm`.
This patch removes the assertion, since `FullSourceLoc` was never intended
to uphold any kind of invariants between the validity of `SourceLocation` and
presence of `SourceManager`.
Reviewed By: arphaman
Differential Revision: https://reviews.llvm.org/D106862
</cut>
Results regressed to (for first_bad == 4aafd5f00c2a772337ec065d4542ef158453a343)
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_abe stage1 -- --set gcc_override_configure=--disable-libsanitizer:
-8
# build_abe linux:
-7
# build_abe glibc:
-6
# build_abe stage2 -- --set gcc_override_configure=--disable-libsanitizer:
-5
# build_llvm true:
-3
# true:
0
# benchmark -- -Oz_LTO
artifacts/build-4aafd5f00c2a772337ec065d4542ef158453a343/results_id:
1
# 470.lbm,lbm_base.default regressed by 104
from (for last_good == 3709822d2602b8b7db2d9bcc0e856f676582f25d)
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_abe stage1 -- --set gcc_override_configure=--disable-libsanitizer:
-8
# build_abe linux:
-7
# build_abe glibc:
-6
# build_abe stage2 -- --set gcc_override_configure=--disable-libsanitizer:
-5
# build_llvm true:
-3
# true:
0
# benchmark -- -Oz_LTO
artifacts/build-3709822d2602b8b7db2d9bcc0e856f676582f25d/results_id:
1
Artifacts of last_good build:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/artifact/artifacts/build-3709822d2602b8b7db2d9bcc0e856f676582f25d/
Results ID of last_good:
apm_64/tcwg_bmk_llvm_apm/bisect-llvm-master-aarch64-spec2k6-Oz_LTO/3746
Artifacts of first_bad build:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/artifact/artifacts/build-4aafd5f00c2a772337ec065d4542ef158453a343/
Results ID of first_bad:
apm_64/tcwg_bmk_llvm_apm/bisect-llvm-master-aarch64-spec2k6-Oz_LTO/3725
Build top page/logs:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/
Configuration details:
Reproduce builds:
<cut>
mkdir investigate-llvm-4aafd5f00c2a772337ec065d4542ef158453a343
cd investigate-llvm-4aafd5f00c2a772337ec065d4542ef158453a343
git clone https://git.linaro.org/toolchain/jenkins-scripts
mkdir -p artifacts/manifests
curl -o artifacts/manifests/build-baseline.sh
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/artifact/artifacts/manifests/build-baseline.sh
--fail
curl -o artifacts/manifests/build-parameters.sh
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/artifact/artifacts/manifests/build-parameters.sh
--fail
curl -o artifacts/test.sh
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/artifact/artifacts/test.sh
--fail
chmod +x artifacts/test.sh
# Reproduce the baseline build (build all pre-requisites)
./jenkins-scripts/tcwg_bmk-build.sh @@ artifacts/manifests/build-baseline.sh
# Save baseline build state (which is then restored in artifacts/test.sh)
mkdir -p ./bisect
rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/
--exclude /llvm/ ./ ./bisect/baseline/
cd llvm
# Reproduce first_bad build
git checkout --detach 4aafd5f00c2a772337ec065d4542ef158453a343
../artifacts/test.sh
# Reproduce last_good build
git checkout --detach 3709822d2602b8b7db2d9bcc0e856f676582f25d
../artifacts/test.sh
cd ..
</cut>
History of pending regressions and results:
https://git.linaro.org/toolchain/ci/base-artifacts.git/log/?h=linaro-local/ci/tcwg_bmk_llvm_apm/llvm-master-aarch64-spec2k6-Oz_LTO
Artifacts:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/artifact/artifacts/
Build log:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-aarch64-spec2k6-Oz_LTO/3/consoleText
Full commit (up to 1000 lines):
<cut>
commit 4aafd5f00c2a772337ec065d4542ef158453a343
Author: Jan Svoboda <jan_svob...@apple.com>
Date: Fri Aug 6 14:46:41 2021 +0200
[clang] Remove misleading assertion in FullSourceLoc
D31709 added an assertion was added to `FullSourceLoc::hasManager()` that
ensured a valid `SourceLocation` is always paired with a `SourceManager`, and
missing `SourceManager` is always paired with an invalid `SourceLocation`.
This appears to be incorrect, since clients never cared about constructing
`FullSourceLoc` to uphold that invariant, or always checking `isValid()` before
calling `hasManager()`.
The assertion started failing when serializing diagnostics pointing into an
explicit module. Explicit modules don't have valid `SourceLocation` for the
`import` statement, since they are "imported" from the command-line argument
`-fmodule-name=x.pcm`.
This patch removes the assertion, since `FullSourceLoc` was never intended
to uphold any kind of invariants between the validity of `SourceLocation` and
presence of `SourceManager`.
Reviewed By: arphaman
Differential Revision: https://reviews.llvm.org/D106862
---
clang/include/clang/Basic/SourceLocation.h | 13 +++++++------
clang/test/Modules/Inputs/explicit-build-diags/a.h | 1 +
.../Modules/Inputs/explicit-build-diags/module.modulemap | 1 +
clang/test/Modules/explicit-build-diags.cpp | 8 ++++++++
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Basic/SourceLocation.h
b/clang/include/clang/Basic/SourceLocation.h
index 540de23b9f55..ba2e9156a2b1 100644
--- a/clang/include/clang/Basic/SourceLocation.h
+++ b/clang/include/clang/Basic/SourceLocation.h
@@ -363,6 +363,10 @@ class FileEntry;
/// A SourceLocation and its associated SourceManager.
///
/// This is useful for argument passing to functions that expect both objects.
+///
+/// This class does not guarantee the presence of either the SourceManager or
+/// a valid SourceLocation. Clients should use `isValid()` and `hasManager()`
+/// before calling the member functions.
class FullSourceLoc : public SourceLocation {
const SourceManager *SrcMgr = nullptr;
@@ -373,13 +377,10 @@ public:
explicit FullSourceLoc(SourceLocation Loc, const SourceManager &SM)
: SourceLocation(Loc), SrcMgr(&SM) {}
- bool hasManager() const {
- bool hasSrcMgr = SrcMgr != nullptr;
- assert(hasSrcMgr == isValid() && "FullSourceLoc has location but no
manager");
- return hasSrcMgr;
- }
+ /// Checks whether the SourceManager is present.
+ bool hasManager() const { return SrcMgr != nullptr; }
- /// \pre This FullSourceLoc has an associated SourceManager.
+ /// \pre hasManager()
const SourceManager &getManager() const {
assert(SrcMgr && "SourceManager is NULL.");
return *SrcMgr;
diff --git a/clang/test/Modules/Inputs/explicit-build-diags/a.h
b/clang/test/Modules/Inputs/explicit-build-diags/a.h
new file mode 100644
index 000000000000..486941dde83b
--- /dev/null
+++ b/clang/test/Modules/Inputs/explicit-build-diags/a.h
@@ -0,0 +1 @@
+void a() __attribute__((deprecated));
diff --git a/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
b/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
new file mode 100644
index 000000000000..bb00c840ce39
--- /dev/null
+++ b/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" }
diff --git a/clang/test/Modules/explicit-build-diags.cpp
b/clang/test/Modules/explicit-build-diags.cpp
new file mode 100644
index 000000000000..4a37dc108a68
--- /dev/null
+++ b/clang/test/Modules/explicit-build-diags.cpp
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clang_cc1 -fmodules -x c++
%S/Inputs/explicit-build-diags/module.modulemap -fmodule-name=a -emit-module -o
%t/a.pcm
+// RUN: %clang_cc1 -fmodules -Wdeprecated-declarations
-fdiagnostics-show-note-include-stack -serialize-diagnostic-file %t/tu.dia \
+// RUN: -I %S/Inputs/explicit-build-diags -fmodule-file=%t/a.pcm
-fsyntax-only %s
+
+#include "a.h"
+
+void foo() { a(); }
</cut>
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-toolchain
