From 9f35d2b564041da3a661b763414b75a51eda9a77 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Thu, 29 Aug 2019 12:47:34 +0000 Subject: [PATCH] [SimplifyCFG] FoldTwoEntryPHINode(): don't bailout on i1 PHI's if we can hoist a 'not' from incoming values Summary: As it can be seen in the tests in D65143/D65144, even though we have formed an '@llvm.umul.with.overflow' and got rid of potential for division-by-zero, the control flow remains, we still have that branch. We have this condition: ``` // Don't fold i1 branches on PHIs which contain binary operators // These can often be turned into switches and other things. if (PN->getType()->isIntegerTy(1) && (isa(PN->getIncomingValue(0)) || isa(PN->getIncomingValue(1)) || isa(IfCond))) return false; ``` which was added back in rL121764 to help with `select` formation i think? That check prevents us to flatten the CFG here, even though we know we no longer need that guard and will be able to drop everything but the '@llvm.umul.with.overflow' + `not`. As it can be seen from tests, we end here because the `not` is being sinked into the PHI's incoming values by InstCombine, so we can't workaround this by hoisting it to after PHI. Thus i suggest that we relax that check to not bailout if we'd get to hoist the `not`. Reviewers: craig.topper, spatel, fhahn, nikic Reviewed By: spatel Subscribers: hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D65147 llvm-svn: 370349 --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 18 +++++- .../unsigned-multiply-overflow-check.ll | 60 +++++++++++++++---- .../unsigned-multiplication-will-overflow.ll | 6 +- 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 33dbbf91cd64..478f32a8f456 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -2329,12 +2329,24 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI, if (!PN) return true; - // Don't fold i1 branches on PHIs which contain binary operators. These can - // often be turned into switches and other things. + // Return true if at least one of these is a 'not', and another is either + // a 'not' too, or a constant. + auto CanHoistNotFromBothValues = [](Value *V0, Value *V1) { + if (!match(V0, m_Not(m_Value()))) + std::swap(V0, V1); + auto Invertible = m_CombineOr(m_Not(m_Value()), m_AnyIntegralConstant()); + return match(V0, m_Not(m_Value())) && match(V1, Invertible); + }; + + // Don't fold i1 branches on PHIs which contain binary operators, unless one + // of the incoming values is an 'not' and another one is freely invertible. + // These can often be turned into switches and other things. if (PN->getType()->isIntegerTy(1) && (isa(PN->getIncomingValue(0)) || isa(PN->getIncomingValue(1)) || - isa(IfCond))) + isa(IfCond)) && + !CanHoistNotFromBothValues(PN->getIncomingValue(0), + PN->getIncomingValue(1))) return false; // If all PHI nodes are promotable, check to make sure that all instructions diff --git a/llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll b/llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll index d81d7c81cdbf..34dbfc4f1865 100644 --- a/llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll +++ b/llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll @@ -104,18 +104,54 @@ define i1 @will_overflow(i64 %arg, i64 %arg1) { ; SIMPLIFYCFG-NEXT: [[T7:%.*]] = xor i1 [[T6]], true ; SIMPLIFYCFG-NEXT: ret i1 [[T7]] ; -; INSTCOMBINE-LABEL: @will_overflow( -; INSTCOMBINE-NEXT: bb: -; INSTCOMBINE-NEXT: [[T0:%.*]] = icmp eq i64 [[ARG:%.*]], 0 -; INSTCOMBINE-NEXT: br i1 [[T0]], label [[BB5:%.*]], label [[BB2:%.*]] -; INSTCOMBINE: bb2: -; INSTCOMBINE-NEXT: [[UMUL:%.*]] = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[ARG]], i64 [[ARG1:%.*]]) -; INSTCOMBINE-NEXT: [[UMUL_OV:%.*]] = extractvalue { i64, i1 } [[UMUL]], 1 -; INSTCOMBINE-NEXT: [[PHITMP:%.*]] = xor i1 [[UMUL_OV]], true -; INSTCOMBINE-NEXT: br label [[BB5]] -; INSTCOMBINE: bb5: -; INSTCOMBINE-NEXT: [[T6:%.*]] = phi i1 [ true, [[BB:%.*]] ], [ [[PHITMP]], [[BB2]] ] -; INSTCOMBINE-NEXT: ret i1 [[T6]] +; INSTCOMBINEONLY-LABEL: @will_overflow( +; INSTCOMBINEONLY-NEXT: bb: +; INSTCOMBINEONLY-NEXT: [[T0:%.*]] = icmp eq i64 [[ARG:%.*]], 0 +; INSTCOMBINEONLY-NEXT: br i1 [[T0]], label [[BB5:%.*]], label [[BB2:%.*]] +; INSTCOMBINEONLY: bb2: +; INSTCOMBINEONLY-NEXT: [[UMUL:%.*]] = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[ARG]], i64 [[ARG1:%.*]]) +; INSTCOMBINEONLY-NEXT: [[UMUL_OV:%.*]] = extractvalue { i64, i1 } [[UMUL]], 1 +; INSTCOMBINEONLY-NEXT: [[PHITMP:%.*]] = xor i1 [[UMUL_OV]], true +; INSTCOMBINEONLY-NEXT: br label [[BB5]] +; INSTCOMBINEONLY: bb5: +; INSTCOMBINEONLY-NEXT: [[T6:%.*]] = phi i1 [ true, [[BB:%.*]] ], [ [[PHITMP]], [[BB2]] ] +; INSTCOMBINEONLY-NEXT: ret i1 [[T6]] +; +; INSTCOMBINESIMPLIFYCFGONLY-LABEL: @will_overflow( +; INSTCOMBINESIMPLIFYCFGONLY-NEXT: bb: +; INSTCOMBINESIMPLIFYCFGONLY-NEXT: [[T0:%.*]] = icmp eq i64 [[ARG:%.*]], 0 +; INSTCOMBINESIMPLIFYCFGONLY-NEXT: [[UMUL:%.*]] = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[ARG]], i64 [[ARG1:%.*]]) +; INSTCOMBINESIMPLIFYCFGONLY-NEXT: [[UMUL_OV:%.*]] = extractvalue { i64, i1 } [[UMUL]], 1 +; INSTCOMBINESIMPLIFYCFGONLY-NEXT: [[PHITMP:%.*]] = xor i1 [[UMUL_OV]], true +; INSTCOMBINESIMPLIFYCFGONLY-NEXT: [[T6:%.*]] = select i1 [[T0]], i1 true, i1 [[PHITMP]] +; INSTCOMBINESIMPLIFYCFGONLY-NEXT: ret i1 [[T6]] +; +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-LABEL: @will_overflow( +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-NEXT: bb: +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-NEXT: [[T0:%.*]] = icmp eq i64 [[ARG:%.*]], 0 +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-NEXT: [[UMUL:%.*]] = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[ARG]], i64 [[ARG1:%.*]]) +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-NEXT: [[UMUL_OV:%.*]] = extractvalue { i64, i1 } [[UMUL]], 1 +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-NEXT: [[PHITMP:%.*]] = xor i1 [[UMUL_OV]], true +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-NEXT: [[T6:%.*]] = or i1 [[T0]], [[PHITMP]] +; INSTCOMBINESIMPLIFYCFGINSTCOMBINE-NEXT: ret i1 [[T6]] +; +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-LABEL: @will_overflow( +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-NEXT: bb: +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-NEXT: [[T0:%.*]] = icmp eq i64 [[ARG:%.*]], 0 +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-NEXT: [[UMUL:%.*]] = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[ARG]], i64 [[ARG1:%.*]]) +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-NEXT: [[UMUL_OV:%.*]] = extractvalue { i64, i1 } [[UMUL]], 1 +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-NEXT: [[PHITMP:%.*]] = xor i1 [[UMUL_OV]], true +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-NEXT: [[T6:%.*]] = select i1 [[T0]], i1 true, i1 [[PHITMP]] +; INSTCOMBINESIMPLIFYCFGCOSTLYONLY-NEXT: ret i1 [[T6]] +; +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-LABEL: @will_overflow( +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-NEXT: bb: +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-NEXT: [[T0:%.*]] = icmp eq i64 [[ARG:%.*]], 0 +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-NEXT: [[UMUL:%.*]] = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[ARG]], i64 [[ARG1:%.*]]) +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-NEXT: [[UMUL_OV:%.*]] = extractvalue { i64, i1 } [[UMUL]], 1 +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-NEXT: [[PHITMP:%.*]] = xor i1 [[UMUL_OV]], true +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-NEXT: [[T6:%.*]] = or i1 [[T0]], [[PHITMP]] +; INSTCOMBINESIMPLIFYCFGCOSTLYINSTCOMBINE-NEXT: ret i1 [[T6]] ; bb: %t0 = icmp eq i64 %arg, 0 diff --git a/llvm/test/Transforms/SimplifyCFG/unsigned-multiplication-will-overflow.ll b/llvm/test/Transforms/SimplifyCFG/unsigned-multiplication-will-overflow.ll index ce0c8296d07b..01a949916b43 100644 --- a/llvm/test/Transforms/SimplifyCFG/unsigned-multiplication-will-overflow.ll +++ b/llvm/test/Transforms/SimplifyCFG/unsigned-multiplication-will-overflow.ll @@ -11,14 +11,10 @@ define i1 @will_overflow(i64 %size, i64 %nmemb) { ; ALL-LABEL: @will_overflow( ; ALL-NEXT: entry: ; ALL-NEXT: [[CMP:%.*]] = icmp eq i64 [[SIZE:%.*]], 0 -; ALL-NEXT: br i1 [[CMP]], label [[LAND_END:%.*]], label [[LAND_RHS:%.*]] -; ALL: land.rhs: ; ALL-NEXT: [[UMUL:%.*]] = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[SIZE]], i64 [[NMEMB:%.*]]) ; ALL-NEXT: [[UMUL_OV:%.*]] = extractvalue { i64, i1 } [[UMUL]], 1 ; ALL-NEXT: [[UMUL_NOT_OV:%.*]] = xor i1 [[UMUL_OV]], true -; ALL-NEXT: br label [[LAND_END]] -; ALL: land.end: -; ALL-NEXT: [[TMP0:%.*]] = phi i1 [ true, [[ENTRY:%.*]] ], [ [[UMUL_NOT_OV]], [[LAND_RHS]] ] +; ALL-NEXT: [[TMP0:%.*]] = select i1 [[CMP]], i1 true, i1 [[UMUL_NOT_OV]] ; ALL-NEXT: ret i1 [[TMP0]] ; entry: