[llvm] r185843 - Fix a SCEV update problem.
Nick Lewycky
nicholas at mxc.ca
Thu Jul 11 23:54:40 PDT 2013
Shuxin Yang wrote:
> Author: shuxin_yang
> Date: Mon Jul 8 12:33:13 2013
> New Revision: 185843
>
> URL: http://llvm.org/viewvc/llvm-project?rev=185843&view=rev
> Log:
> Fix a SCEV update problem.
>
> The symptom is seg-fault, and the root cause is that a SCEV contains a SCEVUnknown
> which has null-pointer to a llvm::Value.
>
> This is how the problem take place:
> ===================================
> 1). In the pristine input IR, there are two relevant instrutions Op1 and Op2,
> Op1's corresponding SCEV (denoted as SCEV(op1)) is a SCEVUnknown, and
> SCEV(Op2) contains SCEV(Op1). None of these instructions are dead.
>
> Op1 : V1 = ...
> ...
> Op2 : V2 = ... // directly or indirectly (data-flow) depends on Op1
>
> 2) Optimizer (LSR in my case) generates an instruction holding the equivalent
> value of Op1, making Op1 dead.
> Op1': V1' = ...
> Op1: V1 = ... ; now dead)
> Op2 : V2 = ... //Now deps on Op1', but the SCEV(Op2) still contains SCEV(Op1)
>
> 3) Op1 is deleted, and call-back function is called to reset
> SCEV(Op1) to indicate it is invalid. However, SCEV(Op2) is not
> invalidated as well.
>
> 4) Following pass get the cached, invalid SCEV(Op2), and try to manipulate it,
> and cause segfault.
>
> The fix:
> ========
> It seems there is no clean yet inexpensive fix. I write to dev-list
> soliciting good solution, unforunately no ack. So, I decide to fix this
> problem in a brute-force way:
>
> When ScalarEvolution::getSCEV is called, check if the cached SCEV
> contains a invalid SCEVUnknow, if yes, remove the cached SCEV, and
> re-evaluate the SCEV from scratch.
>
> I compile buch of big *.c and *.cpp, fortunately, I don't see any increase
> in compile time.
>
> Misc:
> =====
> The reduced test-case has 2357 lines of code+other-stuff, too big to commit.
>
> rdar://14283433
I have a question about the symptom. SCEV(Op1) is invalidated, does that
mean that SCEV(Op2) holds a dangling pointer? If so, don't we still have
a bug if a new SCEV happens to be allocated at the old address?
>
> Modified:
> llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
> llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>
> Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=185843&r1=185842&r2=185843&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Mon Jul 8 12:33:13 2013
> @@ -545,6 +545,10 @@ namespace llvm {
> /// forgetMemoizedResults - Drop memoized information computed for S.
> void forgetMemoizedResults(const SCEV *S);
>
> + /// return false iff given SCEV contains a SCEVUnknown with NULL value-
Capital 'R' please.
> + /// pointer.
> + bool checkValidity(const SCEV *S) const;
> +
> public:
> static char ID; // Pass identification, replacement for typeid
> ScalarEvolution();
>
> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=185843&r1=185842&r2=185843&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Mon Jul 8 12:33:13 2013
> @@ -2715,13 +2715,51 @@ const SCEV *ScalarEvolution::getCouldNot
> return&CouldNotCompute;
> }
>
> +namespace {
> + // Helper class working with SCEVTraversal to figure out if a SCEV contains
> + // a SCEVUnknown with null value-pointer. FindInvalidSCEVUnknown::FindOne
> + // is set iff if find such SCEVUnknown.
> + //
> + struct FindInvalidSCEVUnknown {
> + bool FindOne;
> + FindInvalidSCEVUnknown() { FindOne = false; }
Constructor initializer list?
> + bool follow(const SCEV *S) {
> + switch (S->getSCEVType()) {
> + case scConstant:
> + return false;
> + case scUnknown:
> + if(!cast<SCEVUnknown>(S)->getValue())
Missing space after 'if'.
> + FindOne = true;
> + return false;
> + default:
> + return true;
> + }
> + }
> + bool isDone() const { return FindOne; }
> + };
> +}
> +
> +bool ScalarEvolution::checkValidity(const SCEV *S) const {
> + FindInvalidSCEVUnknown F;
> + SCEVTraversal<FindInvalidSCEVUnknown> ST(F);
> + ST.visitAll(S);
> +
> + return !F.FindOne;
> +}
> +
> /// getSCEV - Return an existing SCEV if it exists, otherwise analyze the
> /// expression and create a new one.
> const SCEV *ScalarEvolution::getSCEV(Value *V) {
> assert(isSCEVable(V->getType())&& "Value is not SCEVable!");
>
> - ValueExprMapType::const_iterator I = ValueExprMap.find_as(V);
> - if (I != ValueExprMap.end()) return I->second;
> + ValueExprMapType::iterator I = ValueExprMap.find_as(V);
> + if (I != ValueExprMap.end()) {
> + const SCEV *S = I->second;
> + if(checkValidity(S))
Missing space after 'if'.
Nick
> + return S;
> + else
> + ValueExprMap.erase(I);
> + }
> const SCEV *S = createSCEV(V);
>
> // The process of creating a SCEV for V may have caused other SCEVs
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list