From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4284 invoked by alias); 2 Oct 2014 05:07:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 4275 invoked by uid 89); 2 Oct 2014 05:07:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-qg0-f46.google.com Received: from mail-qg0-f46.google.com (HELO mail-qg0-f46.google.com) (209.85.192.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 02 Oct 2014 05:07:48 +0000 Received: by mail-qg0-f46.google.com with SMTP id z60so1436098qgd.33 for ; Wed, 01 Oct 2014 22:07:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=lHeiJ2csxtd3TTClliYKRpPYirDi/RhKitV6E1mxJFA=; b=eIr5vChQmQw0pwfdObJRKi7mSalMuXvJAvVrriVlhwL85QYCrZBuKYWPf2Sv+gVUeR sVpQLjJTYpfc8/nVOr5oN+RooUacHj6lHjdMotbFgwYtaManTPcJJsPhg5wWAjPv+YYz 0K6WrFjVcGhdnx6ILMUurPMhCLK2MYFYHM25bQSZorkQ4Rn+IZL9hdAl0g5worN3la8U NDE9resb0fs411M+fsfKNY5kf/dZJSagljpaB5TI5re73jbFK3FbDsKsmvsLQRX2Oo6c 9ytLGFctwo9GokFlAMuk8AQcuX/o25GJjOKHzNhcMKLdVfZqrleMI3H4xg5zuI7Nj9VV j1Pw== X-Gm-Message-State: ALoCoQmml0aaEuvRz4qu82qA54UGlwD9BIu5jVPRP8/faO6/VdA6704d4unm0SaJFh/xzaNrgJTF MIME-Version: 1.0 X-Received: by 10.224.40.193 with SMTP id l1mr82403822qae.14.1412226466457; Wed, 01 Oct 2014 22:07:46 -0700 (PDT) Received: by 10.229.148.1 with HTTP; Wed, 1 Oct 2014 22:07:46 -0700 (PDT) In-Reply-To: <20141001230933.GA12974@kam.mff.cuni.cz> References: <542A32AB.1040708@redhat.com> <20141001152256.GA24862@f1.c.bardezibar.internal> <20141001230933.GA12974@kam.mff.cuni.cz> Date: Thu, 02 Oct 2014 05:07:00 -0000 Message-ID: Subject: Re: [PATCH] Redesign jump threading profile updates From: Teresa Johnson To: Jan Hubicka Cc: Christophe Lyon , Sebastian Pop , Jeff Law , "gcc-patches@gcc.gnu.org" , David Li , mliska@suse.cz, tsaunders@mozilla.com Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg00124.txt.bz2 On Wed, Oct 1, 2014 at 4:09 PM, Jan Hubicka 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 >> >> * 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 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 >> > wrote: >> >> On 1 October 2014 17:22, Sebastian Pop 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 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 -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413