Successfully identified regression in *llvm* in CI configuration
tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O3_LTO. So far, this commit has
regressed CI configurations:
- tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O3_LTO
Culprit:
<cut>
commit 19dc02e99f802922a3af69e802465bee0723b57a
Author: Nikita Popov <nikita....@gmail.com>
Date: Sun Aug 22 18:15:55 2021 +0200
[MergeICmps] Allow sinking past non-load/store
This is a followup to D106591. MergeICmps currently only allows
sinking the loads past either instructions that don't write to
memory at all, or simple loads/stores that don't modify the memory
the loads access.
The "simple loads/stores" part of this check doesn't seem necessary
to me -- AA isModRef() already accurately models any operation
that may clobber the memory. For example, in the adjusted test case
the transform is still fine if the call to @foo() isn't readonly,
but inaccessiblememonly -- in both cases, the call cannot modify
the loaded memory.
Differential Revision: https://reviews.llvm.org/D108517
</cut>
Results regressed to (for first_bad == 19dc02e99f802922a3af69e802465bee0723b57a)
# 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 -- -O3_LTO
artifacts/build-19dc02e99f802922a3af69e802465bee0723b57a/results_id:
1
# 464.h264ref,h264ref_base.default regressed by 105
from (for last_good == da12d88b1c5fc42b49b92fcf94917ca489dd677f)
# 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 -- -O3_LTO
artifacts/build-da12d88b1c5fc42b49b92fcf94917ca489dd677f/results_id:
1
Artifacts of last_good build:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/artifact/artifacts/build-da12d88b1c5fc42b49b92fcf94917ca489dd677f/
Results ID of last_good:
tx1_64/tcwg_bmk_llvm_tx1/bisect-llvm-master-aarch64-spec2k6-O3_LTO/4822
Artifacts of first_bad build:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/artifact/artifacts/build-19dc02e99f802922a3af69e802465bee0723b57a/
Results ID of first_bad:
tx1_64/tcwg_bmk_llvm_tx1/bisect-llvm-master-aarch64-spec2k6-O3_LTO/4807
Build top page/logs:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/
Configuration details:
Reproduce builds:
<cut>
mkdir investigate-llvm-19dc02e99f802922a3af69e802465bee0723b57a
cd investigate-llvm-19dc02e99f802922a3af69e802465bee0723b57a
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_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/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_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/artifact/artifacts/manifests/build-parameters.sh
--fail
curl -o artifacts/test.sh
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/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 19dc02e99f802922a3af69e802465bee0723b57a
../artifacts/test.sh
# Reproduce last_good build
git checkout --detach da12d88b1c5fc42b49b92fcf94917ca489dd677f
../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_tx1/llvm-master-aarch64-spec2k6-O3_LTO
Artifacts:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/artifact/artifacts/
Build log:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O3_LTO/32/consoleText
Full commit (up to 1000 lines):
<cut>
commit 19dc02e99f802922a3af69e802465bee0723b57a
Author: Nikita Popov <nikita....@gmail.com>
Date: Sun Aug 22 18:15:55 2021 +0200
[MergeICmps] Allow sinking past non-load/store
This is a followup to D106591. MergeICmps currently only allows
sinking the loads past either instructions that don't write to
memory at all, or simple loads/stores that don't modify the memory
the loads access.
The "simple loads/stores" part of this check doesn't seem necessary
to me -- AA isModRef() already accurately models any operation
that may clobber the memory. For example, in the adjusted test case
the transform is still fine if the call to @foo() isn't readonly,
but inaccessiblememonly -- in both cases, the call cannot modify
the loaded memory.
Differential Revision: https://reviews.llvm.org/D108517
---
llvm/lib/Transforms/Scalar/MergeICmps.cpp | 14 +-------------
.../Transforms/MergeICmps/X86/split-block-does-work.ll | 2 +-
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/MergeICmps.cpp
b/llvm/lib/Transforms/Scalar/MergeICmps.cpp
index f13f24ad2027..34465c76dd3d 100644
--- a/llvm/lib/Transforms/Scalar/MergeICmps.cpp
+++ b/llvm/lib/Transforms/Scalar/MergeICmps.cpp
@@ -66,15 +66,6 @@ namespace {
#define DEBUG_TYPE "mergeicmps"
-// Returns true if the instruction is a simple load or a simple store
-static bool isSimpleLoadOrStore(const Instruction *I) {
- if (const LoadInst *LI = dyn_cast<LoadInst>(I))
- return LI->isSimple();
- if (const StoreInst *SI = dyn_cast<StoreInst>(I))
- return SI->isSimple();
- return false;
-}
-
// A BCE atom "Binary Compare Expression Atom" represents an integer load
// that is a constant offset from a base value, e.g. `a` or `o.c` in the
example
// at the top.
@@ -244,10 +235,7 @@ bool BCECmpBlock::canSinkBCECmpInst(const Instruction
*Inst,
// If this instruction may clobber the loads and is in middle of the BCE cmp
// block instructions, then bail for now.
if (Inst->mayWriteToMemory()) {
- // Bail if this is not a simple load or store
- if (!isSimpleLoadOrStore(Inst))
- return false;
- // Disallow stores that might alias the BCE operands
+ // Disallow instructions that might modify the BCE operands
MemoryLocation LLoc = MemoryLocation::get(Cmp.Lhs.LoadI);
MemoryLocation RLoc = MemoryLocation::get(Cmp.Rhs.LoadI);
if (isModSet(AA.getModRefInfo(Inst, LLoc)) ||
diff --git a/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll
b/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll
index 0b9663f44980..1e341b92918d 100644
--- a/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll
+++ b/llvm/test/Transforms/MergeICmps/X86/split-block-does-work.ll
@@ -3,7 +3,7 @@
%S = type { i32, i32, i32, i32 }
-declare void @foo(...) readonly
+declare void @foo(...) inaccessiblememonly
; We can split %entry and create a memcmp(16 bytes).
define zeroext i1 @opeq1(
</cut>
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-toolchain
