diff --git a/llvm/include/llvm/Transforms/Utils/GuardUtils.h b/llvm/include/llvm/Transforms/Utils/GuardUtils.h --- a/llvm/include/llvm/Transforms/Utils/GuardUtils.h +++ b/llvm/include/llvm/Transforms/Utils/GuardUtils.h @@ -17,6 +17,7 @@ class BranchInst; class CallInst; class Function; +class LoopInfo; class Value; /// Splits control flow at point of \p Guard, replacing it with explicit branch @@ -32,12 +33,14 @@ /// Given a branch we know is widenable (defined per Analysis/GuardUtils.h), /// widen it such that condition 'NewCond' is also known to hold on the taken /// path. Branch remains widenable after transform. -void widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond); +void widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond, + const LoopInfo &LI); /// Given a branch we know is widenable (defined per Analysis/GuardUtils.h), /// *set* it's condition such that (only) 'Cond' is known to hold on the taken /// path and that the branch remains widenable after transform. -void setWidenableBranchCond(BranchInst *WidenableBR, Value *Cond); +void setWidenableBranchCond(BranchInst *WidenableBR, Value *Cond, + const LoopInfo &LI); } // llvm diff --git a/llvm/lib/Transforms/Scalar/GuardWidening.cpp b/llvm/lib/Transforms/Scalar/GuardWidening.cpp --- a/llvm/lib/Transforms/Scalar/GuardWidening.cpp +++ b/llvm/lib/Transforms/Scalar/GuardWidening.cpp @@ -289,7 +289,7 @@ widenCondCommon(getCondition(ToWiden), NewCondition, InsertPt, Result, InvertCondition); if (isGuardAsWidenableBranch(ToWiden)) { - setWidenableBranchCond(cast(ToWiden), Result); + setWidenableBranchCond(cast(ToWiden), Result, LI); return; } setCondition(ToWiden, Result); diff --git a/llvm/lib/Transforms/Scalar/LoopPredication.cpp b/llvm/lib/Transforms/Scalar/LoopPredication.cpp --- a/llvm/lib/Transforms/Scalar/LoopPredication.cpp +++ b/llvm/lib/Transforms/Scalar/LoopPredication.cpp @@ -1248,7 +1248,7 @@ // context. NewCond = B.CreateFreeze(NewCond); - widenWidenableBranch(WidenableBR, NewCond); + widenWidenableBranch(WidenableBR, NewCond, *LI); Value *OldCond = BI->getCondition(); BI->setCondition(ConstantInt::get(OldCond->getType(), !ExitIfTrue)); diff --git a/llvm/lib/Transforms/Utils/GuardUtils.cpp b/llvm/lib/Transforms/Utils/GuardUtils.cpp --- a/llvm/lib/Transforms/Utils/GuardUtils.cpp +++ b/llvm/lib/Transforms/Utils/GuardUtils.cpp @@ -11,6 +11,7 @@ #include "llvm/Transforms/Utils/GuardUtils.h" #include "llvm/Analysis/GuardUtils.h" +#include "llvm/Analysis/LoopInfo.h" #include "llvm/IR/Function.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/Instructions.h" @@ -78,8 +79,18 @@ } } +#ifndef NDEBUG +static bool isLoopInvariantWidenableCond(Value *WCCond, const Loop &L) { + bool IsLoopInv = L.isLoopInvariant(WCCond); + if (IsLoopInv || !isa(WCCond)) + return IsLoopInv; -void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond) { + return L.hasLoopInvariantOperands(cast(WCCond)); +} +#endif + +void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond, + const LoopInfo &LI) { assert(isWidenableBranch(WidenableBR) && "precondition"); // The tempting trivially option is to produce something like this: @@ -90,6 +101,13 @@ Use *C, *WC; BasicBlock *IfTrueBB, *IfFalseBB; parseWidenableBranch(WidenableBR, C, WC, IfTrueBB, IfFalseBB); +#ifndef NDEBUG + bool IsLoopInvariantWidenableBR = false; + auto *L = LI.getLoopFor(WidenableBR->getParent()); + if (L) + IsLoopInvariantWidenableBR = + isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L); +#endif if (!C) { // br (wc()), ... form IRBuilder<> B(WidenableBR); @@ -102,15 +120,33 @@ // Condition is only guaranteed to dominate branch WCAnd->moveBefore(WidenableBR); } +#ifndef NDEBUG + if (IsLoopInvariantWidenableBR) { + auto *L = LI.getLoopFor(WidenableBR->getParent()); + // NB! This can also be loop-varying if we missed hoisting the IR out, which + // was infact loop-invariant. This would imply a pass-ordering issue and the + // assert should be dealt with as such. + assert(isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L) && + "Loop invariant widenable condition made loop variant!"); + } +#endif assert(isWidenableBranch(WidenableBR) && "preserve widenabiliy"); } -void llvm::setWidenableBranchCond(BranchInst *WidenableBR, Value *NewCond) { +void llvm::setWidenableBranchCond(BranchInst *WidenableBR, Value *NewCond, + const LoopInfo &LI) { assert(isWidenableBranch(WidenableBR) && "precondition"); Use *C, *WC; BasicBlock *IfTrueBB, *IfFalseBB; parseWidenableBranch(WidenableBR, C, WC, IfTrueBB, IfFalseBB); +#ifndef NDEBUG + bool IsLoopInvariantWidenableBR = false; + auto *L = LI.getLoopFor(WidenableBR->getParent()); + if (L) + IsLoopInvariantWidenableBR = + isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L); +#endif if (!C) { // br (wc()), ... form IRBuilder<> B(WidenableBR); @@ -122,5 +158,15 @@ WCAnd->moveBefore(WidenableBR); C->set(NewCond); } +#ifndef NDEBUG + if (IsLoopInvariantWidenableBR) { + auto *L = LI.getLoopFor(WidenableBR->getParent()); + // NB! This can also be loop-varying if we missed hoisting the IR out, which + // was infact loop-invariant. This would imply a pass-ordering issue and the + // assert should be dealt with as such. + assert(isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L) && + "Loop invariant widenable condition made loop variant!"); + } +#endif assert(isWidenableBranch(WidenableBR) && "preserve widenabiliy"); }