From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id 835453877432 for ; Tue, 27 Jul 2021 08:50:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 835453877432 Received: by mail-ed1-x52b.google.com with SMTP id u12so14371625eds.2 for ; Tue, 27 Jul 2021 01:50:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=BWckjd7/uxcCUlA3bvIBP0wdRqIhdfxUXwJNv2Gr7Lo=; b=alfUpo79kBl45YV4j58Uo3/qQ8JzaAHfHYV4ZtB3oTv9YbX++HkcuPk1mIreH7F4y7 AKnWT974Ge3Y0acnPCSae0CwpKaJrFNslTZwy4V+mJKfbb01MvJx42D2srPX4lksOPAP IuYfUUclWXKUTs0UZmH6lPhYxeYhwB+yeENP/xao4PLJlZiUmyDqCbvA3K3hPtd55Zdq m3866T5Z2iu+RK5Hu33R8A3KjDdFaiO8o0ZyCwPPDO39CjFlD4dfaISsn+72vrY096w3 aof0p/89jZz/f8i06LhzPcO7p0TPldUEj5EaCZvFShpDxSJF/gKaFSa9oy5XipVMW2H8 vlfA== X-Gm-Message-State: AOAM531zOAVNzUnfcJom/QE1khs97Fp+sbyEBfxPnI1oJhaOmDGlJJk5 ECwlL6qatVQBcCuHZCUC+Y2fH+RBj69xxmvJGtc= X-Google-Smtp-Source: ABdhPJzJ6y1JBPStISYnQ6lAj2YVzhP3mvUUecFAxtp04Wed7tFdp6wd1PnFWaa3JxTNgtBTufG2bJxZhooWSQSTaB0= X-Received: by 2002:a05:6402:26c5:: with SMTP id x5mr26466032edd.237.1627375853587; Tue, 27 Jul 2021 01:50:53 -0700 (PDT) MIME-Version: 1.0 References: <54c74204-38f5-690a-6f0c-6b9fb5e96756@gmail.com> In-Reply-To: <54c74204-38f5-690a-6f0c-6b9fb5e96756@gmail.com> From: "Bin.Cheng" Date: Tue, 27 Jul 2021 16:50:42 +0800 Message-ID: Subject: Re: 0001-Don-t-skip-prologue-instructions-as-it-could-affect-.patch To: Jeff Law Cc: "bin.cheng" , GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.8 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 autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Tue, 27 Jul 2021 08:50:56 -0000 On Mon, Jul 26, 2021 at 11:07 PM Jeff Law wrote: > > > > On 7/25/2021 7:47 PM, Bin.Cheng wrote: > > On Sat, Jul 24, 2021 at 12:30 AM Jeff Law via Gcc-patches > > wrote: > >> > >> > >> On 7/14/2021 3:14 AM, bin.cheng via Gcc-patches wrote: > >>> Hi, > >>> I ran into a wrong code bug in code with deep template instantiation when working on sdx::simd. > >>> The root cause as described in commit summary is we skip prologue insns in init_alias_analysis. > >>> This simple patch fixes the issue, however, it's hard to reduce a case because of heavy use of > >>> templates. > >>> Bootstrap and test on x86_64, is it OK? > >> It's a clear correctness improvement, but what's unclear to me is why > >> we'd want to skip them in the epilogue either. > > I can only guess, there is nothing to initialize epilogue for because > > no code follows. > Yea, but couldn't the lack of analysis of the epilogue lead to a pass > mis-optimizing code within the epilogue itself? It's not terribly > likely, but it just seems wrong to skip the epilogue like this. > Remember, the aliasing bits are just an analysis phase to find the > aliasing relationships that exist and we don't necessarily know how that > data is going to be used. It may in fact be safe now, but may not be > safe in the future if someone added a late RTL pass that used the > aliasing info in a new way. > > The more I think about it, the more I think we should remove remove this > hunk of code completely. There is some chance for fallout, but I think > it's unlikely. Hi Jeff, Thanks for the suggestion, here is the simple patch removing all of it. diff --git a/gcc/alias.c b/gcc/alias.c index 69e1eb89ac6..099acabca6b 100644 --- a/gcc/alias.c +++ b/gcc/alias.c @@ -3406,14 +3406,6 @@ init_alias_analysis (void) rpo = XNEWVEC (int, n_basic_blocks_for_fn (cfun)); rpo_cnt = pre_and_rev_post_order_compute (NULL, rpo, false); - /* The prologue/epilogue insns are not threaded onto the - insn chain until after reload has completed. Thus, - there is no sense wasting time checking if INSN is in - the prologue/epilogue until after reload has completed. */ - bool could_be_prologue_epilogue = ((targetm.have_prologue () - || targetm.have_epilogue ()) - && reload_completed); - pass = 0; do { @@ -3459,10 +3451,6 @@ init_alias_analysis (void) { rtx note, set; - if (could_be_prologue_epilogue - && prologue_epilogue_contains (insn)) - continue; - /* If this insn has a noalias note, process it, Otherwise, scan for sets. A simple set will have no side effects which could change the base value of any other register. */ No fallouts in bootstrap/test on x86_64. Is it OK? Thanks, bin > > Jeff >