[llvm] d018578 - [InstCombine] Make `isFreeToInvert` check recursively.
Noah Goldstein via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 20 15:59:49 PST 2023
Author: Noah Goldstein
Date: 2023-11-20T17:59:26-06:00
New Revision: d01857803f3593c26118b7f9fe59db71234a3359
URL: https://github.com/llvm/llvm-project/commit/d01857803f3593c26118b7f9fe59db71234a3359
DIFF: https://github.com/llvm/llvm-project/commit/d01857803f3593c26118b7f9fe59db71234a3359.diff
LOG: [InstCombine] Make `isFreeToInvert` check recursively.
Some Instructions (select/min/max) are inverted by just inverting the
operands. So the answer of whether they are free to invert is really
just whether the operands are free to invert.
Differential Revision: https://reviews.llvm.org/D159056
Added:
Modified:
llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index f8b3874267ded3b..3fc16d0e803a6db 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -239,45 +239,67 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
/// uses of V and only keep uses of ~V.
///
/// See also: canFreelyInvertAllUsersOf()
- static bool isFreeToInvert(Value *V, bool WillInvertAllUses) {
+ static bool isFreeToInvertImpl(Value *V, bool WillInvertAllUses,
+ bool &DoesConsume, unsigned Depth) {
+ using namespace llvm::PatternMatch;
// ~(~(X)) -> X.
- if (match(V, m_Not(PatternMatch::m_Value())))
+ if (match(V, m_Not(m_Value()))) {
+ DoesConsume = true;
return true;
+ }
// Constants can be considered to be not'ed values.
- if (match(V, PatternMatch::m_AnyIntegralConstant()))
+ if (match(V, m_AnyIntegralConstant()))
return true;
+ if (Depth++ >= MaxAnalysisRecursionDepth)
+ return false;
+
+ // The rest of the cases require that we invert all uses so don't bother
+ // doing the analysis if we know we can't use the result.
+ if (!WillInvertAllUses)
+ return false;
+
// Compares can be inverted if all of their uses are being modified to use
// the ~V.
if (isa<CmpInst>(V))
- return WillInvertAllUses;
-
- // If `V` is of the form `A + Constant` then `-1 - V` can be folded into
- // `(-1 - Constant) - A` if we are willing to invert all of the uses.
- if (match(V, m_Add(PatternMatch::m_Value(), PatternMatch::m_ImmConstant())))
- return WillInvertAllUses;
-
- // If `V` is of the form `Constant - A` then `-1 - V` can be folded into
- // `A + (-1 - Constant)` if we are willing to invert all of the uses.
- if (match(V, m_Sub(PatternMatch::m_ImmConstant(), PatternMatch::m_Value())))
- return WillInvertAllUses;
-
- // Selects with invertible operands are freely invertible
- if (match(V,
- m_Select(PatternMatch::m_Value(), m_Not(PatternMatch::m_Value()),
- m_Not(PatternMatch::m_Value()))))
- return WillInvertAllUses;
-
- // Min/max may be in the form of intrinsics, so handle those identically
- // to select patterns.
- if (match(V, m_MaxOrMin(m_Not(PatternMatch::m_Value()),
- m_Not(PatternMatch::m_Value()))))
- return WillInvertAllUses;
+ return true;
+
+ Value *A, *B;
+ // If `V` is of the form `A + B` then `-1 - V` can be folded into
+ // `~B - A` or `~A - B` if we are willing to invert all of the uses.
+ if (match(V, m_Add(m_Value(A), m_Value(B))))
+ return isFreeToInvertImpl(A, A->hasOneUse(), DoesConsume, Depth) ||
+ isFreeToInvertImpl(B, B->hasOneUse(), DoesConsume, Depth);
+
+ // If `V` is of the form `A - B` then `-1 - V` can be folded into
+ // `~A + B` if we are willing to invert all of the uses.
+ if (match(V, m_Sub(m_Value(A), m_Value())))
+ return isFreeToInvertImpl(A, A->hasOneUse(), DoesConsume, Depth);
+
+ // LogicOps are special in that we canonicalize them at the cost of an
+ // instruction.
+ bool IsSelect = match(V, m_Select(m_Value(), m_Value(A), m_Value(B))) &&
+ !shouldAvoidAbsorbingNotIntoSelect(*cast<SelectInst>(V));
+ // Selects/min/max with invertible operands are freely invertible
+ if (IsSelect || match(V, m_MaxOrMin(m_Value(A), m_Value(B))))
+ return isFreeToInvertImpl(A, A->hasOneUse(), DoesConsume, Depth) &&
+ isFreeToInvertImpl(B, B->hasOneUse(), DoesConsume, Depth);
return false;
}
+ static bool isFreeToInvert(Value *V, bool WillInvertAllUses,
+ bool &DoesConsume) {
+ DoesConsume = false;
+ return isFreeToInvertImpl(V, WillInvertAllUses, DoesConsume, /*Depth*/ 0);
+ }
+
+ static bool isFreeToInvert(Value *V, bool WillInvertAllUses) {
+ bool Unused;
+ return isFreeToInvert(V, WillInvertAllUses, Unused);
+ }
+
/// Given i1 V, can every user of V be freely adapted if V is changed to !V ?
/// InstCombine's freelyInvertAllUsersOf() must be kept in sync with this fn.
/// NOTE: for Instructions only!
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 65cafbdbe61cff6..f2ae0cb85d94c36 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -2222,14 +2222,17 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
}
// (~X) - (~Y) --> Y - X
- // This is placed after the other reassociations and explicitly excludes a
- // sub-of-sub pattern to avoid infinite looping.
- if (isFreeToInvert(Op0, Op0->hasOneUse()) &&
- isFreeToInvert(Op1, Op1->hasOneUse()) &&
- !match(Op0, m_Sub(m_ImmConstant(), m_Value()))) {
- Value *NotOp0 = Builder.CreateNot(Op0);
- Value *NotOp1 = Builder.CreateNot(Op1);
- return BinaryOperator::CreateSub(NotOp1, NotOp0);
+ {
+ // Need to ensure we can consume at least one of the `not` instructions,
+ // otherwise this can inf loop.
+ bool ConsumesOp0, ConsumesOp1;
+ if (isFreeToInvert(Op0, Op0->hasOneUse(), ConsumesOp0) &&
+ isFreeToInvert(Op1, Op1->hasOneUse(), ConsumesOp1) &&
+ (ConsumesOp0 || ConsumesOp1)) {
+ Value *NotOp0 = Builder.CreateNot(Op0);
+ Value *NotOp1 = Builder.CreateNot(Op1);
+ return BinaryOperator::CreateSub(NotOp1, NotOp0);
+ }
}
auto m_AddRdx = [](Value *&Vec) {
diff --git a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
index 3802036d2a715ac..6df9bb06ca56d2e 100644
--- a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
@@ -1494,14 +1494,13 @@ define i8 @freeToInvert_two_minmax_ops_use3(i8 %x, i8 %y, i8 %z, i8 %w) {
define i8 @sub_not_min_max(i8 %r, i8 %g, i8 %b) {
; CHECK-LABEL: @sub_not_min_max(
-; CHECK-NEXT: [[NOTR:%.*]] = xor i8 [[R:%.*]], -1
; CHECK-NEXT: [[NOTG:%.*]] = xor i8 [[G:%.*]], -1
; CHECK-NEXT: call void @use(i8 [[NOTG]])
; CHECK-NEXT: [[NOTB:%.*]] = xor i8 [[B:%.*]], -1
; CHECK-NEXT: call void @use(i8 [[NOTB]])
-; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smin.i8(i8 [[NOTR]], i8 [[NOTG]])
-; CHECK-NEXT: [[K:%.*]] = call i8 @llvm.smin.i8(i8 [[M]], i8 [[NOTB]])
-; CHECK-NEXT: [[CK:%.*]] = sub i8 [[NOTR]], [[K]]
+; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.smax.i8(i8 [[R:%.*]], i8 [[G]])
+; CHECK-NEXT: [[TMP2:%.*]] = call i8 @llvm.smax.i8(i8 [[TMP1]], i8 [[B]])
+; CHECK-NEXT: [[CK:%.*]] = sub i8 [[TMP2]], [[R]]
; CHECK-NEXT: ret i8 [[CK]]
;
%notr = xor i8 %r, -1
@@ -1523,9 +1522,9 @@ define i8 @sub_not_min_max_uses1(i8 %r, i8 %g, i8 %b) {
; CHECK-NEXT: call void @use(i8 [[NOTG]])
; CHECK-NEXT: [[NOTB:%.*]] = xor i8 [[B:%.*]], -1
; CHECK-NEXT: call void @use(i8 [[NOTB]])
-; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smin.i8(i8 [[NOTR]], i8 [[NOTG]])
-; CHECK-NEXT: [[K:%.*]] = call i8 @llvm.smin.i8(i8 [[M]], i8 [[NOTB]])
-; CHECK-NEXT: [[CK:%.*]] = sub i8 [[NOTR]], [[K]]
+; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.smax.i8(i8 [[R]], i8 [[G]])
+; CHECK-NEXT: [[TMP2:%.*]] = call i8 @llvm.smax.i8(i8 [[TMP1]], i8 [[B]])
+; CHECK-NEXT: [[CK:%.*]] = sub i8 [[TMP2]], [[R]]
; CHECK-NEXT: ret i8 [[CK]]
;
%notr = xor i8 %r, -1
More information about the llvm-commits
mailing list