[llvm] r278161 - Fix the runtime error caused by "Use ValueOffsetPair to enhance value reuse during SCEV expansion".
Steven Wu via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 29 11:11:12 PDT 2016
Hi Wei
I found a fallout from this commit. Here is a reduced testcase:
$ cat bugpoint-reduced-simplified.ll
target datalayout = "e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128"
target triple = "i386-apple-macosx10.12.0"
; Function Attrs: nounwind optsize ssp
define void @Foo() {
entry:
switch i2 undef, label %sw.epilog102 [
i2 -2, label %sw.bb28
]
sw.bb28: ; preds = %entry
%0 = load i8*, i8** undef, align 2
%add.ptr1.i = getelementptr inbounds i8, i8* undef, i32 -1
%add.ptr4.i = getelementptr inbounds i8, i8* %0, i32 -1
%cmp.i = icmp ult i8* undef, %0
%add.ptr4.add.ptr1.i = select i1 %cmp.i, i8* %add.ptr4.i, i8* %add.ptr1.i
br label %while.cond.i
while.cond.i: ; preds = %while.cond.i, %sw.bb28
%currPtr.1.i = phi i8* [ %incdec.ptr.i, %while.cond.i ], [ %add.ptr4.add.ptr1.i, %sw.bb28 ]
%incdec.ptr.i = getelementptr inbounds i8, i8* %currPtr.1.i, i32 1
%1 = load i8, i8* %incdec.ptr.i, align 1
br label %while.cond.i
sw.epilog102: ; preds = %entry
unreachable
}
$ bugpoint-reduced-simplified.ll -loop-reduce -disable-output
Assertion failed: (S1->getType() == S2->getType() && "Cannot create binary operator with two operands of differing type!"), function Create, file /Users/steven/dev/trunk/llvm/lib/IR/Instructions.cpp, line 2137.
It seems SCEV trying to fold i8* %add.ptr4.i into *i8 subtract i32 1 which causes the assertion. Can you take a look?
Thanks
Steven
> On Aug 9, 2016, at 1:40 PM, Wei Mi via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: wmi
> Date: Tue Aug 9 15:40:03 2016
> New Revision: 278161
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278161&view=rev
> Log:
> Fix the runtime error caused by "Use ValueOffsetPair to enhance value reuse during SCEV expansion".
>
> The patch is to fix the bug in PR28705. It was caused by setting wrong return
> value for SCEVExpander::findExistingExpansion. The return values of findExistingExpansion
> have different meanings when the function is used in different ways so it is easy to make
> mistake. The fix creates two new interfaces to replace SCEVExpander::findExistingExpansion,
> and specifies where each interface is expected to be used.
>
> Differential Revision: https://reviews.llvm.org/D22942
>
> Added:
> llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll
> Modified:
> llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h
> llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
> llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
>
> Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h?rev=278161&r1=278160&r2=278161&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h (original)
> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h Tue Aug 9 15:40:03 2016
> @@ -14,6 +14,7 @@
> #ifndef LLVM_ANALYSIS_SCALAREVOLUTIONEXPANDER_H
> #define LLVM_ANALYSIS_SCALAREVOLUTIONEXPANDER_H
>
> +#include "llvm/ADT/Optional.h"
> #include "llvm/Analysis/ScalarEvolutionExpressions.h"
> #include "llvm/Analysis/ScalarEvolutionNormalization.h"
> #include "llvm/Analysis/TargetFolder.h"
> @@ -268,7 +269,15 @@ namespace llvm {
>
> void setChainedPhi(PHINode *PN) { ChainedPhis.insert(PN); }
>
> - /// \brief Try to find LLVM IR value for S available at the point At.
> + /// Try to find existing LLVM IR value for S available at the point At.
> + Value *getExactExistingExpansion(const SCEV *S, const Instruction *At,
> + Loop *L);
> +
> + /// Try to find the ValueOffsetPair for S. The function is mainly
> + /// used to check whether S can be expanded cheaply.
> + /// If this returns a non-None value, we know we can codegen the
> + /// `ValueOffsetPair` into a suitable expansion identical with S
> + /// so that S can be expanded cheaply.
> ///
> /// L is a hint which tells in which loop to look for the suitable value.
> /// On success return value which is equivalent to the expanded S at point
> @@ -276,7 +285,9 @@ namespace llvm {
> ///
> /// Note that this function does not perform an exhaustive search. I.e if it
> /// didn't find any value it does not mean that there is no such value.
> - Value *findExistingExpansion(const SCEV *S, const Instruction *At, Loop *L);
> + ///
> + Optional<ScalarEvolution::ValueOffsetPair>
> + getRelatedExistingExpansion(const SCEV *S, const Instruction *At, Loop *L);
>
> private:
> LLVMContext &getContext() const { return SE.getContext(); }
>
> Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=278161&r1=278160&r2=278161&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Tue Aug 9 15:40:03 2016
> @@ -1892,8 +1892,18 @@ unsigned SCEVExpander::replaceCongruentI
> return NumElim;
> }
>
> -Value *SCEVExpander::findExistingExpansion(const SCEV *S,
> - const Instruction *At, Loop *L) {
> +Value *SCEVExpander::getExactExistingExpansion(const SCEV *S,
> + const Instruction *At, Loop *L) {
> + Optional<ScalarEvolution::ValueOffsetPair> VO =
> + getRelatedExistingExpansion(S, At, L);
> + if (VO && VO.getValue().second == nullptr)
> + return VO.getValue().first;
> + return nullptr;
> +}
> +
> +Optional<ScalarEvolution::ValueOffsetPair>
> +SCEVExpander::getRelatedExistingExpansion(const SCEV *S, const Instruction *At,
> + Loop *L) {
> using namespace llvm::PatternMatch;
>
> SmallVector<BasicBlock *, 4> ExitingBlocks;
> @@ -1911,22 +1921,23 @@ Value *SCEVExpander::findExistingExpansi
> continue;
>
> if (SE.getSCEV(LHS) == S && SE.DT.dominates(LHS, At))
> - return LHS;
> + return ScalarEvolution::ValueOffsetPair(LHS, nullptr);
>
> if (SE.getSCEV(RHS) == S && SE.DT.dominates(RHS, At))
> - return RHS;
> + return ScalarEvolution::ValueOffsetPair(RHS, nullptr);
> }
>
> // Use expand's logic which is used for reusing a previous Value in
> // ExprValueMap.
> - if (Value *Val = FindValueInExprValueMap(S, At).first)
> - return Val;
> + ScalarEvolution::ValueOffsetPair VO = FindValueInExprValueMap(S, At);
> + if (VO.first)
> + return VO;
>
> // There is potential to make this significantly smarter, but this simple
> // heuristic already gets some interesting cases.
>
> // Can not find suitable value.
> - return nullptr;
> + return None;
> }
>
> bool SCEVExpander::isHighCostExpansionHelper(
> @@ -1935,7 +1946,7 @@ bool SCEVExpander::isHighCostExpansionHe
>
> // If we can find an existing value for this scev avaliable at the point "At"
> // then consider the expression cheap.
> - if (At && findExistingExpansion(S, At, L) != nullptr)
> + if (At && getRelatedExistingExpansion(S, At, L))
> return false;
>
> // Zero/One operand expressions
> @@ -1983,7 +1994,7 @@ bool SCEVExpander::isHighCostExpansionHe
> // involving division. This is just a simple search heuristic.
> if (!At)
> At = &ExitingBB->back();
> - if (!findExistingExpansion(
> + if (!getRelatedExistingExpansion(
> SE.getAddExpr(S, SE.getConstant(S->getType(), 1)), At, L))
> return true;
> }
>
> Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=278161&r1=278160&r2=278161&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Tue Aug 9 15:40:03 2016
> @@ -481,7 +481,7 @@ Value *IndVarSimplify::expandSCEVIfNeede
> Type *ResultTy) {
> // Before expanding S into an expensive LLVM expression, see if we can use an
> // already existing value as the expansion for S.
> - if (Value *ExistingValue = Rewriter.findExistingExpansion(S, InsertPt, L))
> + if (Value *ExistingValue = Rewriter.getExactExistingExpansion(S, InsertPt, L))
> if (ExistingValue->getType() == ResultTy)
> return ExistingValue;
>
>
> Added: llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll?rev=278161&view=auto
> ==============================================================================
> --- llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll (added)
> +++ llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll Tue Aug 9 15:40:03 2016
> @@ -0,0 +1,41 @@
> +; PR28705
> +; RUN: opt < %s -indvars -S | FileCheck %s
> +
> +; Check IndVarSimplify replaces the exitval use of the induction var "%inc.i.i"
> +; with "%.sroa.speculated + 1".
> +;
> +; CHECK-LABEL: @foo(
> +; CHECK: %[[EXIT:.+]] = sub i32 %.sroa.speculated, -1
> +; CHECK: %DB.sroa.9.0.lcssa = phi i32 [ 1, %entry ], [ %[[EXIT]], %loopexit ]
> +;
> +define void @foo(i32 %sub.ptr.div.i, i8* %ref.i1174) local_unnamed_addr {
> +entry:
> + %cmp.i1137 = icmp ugt i32 %sub.ptr.div.i, 3
> + %.sroa.speculated = select i1 %cmp.i1137, i32 3, i32 %sub.ptr.div.i
> + %cmp6483126 = icmp eq i32 %.sroa.speculated, 0
> + br i1 %cmp6483126, label %XZ.exit, label %for.body650.lr.ph
> +
> +for.body650.lr.ph:
> + br label %for.body650
> +
> +loopexit:
> + %inc.i.i.lcssa = phi i32 [ %inc.i.i, %for.body650 ]
> + br label %XZ.exit
> +
> +XZ.exit:
> + %DB.sroa.9.0.lcssa = phi i32 [ 1, %entry ], [ %inc.i.i.lcssa, %loopexit ]
> + br label %end
> +
> +for.body650:
> + %iv = phi i32 [ 0, %for.body650.lr.ph ], [ %inc655, %for.body650 ]
> + %iv2 = phi i32 [ 1, %for.body650.lr.ph ], [ %inc.i.i, %for.body650 ]
> + %arrayidx.i.i1105 = getelementptr inbounds i8, i8* %ref.i1174, i32 %iv2
> + store i8 7, i8* %arrayidx.i.i1105, align 1
> + %inc.i.i = add i32 %iv2, 1
> + %inc655 = add i32 %iv, 1
> + %cmp648 = icmp eq i32 %inc655, %.sroa.speculated
> + br i1 %cmp648, label %loopexit, label %for.body650
> +
> +end:
> + ret void
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list