[InstCombine] Clean up bitwise folds without one-use check #80587

dtcxzyw · 2024-02-04T07:09:18Z

This patch removes some bitwise folds that fail to check the one-use constraint on the operands.
See also the comments #77231 (comment).

llvmbot · 2024-02-04T07:09:49Z

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch removes some bitwise folds that fail to check the one-use constraint on the operands.
See also the comments #77231 (comment).

Full diff: https://github.com/llvm/llvm-project/pull/80587.diff

3 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (-16)
(modified) llvm/test/Transforms/InstCombine/add.ll (+1-1)
(modified) llvm/test/Transforms/InstCombine/or.ll (+116-4)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 1cfa797be2207..4a81b68b77fe6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3565,22 +3565,6 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
     if (match(Op1, m_Xor(m_Specific(B), m_Specific(A))))
       return BinaryOperator::CreateOr(Op1, C);
 
-  // ((A & B) ^ C) | B -> C | B
-  if (match(Op0, m_c_Xor(m_c_And(m_Value(A), m_Specific(Op1)), m_Value(C))))
-    return BinaryOperator::CreateOr(C, Op1);
-
-  // B | ((A & B) ^ C) -> B | C
-  if (match(Op1, m_c_Xor(m_c_And(m_Value(A), m_Specific(Op0)), m_Value(C))))
-    return BinaryOperator::CreateOr(Op0, C);
-
-  // ((B | C) & A) | B -> B | (A & C)
-  if (match(Op0, m_c_And(m_c_Or(m_Specific(Op1), m_Value(C)), m_Value(A))))
-    return BinaryOperator::CreateOr(Op1, Builder.CreateAnd(A, C));
-
-  // B | ((B | C) & A) -> B | (A & C)
-  if (match(Op1, m_c_And(m_c_Or(m_Specific(Op0), m_Value(C)), m_Value(A))))
-    return BinaryOperator::CreateOr(Op0, Builder.CreateAnd(A, C));
-
   if (Instruction *DeMorgan = matchDeMorgansLaws(I, *this))
     return DeMorgan;
 
