[llvm] bc49103 - [InstCombine] Don't handle constants in de morgan folds (PR63791)

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 11 06:19:01 PDT 2023


Author: Nikita Popov
Date: 2023-07-11T15:18:53+02:00
New Revision: bc491030154f2583f3ea0d22ee0b249f45cedbac

URL: https://github.com/llvm/llvm-project/commit/bc491030154f2583f3ea0d22ee0b249f45cedbac
DIFF: https://github.com/llvm/llvm-project/commit/bc491030154f2583f3ea0d22ee0b249f45cedbac.diff

LOG: [InstCombine] Don't handle constants in de morgan folds (PR63791)

If the and/or operand is an immediate constant, it will get folded
away anyway. Don't try to freely invert those operands.

A particularly degenerate case of this arises when both operands
are constant and the result is a constant, in which case we try
to invert users of a constant, resulting in an assertion failure.

Fixes https://github.com/llvm/llvm-project/issues/63791.

Added: 
    llvm/test/Transforms/InstCombine/pr63791.ll

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index ed3297541c1138..c06d5c75963447 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3996,20 +3996,15 @@ static Instruction *canonicalizeAbs(BinaryOperator &Xor,
 
 static bool canFreelyInvert(InstCombiner &IC, Value *Op,
                             Instruction *IgnoredUser) {
-  if (!IC.isFreeToInvert(Op, /*WillInvertAllUses=*/true))
-    return false;
-  return match(Op, m_ImmConstant()) ||
-         (isa<Instruction>(Op) && InstCombiner::canFreelyInvertAllUsersOf(
-                                      cast<Instruction>(Op), IgnoredUser));
+  auto *I = dyn_cast<Instruction>(Op);
+  return I && IC.isFreeToInvert(I, /*WillInvertAllUses=*/true) &&
+         InstCombiner::canFreelyInvertAllUsersOf(I, IgnoredUser);
 }
 
 static Value *freelyInvert(InstCombinerImpl &IC, Value *Op,
                            Instruction *IgnoredUser) {
-  if (auto *C = dyn_cast<Constant>(Op))
-    return ConstantExpr::getNot(C);
-
-  IC.Builder.SetInsertPoint(
-      &*cast<Instruction>(Op)->getInsertionPointAfterDef());
+  auto *I = cast<Instruction>(Op);
+  IC.Builder.SetInsertPoint(&*I->getInsertionPointAfterDef());
   Value *NotOp = IC.Builder.CreateNot(Op, Op->getName() + ".not");
   Op->replaceUsesWithIf(NotOp,
                         [NotOp](Use &U) { return U.getUser() != NotOp; });

diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 971b17d2ce54b0..4e731390533fe3 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1082,6 +1082,7 @@ Value *InstCombinerImpl::SimplifySelectsFeedingBinaryOp(BinaryOperator &I,
 /// Freely adapt every user of V as-if V was changed to !V.
 /// WARNING: only if canFreelyInvertAllUsersOf() said this can be done.
 void InstCombinerImpl::freelyInvertAllUsersOf(Value *I, Value *IgnoredUser) {
+  assert(!isa<Constant>(I) && "Shouldn't invert users of constant");
   for (User *U : make_early_inc_range(I->users())) {
     if (U == IgnoredUser)
       continue; // Don't consider this user.

diff  --git a/llvm/test/Transforms/InstCombine/pr63791.ll b/llvm/test/Transforms/InstCombine/pr63791.ll
new file mode 100644
index 00000000000000..a489b2e3e6221a
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr63791.ll
@@ -0,0 +1,59 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Make sure we don't crash.
+
+ at j = internal constant i32 4
+
+define void @y() {
+; CHECK-LABEL: define void @y() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_COND_I:%.*]]
+; CHECK:       for.cond.i:
+; CHECK-NEXT:    br i1 false, label [[FOR_COND_I]], label [[FOR_COND5_PREHEADER_I:%.*]]
+; CHECK:       for.cond1.loopexit.i:
+; 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:       for.inc19.i:
+; CHECK-NEXT:    br i1 false, label [[FOR_INC19_I]], label [[FOR_COND1_LOOPEXIT_I]]
+;
+entry:
+  br label %for.cond.i
+
+for.cond.i:                                       ; preds = %for.cond1.loopexit.i, %for.cond.i, %entry
+  %phi1 = phi ptr [ @j, %entry ], [ @j, %for.cond.i ], [ null, %for.cond1.loopexit.i ]
+  %load1 = load i32, ptr %phi1, align 4
+  br i1 false, label %for.cond.i, label %for.cond5.preheader.i
+
+for.cond1.loopexit.i:                             ; preds = %for.inc19.i, %for.cond5.preheader.i
+  %load2 = load i32, ptr null, align 4
+  %conv.i = sext i32 %load2 to i64
+  %tobool4.not.i = icmp eq i64 0, %conv.i
+  br i1 %tobool4.not.i, label %for.cond.i, label %for.cond5.preheader.i
+
+for.cond5.preheader.i:                            ; preds = %for.cond1.loopexit.i, %for.cond.i
+  %phi = phi i32 [ 0, %for.cond1.loopexit.i ], [ %load1, %for.cond.i ]
+  %sub6.i = or i32 %load1, 1
+  %cmp5.i.i = icmp sgt i32 %load1, 41
+  %a.1.3.i.i = select i1 %cmp5.i.i, i32 %sub6.i, i32 0
+  %add.4.i.i = add i32 %a.1.3.i.i, 1
+  %cmp8.4.i.i = icmp sgt i32 %a.1.3.i.i, 0
+  %spec.select.4.i.i1 = select i1 %cmp8.4.i.i, i32 %add.4.i.i, i32 0
+  %cmp11.4.i.i = icmp eq i32 %add.4.i.i, 0
+  %add.5.i.i = or i32 %add.4.i.i, %sub6.i
+  %cmp8.5.i.i = icmp sgt i32 %add.5.i.i, 0
+  %sel = select i1 %cmp5.i.i, i1 %cmp8.5.i.i, i1 false
+  %i.1.5.i.i = select i1 %sel, i32 0, i32 %spec.select.4.i.i1
+  %spec.select = select i1 %cmp11.4.i.i, i32 0, i32 %i.1.5.i.i
+  %add8.i = or i32 %spec.select, -1020499714
+  %cmp.i3 = icmp sgt i32 %add8.i, -1020499638
+  br i1 %cmp.i3, label %for.inc19.i, label %for.cond1.loopexit.i
+
+for.inc19.i:                                      ; preds = %for.inc19.i, %for.cond5.preheader.i
+  br i1 %sel, label %for.inc19.i, label %for.cond1.loopexit.i
+
+; uselistorder directives
+  uselistorder i32 %load1, { 1, 0, 2 }
+}


        


More information about the llvm-commits mailing list