On Wed, Oct 1, 2014 at 4:09 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> The block frequencies are very small in this case leading to rounding
>> errors when computing the edge frequency. The rounding error was then
>> propagated into the recomputed probabilities, leading to insanities in
>> the outgoing edge probabilities on the jump threading path. Eventually
>> during rtl expansion these insanities somehow caused an ICE.
>
> Concerning the never ending rounoff issues, I thing we ought to progress
> on turning counts, freqs and probabilities into a type that eliminates
> most of this fun.
>
> I see basically two options - first would be to take wide_int and build
> a fixed point type template around it (so one can chose precisions),
> other would be to have software emulated floating point type.
>
> Actually I think the second will be less troublesome - FP seems to fit
> the bill very well. We already have sreal.h that does the job for
> propagation of frequencies. What about turning it ito C++ type with
> operations on it and reroganizing code to use it?
> (we may consider making the base 64bit, as opposed to HOST_WIDE_INT
> right now)
>
> Or are there other options I missed? It would be very cool if someone
> could volunteer and implement the basic datatype and possibly start with
> convertion.
>
> I think we could do type by type, first redefining counts from gcov_t
> and relying on conversion to gcov_t to work to not having to update all
> uses at once. THen we can just update the code pass by pass.
>
> One nice thing we could add into the type is to make difference in betwen
> known zeros (i.e. read from profile) and zeros that were results of updates
> that may or may not be precise. THis should hopefully solve the bad
> iteraction of Martin's code ordering and Theresa's BB-reorder work.
Yes, this would avoid a number of headaches with roundoff errors I've
seen and worked around in various contexts. Using C++ to implement the
sreal type and provide the operators seems like a good option. I had
hoped to work on this after we talked about it in the past but haven't
had the bandwidth unfortunately.
Teresa
>
> Honza
>>
>> (Before this patch the probabilities weren't even being updated,
>> leading to insanities in other cases where they needed to be updated.)
>>
>> Specifically, in this case we had a block with frequency = 1. It had
>> two outgoing edges each with probability 50%. Because the
>> EDGE_FREQUENCY computation uses a rounding divide, the outgoing edge
>> frequencies were both computed as 1. Later use of these block and edge
>> frequencies to recompute the probability lead to both outgoing edges
>> getting 100%.
>>
>> To address this, in the routine freqs_to_counts_path can simply scale
>> up the frequencies when recording them in the count field. I added a
>> scale by REG_BR_PROB_BASE. The largest count possible would therefore
>> be REG_BR_PROB_BASE * BB_FREQ_MAX which is 10000 * 10000 = 100mil
>> which safely fits in gcov_type.
>>
>> Here is the patch I am testing. Confirmed it fixes the reported issue.
>> Currently bootstrapping and testing on x86_64-unknown-linux-gnu. Ok
>> for trunk if it passes?
>>
>> (The whitespace is getting messed up when I copy the patch in here -
>> the indentations do line up in the patch.)
>>
>> Thanks,
>> Teresa
>>
>> 2014-10-01 Teresa Johnson <tejohn...@google.com>
>>
>> * tree-ssa-threadupdate.c (freqs_to_counts_path): Scale frequencies
>> up when synthesizing counts to avoid rounding errors.
>>
>> Index: tree-ssa-threadupdate.c
>> ===================================================================
>> --- tree-ssa-threadupdate.c (revision 215739)
>> +++ tree-ssa-threadupdate.c (working copy)
>> @@ -979,7 +979,11 @@ freqs_to_counts_path (struct redirection_data *rd)
>> FOR_EACH_EDGE (ein, ei, e->dest->preds)
>> {
>> gcc_assert (!ein->count);
>> - ein->count = EDGE_FREQUENCY (ein);
>> + /* Scale up the frequency by REG_BR_PROB_BASE, to avoid rounding
>> + errors applying the probability when the frequencies are very
>> + small. */
>> + ein->count = apply_probability (ein->src->frequency *
>> REG_BR_PROB_BASE,
>> + ein->probability);
>> }
>>
>> for (unsigned int i = 1; i < path->length (); i++)
>> @@ -987,11 +991,13 @@ freqs_to_counts_path (struct redirection_data *rd)
>> edge epath = (*path)[i]->e;
>> gcc_assert (!epath->count);
>> edge esucc;
>> + /* Scale up the frequency by REG_BR_PROB_BASE, to avoid rounding
>> + errors applying the edge probability when the frequencies are very
>> + small. */
>> + epath->src->count = epath->src->frequency * REG_BR_PROB_BASE;
>> FOR_EACH_EDGE (esucc, ei, epath->src->succs)
>> - {
>> - esucc->count = EDGE_FREQUENCY (esucc);
>> - }
>> - epath->src->count = epath->src->frequency;
>> + esucc->count = apply_probability (esucc->src->count,
>> + esucc->probability);
>> }
>> }
>>
>>
>> On Wed, Oct 1, 2014 at 8:29 AM, Teresa Johnson <tejohn...@google.com> wrote:
>> > I got the preprocessed source. With the aarch64 cross-compiler I built
>> > I am able to reproduce the ICE. Looking at it now.
>> >
>> > Thanks,
>> > Teresa
>> >
>> > On Wed, Oct 1, 2014 at 8:25 AM, Christophe Lyon
>> > <christophe.l...@linaro.org> wrote:
>> >> On 1 October 2014 17:22, Sebastian Pop <seb...@gmail.com> wrote:
>> >>> Christophe Lyon wrote:
>> >>>> Since this commit, I can see all my builds for arm*linux* and
>> >>>> aarch64*linux* fail while building glibc:
>> >>>>
>> >>>> /tmp/3496222_18.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/bin/aarch64-none-linux-gnu-gcc
>> >>>> iso-2022-cn.c -c -std=gnu99 -fgnu89-inline -O2 -Wall -Win
>> >>>> line -Wundef -Wwrite-strings -fmerge-all-constants -frounding-math -g
>> >>>> -Wstrict-prototypes -fPIC -I../include
>> >>>> -I/tmp/3496222_18.tmpdir/aci-gcc-f
>> >>>> sf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/iconvdata
>> >>>> -I/tmp/3496222_18.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux
>> >>>> -gnu/glibc-1 -I../sysdeps/unix/sysv/linux/aarch64/nptl
>> >>>> -I../sysdeps/unix/sysv/linux/aarch64
>> >>>> -I../sysdeps/unix/sysv/linux/generic -I../sysdeps/unix/s
>> >>>> ysv/linux/wordsize-64 -I../nptl/sysdeps/unix/sysv/linux
>> >>>> -I../nptl/sysdeps/pthread -I../sysdeps/pthread
>> >>>> -I../sysdeps/unix/sysv/linux -I../sysdeps/gn
>> >>>> u -I../sysdeps/unix/inet -I../nptl/sysdeps/unix/sysv
>> >>>> -I../sysdeps/unix/sysv -I../nptl/sysdeps/unix -I../sysdeps/unix
>> >>>> -I../sysdeps/posix -I../sysd
>> >>>> eps/aarch64/fpu -I../sysdeps/aarch64/nptl -I../sysdeps/aarch64
>> >>>> -I../sysdeps/wordsize-64 -I../sysdeps/ieee754/ldbl-128
>> >>>> -I../sysdeps/ieee754/dbl-64/w
>> >>>> ordsize-64 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32
>> >>>> -I../sysdeps/aarch64/soft-fp -I../sysdeps/ieee754
>> >>>> -I../sysdeps/generic -I../npt
>> >>>> l -I.. -I../libio -I. -nostdinc -isystem
>> >>>> /tmp/3496222_18.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/lib/gcc/aarch64-none-linux-gnu/5.0.0/include
>> >>>> -i
>> >>>> system
>> >>>> /tmp/3496222_18.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/lib/gcc/aarch64-none-linux-gnu/5.0.0/include-fixed
>> >>>> -isystem /tmp/3496222_18.tmpdir
>> >>>> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/sysroot-aarch64-none-linux-gnu/usr/include
>> >>>> -D_LIBC_REENTRANT -include ../include/libc-symbols.h -DPIC -DSHARED
>> >>>> -DNOT_IN_libc -o
>> >>>> /tmp/3496222_18.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/iconvdata/iso-2022-cn.os
>> >>>> -MD -MP -MF /tmp/3
>> >>>> 496222_18.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/iconvdata/iso-2022-cn.os.dt
>> >>>> -MT /tmp/3496222_18.tmpdir/aci-gcc-fsf
>> >>>> /builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/iconvdata/iso-2022-cn.os
>> >>>>
>> >>>> In file included from iso-2022-cn.c:407:0:
>> >>>> ../iconv/skeleton.c: In function 'gconv':
>> >>>> ../iconv/skeleton.c:800:1: internal compiler error: in
>> >>>> check_probability, at basic-block.h:959
>> >>>> 0xe4e2fb find_many_sub_basic_blocks(simple_bitmap_def*)
>> >>>>
>> >>>> /tmp/3496222_18.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/basic-block.h:959
>> >>>> 0x6623f0 execute
>> >>>>
>> >>>> /tmp/3496222_18.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:5916
>> >>>> Please submit a full bug report,
>> >>>> with preprocessed source if appropriate.
>> >>>> Please include the complete backtrace with any bug report.
>> >>>> See <http://gcc.gnu.org/bugs.html> for instructions.
>> >>>>
>> >>>> Can you have a look?
>> >>>
>> >>> It would help if you could attach a preprocessed file.
>> >>>
>> >> I did it in a followup mail, but the list server rejected it because
>> >> it was too large.
>> >> I suppose Teresa did receive it though.
>> >>
>> >> Not sure whether I can attach it in .xz format? Is this allowed?
>> >>
>> >> Thanks
>> >> Christophe.
>> >>
>> >>> Thanks,
>> >>> Sebastian
>> >
>> >
>> >
>> > --
>> > Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
