From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa30.google.com (mail-vk1-xa30.google.com [IPv6:2607:f8b0:4864:20::a30]) by sourceware.org (Postfix) with ESMTPS id 233803856242 for ; Thu, 12 May 2022 07:34:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 233803856242 Received: by mail-vk1-xa30.google.com with SMTP id o132so2232324vko.11 for ; Thu, 12 May 2022 00:34:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=B3QGHC1kfZzFmlqIzP98HwgQZsIq3CE8OwwzAmwJ878=; b=WodG0rDbnAifckHZDqQ6VTlIsYGT989yBdMFs4obqa28s7cdjNSiv89LDEx2D+WSrv d/uGefyAreSW41q7eqLzXw4Xnjt8eYJqUksRq8YMZWdReuqAV/4M/bWCse1r136j6XvV cCCb9/UBSvkmCv0Y3zsyfH4t5ETlFHOBeHTnY6QS1TOzcNo9z8B7U5JxjyLeOFfwLJJ3 Z55w84gX02xj3nVSXRTmQDD8Fq+p0LvffXyoDAPMeULxI6f0ZM5EVNcPFOaR1+/picMD AC0zR5jtyj9jeXsEcN8nfWEkiMjYYeBjuI4nQIy8NqppaqhRo2PVelEqhMFXyiJjtvFN DXmQ== X-Gm-Message-State: AOAM530C4gtS+NkZljXh1WskFz7cZTJa+vaFsLSjq5syF9MnUAP4ngS3 mNObrw1DQ18Sk8FSu3urZLl35pnxycnwZJQdyLI= X-Google-Smtp-Source: ABdhPJxj4QP73Na6yJQXZKlM1W5tLkvx0C/OgcPCcIs84MS3Qbme1/NTFlnOayoShMQZICmuCvyOoYH41xYGLhqOlY4= X-Received: by 2002:a1f:aa16:0:b0:345:df50:5f89 with SMTP id t22-20020a1faa16000000b00345df505f89mr15975092vke.21.1652340865432; Thu, 12 May 2022 00:34:25 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Thu, 12 May 2022 09:34:13 +0200 Message-ID: Subject: Re: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator To: Eugene Rozenfeld Cc: Jan Hubicka , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 May 2022 07:34:27 -0000 On Thu, May 12, 2022 at 3:37 AM Eugene Rozenfeld wrote: > > In my case this is not exactly what the FIXME in the comment above says. The count is 0 even before > the initial scaling happens. I hit this case with some changes I'm working on to enable per-instruction discriminators for AutoFDO. > > I looked into saving/restoring the old counts but it would be awkward to do. The initial scaling happens here: > > if (skip_vector) > { > split_edge (loop_preheader_edge (loop)); > > /* Due to the order in which we peel prolog and epilog, we first > propagate probability to the whole loop. The purpose is to > avoid adjusting probabilities of both prolog and vector loops > separately. Note in this case, the probability of epilog loop > needs to be scaled back later. */ > basic_block bb_before_loop = loop_preheader_edge (loop)->src; > if (prob_vector.initialized_p ()) > { > scale_bbs_frequencies (&bb_before_loop, 1, prob_vector); > scale_loop_profile (loop, prob_vector, 0); > } > } > > The scaling happens before we do the cloning for the epilog so to save/restore the counts > we would need to maintain a mapping between the original basic blocks and the cloned basic blocks in the epilog. There is one already - after the epilogue is copied you can use get_bb_original (epilouge_bb) to get at the block it was copied from. It could also be that we can rely on the basic-block order to be preserved between the copies (I _think_ that would work ... but then assert () for this using get_bb_original ()). That means the simplest fix would be to have an auto_vec and initialize that from the BB counts in loop body order (we also have exactly two BBs in all peeled loops ... But note we only scaled the scalar if-converted loop but eventually used the not if-coverted copy for the epilogue (if not vectorizing the epilogue), so indiscriminate scaling back looks wrong in some cases (I'd have to double-check the details here). > I'd like to get my simple fix in since it makes things better even if it doesn't address the issue mentioned > In the FIXME. But don't you need to check that bbs[i]->count.apply_probability (prob_vector) is not zero instead of checking that bbs[i].count is not zero? Richard. > -----Original Message----- > From: Richard Biener > Sent: Monday, May 09, 2022 12:42 AM > To: Eugene Rozenfeld ; Jan Hubicka > Cc: gcc-patches@gcc.gnu.org > Subject: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator > > On Fri, May 6, 2022 at 10:32 PM Eugene Rozenfeld via Gcc-patches wrote: > > > > Calling count.apply_scale with a 0 denominator causes an assert. > > This change guards against that. > > > > Tested on x86_64-pc-linux-gnu. > > > > gcc/ChangeLog: > > * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying scale with 0 denominator. > > --- > > gcc/tree-vect-loop-manip.cc | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc > > index 1d4337eb261..db54ae69e45 100644 > > --- a/gcc/tree-vect-loop-manip.cc > > +++ b/gcc/tree-vect-loop-manip.cc > > @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, > > get lost if we scale down to 0. */ > > basic_block *bbs = get_loop_body (epilog); > > for (unsigned int i = 0; i < epilog->num_nodes; i++) > > - bbs[i]->count = bbs[i]->count.apply_scale > > - (bbs[i]->count, > > - bbs[i]->count.apply_probability > > - (prob_vector)); > > + if (bbs[i]->count.nonzero_p ()) > > + bbs[i]->count = bbs[i]->count.apply_scale > > + (bbs[i]->count, > > + bbs[i]->count.apply_probability > > + (prob_vector)); > > So exactly what the FIXME in the comment above says happens. It > might be better > to save/restore the old counts if the intent is to get them back. I'm not exactly sure where the other scaling happens though. > > Richard. > > > > > free (bbs); > > } > > > > -- > > 2.25.1