Successfully identified regression in *llvm* in CI configuration tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O2. So far, this commit has regressed CI configurations: - tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O2
Culprit: <cut> commit a0a9c9e188f5b97ff8b74287d1536f57ec5dda54 Author: Sanjay Patel spatel@rotateright.com Date: Wed Aug 11 12:41:47 2021 -0400
[InstCombine] avoid breaking up min/max (cmp+sel) idioms
This is a quick fix for a motivating case that looks like this: https://godbolt.org/z/GeMqzMc38
As noted, we might be able to restore the min/max patterns with select folds, or we just wait for this to become easier with canonicalization to min/max intrinsics. </cut>
Results regressed to (for first_bad == a0a9c9e188f5b97ff8b74287d1536f57ec5dda54) # 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 artifacts/build-a0a9c9e188f5b97ff8b74287d1536f57ec5dda54/results_id: 1 # 464.h264ref,h264ref_base.default regressed by 106 # 464.h264ref,[.] FastFullPelBlockMotionSearch regressed by 146
from (for last_good == 5bf4ab0e79e1a8552019918a662bdf7af8b3825a) # 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 artifacts/build-5bf4ab0e79e1a8552019918a662bdf7af8b3825a/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/3875 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/3877 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-a0a9c9e188f5b97ff8b74287d1536f57ec5dda54 cd investigate-llvm-a0a9c9e188f5b97ff8b74287d1536f57ec5dda54
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 a0a9c9e188f5b97ff8b74287d1536f57ec5dda54 ../artifacts/test.sh
# Reproduce last_good build git checkout --detach 5bf4ab0e79e1a8552019918a662bdf7af8b3825a ../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 a0a9c9e188f5b97ff8b74287d1536f57ec5dda54 Author: Sanjay Patel spatel@rotateright.com Date: Wed Aug 11 12:41:47 2021 -0400
[InstCombine] avoid breaking up min/max (cmp+sel) idioms
This is a quick fix for a motivating case that looks like this: https://godbolt.org/z/GeMqzMc38
As noted, we might be able to restore the min/max patterns with select folds, or we just wait for this to become easier with canonicalization to min/max intrinsics. --- llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | 12 +++++++++--- llvm/test/Transforms/InstCombine/icmp-add.ll | 13 ++++++------- 2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp index 2e20bca300d3..71037616585c 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -5755,9 +5755,6 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) { if (Instruction *Res = foldICmpWithDominatingICmp(I)) return Res;
- if (Instruction *Res = foldICmpBinOp(I, Q)) - return Res; - if (Instruction *Res = foldICmpUsingKnownBits(I)) return Res;
@@ -5803,6 +5800,15 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) { } }
+ // The folds in here may rely on wrapping flags and special constants, so + // they can break up min/max idioms in some cases but not seemingly similar + // patterns. + // FIXME: It may be possible to enhance select folding to make this + // unnecessary. It may also be moot if we canonicalize to min/max + // intrinsics. + if (Instruction *Res = foldICmpBinOp(I, Q)) + return Res; + if (Instruction *Res = foldICmpInstWithConstant(I)) return Res;
diff --git a/llvm/test/Transforms/InstCombine/icmp-add.ll b/llvm/test/Transforms/InstCombine/icmp-add.ll index 187e0ad1a31b..1750b5685c50 100644 --- a/llvm/test/Transforms/InstCombine/icmp-add.ll +++ b/llvm/test/Transforms/InstCombine/icmp-add.ll @@ -972,7 +972,6 @@ define i1 @slt_offset_nsw(i8 %a, i8 %c) { ret i1 %ov }
-; FIXME: ; In the following 4 tests, we could push the inc/dec ; through the min/max, but we should not break up the ; min/max idiom by using different icmp and select @@ -980,9 +979,9 @@ define i1 @slt_offset_nsw(i8 %a, i8 %c) {
define i32 @increment_max(i32 %x) { ; CHECK-LABEL: @increment_max( -; CHECK-NEXT: [[A:%.*]] = add nsw i32 [[X:%.*]], 1 -; CHECK-NEXT: [[C_INV:%.*]] = icmp slt i32 [[X]], 0 -; CHECK-NEXT: [[S:%.*]] = select i1 [[C_INV]], i32 0, i32 [[A]] +; CHECK-NEXT: [[TMP1:%.*]] = icmp sgt i32 [[X:%.*]], -1 +; CHECK-NEXT: [[TMP2:%.*]] = select i1 [[TMP1]], i32 [[X]], i32 -1 +; CHECK-NEXT: [[S:%.*]] = add nsw i32 [[TMP2]], 1 ; CHECK-NEXT: ret i32 [[S]] ; %a = add nsw i32 %x, 1 @@ -1019,9 +1018,9 @@ define i32 @increment_min(i32 %x) {
define i32 @decrement_min(i32 %x) { ; CHECK-LABEL: @decrement_min( -; CHECK-NEXT: [[A:%.*]] = add nsw i32 [[X:%.*]], -1 -; CHECK-NEXT: [[C_INV:%.*]] = icmp sgt i32 [[X]], 0 -; CHECK-NEXT: [[S:%.*]] = select i1 [[C_INV]], i32 0, i32 [[A]] +; CHECK-NEXT: [[TMP1:%.*]] = icmp slt i32 [[X:%.*]], 1 +; CHECK-NEXT: [[TMP2:%.*]] = select i1 [[TMP1]], i32 [[X]], i32 1 +; CHECK-NEXT: [[S:%.*]] = add nsw i32 [[TMP2]], -1 ; CHECK-NEXT: ret i32 [[S]] ; %a = add nsw i32 %x, -1 </cut>