[llvm] r278161 - Fix the runtime error caused by "Use ValueOffsetPair to enhance value reuse during SCEV expansion".
Wei Mi via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 9 13:40:03 PDT 2016
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
+}
More information about the llvm-commits
mailing list