diff --git a/llvm/test/Transforms/InstCombine/add.ll b/llvm/test/Transforms/InstCombine/add.ll
index 6242fc6f528a4..61a5ec3e4bb4f 100644
--- a/llvm/test/Transforms/InstCombine/add.ll
+++ b/llvm/test/Transforms/InstCombine/add.ll
@@ -1808,7 +1808,7 @@ define i8 @add_xor_and_var_extra_use(i8 noundef %x, i8 noundef %y) {
 ; CHECK-NEXT:    call void @use(i8 [[AND]])
 ; CHECK-NEXT:    [[XOR:%.*]] = xor i8 [[AND]], [[Y]]
 ; CHECK-NEXT:    call void @use(i8 [[XOR]])
-; CHECK-NEXT:    [[ADD:%.*]] = or i8 [[Y]], [[X]]
+; CHECK-NEXT:    [[ADD:%.*]] = or disjoint i8 [[XOR]], [[X]]
 ; CHECK-NEXT:    ret i8 [[ADD]]
 ;
   %and = and i8 %x, %y
diff --git a/llvm/test/Transforms/InstCombine/or.ll b/llvm/test/Transforms/InstCombine/or.ll
index 2238b6bcc5653..9ead7f2aae6b0 100644
--- a/llvm/test/Transforms/InstCombine/or.ll
+++ b/llvm/test/Transforms/InstCombine/or.ll
@@ -743,7 +743,7 @@ define i32 @test40d(i32 %a, i32 %b) {
 
 define i32 @test45(i32 %x, i32 %y, i32 %z) {
 ; CHECK-LABEL: @test45(
-; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[X:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[Z:%.*]], [[X:%.*]]
 ; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[TMP1]], [[Y:%.*]]
 ; CHECK-NEXT:    ret i32 [[OR1]]
 ;
@@ -753,10 +753,40 @@ define i32 @test45(i32 %x, i32 %y, i32 %z) {
   ret i32 %or1
 }
 
+define i32 @test45_uses1(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @test45_uses1(
+; CHECK-NEXT:    [[OR:%.*]] = or i32 [[Y:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    call void @use(i32 [[OR]])
+; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[Z]], [[X:%.*]]
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[TMP1]], [[Y]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %or = or i32 %y, %z
+  call void @use(i32 %or)
+  %and = and i32 %x, %or
+  %or1 = or i32 %and, %y
+  ret i32 %or1
+}
+
+define i32 @test45_uses2(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @test45_uses2(
+; CHECK-NEXT:    [[OR:%.*]] = or i32 [[Y:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[OR]], [[X:%.*]]
+; CHECK-NEXT:    call void @use(i32 [[AND]])
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[AND]], [[Y]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %or = or i32 %y, %z
+  %and = and i32 %x, %or
+  call void @use(i32 %and)
+  %or1 = or i32 %and, %y
+  ret i32 %or1
+}
+
 define i32 @test45_commuted1(i32 %x, i32 %y, i32 %z) {
 ; CHECK-LABEL: @test45_commuted1(
 ; CHECK-NEXT:    [[YY:%.*]] = mul i32 [[Y:%.*]], [[Y]]
-; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[X:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[Z:%.*]], [[X:%.*]]
 ; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[YY]], [[TMP1]]
 ; CHECK-NEXT:    ret i32 [[OR1]]
 ;
@@ -772,7 +802,7 @@ define i32 @test45_commuted2(i32 %x, i32 %y, i32 %z) {
 ; CHECK-NEXT:    [[YY:%.*]] = mul i32 [[Y:%.*]], [[Y]]
 ; CHECK-NEXT:    [[XX:%.*]] = mul i32 [[X:%.*]], [[X]]
 ; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[XX]], [[Z:%.*]]
-; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[YY]], [[TMP1]]
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[TMP1]], [[YY]]
 ; CHECK-NEXT:    ret i32 [[OR1]]
 ;
   %yy = mul i32 %y, %y ; thwart complexity-based ordering
@@ -788,7 +818,7 @@ define i32 @test45_commuted3(i32 %x, i32 %y, i32 %z) {
 ; CHECK-NEXT:    [[YY:%.*]] = mul i32 [[Y:%.*]], [[Y]]
 ; CHECK-NEXT:    [[ZZ:%.*]] = mul i32 [[Z:%.*]], [[Z]]
 ; CHECK-NEXT:    [[TMP1:%.*]] = and i32 [[ZZ]], [[X:%.*]]
-; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[YY]], [[TMP1]]
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[TMP1]], [[YY]]
 ; CHECK-NEXT:    ret i32 [[OR1]]
 ;
   %yy = mul i32 %y, %y ; thwart complexity-based ordering
@@ -1901,3 +1931,85 @@ define i32 @test_or_add_xor(i32 %a, i32 %b, i32 %c) {
   %or = or i32 %add, %a
   ret i32 %or
 }
+
+define i32 @or_xor_and(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @or_xor_and(
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %and = and i32 %y, %z
+  %xor = xor i32 %x, %and
+  %or1 = or i32 %xor, %y
+  ret i32 %or1
+}
+
+define i32 @or_xor_and_uses1(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @or_xor_and_uses1(
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[Y:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    call void @use(i32 [[AND]])
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[X:%.*]], [[Y]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %and = and i32 %y, %z
+  call void @use(i32 %and)
+  %xor = xor i32 %x, %and
+  %or1 = or i32 %xor, %y
+  ret i32 %or1
+}
+
+define i32 @or_xor_and_uses2(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @or_xor_and_uses2(
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[Y:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    [[XOR:%.*]] = xor i32 [[AND]], [[X:%.*]]
+; CHECK-NEXT:    call void @use(i32 [[XOR]])
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[XOR]], [[Y]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %and = and i32 %y, %z
+  %xor = xor i32 %x, %and
+  call void @use(i32 %xor)
+  %or1 = or i32 %xor, %y
+  ret i32 %or1
+}
+
+define i32 @or_xor_and_commuted1(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @or_xor_and_commuted1(
+; CHECK-NEXT:    [[YY:%.*]] = mul i32 [[Y:%.*]], [[Y]]
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[YY]], [[X:%.*]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %yy = mul i32 %y, %y ; thwart complexity-based ordering
+  %and = and i32 %yy, %z
+  %xor = xor i32 %and, %x
+  %or1 = or i32 %yy, %xor
+  ret i32 %or1
+}
+
+define i32 @or_xor_and_commuted2(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @or_xor_and_commuted2(
+; CHECK-NEXT:    [[YY:%.*]] = mul i32 [[Y:%.*]], [[Y]]
+; CHECK-NEXT:    [[XX:%.*]] = mul i32 [[X:%.*]], [[X]]
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[XX]], [[YY]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %yy = mul i32 %y, %y ; thwart complexity-based ordering
+  %xx = mul i32 %x, %x ; thwart complexity-based ordering
+  %and = and i32 %yy, %z
+  %xor = xor i32 %xx, %and
+  %or1 = or i32 %xor, %yy
+  ret i32 %or1
+}
+
+define i32 @or_xor_and_commuted3(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: @or_xor_and_commuted3(
+; CHECK-NEXT:    [[YY:%.*]] = mul i32 [[Y:%.*]], [[Y]]
+; CHECK-NEXT:    [[OR1:%.*]] = or i32 [[YY]], [[X:%.*]]
+; CHECK-NEXT:    ret i32 [[OR1]]
+;
+  %yy = mul i32 %y, %y ; thwart complexity-based ordering
+  %zz = mul i32 %z, %z ; thwart complexity-based ordering
+  %and = and i32 %zz, %yy
+  %xor = xor i32 %and, %x
+  %or1 = or i32 %xor, %yy
+  ret i32 %or1
+}

PR Link: llvm/llvm-project#80587

nikic

I think you may be able to drop

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Lines 3621 to 3630 in 616ca00

    
           // A | (~A ^ B) --> ~B | A 
        
           // B | (A ^ ~B) --> ~A | B 
        
           if (Op1->hasOneUse() && match(A, m_Not(m_Specific(Op0)))) { 
        
             Value *NotB = Builder.CreateNot(B, B->getName() + ".not"); 
        
             return BinaryOperator::CreateOr(NotB, Op0); 
        
           } 
        
           if (Op1->hasOneUse() && match(B, m_Not(m_Specific(Op0)))) { 
        
             Value *NotA = Builder.CreateNot(A, A->getName() + ".not"); 
        
             return BinaryOperator::CreateOr(NotA, Op0); 
        
           }

as redundant? It has a one-use check, so I think it is subsumed by your fold.

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

dtcxzyw · 2024-02-04T12:05:47Z

I think you may be able to drop

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Lines 3621 to 3630 in 616ca00

// A | (~A ^ B) --> ~B | A

// B | (A ^ ~B) --> ~A | B

if (Op1->hasOneUse() && match(A, m_Not(m_Specific(Op0)))) {

Value *NotB = Builder.CreateNot(B, B->getName() + ".not");

return BinaryOperator::CreateOr(NotB, Op0);

}

if (Op1->hasOneUse() && match(B, m_Not(m_Specific(Op0)))) {

Value *NotA = Builder.CreateNot(A, A->getName() + ".not");

return BinaryOperator::CreateOr(NotA, Op0);

}

as redundant? It has a one-use check, so I think it is subsumed by your fold.

Done

nikic

LGTM

dtcxzyw requested a review from goldsteinn February 4, 2024 07:09

dtcxzyw requested a review from nikic as a code owner February 4, 2024 07:09

llvmbot added the llvm:transforms label Feb 4, 2024

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Feb 4, 2024

pre-commit: test PR80587

6bf514c

PR Link: llvm/llvm-project#80587

dtcxzyw mentioned this pull request Feb 4, 2024

pre-commit: test PR80587 dtcxzyw/llvm-opt-benchmark#186

Closed

nikic reviewed Feb 4, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Show resolved Hide resolved

dtcxzyw force-pushed the cleanup-bit-fold branch from 616ca00 to f721409 Compare February 4, 2024 12:05

nikic approved these changes Feb 7, 2024

View reviewed changes

dtcxzyw added 2 commits February 8, 2024 00:40

[InstCombine] Add pre-commit tests. NFC.

e2897e4

[InstCombine] Clean up bitwise folds without one-use check

1cebf05

dtcxzyw force-pushed the cleanup-bit-fold branch from f721409 to 1cebf05 Compare February 7, 2024 16:59

dtcxzyw merged commit 65bf93d into llvm:main Feb 7, 2024
3 of 4 checks passed

dtcxzyw deleted the cleanup-bit-fold branch February 7, 2024 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[InstCombine] Clean up bitwise folds without one-use check #80587

[InstCombine] Clean up bitwise folds without one-use check #80587

dtcxzyw commented Feb 4, 2024

llvmbot commented Feb 4, 2024

nikic left a comment

dtcxzyw commented Feb 4, 2024

nikic left a comment

	// A \| (~A ^ B) --> ~B \| A
	// B \| (A ^ ~B) --> ~A \| B
	if (Op1->hasOneUse() && match(A, m_Not(m_Specific(Op0)))) {
	Value *NotB = Builder.CreateNot(B, B->getName() + ".not");
	return BinaryOperator::CreateOr(NotB, Op0);
	}
	if (Op1->hasOneUse() && match(B, m_Not(m_Specific(Op0)))) {
	Value *NotA = Builder.CreateNot(A, A->getName() + ".not");
	return BinaryOperator::CreateOr(NotA, Op0);
	}

[InstCombine] Clean up bitwise folds without one-use check #80587

[InstCombine] Clean up bitwise folds without one-use check #80587

Conversation

dtcxzyw commented Feb 4, 2024

llvmbot commented Feb 4, 2024

nikic left a comment

Choose a reason for hiding this comment

dtcxzyw commented Feb 4, 2024

nikic left a comment

Choose a reason for hiding this comment