Skip to content

Commit 65d0c77

Browse files
committed
[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.
1 parent 32679e1 commit 65d0c77

File tree

2 files changed

+12
-12
lines changed

2 files changed

+12
-12
lines changed

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,10 @@ static std::optional<std::pair<unsigned, unsigned>> getMaskedTypeForICmpPair(
270270
E = R2;
271271
Ok = true;
272272
}
273+
274+
// Avoid matching against the -1 value we created for unmasked operand.
275+
if (Ok && match(A, m_AllOnes()))
276+
Ok = false;
273277
}
274278

275279
// Bail if RHS was a icmp that can't be decomposed into an equality.

llvm/test/Transforms/InstCombine/bit-checks.ll

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -809,12 +809,10 @@ define i32 @main7a_logical(i32 %argc, i32 %argc2, i32 %argc3) {
809809
define i32 @main7b(i32 %argc, i32 %argc2, i32 %argc3x) {
810810
; CHECK-LABEL: @main7b(
811811
; CHECK-NEXT: [[ARGC3:%.*]] = mul i32 [[ARGC3X:%.*]], 42
812-
; CHECK-NEXT: [[AND1:%.*]] = and i32 [[ARGC:%.*]], [[ARGC2:%.*]]
813-
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[ARGC2]], [[AND1]]
814-
; CHECK-NEXT: [[AND2:%.*]] = and i32 [[ARGC]], [[ARGC3]]
815-
; CHECK-NEXT: [[TOBOOL3:%.*]] = icmp ne i32 [[ARGC3]], [[AND2]]
816-
; CHECK-NEXT: [[AND_COND_NOT:%.*]] = or i1 [[TOBOOL]], [[TOBOOL3]]
817-
; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND_NOT]] to i32
812+
; CHECK-NEXT: [[TMP1:%.*]] = or i32 [[ARGC2:%.*]], [[ARGC3]]
813+
; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[ARGC:%.*]], [[TMP1]]
814+
; CHECK-NEXT: [[AND_COND:%.*]] = icmp ne i32 [[TMP2]], [[TMP1]]
815+
; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND]] to i32
818816
; CHECK-NEXT: ret i32 [[STOREMERGE]]
819817
;
820818
%argc3 = mul i32 %argc3x, 42 ; thwart complexity-based canonicalization
@@ -850,12 +848,10 @@ define i32 @main7b_logical(i32 %argc, i32 %argc2, i32 %argc3) {
850848
define i32 @main7c(i32 %argc, i32 %argc2, i32 %argc3x) {
851849
; CHECK-LABEL: @main7c(
852850
; CHECK-NEXT: [[ARGC3:%.*]] = mul i32 [[ARGC3X:%.*]], 42
853-
; CHECK-NEXT: [[AND1:%.*]] = and i32 [[ARGC2:%.*]], [[ARGC:%.*]]
854-
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[ARGC2]], [[AND1]]
855-
; CHECK-NEXT: [[AND2:%.*]] = and i32 [[ARGC3]], [[ARGC]]
856-
; CHECK-NEXT: [[TOBOOL3:%.*]] = icmp ne i32 [[ARGC3]], [[AND2]]
857-
; CHECK-NEXT: [[AND_COND_NOT:%.*]] = or i1 [[TOBOOL]], [[TOBOOL3]]
858-
; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND_NOT]] to i32
851+
; CHECK-NEXT: [[TMP1:%.*]] = or i32 [[ARGC2:%.*]], [[ARGC3]]
852+
; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[ARGC:%.*]], [[TMP1]]
853+
; CHECK-NEXT: [[AND_COND:%.*]] = icmp ne i32 [[TMP2]], [[TMP1]]
854+
; CHECK-NEXT: [[STOREMERGE:%.*]] = zext i1 [[AND_COND]] to i32
859855
; CHECK-NEXT: ret i32 [[STOREMERGE]]
860856
;
861857
%argc3 = mul i32 %argc3x, 42 ; thwart complexity-based canonicalization

0 commit comments

Comments
 (0)