From: Roman Lebedev Date: Tue, 20 Dec 2022 15:15:19 +0000 (+0300) Subject: [InstCombine] Fix inversion of constants X-Git-Tag: upstream/17.0.6~23106 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e51b7bff192f5c2712009a28eed1544ff45913c1;p=platform%2Fupstream%2Fllvm.git [InstCombine] Fix inversion of constants `canFreelyInvertAllUsersOf()`, in general, does not make sense for constants, and constant expressions are likely even more problematic. For those, we just want to create a simple constant expression and be done. Fixes https://github.com/llvm/llvm-project/issues/59613 --- diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h index 92df0e4..a8763855 100644 --- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h +++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h @@ -273,9 +273,10 @@ public: /// 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! /// /// See also: isFreeToInvert() - static bool canFreelyInvertAllUsersOf(Value *V, Value *IgnoredUser) { + static bool canFreelyInvertAllUsersOf(Instruction *V, Value *IgnoredUser) { // Look at every user of V. for (Use &U : V->uses()) { if (U.getUser() == IgnoredUser) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp index 5634008..7843435 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp @@ -3678,17 +3678,24 @@ bool InstCombinerImpl::sinkNotIntoLogicalOp(Instruction &I) { // And can the operands be adapted? for (Value *Op : {Op0, Op1}) - if (!InstCombiner::isFreeToInvert(Op, /*WillInvertAllUses=*/true) || - !InstCombiner::canFreelyInvertAllUsersOf(Op, /*IgnoredUser=*/&I)) + if (!(InstCombiner::isFreeToInvert(Op, /*WillInvertAllUses=*/true) && + (isa(Op) || + InstCombiner::canFreelyInvertAllUsersOf(cast(Op), + /*IgnoredUser=*/&I)))) return false; for (Value **Op : {&Op0, &Op1}) { - Builder.SetInsertPoint( - &*cast(*Op)->getInsertionPointAfterDef()); - Value *NotOp = Builder.CreateNot(*Op, (*Op)->getName() + ".not"); - (*Op)->replaceUsesWithIf(NotOp, - [NotOp](Use &U) { return U.getUser() != NotOp; }); - freelyInvertAllUsersOf(NotOp, /*IgnoredUser=*/&I); + Value *NotOp; + if (auto *C = dyn_cast(*Op)) { + NotOp = ConstantExpr::getNot(C); + } else { + Builder.SetInsertPoint( + &*cast(*Op)->getInsertionPointAfterDef()); + NotOp = Builder.CreateNot(*Op, (*Op)->getName() + ".not"); + (*Op)->replaceUsesWithIf( + NotOp, [NotOp](Use &U) { return U.getUser() != NotOp; }); + freelyInvertAllUsersOf(NotOp, /*IgnoredUser=*/&I); + } *Op = NotOp; } @@ -3726,12 +3733,15 @@ bool InstCombinerImpl::sinkNotIntoOtherHandOfLogicalOp(Instruction &I) { Value **OpToInvert = nullptr; if (match(Op0, m_Not(m_Value(NotOp0))) && InstCombiner::isFreeToInvert(Op1, /*WillInvertAllUses=*/true) && - InstCombiner::canFreelyInvertAllUsersOf(Op1, /*IgnoredUser=*/&I)) { + (isa(Op1) || InstCombiner::canFreelyInvertAllUsersOf( + cast(Op1), /*IgnoredUser=*/&I))) { Op0 = NotOp0; OpToInvert = &Op1; } else if (match(Op1, m_Not(m_Value(NotOp1))) && InstCombiner::isFreeToInvert(Op0, /*WillInvertAllUses=*/true) && - InstCombiner::canFreelyInvertAllUsersOf(Op0, /*IgnoredUser=*/&I)) { + (isa(Op0) || + InstCombiner::canFreelyInvertAllUsersOf(cast(Op0), + /*IgnoredUser=*/&I))) { Op1 = NotOp1; OpToInvert = &Op0; } else @@ -3741,15 +3751,19 @@ bool InstCombinerImpl::sinkNotIntoOtherHandOfLogicalOp(Instruction &I) { if (!InstCombiner::canFreelyInvertAllUsersOf(&I, /*IgnoredUser=*/nullptr)) return false; - Builder.SetInsertPoint( - &*cast(*OpToInvert)->getInsertionPointAfterDef()); - Value *NotOpToInvert = - Builder.CreateNot(*OpToInvert, (*OpToInvert)->getName() + ".not"); - (*OpToInvert)->replaceUsesWithIf(NotOpToInvert, [NotOpToInvert](Use &U) { - return U.getUser() != NotOpToInvert; - }); - freelyInvertAllUsersOf(NotOpToInvert, /*IgnoredUser=*/&I); - *OpToInvert = NotOpToInvert; + if (auto *C = dyn_cast(*OpToInvert)) { + *OpToInvert = ConstantExpr::getNot(C); + } else { + Builder.SetInsertPoint( + &*cast(*OpToInvert)->getInsertionPointAfterDef()); + Value *NotOpToInvert = + Builder.CreateNot(*OpToInvert, (*OpToInvert)->getName() + ".not"); + (*OpToInvert)->replaceUsesWithIf(NotOpToInvert, [NotOpToInvert](Use &U) { + return U.getUser() != NotOpToInvert; + }); + freelyInvertAllUsersOf(NotOpToInvert, /*IgnoredUser=*/&I); + *OpToInvert = NotOpToInvert; + } Builder.SetInsertPoint(&*I.getInsertionPointAfterDef()); Value *NewBinOp; @@ -3861,8 +3875,9 @@ Instruction *InstCombinerImpl::foldNot(BinaryOperator &I) { // not (cmp A, B) = !cmp A, B CmpInst::Predicate Pred; if (match(NotOp, m_Cmp(Pred, m_Value(), m_Value())) && - (NotOp->hasOneUse() || InstCombiner::canFreelyInvertAllUsersOf( - NotOp, /*IgnoredUser=*/nullptr))) { + (NotOp->hasOneUse() || + InstCombiner::canFreelyInvertAllUsersOf(cast(NotOp), + /*IgnoredUser=*/nullptr))) { cast(NotOp)->setPredicate(CmpInst::getInversePredicate(Pred)); freelyInvertAllUsersOf(NotOp); return &I; diff --git a/llvm/test/Transforms/InstCombine/pr59613.ll b/llvm/test/Transforms/InstCombine/pr59613.ll new file mode 100644 index 0000000..a669a0d --- /dev/null +++ b/llvm/test/Transforms/InstCombine/pr59613.ll @@ -0,0 +1,16 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt < %s -passes=instcombine -S | FileCheck %s + +; This used to crash, depending on the particular worklist iteration order. +define void @pr59613(<6 x i16> %0) { +; CHECK-LABEL: @pr59613( +; CHECK-NEXT: store <6 x i16> poison, ptr null, align 4294967296 +; CHECK-NEXT: ret void +; + %cmp1 = icmp ne <6 x i16> %0, zeroinitializer + %or = or <6 x i1> , %cmp1 + %sext = sext <6 x i1> %or to <6 x i16> + %not = xor <6 x i16> %sext, + store <6 x i16> %not, ptr null, align 16 + ret void +}