[TCWG CI] Regression caused by llvm: [InstCombine] fold icmp with sub and bool

* [TCWG CI] Regression caused by llvm: [InstCombine] fold icmp with sub and bool
@ 2022-05-23  1:10 ci_notify
  2022-05-23 15:14 ` Nathan Chancellor
  0 siblings, 1 reply; 2+ messages in thread
From: ci_notify @ 2022-05-23  1:10 UTC (permalink / raw)
  To: Sanjay Patel; +Cc: llvm

[-- Attachment #1: Type: text/plain, Size: 8161 bytes --]

[TCWG CI] Regression caused by llvm: [InstCombine] fold icmp with sub and bool:
commit 4069cccf3b4ff4afb743d3d371ead9e2d5491e3a
Author: Sanjay Patel <spatel@rotateright.com>

    [InstCombine] fold icmp with sub and bool

Results regressed to
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_llvm:
-5
# build_abe qemu:
-2
# linux_n_obj:
546
# First few build errors in logs:
# 00:01:23 clang-15: error: clang frontend command failed with exit code 134 (use -v to see invocation)
# 00:01:23 make[1]: *** [scripts/Makefile.build:295: block/bio.o] Error 134
# 00:01:32 make: *** [Makefile:2000: block] Error 2

from
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_llvm:
-5
# build_abe qemu:
-2
# linux_n_obj:
563
# linux build successful:
all

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

This commit has regressed these CI configurations:
 - tcwg_kernel/llvm-master-arm-next-allnoconfig

First_bad build: https://ci.linaro.org/job/tcwg_kernel-llvm-bisect-llvm-master-arm-next-allnoconfig/3/artifact/artifacts/build-4069cccf3b4ff4afb743d3d371ead9e2d5491e3a/
Last_good build: https://ci.linaro.org/job/tcwg_kernel-llvm-bisect-llvm-master-arm-next-allnoconfig/3/artifact/artifacts/build-aa9acb51f69a862547699c5f7fa5c6f6c1353253/
Baseline build: https://ci.linaro.org/job/tcwg_kernel-llvm-bisect-llvm-master-arm-next-allnoconfig/3/artifact/artifacts/build-baseline/
Even more details: https://ci.linaro.org/job/tcwg_kernel-llvm-bisect-llvm-master-arm-next-allnoconfig/3/artifact/artifacts/

Reproduce builds:
<cut>
mkdir investigate-llvm-4069cccf3b4ff4afb743d3d371ead9e2d5491e3a
cd investigate-llvm-4069cccf3b4ff4afb743d3d371ead9e2d5491e3a

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests and test.sh script
mkdir -p artifacts/manifests
curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_kernel-llvm-bisect-llvm-master-arm-next-allnoconfig/3/artifact/artifacts/manifests/build-baseline.sh --fail
curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_kernel-llvm-bisect-llvm-master-arm-next-allnoconfig/3/artifact/artifacts/manifests/build-parameters.sh --fail
curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_kernel-llvm-bisect-llvm-master-arm-next-allnoconfig/3/artifact/artifacts/test.sh --fail
chmod +x artifacts/test.sh

# Reproduce the baseline build (build all pre-requisites)
./jenkins-scripts/tcwg_kernel-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 4069cccf3b4ff4afb743d3d371ead9e2d5491e3a
../artifacts/test.sh

# Reproduce last_good build
git checkout --detach aa9acb51f69a862547699c5f7fa5c6f6c1353253
../artifacts/test.sh

cd ..
</cut>

Full commit (up to 1000 lines):
<cut>
commit 4069cccf3b4ff4afb743d3d371ead9e2d5491e3a
Author: Sanjay Patel <spatel@rotateright.com>
Date:   Sun May 22 11:02:28 2022 -0400

    [InstCombine] fold icmp with sub and bool
    
    This is the specific pattern seen in #53432, but it can be extended
    in multiple ways:
    1. The 'zext' could be an 'and'
    2. The 'sub' could be some other binop with a similar ==0 property (udiv).
    
    There might be some way to generalize using knownbits, but that
    would require checking that the 'bool' value is created with
    some instruction that can be replaced with new icmp+logic.
    
    https://alive2.llvm.org/ce/z/-KCfpa
---
 .../Transforms/InstCombine/InstCombineCompares.cpp | 26 ++++++++++++++++++++++
 llvm/test/Transforms/InstCombine/icmp-range.ll     | 17 ++++++++------
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 4f20a0699ec5..e2a6b6b1495b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -5631,6 +5631,29 @@ Instruction *InstCombinerImpl::foldICmpUsingKnownBits(ICmpInst &I) {
   return nullptr;
 }
 
+/// If one operand of an icmp is effectively a bool (value range of {0,1}),
+/// then try to reduce patterns based on that limit.
+static Instruction *foldICmpUsingBoolRange(ICmpInst &I,
+                                           InstCombiner::BuilderTy &Builder) {
+  Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
+  const ICmpInst::Predicate Pred = I.getPredicate();
+
+  Value *X, *Y, *Z;
+  if (Pred != ICmpInst::ICMP_ULT || !match(Op0, m_Sub(m_Value(X), m_Value(Y))))
+    return nullptr;
+
+  unsigned ExtraUses = !Op0->hasOneUse() + !Op1->hasOneUse();
+
+  // Sub must be 0 and bool must be true for "ULT":
+  // (sub X, Y) <u (zext i1 Z) --> (X == Y) && Z
+  if (match(Op1, m_ZExt(m_Value(Z))) && ExtraUses < 2) {
+    Value *EqXY = Builder.CreateICmpEQ(X, Y);
+    return BinaryOperator::CreateAnd(EqXY, Z);
+  }
+
+  return nullptr;
+}
+
 llvm::Optional<std::pair<CmpInst::Predicate, Constant *>>
 InstCombiner::getFlippedStrictnessPredicateAndConstant(CmpInst::Predicate Pred,
                                                        Constant *C) {
@@ -6058,6 +6081,9 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) {
   if (Instruction *Res = foldICmpWithDominatingICmp(I))
     return Res;
 
+  if (Instruction *Res = foldICmpUsingBoolRange(I, Builder))
+    return Res;
+
   if (Instruction *Res = foldICmpUsingKnownBits(I))
     return Res;
 
diff --git a/llvm/test/Transforms/InstCombine/icmp-range.ll b/llvm/test/Transforms/InstCombine/icmp-range.ll
index b67356675eba..a29d4cd81b9d 100644
--- a/llvm/test/Transforms/InstCombine/icmp-range.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-range.ll
@@ -173,9 +173,8 @@ define i1 @test_two_ranges3(i32* nocapture readonly %arg1, i32* nocapture readon
 
 define i1 @sub_ult_zext(i1 %b, i8 %x, i8 %y) {
 ; CHECK-LABEL: @sub_ult_zext(
-; CHECK-NEXT:    [[Z:%.*]] = zext i1 [[B:%.*]] to i8
-; CHECK-NEXT:    [[S:%.*]] = sub i8 [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = icmp ult i8 [[S]], [[Z]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[TMP1]], [[B:%.*]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %z = zext i1 %b to i8
@@ -188,8 +187,8 @@ define i1 @sub_ult_zext_use1(i1 %b, i8 %x, i8 %y) {
 ; CHECK-LABEL: @sub_ult_zext_use1(
 ; CHECK-NEXT:    [[Z:%.*]] = zext i1 [[B:%.*]] to i8
 ; CHECK-NEXT:    call void @use(i8 [[Z]])
-; CHECK-NEXT:    [[S:%.*]] = sub i8 [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = icmp ult i8 [[S]], [[Z]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i8 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[TMP1]], [[B]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %z = zext i1 %b to i8
@@ -201,10 +200,10 @@ define i1 @sub_ult_zext_use1(i1 %b, i8 %x, i8 %y) {
 
 define <2 x i1> @zext_ugt_sub_use2(<2 x i1> %b, <2 x i8> %x, <2 x i8> %y) {
 ; CHECK-LABEL: @zext_ugt_sub_use2(
-; CHECK-NEXT:    [[Z:%.*]] = zext <2 x i1> [[B:%.*]] to <2 x i8>
 ; CHECK-NEXT:    [[S:%.*]] = sub <2 x i8> [[X:%.*]], [[Y:%.*]]
 ; CHECK-NEXT:    call void @use_vec(<2 x i8> [[S]])
-; CHECK-NEXT:    [[R:%.*]] = icmp ult <2 x i8> [[S]], [[Z]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq <2 x i8> [[X]], [[Y]]
+; CHECK-NEXT:    [[R:%.*]] = and <2 x i1> [[TMP1]], [[B:%.*]]
 ; CHECK-NEXT:    ret <2 x i1> [[R]]
 ;
   %z = zext <2 x i1> %b to <2 x i8>
@@ -214,6 +213,8 @@ define <2 x i1> @zext_ugt_sub_use2(<2 x i1> %b, <2 x i8> %x, <2 x i8> %y) {
   ret <2 x i1> %r
 }
 
+; negative test - too many extra uses
+
 define i1 @sub_ult_zext_use3(i1 %b, i8 %x, i8 %y) {
 ; CHECK-LABEL: @sub_ult_zext_use3(
 ; CHECK-NEXT:    [[Z:%.*]] = zext i1 [[B:%.*]] to i8
@@ -231,6 +232,8 @@ define i1 @sub_ult_zext_use3(i1 %b, i8 %x, i8 %y) {
   ret i1 %r
 }
 
+; negative test - wrong predicate
+
 define i1 @sub_ule_zext(i1 %b, i8 %x, i8 %y) {
 ; CHECK-LABEL: @sub_ule_zext(
 ; CHECK-NEXT:    [[Z:%.*]] = zext i1 [[B:%.*]] to i8
</cut>

^ permalink raw reply related	[flat|nested] 2+ messages in thread