On Tue, Nov 5, 2013 at 6:42 AM, Trevor Saunders <tsaund...@mozilla.com> wrote:
> On Mon, Nov 04, 2013 at 01:29:10PM +0100, Richard Biener wrote:
>> On Wed, Oct 30, 2013 at 8:17 PM, Toon Moene <t...@moene.org> wrote:
>> > Consider this:
>> >
>> > http://gcc.gnu.org/ml/gcc-testresults/2013-10/msg02329.html
>> >
>> > and
>> >
>> > http://gcc.gnu.org/ml/gcc-testresults/2013-10/msg02258.html
>> >
>> > /scratch/toon/bd5894/./prev-gcc/xg++ -B/scratch/toon/bd5894/./prev-gcc/
>> > -B/home/toon/compilers/install/x86_64-unknown-linux-gnu/bin/ -nostdinc++
>> > -B/scratch/toon/bd5894/prev-x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs
>> > -B/scratch/toon/bd5894/prev-x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs
>> > -I/scratch/toon/bd5894/prev-x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu
>> > -I/scratch/toon/bd5894/prev-x86_64-unknown-linux-gnu/libstdc++-v3/include
>> > -I/home/toon/compilers/gcc/libstdc++-v3/libsupc++
>> > -L/scratch/toon/bd5894/prev-x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs
>> > -L/scratch/toon/bd5894/prev-x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs
>> > -g -O2 -flto=jobserver -frandom-seed=1 -DIN_GCC -fno-exceptions
>> > -fno-rtti
>> > -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
>> > -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long
>> > -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common
>> > -DHAVE_CONFIG_H -static-libstdc++ -static-libgcc -o cc1plus \
>> > cp/cp-lang.o c-family/stub-objc.o cp/call.o cp/decl.o
>> > cp/expr.o cp/pt.o cp/typeck2.o cp/class.o cp/decl2.o cp/error.o cp/lex.o
>> > cp/parser.o cp/ptree.o cp/rtti.o cp/typeck.o cp/cvt.o cp/except.o
>> > cp/friend.o cp/init.o cp/method.o cp/search.o cp/semantics.o cp/tree.o
>> > cp/repo.o cp/dump.o cp/optimize.o cp/mangle.o cp/cp-objcp-common.o
>> > cp/name-lookup.o cp/cxx-pretty-print.o cp/cp-gimplify.o
>> > cp/cp-array-notation.o cp/lambda.o cp/vtable-class-hierarchy.o attribs.o
>> > incpath.o c-family/c-common.o c-family/c-cppbuiltin.o c-family/c-dump.o
>> > c-family/c-format.o c-family/c-gimplify.o c-family/c-lex.o c-family/c-omp.o
>> > c-family/c-opts.o c-family/c-pch.o c-family/c-ppoutput.o
>> > c-family/c-pragma.o
>> > c-family/c-pretty-print.o c-family/c-semantics.o c-family/c-ada-spec.o
>> > c-family/array-notation-common.o c-family/c-ubsan.o i386-c.o glibc-c.o
>> > cc1plus-checksum.o libbackend.a main.o tree-browser.o libcommon-target.a
>> > libcommon.a ../libcpp/libcpp.a ../libdecnumber/libdecnumber.a libcommon.a
>> > ../libcpp/libcpp.a ../libbacktrace/.libs/libbacktrace.a
>> > ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a -lcloog-isl -lisl
>> > -lmpc -lmpfr -lgmp -rdynamic -ldl -L../zlib -lz
>> > In function 'release',
>> > inlined from '_ZN7va_heap7reserveIbEEvRP3vecIT_S_8vl_embedEjb.part.95'
>> > at /home/toon/compilers/gcc/gcc/vec.h:288:7,
>> > inlined from 'reserve',
>> > inlined from '_ZN3vecIb7va_heap6vl_ptrE7reserveEjb.part.96' at
>> > /home/toon/compilers/gcc/gcc/vec.h:1367:3,
>> > inlined from 'reserve.constprop',
>> > inlined from 'reserve_exact' at
>> > /home/toon/compilers/gcc/gcc/vec.h:1387:45,
>> > inlined from 'safe_grow' at /home/toon/compilers/gcc/gcc/vec.h:1515:3,
>> > inlined from 'safe_grow_cleared' at
>> > /home/toon/compilers/gcc/gcc/vec.h:1529:3,
>> > inlined from 'vect_bb_vectorization_profitable_p' at
>> > /home/toon/compilers/gcc/gcc/tree-vect-slp.c:2027:0,
>> > inlined from 'vect_slp_analyze_bb_1' at
>> > /home/toon/compilers/gcc/gcc/tree-vect-slp.c:2174:55,
>> > inlined from 'vect_slp_analyze_bb' at
>> > /home/toon/compilers/gcc/gcc/tree-vect-slp.c:2229:44:
>> > /home/toon/compilers/gcc/gcc/vec.h:316:3: error: attempt to free a non-heap
>> > object 'life' [-Werror=free-nonheap-object]
>> > ::free (v);
>> > ^
>> > lto1: all warnings being treated as errors
>>
>> Obviously fallout of the stack-vector rewrite.
>>
>> stack_vec<bool, 20> life;
>> life.safe_grow_cleared (SLP_INSTANCE_GROUP_SIZE (instance));
>>
>> doesn't make much sense anyway ... why is the initial size only
>> allowed as a template parameter? But then safe_grow shouldn't free
>> the stack storage.
>
> So, I'm still confused how exactly this happens, presumably we must be
> trying to reserve a vector of length zero and consequently hit that code
> in va_heap::reserve to free the vector if we're supposed to reserve 0
> elements, but that doesn't make sense because the first check in
> vec::reserve should have failed if that was the case. Unfortunately I
> have no clue how to get dumps for optimization happening during lto or
> otherwise debug the compiler during lto.
The backtrace suggests that the error may be from expanding code
that is dead at runtime but was not detected as so. IPA-CP likely
propagated the storage object and partial inlining obfuscated the code
enough to hide the fact that it is dead ...
> However I think I've convinced myself this code to have
> vec_prefix::calculate_allocation return 0 if !prefix & !reserve should
> never be hit, so I think we can remove it and infact that seems to get
> me through atleast one round of lto.
>
> I'll run a full lto bootstrap and standard bootstrap on x86_64-linux-gnu
> with test suite, assuming those are ok is the below ok?
Ok.
Thanks,
Richard.
> Trev
>
> 2013-11-4 Trevor Saunders <tsaund...@mozilla.com>
>
> * vec.c (vec_prefix::calculate_allocation): Don't try to handle the
> case of no prefix and reserving zero slots, because when that's the
> case we'll never get here.
> * vec.h (va_heap::reserve): Don't try and handle
> vec_prefix::calculate_allocation returning zero because that should
> never happen.
>
> diff --git a/gcc/vec.c b/gcc/vec.c
> index f3c3315..78252e0 100644
> --- a/gcc/vec.c
> +++ b/gcc/vec.c
> @@ -187,9 +187,7 @@ vec_prefix::calculate_allocation (vec_prefix *pfx,
> unsigned reserve,
> num = pfx->m_num;
> }
> else if (!reserve)
> - /* If there's no vector, and we've not requested anything, then we
> - will create a NULL vector. */
> - return 0;
> + gcc_unreachable ();
>
> /* We must have run out of room. */
> gcc_assert (alloc - num < reserve);
> diff --git a/gcc/vec.h b/gcc/vec.h
> index f97e022..b1ebda4 100644
> --- a/gcc/vec.h
> +++ b/gcc/vec.h
> @@ -283,11 +283,7 @@ va_heap::reserve (vec<T, va_heap, vl_embed> *&v,
> unsigned reserve, bool exact
> {
> unsigned alloc
> = vec_prefix::calculate_allocation (v ? &v->m_vecpfx : 0, reserve,
> exact);
> - if (!alloc)
> - {
> - release (v);
> - return;
> - }
> + gcc_assert (alloc);
>
> if (GATHER_STATISTICS && v)
> v->m_vecpfx.release_overhead ();
> --
> 1.8.4.2
>
>>
>> Richard.
>>
>> > --
>> > Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290
>> > Saturnushof 14, 3738 XG Maartensdijk, The Netherlands
>> > At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/
>> > Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news
