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 <spa...@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-aarch64-spec2k6-O2/15/artifact/artifacts/build-5bf4ab0e79e1a8552019918a662bdf7af8b3825a/
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-aarch64-spec2k6-O2/15/artifact/artifacts/build-a0a9c9e188f5b97ff8b74287d1536f57ec5dda54/
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-aarch64-spec2k6-O2/15/

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-aarch64-spec2k6-O2/15/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/15/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/15/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 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/ci/tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O2

Artifacts: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2/15/artifact/artifacts/
Build log: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2/15/consoleText

Full commit (up to 1000 lines):
<cut>
commit a0a9c9e188f5b97ff8b74287d1536f57ec5dda54
Author: Sanjay Patel <spa...@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>
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-toolchain

Reply via email to