[llvm-commits] [llvm] r155722 - /llvm/trunk/lib/Transforms/Scalar/GVN.cpp

[David Blaikie]

On Fri, Apr 27, 2012 at 11:09 AM, Mon P Wang <wangmp at apple.com> wrote:
> Author: wangmp
> Date: Fri Apr 27 13:09:28 2012
> New Revision: 155722
>
> URL: http://llvm.org/viewvc/llvm-project?rev=155722&view=rev
> Log:
> Add an early bailout to IsValueFullyAvailableInBlock from deeply nested blocks.
> The limit is set to an arbitrary 1000 recursion depth to avoid stack overflow
> issues. <rdar://problem/11286839>.
>
>
> Modified:
>    llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=155722&r1=155721&r2=155722&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Fri Apr 27 13:09:28 2012
> @@ -59,6 +59,11 @@
>                                cl::init(true), cl::Hidden);
>  static cl::opt<bool> EnableLoadPRE("enable-load-pre", cl::init(true));
>
> +// Maximum allowed recursion depth.
> +static cl::opt<int>
> +MaxRecurseDepth("max-recurse-depth", cl::Hidden, cl::init(1000), cl::ZeroOrMore,
> +                cl::desc("Max recurse depth (default = 1000)"));
> +
>  //===----------------------------------------------------------------------===//
>  //                         ValueTable Class
>  //===----------------------------------------------------------------------===//
> @@ -647,7 +652,11 @@
>  ///   3) we are speculating for this block and have used that to speculate for
>  ///      other blocks.
>  static bool IsValueFullyAvailableInBlock(BasicBlock *BB,
> -                            DenseMap<BasicBlock*, char> &FullyAvailableBlocks) {
> +                            DenseMap<BasicBlock*, char> &FullyAvailableBlocks,
> +                            uint32_t RecurseDepth) {
> +  if (RecurseDepth > MaxRecurseDepth)

This resulted in a clang warning due to comparison of signed and
unsigned. We tend to keep the Clang build of LLVM+Clang clean of any
warnings (many of us build with -Werror, so this can be a blocking
issue) so I went ahead & changed MaxRecurseDepth to be a uint32_t
option (instead of an int option) in 155727.

Thanks,
- David

> +    return false;
> +
>   // Optimistically assume that the block is fully available and check to see
>   // if we already know about this block in one lookup.
>   std::pair<DenseMap<BasicBlock*, char>::iterator, char> IV =
> @@ -673,7 +682,7 @@
>     // If the value isn't fully available in one of our predecessors, then it
>     // isn't fully available in this block either.  Undo our previous
>     // optimistic assumption and bail out.
> -    if (!IsValueFullyAvailableInBlock(*PI, FullyAvailableBlocks))
> +    if (!IsValueFullyAvailableInBlock(*PI, FullyAvailableBlocks,RecurseDepth+1))
>       goto SpeculationFailure;
>
>   return true;
> @@ -1570,7 +1579,7 @@
>   for (pred_iterator PI = pred_begin(LoadBB), E = pred_end(LoadBB);
>        PI != E; ++PI) {
>     BasicBlock *Pred = *PI;
> -    if (IsValueFullyAvailableInBlock(Pred, FullyAvailableBlocks)) {
> +    if (IsValueFullyAvailableInBlock(Pred, FullyAvailableBlocks, 0)) {
>       continue;
>     }
>     PredLoads[Pred] = 0;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits