Fix a SCEV update problem.
authorShuxin Yang <shuxin.llvm@gmail.com>
Mon, 8 Jul 2013 17:33:13 +0000 (17:33 +0000)
committerShuxin Yang <shuxin.llvm@gmail.com>
Mon, 8 Jul 2013 17:33:13 +0000 (17:33 +0000)
 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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@185843 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Analysis/ScalarEvolution.h
lib/Analysis/ScalarEvolution.cpp

index 349447fbbb624ef66923d40a1ee59112593b87e4..2af6d897234924fe0f6c3d02f190b9845347d7d8 100644 (file)
@@ -545,6 +545,10 @@ namespace llvm {
     /// forgetMemoizedResults - Drop memoized information computed for S.
     void forgetMemoizedResults(const SCEV *S);
 
     /// forgetMemoizedResults - Drop memoized information computed for S.
     void forgetMemoizedResults(const SCEV *S);
 
+    /// return false iff given SCEV contains a SCEVUnknown with NULL value-
+    /// pointer.
+    bool checkValidity(const SCEV *S) const;
+
   public:
     static char ID; // Pass identification, replacement for typeid
     ScalarEvolution();
   public:
     static char ID; // Pass identification, replacement for typeid
     ScalarEvolution();
index 6328e1ad8b7de5e7d30f1f09b7f643afc0ac46f5..0bff21e7c3e5f24c1891feed1966ac57ae094fd7 100644 (file)
@@ -2715,13 +2715,51 @@ const SCEV *ScalarEvolution::getCouldNotCompute() {
   return &CouldNotCompute;
 }
 
   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; }
+    bool follow(const SCEV *S) {
+      switch (S->getSCEVType()) {
+      case scConstant:
+        return false;
+      case scUnknown:
+        if(!cast<SCEVUnknown>(S)->getValue())
+          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!");
 
 /// 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))
+      return S;
+    else
+      ValueExprMap.erase(I);
+  }
   const SCEV *S = createSCEV(V);
 
   // The process of creating a SCEV for V may have caused other SCEVs
   const SCEV *S = createSCEV(V);
 
   // The process of creating a SCEV for V may have caused other SCEVs