From b212eb7159b40c98b3c40619b82b996fb903282b Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Wed, 8 Jan 2020 11:21:21 +0100 Subject: [PATCH] Revert "[InstCombine] fold zext of masked bit set/clear" This reverts commit a041c4ec6f7aa659b235cb67e9231a05e0a33b7d. This looks like a non-trivial change and there has been no code reviews (at least there were no phabricator revisions attached to the commit description). It is also causing a regression in one of our downstream integration tests, we haven't been able to come up with a minimal reproducer yet. --- .../Transforms/InstCombine/InstCombineCasts.cpp | 20 +------ llvm/test/Transforms/InstCombine/zext.ll | 65 ++++++++++------------ 2 files changed, 31 insertions(+), 54 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp index b9be418..3ba56bb 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -922,24 +922,10 @@ Instruction *InstCombiner::transformZExtICmp(ICmpInst *Cmp, ZExtInst &Zext, } } + // icmp ne A, B is equal to xor A, B when A and B only really have one bit. + // It is also profitable to transform icmp eq into not(xor(A, B)) because that + // may lead to additional simplifications. if (Cmp->isEquality() && Zext.getType() == Cmp->getOperand(0)->getType()) { - // Test if a bit is clear/set using a shifted-one mask: - // zext (icmp eq (and X, (1 << ShAmt)), 0) --> and (lshr (not X), ShAmt), 1 - // zext (icmp ne (and X, (1 << ShAmt)), 0) --> and (lshr X, ShAmt), 1 - Value *X, *ShAmt; - if (Cmp->hasOneUse() && match(Cmp->getOperand(1), m_ZeroInt()) && - match(Cmp->getOperand(0), - m_OneUse(m_c_And(m_Shl(m_One(), m_Value(ShAmt)), m_Value(X))))) { - if (Cmp->getPredicate() == ICmpInst::ICMP_EQ) - X = Builder.CreateNot(X); - Value *Lshr = Builder.CreateLShr(X, ShAmt); - Value *And1 = Builder.CreateAnd(Lshr, ConstantInt::get(X->getType(), 1)); - return replaceInstUsesWith(Zext, And1); - } - - // icmp ne A, B is equal to xor A, B when A and B only really have one bit. - // It is also profitable to transform icmp eq into not(xor(A, B)) because - // that may lead to additional simplifications. if (IntegerType *ITy = dyn_cast(Zext.getType())) { Value *LHS = Cmp->getOperand(0); Value *RHS = Cmp->getOperand(1); diff --git a/llvm/test/Transforms/InstCombine/zext.ll b/llvm/test/Transforms/InstCombine/zext.ll index 9351f5c..1dbb9ff 100644 --- a/llvm/test/Transforms/InstCombine/zext.ll +++ b/llvm/test/Transforms/InstCombine/zext.ll @@ -177,9 +177,11 @@ declare void @use32(i32) define i32 @masked_bit_set(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_set( -; CHECK-NEXT: [[TMP1:%.*]] = lshr i32 [[X:%.*]], [[Y:%.*]] -; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[TMP1]], 1 -; CHECK-NEXT: ret i32 [[TMP2]] +; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] +; CHECK-NEXT: [[AND:%.*]] = and i32 [[SH1]], [[X:%.*]] +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[AND]], 0 +; CHECK-NEXT: [[R:%.*]] = zext i1 [[CMP]] to i32 +; CHECK-NEXT: ret i32 [[R]] ; %sh1 = shl i32 1, %y %and = and i32 %sh1, %x @@ -190,10 +192,11 @@ define i32 @masked_bit_set(i32 %x, i32 %y) { define <2 x i32> @masked_bit_clear(<2 x i32> %x, <2 x i32> %y) { ; CHECK-LABEL: @masked_bit_clear( -; CHECK-NEXT: [[TMP1:%.*]] = xor <2 x i32> [[X:%.*]], -; CHECK-NEXT: [[TMP2:%.*]] = lshr <2 x i32> [[TMP1]], [[Y:%.*]] -; CHECK-NEXT: [[TMP3:%.*]] = and <2 x i32> [[TMP2]], -; CHECK-NEXT: ret <2 x i32> [[TMP3]] +; CHECK-NEXT: [[SH1:%.*]] = shl <2 x i32> , [[Y:%.*]] +; CHECK-NEXT: [[AND:%.*]] = and <2 x i32> [[SH1]], [[X:%.*]] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i32> [[AND]], zeroinitializer +; CHECK-NEXT: [[R:%.*]] = zext <2 x i1> [[CMP]] to <2 x i32> +; CHECK-NEXT: ret <2 x i32> [[R]] ; %sh1 = shl <2 x i32> , %y %and = and <2 x i32> %sh1, %x @@ -205,9 +208,11 @@ define <2 x i32> @masked_bit_clear(<2 x i32> %x, <2 x i32> %y) { define <2 x i32> @masked_bit_set_commute(<2 x i32> %px, <2 x i32> %y) { ; CHECK-LABEL: @masked_bit_set_commute( ; CHECK-NEXT: [[X:%.*]] = srem <2 x i32> , [[PX:%.*]] -; CHECK-NEXT: [[TMP1:%.*]] = lshr <2 x i32> [[X]], [[Y:%.*]] -; CHECK-NEXT: [[TMP2:%.*]] = and <2 x i32> [[TMP1]], -; CHECK-NEXT: ret <2 x i32> [[TMP2]] +; CHECK-NEXT: [[SH1:%.*]] = shl <2 x i32> , [[Y:%.*]] +; CHECK-NEXT: [[AND:%.*]] = and <2 x i32> [[X]], [[SH1]] +; CHECK-NEXT: [[CMP:%.*]] = icmp ne <2 x i32> [[AND]], zeroinitializer +; CHECK-NEXT: [[R:%.*]] = zext <2 x i1> [[CMP]] to <2 x i32> +; CHECK-NEXT: ret <2 x i32> [[R]] ; %x = srem <2 x i32> , %px ; thwart complexity-based canonicalization %sh1 = shl <2 x i32> , %y @@ -220,10 +225,11 @@ define <2 x i32> @masked_bit_set_commute(<2 x i32> %px, <2 x i32> %y) { define i32 @masked_bit_clear_commute(i32 %px, i32 %y) { ; CHECK-LABEL: @masked_bit_clear_commute( ; CHECK-NEXT: [[X:%.*]] = srem i32 42, [[PX:%.*]] -; CHECK-NEXT: [[TMP1:%.*]] = xor i32 [[X]], -1 -; CHECK-NEXT: [[TMP2:%.*]] = lshr i32 [[TMP1]], [[Y:%.*]] -; CHECK-NEXT: [[TMP3:%.*]] = and i32 [[TMP2]], 1 -; CHECK-NEXT: ret i32 [[TMP3]] +; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] +; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], [[SH1]] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[AND]], 0 +; CHECK-NEXT: [[R:%.*]] = zext i1 [[CMP]] to i32 +; CHECK-NEXT: ret i32 [[R]] ; %x = srem i32 42, %px ; thwart complexity-based canonicalization %sh1 = shl i32 1, %y @@ -237,9 +243,10 @@ define i32 @masked_bit_set_use1(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_set_use1( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] ; CHECK-NEXT: call void @use32(i32 [[SH1]]) -; CHECK-NEXT: [[TMP1:%.*]] = lshr i32 [[X:%.*]], [[Y]] -; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[TMP1]], 1 -; CHECK-NEXT: ret i32 [[TMP2]] +; CHECK-NEXT: [[AND:%.*]] = and i32 [[SH1]], [[X:%.*]] +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[AND]], 0 +; CHECK-NEXT: [[R:%.*]] = zext i1 [[CMP]] to i32 +; CHECK-NEXT: ret i32 [[R]] ; %sh1 = shl i32 1, %y call void @use32(i32 %sh1) @@ -249,8 +256,6 @@ define i32 @masked_bit_set_use1(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @masked_bit_set_use2(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_set_use2( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] @@ -268,8 +273,6 @@ define i32 @masked_bit_set_use2(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @masked_bit_set_use3(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_set_use3( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] @@ -291,10 +294,10 @@ define i32 @masked_bit_clear_use1(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_clear_use1( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] ; CHECK-NEXT: call void @use32(i32 [[SH1]]) -; CHECK-NEXT: [[TMP1:%.*]] = xor i32 [[X:%.*]], -1 -; CHECK-NEXT: [[TMP2:%.*]] = lshr i32 [[TMP1]], [[Y]] -; CHECK-NEXT: [[TMP3:%.*]] = and i32 [[TMP2]], 1 -; CHECK-NEXT: ret i32 [[TMP3]] +; CHECK-NEXT: [[AND:%.*]] = and i32 [[SH1]], [[X:%.*]] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[AND]], 0 +; CHECK-NEXT: [[R:%.*]] = zext i1 [[CMP]] to i32 +; CHECK-NEXT: ret i32 [[R]] ; %sh1 = shl i32 1, %y call void @use32(i32 %sh1) @@ -304,8 +307,6 @@ define i32 @masked_bit_clear_use1(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @masked_bit_clear_use2(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_clear_use2( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] @@ -323,8 +324,6 @@ define i32 @masked_bit_clear_use2(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @masked_bit_clear_use3(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_clear_use3( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] @@ -342,8 +341,6 @@ define i32 @masked_bit_clear_use3(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @masked_bits_set(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bits_set( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 3, [[Y:%.*]] @@ -359,8 +356,6 @@ define i32 @masked_bits_set(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @div_bit_set(i32 %x, i32 %y) { ; CHECK-LABEL: @div_bit_set( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] @@ -376,8 +371,6 @@ define i32 @div_bit_set(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @masked_bit_set_nonzero_cmp(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_set_nonzero_cmp( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] @@ -393,8 +386,6 @@ define i32 @masked_bit_set_nonzero_cmp(i32 %x, i32 %y) { ret i32 %r } -; Negative test - define i32 @masked_bit_wrong_pred(i32 %x, i32 %y) { ; CHECK-LABEL: @masked_bit_wrong_pred( ; CHECK-NEXT: [[SH1:%.*]] = shl i32 1, [[Y:%.*]] -- 2.7.4