[InstCombine] Support and/or in `getFreelyInvertedImpl` using DeMorgan's Law #85193

dtcxzyw · 2024-03-14T07:53:29Z

This patch adds the support for and/or in getFreelyInvertedImpl using DeMorgan's Law:

(~(A | B)) -> (~A & ~B)
(~(A & B)) -> (~A | ~B)

Alive2: https://alive2.llvm.org/ce/z/Uig8-j

llvmbot · 2024-03-14T07:54:03Z

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch adds the support for and/or in getFreelyInvertedImpl using DeMorgan's Law:

(~(A | B)) -&gt; (~A &amp; ~B)
(~(A &amp; B)) -&gt; (~A | ~B)

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

4 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+26)
(modified) llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll (+7-9)
(modified) llvm/test/Transforms/InstCombine/not.ll (+167)
(modified) llvm/test/Transforms/InstCombine/pr63791.ll (+1-1)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 1688005de2104d..8ee4979fc17892 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2596,6 +2596,32 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
     return nullptr;
   }
 
+  // De Morgan's Laws:
+  // (~(A | B)) -> (~A & ~B)
+  // (~(A & B)) -> (~A | ~B)
+  auto TryInvertAndOrUsingDeMorgan = [&](Instruction::BinaryOps Opcode,
+                                         Value *A, Value *B) -> Value * {
+    bool LocalDoesConsume = DoesConsume;
+    if (!getFreelyInvertedImpl(B, B->hasOneUse(), /*Builder=*/nullptr,
+                               LocalDoesConsume, Depth))
+      return nullptr;
+    if (auto *NotA = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
+                                           LocalDoesConsume, Depth)) {
+      auto *NotB = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
+                                         LocalDoesConsume, Depth);
+      DoesConsume = LocalDoesConsume;
+      return Builder ? Builder->CreateBinOp(Opcode, NotA, NotB) : NonNull;
+    }
+
+    return nullptr;
+  };
+
+  if (match(V, m_Or(m_Value(A), m_Value(B))))
+    return TryInvertAndOrUsingDeMorgan(Instruction::And, A, B);
+
+  if (match(V, m_And(m_Value(A), m_Value(B))))
+    return TryInvertAndOrUsingDeMorgan(Instruction::Or, A, B);
+
   return nullptr;
 }
 
