On Fri, Oct 25, 2013 at 03:30:47PM -0400, Diego Novillo wrote: > On 2013-10-10 14:07 , tsaund...@mozilla.com wrote: > >This makes the implementation of stack vectors simpler and easier to use. > >This > > > >works by making the size of the on stack storage a template argument, so the > >size is embedded in the type. This allows you to implicitly convert a > >stack_vec<T, N> to a vec<T, va_heap> *, and it will just work. Because > >there's > >no need to support stack vectors in unions we can make them be a more normal > >c++ class with a constructor and destructor that are nontrivial. > > > > Thanks. This looks much simpler, indeed. The patch is fine to > commit. Just a couple of observations/questions:
I don't have commit access, so can someone check it in for me? I bootstrapped and got no changes in regression tests two weeks ago, but haven't checked it since if that helps. > >@@ -638,7 +638,7 @@ propagate_threaded_block_debug_into (basic_block dest, > >basic_block src) > > i++; > > } > >- vec<tree, va_stack> fewvars = vNULL; > >+ stack_vec<tree, alloc_count> fewvars; > > pointer_set_t *vars = NULL; > > Hm, what will happen now if alloc_count == 0? If I'm following the > logic, this is tied to the presence of the 'vars' local, so it seems > that we are fine. Otherwise, the quick_push operation will run into > trouble. The first quick_push is ok because if we were going to add more than alloc_count vars we would have created the pointer set, and the second is safe because we explicitly check the vectors length is less than alloc_count. > > /* If we're already starting with 3/4 of alloc_count, go for a > >@@ -646,8 +646,6 @@ propagate_threaded_block_debug_into (basic_block dest, > >basic_block src) > > VEC. */ > > if (i * 4 > alloc_count * 3) > > vars = pointer_set_create (); > >- else if (alloc_count) > >- vec_stack_alloc (tree, fewvars, alloc_count); > > /* Now go through the initial debug stmts in DEST again, this time > > actually inserting in VARS or FEWVARS. Don't bother checking for > >diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c > >index 574446a..4a14607 100644 > >--- a/gcc/tree-vect-loop-manip.c > >+++ b/gcc/tree-vect-loop-manip.c > >@@ -107,7 +107,7 @@ typedef struct > > with a PHI DEF that would soon become non-dominant, and when we got > > to the suitable one, it wouldn't have anything to substitute any > > more. */ > >-static vec<adjust_info, va_stack> adjust_vec; > >+static vec<adjust_info, va_heap> adjust_vec; > > A file global was declared as a stack vector? Sigh. indeed, istr checking and it could be moved to the stack, but involved more refactering than seemed to make sense here. Trev > > > > Diego.
