[llvm] [InstCombine] Teach tryFactorization to treat a disjoint Or like an Add. (PR #75691)

[Craig Topper via llvm-commits]

https://github.com/topperc created https://github.com/llvm/llvm-project/pull/75691

If the InnerOpcode is a Mul. This replaces a combine in visitOr that was not handling all commuted cases.

This does change one of the negative cases for the original combine. We now introduce a second Mul, but the same thing already happens if the Or was an Add.

>From e8e1fbfb92465828eeee36f663cb15e1b43163bc Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 15 Dec 2023 22:15:40 -0800
Subject: [PATCH] [InstCombine] Teach tryFactorization to treat a disjoint Or
 like an Add.

If the InnerOpcode is a Mul. This replaces a combine in visitOr that
was not handling all commuted cases.

This does change one of the negative cases for the original combine. We
now introduce a second Mul, but the same thing already happens if
the Or was an Add.
---
 llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp  | 8 --------
 llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 5 +++++
 llvm/test/Transforms/InstCombine/or.ll                   | 8 ++++----
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 31db1d3164b772..3ba2f19f010f97 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3391,14 +3391,6 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
     return BinaryOperator::CreateXor(Or, ConstantInt::get(Ty, *CV));
   }
 
-  // If the operands have no common bits set:
-  // or (mul X, Y), X --> add (mul X, Y), X --> mul X, (Y + 1)
-  if (match(&I, m_c_DisjointOr(m_OneUse(m_Mul(m_Value(X), m_Value(Y))),
-                               m_Deferred(X)))) {
-    Value *IncrementY = Builder.CreateAdd(Y, ConstantInt::get(Ty, 1));
-    return BinaryOperator::CreateMul(X, IncrementY);
-  }
-
   // X | (X ^ Y) --> X | Y (4 commuted patterns)
   if (match(&I, m_c_Or(m_Value(X), m_c_Xor(m_Deferred(X), m_Value(Y)))))
     return BinaryOperator::CreateOr(X, Y);
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index a7ddadc25de43c..a5f4b8e40622cd 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -646,6 +646,11 @@ static Value *tryFactorization(BinaryOperator &I, const SimplifyQuery &SQ,
   Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
   Instruction::BinaryOps TopLevelOpcode = I.getOpcode();
 
+  // Treat a disjoint Or like an Add if the inner opcode is a Mul.
+  if (I.getOpcode() == Instruction::Or && InnerOpcode == Instruction::Mul &&
+      cast<PossiblyDisjointInst>(I).isDisjoint())
+    TopLevelOpcode = Instruction::Add;
+
   // Does "X op' Y" always equal "Y op' X"?
   bool InnerCommutative = Instruction::isCommutative(InnerOpcode);
 
diff --git a/llvm/test/Transforms/InstCombine/or.ll b/llvm/test/Transforms/InstCombine/or.ll
index 805546099398b4..757f8b965ea68d 100644
--- a/llvm/test/Transforms/InstCombine/or.ll
+++ b/llvm/test/Transforms/InstCombine/or.ll
@@ -1515,8 +1515,8 @@ define i32 @mul_no_common_bits_commute2(i32 %p1, i32 %p2) {
 ; CHECK-LABEL: @mul_no_common_bits_commute2(
 ; CHECK-NEXT:    [[X:%.*]] = and i32 [[P1:%.*]], 7
 ; CHECK-NEXT:    [[Y:%.*]] = shl i32 [[P2:%.*]], 3
-; CHECK-NEXT:    [[M:%.*]] = mul i32 [[Y]], [[X]]
-; CHECK-NEXT:    [[R:%.*]] = or disjoint i32 [[M]], [[X]]
+; CHECK-NEXT:    [[M1:%.*]] = or disjoint i32 [[Y]], 1
+; CHECK-NEXT:    [[R:%.*]] = mul i32 [[M1]], [[X]]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %x = and i32 %p1, 7
@@ -1566,14 +1566,14 @@ define i32 @mul_no_common_bits_uses(i32 %p1, i32 %p2) {
   ret i32 %r
 }
 
-; negative test - probably not good to create an extra mul
+; TODO: is it good to create an extra mul?
 
 define i32 @mul_no_common_bits_const_op_uses(i32 %p) {
 ; CHECK-LABEL: @mul_no_common_bits_const_op_uses(
 ; CHECK-NEXT:    [[X:%.*]] = and i32 [[P:%.*]], 7
 ; CHECK-NEXT:    [[M:%.*]] = mul nuw nsw i32 [[X]], 24
 ; CHECK-NEXT:    call void @use(i32 [[M]])
-; CHECK-NEXT:    [[R:%.*]] = or disjoint i32 [[M]], [[X]]
+; CHECK-NEXT:    [[R:%.*]] = mul nuw nsw i32 [[X]], 25
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %x = and i32 %p, 7