<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 3, 2020 at 8:31 PM Roman Lebedev via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Roman Lebedev<br>
Date: 2020-11-03T22:30:51+03:00<br>
New Revision: 70472f34b289d663a9f70740dfc9ae32fc89e077<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/70472f34b289d663a9f70740dfc9ae32fc89e077" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/70472f34b289d663a9f70740dfc9ae32fc89e077</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/70472f34b289d663a9f70740dfc9ae32fc89e077.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/70472f34b289d663a9f70740dfc9ae32fc89e077.diff</a><br>
<br>
LOG: [Reassociate] Convert `add`-like `or`'s into an `add`'s to allow reassociation<br>
<br>
InstCombine is quite aggressive in doing the opposite transform,<br>
folding `add` of operands with no common bits set into an `or`,<br>
and that not many things support that new pattern..<br>
<br>
In this case, teaching Reassociate about it is easy,<br>
there's preexisting art for `sub`/`shl`:<br>
just convert such an `or` into an `add`:<br>
<a href="https://rise4fun.com/Alive/Xlyv" rel="noreferrer" target="_blank">https://rise4fun.com/Alive/Xlyv</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
llvm/lib/Transforms/Scalar/Reassociate.cpp<br>
llvm/test/Transforms/Reassociate/add-like-or.ll<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp<br>
index ba7f367267fe..164971c916d4 100644<br>
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp<br>
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp<br>
@@ -920,6 +920,24 @@ static Value *NegateValue(Value *V, Instruction *BI,<br>
return NewNeg;<br>
}<br>
<br>
+/// If we have (X|Y), and iff X and Y have no common bits set,<br>
+/// transform this into (X+Y) to allow arithmetics reassociation.<br>
+static BinaryOperator *ConvertOrWithNoCommonBitsToAdd(Instruction *Or) {<br>
+ // Convert an or into an add.<br>
+ BinaryOperator *New =<br>
+ CreateAdd(Or->getOperand(0), Or->getOperand(1), "", Or, Or);<br>
+ New->setHasNoSignedWrap();<br>
+ New->setHasNoUnsignedWrap();<br>
+ New->takeName(Or);<br>
+<br>
+ // Everyone now refers to the add instruction.<br>
+ Or->replaceAllUsesWith(New);<br>
+ New->setDebugLoc(Or->getDebugLoc());<br>
+<br>
+ LLVM_DEBUG(dbgs() << "Converted or into an add: " << *New << '\n');<br>
+ return New;<br>
+}<br>
+<br>
/// Return true if we should break up this subtract of X-Y into (X + -Y).<br>
static bool ShouldBreakUpSubtract(Instruction *Sub) {<br>
// If this is a negation, we can't split it up!<br>
@@ -2116,6 +2134,18 @@ void ReassociatePass::OptimizeInst(Instruction *I) {<br>
if (I->getType()->isIntegerTy(1))<br>
return;<br>
<br>
+ // If this is a bitwise or instruction of operands<br>
+ // with no common bits set, convert it to X+Y.<br>
+ if (I->getOpcode() == Instruction::Or &&<br>
+ haveNoCommonBitsSet(I->getOperand(0), I->getOperand(1),<br>
+ I->getModule()->getDataLayout(), /*AC=*/nullptr, I,<br>
+ /*DT=*/nullptr)) {<br>
+ Instruction *NI = ConvertOrWithNoCommonBitsToAdd(I);<br>
+ RedoInsts.insert(I);<br>
+ MadeChange = true;<br>
+ I = NI;<br>
+ }<br>
+<br>
// If this is a subtract instruction which is not already in negate form,<br>
// see if we can convert it to X+-Y.<br>
if (I->getOpcode() == Instruction::Sub) {<br>
<br>
diff --git a/llvm/test/Transforms/Reassociate/add-like-or.ll b/llvm/test/Transforms/Reassociate/add-like-or.ll<br>
index 25f2c0563da9..13477ad5a491 100644<br>
--- a/llvm/test/Transforms/Reassociate/add-like-or.ll<br>
+++ b/llvm/test/Transforms/Reassociate/add-like-or.ll<br>
@@ -17,7 +17,7 @@ define i32 @test1(i32 %a, i32 %b) {<br>
define i32 @test2(i32 %x) {<br>
; CHECK-LABEL: @test2(<br>
; CHECK-NEXT: [[X_NUMLZ:%.*]] = tail call i32 @llvm.ctlz.i32(i32 [[X:%.*]], i1 true), [[RNG0:!range !.*]]<br>
-; CHECK-NEXT: [[RES:%.*]] = or i32 [[X_NUMLZ]], -32<br>
+; CHECK-NEXT: [[RES:%.*]] = add nuw nsw i32 [[X_NUMLZ]], -32<br>
; CHECK-NEXT: ret i32 [[RES]]<br>
;<br>
%x.numlz = tail call i32 @llvm.ctlz.i32(i32 %x, i1 true), !range !0<br>
@@ -29,9 +29,8 @@ define i32 @test2(i32 %x) {<br>
define i32 @test3(i32 %x, i32 %bit) {<br>
; CHECK-LABEL: @test3(<br>
; CHECK-NEXT: [[X_NUMLZ:%.*]] = tail call i32 @llvm.ctlz.i32(i32 [[X:%.*]], i1 true), [[RNG0]]<br>
-; CHECK-NEXT: [[ZERO_MINUS_X_NUMACTIVEBITS:%.*]] = or i32 [[X_NUMLZ]], -32<br>
-; CHECK-NEXT: [[BIT_PLUS_ONE:%.*]] = add i32 [[BIT:%.*]], 1<br>
-; CHECK-NEXT: [[RES:%.*]] = add i32 [[BIT_PLUS_ONE]], [[ZERO_MINUS_X_NUMACTIVEBITS]]<br>
+; CHECK-NEXT: [[BIT_PLUS_ONE:%.*]] = add i32 [[BIT:%.*]], -31<br>
+; CHECK-NEXT: [[RES:%.*]] = add i32 [[BIT_PLUS_ONE]], [[X_NUMLZ]]<br>
; CHECK-NEXT: ret i32 [[RES]]<br>
;<br>
%x.numlz = tail call i32 @llvm.ctlz.i32(i32 %x, i1 true), !range !0<br></blockquote><div><br></div><div>Not familiar with this transform, so possibly stupid question: The way I'm reading this change, you are unconditionally reverting the InstCombine canonicalization. The other transform you mention (shl -> mul) first checks whether doing this will result in reassociation opportunities. I'm wondering if the same shouldn't be done here as well?</div><div><br></div><div>Regards,<br></div><div>Nikita<br></div></div></div>