Message ID | CAAe5K+XWZEJPkhnM_2TFmh0VN9u_qXpVGfddUZ71YZkD8kMfXg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Wed, 2014-10-01 at 13:04 -0700, Teresa Johnson wrote: > 2014-10-01 Teresa Johnson <tejohnson@google.com> > > * tree-ssa-threadupdate.c (freqs_to_counts_path): Scale frequencies > up when synthesizing counts to avoid rounding errors. I tried this patch on my MIPS toolchain build but GCC is still failing while building glibc. iconv_open.c: In function 'iconv_open': iconv_open.c:32:1: error: verify_flow_info: Wrong probability of edge 14->18 -920374544 iconv_open (const char *tocode, const char *fromcode) ^ iconv_open.c:32:1: error: verify_flow_info: Wrong probability of edge 14->19 -903724544 iconv_open.c:32:1: internal compiler error: verify_flow_info failed 0x626a70 verify_flow_info() /scratch/sellcey/repos/nightly/src/gcc/gcc/cfghooks.c:260 0x9f36e4 cleanup_tree_cfg_noloop /scratch/sellcey/repos/nightly/src/gcc/gcc/tree-cfgcleanup.c:765 0x9f36e4 cleanup_tree_cfg() /scratch/sellcey/repos/nightly/src/gcc/gcc/tree-cfgcleanup.c:814 0x8eeff4 execute_function_todo /scratch/sellcey/repos/nightly/src/gcc/gcc/passes.c:1704 0x8efb23 execute_todo /scratch/sellcey/repos/nightly/src/gcc/gcc/passes.c:1808 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.
> 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. 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 <tejohnson@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 <tejohnson@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.lyon@linaro.org> wrote: > >> On 1 October 2014 17:22, Sebastian Pop <sebpop@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 | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Wed, Oct 1, 2014 at 3:46 PM, Steve Ellcey <sellcey@mips.com> wrote: > On Wed, 2014-10-01 at 13:04 -0700, Teresa Johnson wrote: > >> 2014-10-01 Teresa Johnson <tejohnson@google.com> >> >> * tree-ssa-threadupdate.c (freqs_to_counts_path): Scale frequencies >> up when synthesizing counts to avoid rounding errors. > > I tried this patch on my MIPS toolchain build but GCC is still failing > while building glibc. > > iconv_open.c: In function 'iconv_open': > iconv_open.c:32:1: error: verify_flow_info: Wrong probability of edge > 14->18 -920374544 > iconv_open (const char *tocode, const char *fromcode) > ^ > iconv_open.c:32:1: error: verify_flow_info: Wrong probability of edge > 14->19 -903724544 > iconv_open.c:32:1: internal compiler error: verify_flow_info failed > 0x626a70 verify_flow_info() > /scratch/sellcey/repos/nightly/src/gcc/gcc/cfghooks.c:260 > 0x9f36e4 cleanup_tree_cfg_noloop > /scratch/sellcey/repos/nightly/src/gcc/gcc/tree-cfgcleanup.c:765 > 0x9f36e4 cleanup_tree_cfg() > /scratch/sellcey/repos/nightly/src/gcc/gcc/tree-cfgcleanup.c:814 > 0x8eeff4 execute_function_todo > /scratch/sellcey/repos/nightly/src/gcc/gcc/passes.c:1704 > 0x8efb23 execute_todo > /scratch/sellcey/repos/nightly/src/gcc/gcc/passes.c:1808 > 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. > Will take a look. In case I can't reproduce it with the aarch64 cross-compiler or x86_64, can you give me the preprocessed source? And also the instructions for how to configure for MIPS. Teresa
On Wed, Oct 1, 2014 at 4:09 PM, Jan Hubicka <hubicka@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 <tejohnson@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 <tejohnson@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.lyon@linaro.org> wrote: >> >> On 1 October 2014 17:22, Sebastian Pop <sebpop@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 | tejohnson@google.com | 408-460-2413 >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Wed, Oct 1, 2014 at 10:02 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Wed, Oct 1, 2014 at 3:46 PM, Steve Ellcey <sellcey@mips.com> wrote: >> On Wed, 2014-10-01 at 13:04 -0700, Teresa Johnson wrote: >> >>> 2014-10-01 Teresa Johnson <tejohnson@google.com> >>> >>> * tree-ssa-threadupdate.c (freqs_to_counts_path): Scale frequencies >>> up when synthesizing counts to avoid rounding errors. >> >> I tried this patch on my MIPS toolchain build but GCC is still failing >> while building glibc. >> >> iconv_open.c: In function 'iconv_open': >> iconv_open.c:32:1: error: verify_flow_info: Wrong probability of edge >> 14->18 -920374544 >> iconv_open (const char *tocode, const char *fromcode) >> ^ >> iconv_open.c:32:1: error: verify_flow_info: Wrong probability of edge >> 14->19 -903724544 >> iconv_open.c:32:1: internal compiler error: verify_flow_info failed >> 0x626a70 verify_flow_info() >> /scratch/sellcey/repos/nightly/src/gcc/gcc/cfghooks.c:260 >> 0x9f36e4 cleanup_tree_cfg_noloop >> /scratch/sellcey/repos/nightly/src/gcc/gcc/tree-cfgcleanup.c:765 >> 0x9f36e4 cleanup_tree_cfg() >> /scratch/sellcey/repos/nightly/src/gcc/gcc/tree-cfgcleanup.c:814 >> 0x8eeff4 execute_function_todo >> /scratch/sellcey/repos/nightly/src/gcc/gcc/passes.c:1704 >> 0x8efb23 execute_todo >> /scratch/sellcey/repos/nightly/src/gcc/gcc/passes.c:1808 >> 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. >> > > Will take a look. In case I can't reproduce it with the aarch64 > cross-compiler or x86_64, can you give me the preprocessed source? And > also the instructions for how to configure for MIPS. Hi Steve, Please do send me the preprocessed source and if possible also instructions for configuring a MIPS cross-compiler. I haven't been able to reproduce this with either x86_64 or the aarch64 cross-compiler I built yesterday. But I can't reproduce Christophe's failure either with my aarch64-configured glibc, so my version or headers must be different. Thanks! Teresa > > Teresa > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Wed, 2014-10-01 at 22:02 -0700, Teresa Johnson wrote: > Will take a look. In case I can't reproduce it with the aarch64 > cross-compiler or x86_64, can you give me the preprocessed source? And > also the instructions for how to configure for MIPS. > > Teresa Never mind, I messed up the patch when applying it. It looks good on MIPS now. I haven't done a complete build and test but the ICE went away when I re-applied the patch. Steve Ellcey sellcey@mips.com
On Thu, Oct 2, 2014 at 8:45 AM, Steve Ellcey <sellcey@mips.com> wrote: > On Wed, 2014-10-01 at 22:02 -0700, Teresa Johnson wrote: > >> Will take a look. In case I can't reproduce it with the aarch64 >> cross-compiler or x86_64, can you give me the preprocessed source? And >> also the instructions for how to configure for MIPS. >> >> Teresa > > Never mind, I messed up the patch when applying it. It looks good on > MIPS now. I haven't done a complete build and test but the ICE went > away when I re-applied the patch. Ok, phew. =) Thanks for the update. Jeff or Honza, did the patch look ok for trunk? Regression tests pass. Thanks, Teresa > > Steve Ellcey > sellcey@mips.com >
On 10/01/14 14:04, Teresa Johnson 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. > > (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 <tejohnson@google.com> > > * tree-ssa-threadupdate.c (freqs_to_counts_path): Scale frequencies > up when synthesizing counts to avoid rounding errors. OK. jeff
On 01/10/14 21:04, Teresa Johnson 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. That's exactly what I have observed while investigation this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61529 In this ICE while building glibc, we have frequency = 16, but we have even probabilities for 45 successors. The count for each outgoing edge is rounding to 0. Later in the update_joiner_offpath_counts() function, the count for each edge is re-calculated to be equal to e->src->count, thus the probability for each outgoing edge is 100% when probability is recomputed. This patch fixed the bug. Thank you! > > (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 <tejohnson@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 <tejohnson@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.lyon@linaro.org> wrote: >>> On 1 October 2014 17:22, Sebastian Pop <sebpop@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 | tejohnson@google.com | 408-460-2413 > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >
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); } }