[llvm-commits] [llvm] r109570 - /llvm/trunk/lib/Analysis/ScalarEvolution.cpp

[Daniel Dunbar]

Hi Dan,

On Tue, Jul 27, 2010 at 6:09 PM, Dan Gohman <gohman at apple.com> wrote:
> Author: djg
> Date: Tue Jul 27 20:09:07 2010
> New Revision: 109570
>
> URL: http://llvm.org/viewvc/llvm-project?rev=109570&view=rev
> Log:
> Make SCEVCallbackVH::allUsesReplacedWith update the old SCEVUnknown
> object, as it may still be referenced by SCEVs not cleaned up by the
> use list traversal.
>
> Also, in ScalarEvolution::forgetValue, only check for a SCEVUnknown
> object for the original value, not for any value in the use list,
> because other SCEVUnknown values aren't necessary obsolete at that
> point.
>
> Modified:
>    llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>
> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=109570&r1=109569&r2=109570&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Tue Jul 27 20:09:07 2010
> @@ -3664,6 +3664,26 @@
>  /// changed a value in a way that may effect its value, or which may
>  /// disconnect it from a def-use chain linking it to a loop.
>  void ScalarEvolution::forgetValue(Value *V) {
> +  // If there's a SCEVUnknown tying this value into the SCEV
> +  // space, remove it from the folding set map. The SCEVUnknown
> +  // object and any other SCEV objects which reference it
> +  // (transitively) remain allocated, effectively leaked until
> +  // the underlying BumpPtrAllocator is freed.
> +  //
> +  // This permits SCEV pointers to be used as keys in maps
> +  // such as the ValuesAtScopes map.
> +  FoldingSetNodeID ID;
> +  ID.AddInteger(scUnknown);
> +  ID.AddPointer(V);
> +  void *IP;
> +  if (SCEV *S = UniqueSCEVs.FindNodeOrInsertPos(ID, IP)) {
> +    UniqueSCEVs.RemoveNode(S);
> +
> +    // This isn't necessary, but we might as well remove the
> +    // value from the ValuesAtScopes map too.
> +    ValuesAtScopes.erase(S);
> +  }
> +
>   Instruction *I = dyn_cast<Instruction>(V);
>   if (!I) return;
>
> @@ -3685,26 +3705,6 @@
>         ConstantEvolutionLoopExitValue.erase(PN);
>     }
>
> -    // If there's a SCEVUnknown tying this value into the SCEV
> -    // space, remove it from the folding set map. The SCEVUnknown
> -    // object and any other SCEV objects which reference it
> -    // (transitively) remain allocated, effectively leaked until
> -    // the underlying BumpPtrAllocator is freed.
> -    //
> -    // This permits SCEV pointers to be used as keys in maps
> -    // such as the ValuesAtScopes map.
> -    FoldingSetNodeID ID;
> -    ID.AddInteger(scUnknown);
> -    ID.AddPointer(I);
> -    void *IP;
> -    if (SCEV *S = UniqueSCEVs.FindNodeOrInsertPos(ID, IP)) {
> -      UniqueSCEVs.RemoveNode(S);
> -
> -      // This isn't necessary, but we might as well remove the
> -      // value from the ValuesAtScopes map too.
> -      ValuesAtScopes.erase(S);
> -    }
> -
>     PushDefUseChildren(I, Worklist);
>   }
>  }
> @@ -5690,15 +5690,32 @@
>   // this now dangles!
>  }
>
> -void ScalarEvolution::SCEVCallbackVH::allUsesReplacedWith(Value *) {
> +void ScalarEvolution::SCEVCallbackVH::allUsesReplacedWith(Value *V) {
>   assert(SE && "SCEVCallbackVH called with a null ScalarEvolution!");
>
> +  Value *Old = getValPtr();
> +
> +  // If there's a SCEVUnknown tying this value into the SCEV
> +  // space, replace the SCEVUnknown's value with the new value
> +  // for the benefit of any SCEVs still referencing it, and
> +  // and remove it from the folding set map so that new scevs
> +  // don't reference it.
> +  FoldingSetNodeID ID;
> +  ID.AddInteger(scUnknown);
> +  ID.AddPointer(Old);
> +  void *IP;
> +  if (SCEVUnknown *S = cast_or_null<SCEVUnknown>(
> +        SE->UniqueSCEVs.FindNodeOrInsertPos(ID, IP))) {
> +    S->V = V;

gcc-4.0 seems unhappy with this:
--
ScalarEvolution.cpp: In member function 'virtual void
llvm::ScalarEvolution::SCEVCallbackVH::allUsesReplacedWith(llvm::Value*)':
/Users/buildslave/zorg/buildbot/smooshlab/slave/build.clang-i386-darwin9/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h:528:
error: 'llvm::Value* llvm::SCEVUnknown::V' is private
ScalarEvolution.cpp:5709: error: within this context
--

 - Daniel

> +    SE->UniqueSCEVs.RemoveNode(S);
> +    SE->ValuesAtScopes.erase(S);
> +  }
> +
>   // Forget all the expressions associated with users of the old value,
>   // so that future queries will recompute the expressions using the new
>   // value.
>   SmallVector<User *, 16> Worklist;
>   SmallPtrSet<User *, 8> Visited;
> -  Value *Old = getValPtr();
>   for (Value::use_iterator UI = Old->use_begin(), UE = Old->use_end();
>        UI != UE; ++UI)
>     Worklist.push_back(*UI);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>