This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Redesign jump threading profile updates


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 Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]