Successfully identified regression in *llvm* in CI configuration
tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O2_LTO. So far, this commit has
regressed CI configurations:
- tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O2_LTO
Culprit:
<cut>
commit 643ce61fb3c2c730b7ecead4a489eaeef3f053ea
Author: Akira Hatanaka <ahatan...@apple.com>
Date: Wed Aug 11 12:55:28 2021 -0700
[ObjC][ARC] Don't form a StoreStrong call if it is unsafe to move the
release call
findSafeStoreForStoreStrongContraction checks whether it's safe to move
the release call to the store by inspecting all instructions between the
two, but was ignoring retain instructions. This was causing objects to
be released and deallocated before they were retained.
rdar://81668577
</cut>
Results regressed to (for first_bad == 643ce61fb3c2c730b7ecead4a489eaeef3f053ea)
# 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 -- -O2_LTO
artifacts/build-643ce61fb3c2c730b7ecead4a489eaeef3f053ea/results_id:
1
# 433.milc,milc_base.default regressed by 103
from (for last_good == 767496d19cb9a1fbba57ff08095faa161998ee36)
# 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 -- -O2_LTO
artifacts/build-767496d19cb9a1fbba57ff08095faa161998ee36/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-O2_LTO/24/artifact/artifacts/build-767496d19cb9a1fbba57ff08095faa161998ee36/
Results ID of last_good:
tx1_64/tcwg_bmk_llvm_tx1/bisect-llvm-master-aarch64-spec2k6-O2_LTO/4172
Artifacts of first_bad build:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/24/artifact/artifacts/build-643ce61fb3c2c730b7ecead4a489eaeef3f053ea/
Results ID of first_bad:
tx1_64/tcwg_bmk_llvm_tx1/bisect-llvm-master-aarch64-spec2k6-O2_LTO/4171
Build top page/logs:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/24/
Configuration details:
Reproduce builds:
<cut>
mkdir investigate-llvm-643ce61fb3c2c730b7ecead4a489eaeef3f053ea
cd investigate-llvm-643ce61fb3c2c730b7ecead4a489eaeef3f053ea
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-O2_LTO/24/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-O2_LTO/24/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-O2_LTO/24/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 643ce61fb3c2c730b7ecead4a489eaeef3f053ea
../artifacts/test.sh
# Reproduce last_good build
git checkout --detach 767496d19cb9a1fbba57ff08095faa161998ee36
../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-O2_LTO
Artifacts:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/24/artifact/artifacts/
Build log:
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/24/consoleText
Full commit (up to 1000 lines):
<cut>
commit 643ce61fb3c2c730b7ecead4a489eaeef3f053ea
Author: Akira Hatanaka <ahatan...@apple.com>
Date: Wed Aug 11 12:55:28 2021 -0700
[ObjC][ARC] Don't form a StoreStrong call if it is unsafe to move the
release call
findSafeStoreForStoreStrongContraction checks whether it's safe to move
the release call to the store by inspecting all instructions between the
two, but was ignoring retain instructions. This was causing objects to
be released and deallocated before they were retained.
rdar://81668577
---
llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp | 21 ++++++++++++---------
.../test/Transforms/ObjCARC/contract-storestrong.ll | 19 +++++++++++++++++++
2 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
b/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
index 62161b5b6b40..577973c80601 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
@@ -226,13 +226,6 @@ static StoreInst
*findSafeStoreForStoreStrongContraction(LoadInst *Load,
// of Inst.
ARCInstKind Class = GetBasicARCInstKind(Inst);
- // If Inst is an unrelated retain, we don't care about it.
- //
- // TODO: This is one area where the optimization could be made more
- // aggressive.
- if (IsRetain(Class))
- continue;
-
// If we have seen the store, but not the release...
if (Store) {
// We need to make sure that it is safe to move the release from its
@@ -248,8 +241,18 @@ static StoreInst
*findSafeStoreForStoreStrongContraction(LoadInst *Load,
return nullptr;
}
- // Ok, now we know we have not seen a store yet. See if Inst can write to
- // our load location, if it can not, just ignore the instruction.
+ // Ok, now we know we have not seen a store yet.
+
+ // If Inst is a retain, we don't care about it as it doesn't prevent moving
+ // the load to the store.
+ //
+ // TODO: This is one area where the optimization could be made more
+ // aggressive.
+ if (IsRetain(Class))
+ continue;
+
+ // See if Inst can write to our load location, if it can not, just ignore
+ // the instruction.
if (!isModSet(AA->getModRefInfo(Inst, Loc)))
continue;
diff --git a/llvm/test/Transforms/ObjCARC/contract-storestrong.ll
b/llvm/test/Transforms/ObjCARC/contract-storestrong.ll
index eff0a6fdf900..9c45e3334d83 100644
--- a/llvm/test/Transforms/ObjCARC/contract-storestrong.ll
+++ b/llvm/test/Transforms/ObjCARC/contract-storestrong.ll
@@ -256,6 +256,25 @@ define i8* @test13(i8* %a0, i8* %a1, i8** %addr, i8* %new)
{
ret i8* %retained
}
+; Cannot form a storeStrong call because it's unsafe to move the release call
to
+; the store.
+
+; CHECK-LABEL: define void @test14(
+; CHECK: %[[V0:.*]] = load i8*, i8** %a
+; CHECK: %[[V1:.*]] = call i8* @llvm.objc.retain(i8* %p)
+; CHECK: store i8* %[[V1]], i8** %a
+; CHECK: %[[V2:.*]] = call i8* @llvm.objc.retain(i8* %[[V0]])
+; CHECK: call void @llvm.objc.release(i8* %[[V2]])
+
+define void @test14(i8** %a, i8* %p) {
+ %v0 = load i8*, i8** %a, align 8
+ %v1 = call i8* @llvm.objc.retain(i8* %p)
+ store i8* %p, i8** %a, align 8
+ %v2 = call i8* @llvm.objc.retain(i8* %v0)
+ call void @llvm.objc.release(i8* %v0)
+ ret void
+}
+
!0 = !{}
; CHECK: attributes [[NUW]] = { nounwind }
</cut>
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-toolchain