diff --git a/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll b/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll
index 07060922895846..410b6c29187b22 100644
--- a/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-and-lowbit-mask.ll
@@ -226,12 +226,11 @@ define i1 @src_is_mask_shl_lshr(i8 %x_in, i8 %y, i1 %cond) {
 
 define i1 @src_is_mask_shl_lshr_fail_not_allones(i8 %x_in, i8 %y, i1 %cond) {
 ; CHECK-LABEL: @src_is_mask_shl_lshr_fail_not_allones(
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
 ; CHECK-NEXT:    [[TMP1:%.*]] = lshr i8 -1, [[Y:%.*]]
 ; CHECK-NEXT:    [[MASK:%.*]] = and i8 [[TMP1]], -2
-; CHECK-NEXT:    [[NOTMASK:%.*]] = xor i8 [[MASK]], -1
-; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X]], [[NOTMASK]]
-; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[AND]], 0
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i8 [[X_IN:%.*]], -124
+; CHECK-NEXT:    [[TMP3:%.*]] = or i8 [[TMP2]], [[MASK]]
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[TMP3]], -1
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
@@ -572,11 +571,10 @@ define i1 @src_is_notmask_neg_p2(i8 %x_in, i8 %y) {
 
 define i1 @src_is_notmask_neg_p2_fail_not_invertable(i8 %x_in, i8 %y) {
 ; CHECK-LABEL: @src_is_notmask_neg_p2_fail_not_invertable(
-; CHECK-NEXT:    [[X:%.*]] = xor i8 [[X_IN:%.*]], 123
-; CHECK-NEXT:    [[TMP1:%.*]] = add i8 [[Y:%.*]], -1
-; CHECK-NEXT:    [[TMP2:%.*]] = xor i8 [[Y]], -1
-; CHECK-NEXT:    [[TMP3:%.*]] = and i8 [[TMP1]], [[TMP2]]
-; CHECK-NEXT:    [[R:%.*]] = icmp ule i8 [[X]], [[TMP3]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X_IN:%.*]], -124
+; CHECK-NEXT:    [[TMP2:%.*]] = sub i8 0, [[Y:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = or i8 [[TMP2]], [[Y]]
+; CHECK-NEXT:    [[R:%.*]] = icmp uge i8 [[TMP1]], [[TMP3]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %x = xor i8 %x_in, 123
diff --git a/llvm/test/Transforms/InstCombine/not.ll b/llvm/test/Transforms/InstCombine/not.ll
index f277d13eee930c..0835358682b808 100644
--- a/llvm/test/Transforms/InstCombine/not.ll
+++ b/llvm/test/Transforms/InstCombine/not.ll
@@ -3,6 +3,8 @@
 
 declare void @use1(i1)
 declare void @use8(i8)
+declare void @f1()
+declare void @f2()
 
 define i32 @test1(i32 %A) {
 ; CHECK-LABEL: @test1(
@@ -858,3 +860,168 @@ define i32 @test_zext(i32 %a, i32 %b){
   %not = xor i32 %add, -1
   ret i32 %not
 }
+
+define void @test_invert_demorgan_or(i32 %a, i32 %b, i1 %cond) {
+; CHECK-LABEL: @test_invert_demorgan_or(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i32 [[B:%.*]], 0
+; CHECK-NEXT:    [[CMP3:%.*]] = icmp eq i32 [[B1:%.*]], 0
+; CHECK-NEXT:    [[OR_NOT1:%.*]] = and i1 [[CMP2]], [[CMP3]]
+; CHECK-NEXT:    [[MERGE:%.*]] = and i1 [[OR_NOT1]], [[COND:%.*]]
+; CHECK-NEXT:    br i1 [[MERGE]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    call void @f1()
+; CHECK-NEXT:    unreachable
+; CHECK:       if.else:
+; CHECK-NEXT:    call void @f2()
+; CHECK-NEXT:    unreachable
+;
+entry:
+  %cmp1 = icmp eq i32 %a, 0
+  %cmp2 = icmp ne i32 %b, 0
+  %or = or i1 %cmp1, %cmp2
+  %not = xor i1 %cond, true
+  %merge = or i1 %not, %or
+  br i1 %merge, label %if.then, label %if.else
+if.then:
+  call void @f1()
+  unreachable
+if.else:
+  call void @f2()
+  unreachable
+}
+
+define i1 @test_invert_demorgan_or2(i64 %a, i64 %b, i64 %c) {
+; CHECK-LABEL: @test_invert_demorgan_or2(
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ult i64 [[A:%.*]], 24
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ult i64 [[B:%.*]], 60
+; CHECK-NEXT:    [[OR1_NOT1:%.*]] = and i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT:    [[CMP3:%.*]] = icmp ult i64 [[C:%.*]], 60
+; CHECK-NEXT:    [[NOT:%.*]] = and i1 [[OR1_NOT1]], [[CMP3]]
+; CHECK-NEXT:    ret i1 [[NOT]]
+;
+  %cmp1 = icmp ugt i64 %a, 23
+  %cmp2 = icmp ugt i64 %b, 59
+  %or1 = or i1 %cmp1, %cmp2
+  %cmp3 = icmp ugt i64 %c, 59
+  %or2 = or i1 %or1, %cmp3
+  %not = xor i1 %or2, true
+  ret i1 %not
+}
+
+define i1 @test_invert_demorgan_or3(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_invert_demorgan_or3(
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i32 [[A:%.*]], 178206
+; CHECK-NEXT:    [[TMP1:%.*]] = add i32 [[B:%.*]], -196608
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ult i32 [[TMP1]], -1506
+; CHECK-NEXT:    [[TMP2:%.*]] = add i32 [[B]], -917760
+; CHECK-NEXT:    [[CMP3:%.*]] = icmp ult i32 [[TMP2]], -716213
+; CHECK-NEXT:    [[TMP3:%.*]] = add i32 [[B]], -1114112
+; CHECK-NEXT:    [[CMP4:%.*]] = icmp ult i32 [[TMP3]], -196112
+; CHECK-NEXT:    [[OR1_NOT2:%.*]] = and i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT:    [[OR2_NOT1:%.*]] = and i1 [[OR1_NOT2]], [[CMP3]]
+; CHECK-NEXT:    [[NOT:%.*]] = and i1 [[OR2_NOT1]], [[CMP4]]
+; CHECK-NEXT:    ret i1 [[NOT]]
+;
+  %cmp1 = icmp eq i32 %a, 178206
+  %v1 = add i32 %b, -195102
+  %cmp2 = icmp ult i32 %v1, 1506
+  %v2 = add i32 %b, -201547
+  %cmp3 = icmp ult i32 %v2, 716213
+  %v3 = add i32 %b, -918000
+  %cmp4 = icmp ult i32 %v3, 196112
+  %or1 = or i1 %cmp1, %cmp2
+  %or2 = or i1 %or1, %cmp3
+  %or3 = or i1 %or2, %cmp4
+  %not = xor i1 %or3, true
+  ret i1 %not
+}
+
+define i1 @test_invert_demorgan_and(i32 %a, i32 %b, i1 %cond) {
+; CHECK-LABEL: @test_invert_demorgan_and(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i32 [[B:%.*]], 0
+; CHECK-NEXT:    [[CMP3:%.*]] = icmp eq i32 [[B1:%.*]], 0
+; CHECK-NEXT:    [[AND_NOT1:%.*]] = or i1 [[CMP2]], [[CMP3]]
+; CHECK-NEXT:    [[MERGE:%.*]] = or i1 [[AND_NOT1]], [[COND:%.*]]
+; CHECK-NEXT:    br i1 [[MERGE]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    call void @f1()
+; CHECK-NEXT:    unreachable
+; CHECK:       if.else:
+; CHECK-NEXT:    call void @f2()
+; CHECK-NEXT:    unreachable
+;
+entry:
+  %cmp1 = icmp eq i32 %a, 0
+  %cmp2 = icmp ne i32 %b, 0
+  %and = and i1 %cmp1, %cmp2
+  %not = xor i1 %cond, true
+  %merge = and i1 %not, %and
+  br i1 %merge, label %if.then, label %if.else
+if.then:
+  call void @f1()
+  unreachable
+if.else:
+  call void @f2()
+  unreachable
+}
+
+define i64 @test_invert_demorgan_and2(i64 %x) {
+; CHECK-LABEL: @test_invert_demorgan_and2(
+; CHECK-NEXT:    [[TMP1:%.*]] = sub i64 0, [[X:%.*]]
+; CHECK-NEXT:    [[SUB:%.*]] = or i64 [[TMP1]], -9223372036854775808
+; CHECK-NEXT:    ret i64 [[SUB]]
+;
+  %add = add i64 %x, 9223372036854775807
+  %and = and i64 %add, 9223372036854775807
+  %sub = xor i64 %and, -1
+  ret i64 %sub
+}
+
+define i1 @test_invert_demorgan_and3(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_invert_demorgan_and3(
+; CHECK-NEXT:    [[ADD:%.*]] = sub i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[ADD]], 4095
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[AND]], 4095
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %not = xor i32 %a, -1
+  %add = add i32 %b, %not
+  %and = and i32 %add, 4095
+  %cmp = icmp eq i32 %and, 0
+  ret i1 %cmp
+}
+
+define i1 @test_invert_demorgan_and_multiuse(i32 %a, i32 %b, i1 %cond) {
+; CHECK-LABEL: @test_invert_demorgan_and_multiuse(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i32 [[A:%.*]], 0
+; CHECK-NEXT:    call void @use1(i1 [[CMP1]])
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i32 [[B:%.*]], 0
+; CHECK-NEXT:    [[NOT:%.*]] = xor i1 [[COND:%.*]], true
+; CHECK-NEXT:    [[TMP0:%.*]] = and i1 [[CMP2]], [[NOT]]
+; CHECK-NEXT:    [[MERGE:%.*]] = and i1 [[TMP0]], [[CMP1]]
+; CHECK-NEXT:    br i1 [[MERGE]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    call void @f1()
+; CHECK-NEXT:    unreachable
+; CHECK:       if.else:
+; CHECK-NEXT:    call void @f2()
+; CHECK-NEXT:    unreachable
+;
+entry:
+  %cmp1 = icmp eq i32 %a, 0
+  call void @use1(i1 %cmp1)
+  %cmp2 = icmp ne i32 %b, 0
+  %and = and i1 %cmp1, %cmp2
+  %not = xor i1 %cond, true
+  %merge = and i1 %not, %and
+  br i1 %merge, label %if.then, label %if.else
+if.then:
+  call void @f1()
+  unreachable
+if.else:
+  call void @f2()
+  unreachable
+}
diff --git a/llvm/test/Transforms/InstCombine/pr63791.ll b/llvm/test/Transforms/InstCombine/pr63791.ll
index 78cc1130fb33f0..73a559f9892612 100644
--- a/llvm/test/Transforms/InstCombine/pr63791.ll
+++ b/llvm/test/Transforms/InstCombine/pr63791.ll
@@ -15,7 +15,7 @@ define void @y() {
 ; CHECK-NEXT:    store i1 true, ptr poison, align 1
 ; CHECK-NEXT:    br i1 poison, label [[FOR_COND_I]], label [[FOR_COND5_PREHEADER_I]]
 ; CHECK:       for.cond5.preheader.i:
-; CHECK-NEXT:    br i1 false, label [[FOR_INC19_I:%.*]], label [[FOR_COND1_LOOPEXIT_I:%.*]]
+; CHECK-NEXT:    br i1 true, label [[FOR_COND1_LOOPEXIT_I:%.*]], label [[FOR_INC19_I:%.*]]
 ; CHECK:       for.inc19.i:
 ; CHECK-NEXT:    br i1 poison, label [[FOR_INC19_I]], label [[FOR_COND1_LOOPEXIT_I]]
 ;

PR Link: llvm/llvm-project#85193

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

…n's Law

PR Link: llvm/llvm-project#85193

nikic · 2024-03-14T15:15:51Z

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

+
+  if (match(V, m_LogicalAnd(m_Value(A), m_Value(B))))
+    return TryInvertAndOrUsingDeMorgan(Instruction::Or, /*IsLogical=*/true, A,
+                                       B);


I'd just keep the m_Logical cases and then do bool IsLogical = isa<SelectInst> inside the helper.

m_Logical only matches and/or with boolean types.

llvm-project/llvm/include/llvm/IR/PatternMatch.h

Lines 2739 to 2757 in f2d02ce

template <typename LHS, typename RHS, unsigned Opcode, bool Commutable = false>

struct LogicalOp_match {

LHS L;

RHS R;

LogicalOp_match(const LHS &L, const RHS &R) : L(L), R(R) {}

template <typename T> bool match(T *V) {

auto *I = dyn_cast<Instruction>(V);

if (!I || !I->getType()->isIntOrIntVectorTy(1))

return false;

if (I->getOpcode() == Opcode) {

auto *Op0 = I->getOperand(0);

auto *Op1 = I->getOperand(1);

return (L.match(Op0) && R.match(Op1)) ||

(Commutable && L.match(Op1) && R.match(Op0));

}

Note the select code intentionally skips logicaland/logicalor patterns via: shouldAvoidAbsorbingNotIntoSelect

Yeah, logical ors/ands will be handled here :)

Note the select code intentionally skips logicaland/logicalor patterns via: shouldAvoidAbsorbingNotIntoSelect

Yeah, logical ors/ands will be handled here :)

I had misread nikics comment, thought he meant just let the select logic handle this. Delete when I saw my err :)

goldsteinn · 2024-03-14T17:31:30Z

LGTM, wait on nikic

nikic

LGTM

vitalybuka · 2024-03-27T22:35:43Z

We probably need to revert this one before figuring out what to do about #86880

thurstond · 2024-03-27T22:56:52Z

We probably need to revert this one before figuring out what to do about #86880

topperc mentioned in #86880 (comment) that it "post-RA machine copy propagation removed a copy that was needed to make the code correct", so it's probably not worthwhile reverting this innocent patch that just happened to unluckily trigger the bug.

topperc · 2024-03-27T22:58:18Z

We probably need to revert this one before figuring out what to do about #86880

topperc mentioned in #86880 (comment) that it "post-RA machine copy propagation removed a copy that was needed to make the code correct", so it's probably not worthwhile reverting this innocent patch that just happened to unluckily trigger the bug.

I'm debugging MCP now.

thurstond · 2024-03-27T23:00:29Z

Thank you @topperc! :-)

dtcxzyw requested a review from goldsteinn March 14, 2024 07:53

dtcxzyw requested a review from nikic as a code owner March 14, 2024 07:53

llvmbot added the llvm:transforms label Mar 14, 2024

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

pre-commit: test PR85193

49ca995

PR Link: llvm/llvm-project#85193

dtcxzyw mentioned this pull request Mar 14, 2024

pre-commit: test PR85193 dtcxzyw/llvm-opt-benchmark#403

Closed

nikic reviewed Mar 14, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp Outdated Show resolved Hide resolved

dtcxzyw added 2 commits March 14, 2024 19:40

[InstCombine] Add pre-commit tests. NFC.

c697d80

[InstCombine] Support and/or in getFreelyInvertedImpl using DeMorga…

00d0e75

…n's Law

dtcxzyw force-pushed the perf/freely-invert-demorgan branch from 9a44f5d to 00d0e75 Compare March 14, 2024 11:46

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

pre-commit: test PR85193

cad906f

PR Link: llvm/llvm-project#85193

nikic reviewed Mar 14, 2024

View reviewed changes

nikic approved these changes Mar 15, 2024

View reviewed changes

dtcxzyw merged commit cf5cd98 into llvm:main Mar 15, 2024
4 checks passed

dtcxzyw deleted the perf/freely-invert-demorgan branch March 15, 2024 11:10

thurstond mentioned this pull request Mar 27, 2024

Frame pointer corrupted by __cpuid after #85193 #86880

Closed

svs-quic mentioned this pull request May 8, 2024

Regression on trunk: missed optimization for dead code, work as expected on clang-18 #91431

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[InstCombine] Support and/or in `getFreelyInvertedImpl` using DeMorgan's Law #85193

[InstCombine] Support and/or in `getFreelyInvertedImpl` using DeMorgan's Law #85193

dtcxzyw commented Mar 14, 2024 •

edited

llvmbot commented Mar 14, 2024

nikic Mar 14, 2024

dtcxzyw Mar 14, 2024

dtcxzyw Mar 14, 2024

goldsteinn Mar 14, 2024

goldsteinn commented Mar 14, 2024

nikic left a comment

vitalybuka commented Mar 27, 2024

thurstond commented Mar 27, 2024

topperc commented Mar 27, 2024

thurstond commented Mar 27, 2024

	template <typename LHS, typename RHS, unsigned Opcode, bool Commutable = false>
	struct LogicalOp_match {
	LHS L;
	RHS R;

	LogicalOp_match(const LHS &L, const RHS &R) : L(L), R(R) {}

	template <typename T> bool match(T *V) {
	auto *I = dyn_cast<Instruction>(V);
	if (!I \|\| !I->getType()->isIntOrIntVectorTy(1))
	return false;

	if (I->getOpcode() == Opcode) {
	auto *Op0 = I->getOperand(0);
	auto *Op1 = I->getOperand(1);
	return (L.match(Op0) && R.match(Op1)) \|\|
	(Commutable && L.match(Op1) && R.match(Op0));
	}

[InstCombine] Support and/or in getFreelyInvertedImpl using DeMorgan's Law #85193

[InstCombine] Support and/or in getFreelyInvertedImpl using DeMorgan's Law #85193

Conversation

dtcxzyw commented Mar 14, 2024 • edited

llvmbot commented Mar 14, 2024

nikic Mar 14, 2024

Choose a reason for hiding this comment

dtcxzyw Mar 14, 2024

Choose a reason for hiding this comment

dtcxzyw Mar 14, 2024

Choose a reason for hiding this comment

goldsteinn Mar 14, 2024

Choose a reason for hiding this comment

goldsteinn commented Mar 14, 2024

nikic left a comment

Choose a reason for hiding this comment

vitalybuka commented Mar 27, 2024

thurstond commented Mar 27, 2024

topperc commented Mar 27, 2024

thurstond commented Mar 27, 2024

[InstCombine] Support and/or in `getFreelyInvertedImpl` using DeMorgan's Law #85193

[InstCombine] Support and/or in `getFreelyInvertedImpl` using DeMorgan's Law #85193

dtcxzyw commented Mar 14, 2024 •

edited