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 ahatanaka@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-a... 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-a... 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-a...

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-a... --fail curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-a... --fail curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-a... --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/c...

Artifacts: https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-a... Build log: https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-a...

Full commit (up to 1000 lines): <cut> commit 643ce61fb3c2c730b7ecead4a489eaeef3f053ea Author: Akira Hatanaka ahatanaka@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>