From 65d0c7787b748d989b8191a54c6b30200bcab8b8 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 20 Aug 2024 16:07:07 +0200 Subject: [PATCH] [InstCombine] Remove AllOnes fallbacks in getMaskedTypeForICmpPair() getMaskedTypeForICmpPair() tries to model non-and operands as x & -1. However, this can end up confusing the matching logic, by picking the -1 operand as the "common" operand, resulting in a successful, but useless, match. This is what causes commutation failures for some of the optimizations driven by this function. Fix this by removing this -1 fallback entirely. We don't seem to have any test coverage that demonstrates why it would be needed. --- .../InstCombine/InstCombineAndOrXor.cpp | 4 ++++ .../test/Transforms/InstCombine/bit-checks.ll | 20 ++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp index b703bc7d04db5..0af06c7e463f8 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp @@ -270,6 +270,10 @@ static std::optional> getMaskedTypeForICmpPair( E = R2; Ok = true; } + + // Avoid matching against the -1 value we created for unmasked operand. + if (Ok && match(A, m_AllOnes())) + Ok = false; } // Bail if RHS was a icmp that can't be decomposed into an equality. diff --git a/llvm/test/Transforms/InstCombine/bit-checks.ll b/llvm/test/Transforms/InstCombine/bit-checks.ll index d1b6104085370..3e3426d951eb9 100644 --- a/llvm/test/Transforms/InstCombine/bit-checks.ll +++ b/llvm/test/Transforms/InstCombine/bit-checks.ll @@ -809,12 +809,10 @@ define i32 @main7a_logical(i32 %argc, i32 %argc2, i32 %argc3) { define i32 @main7b(i32 %argc, i32 %argc2, i32 %argc3x) { ; CHECK-LABEL: @main7b( ; CHECK-NEXT: [[ARGC3:%.*]] = mul i32 [[ARGC3X:%.*]], 42 -; CHECK-NEXT: [[AND1:%.*]] = and i32 [[ARGC:%.*]], [[ARGC2:%.*]] -; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[ARGC2]], [[AND1]] -; CHECK-NEXT: [[AND2:%.*]] = and i32 [[ARGC]], [[ARGC3]] -; CHECK-NEXT: [[TOBOOL3:%.*]] = icmp ne i32 [[ARGC3]], [[AND2]] -; CHECK-NEXT: [[AND_COND_NOT:%.*]] = or i1 [[TOBOOL]], [[TOBOOL3]] -; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND_NOT]] to i32 +; CHECK-NEXT: [[TMP1:%.*]] = or i32 [[ARGC2:%.*]], [[ARGC3]] +; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[ARGC:%.*]], [[TMP1]] +; CHECK-NEXT: [[AND_COND:%.*]] = icmp ne i32 [[TMP2]], [[TMP1]] +; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND]] to i32 ; CHECK-NEXT: ret i32 [[STOREMERGE]] ; %argc3 = mul i32 %argc3x, 42 ; thwart complexity-based canonicalization @@ -850,12 +848,10 @@ define i32 @main7b_logical(i32 %argc, i32 %argc2, i32 %argc3) { define i32 @main7c(i32 %argc, i32 %argc2, i32 %argc3x) { ; CHECK-LABEL: @main7c( ; CHECK-NEXT: [[ARGC3:%.*]] = mul i32 [[ARGC3X:%.*]], 42 -; CHECK-NEXT: [[AND1:%.*]] = and i32 [[ARGC2:%.*]], [[ARGC:%.*]] -; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[ARGC2]], [[AND1]] -; CHECK-NEXT: [[AND2:%.*]] = and i32 [[ARGC3]], [[ARGC]] -; CHECK-NEXT: [[TOBOOL3:%.*]] = icmp ne i32 [[ARGC3]], [[AND2]] -; CHECK-NEXT: [[AND_COND_NOT:%.*]] = or i1 [[TOBOOL]], [[TOBOOL3]] -; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND_NOT]] to i32 +; CHECK-NEXT: [[TMP1:%.*]] = or i32 [[ARGC2:%.*]], [[ARGC3]] +; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[ARGC:%.*]], [[TMP1]] +; CHECK-NEXT: [[AND_COND:%.*]] = icmp ne i32 [[TMP2]], [[TMP1]] +; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND]] to i32 ; CHECK-NEXT: ret i32 [[STOREMERGE]] ; %argc3 = mul i32 %argc3x, 42 ; thwart complexity-based canonicalization