From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by sourceware.org (Postfix) with ESMTPS id 65FC8384B13D for ; Tue, 27 Jul 2021 15:21:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 65FC8384B13D Received: by mail-pj1-x1035.google.com with SMTP id ca5so5879425pjb.5 for ; Tue, 27 Jul 2021 08:21:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=kcP4TQpz3YTvX54EkXa2vd6QdZSngToj6bqH+8bQ5Kk=; b=qIgDiYM6vVYfZBXsbw5HfhdAa+Li7YsTuRuryVwOqmtSy9gRuXifN/gl2eGQiWWOtJ W2eNEo08aaHNlYvf9WiTl3VP5JZMgRSNz7DQSZ2vYazQ5vnYnBjMdHP4DMDs6o7InqF2 /Fld9AsXTjGXEqbvEvEV0Ul7gDeErXIthtaeNxolPNkT7L3kthN5ADFHVQLc5DCbmIay uTY3Kia3fWlcovaOgrvGEJ0Djnp868n49O8+qssyjF+7SNklF96NC89ibtYFayLw9BHD DjmqiCnZRYMGXKHflxUGPP36Is8m0UkeOKXktJxEE7X2BNBNdXvz0m50ynMgV3llvleH Q1NQ== X-Gm-Message-State: AOAM533pIXuxeZ1aLodIJfRsuk8Hdj1iuFOuIvbn8BDkbVlmfA3sGu2E yzdgWNRtkCNZE+IurSh3aL3Nz6YObw8= X-Google-Smtp-Source: ABdhPJzxqWEqXa6TdAhFXf+L+HgsrEaj61wo+SVeVut7+s6aX2ZNNrr7rp9tEm3zDQhmJtx7hP4C2A== X-Received: by 2002:a17:90a:4093:: with SMTP id l19mr22923370pjg.118.1627399309378; Tue, 27 Jul 2021 08:21:49 -0700 (PDT) Received: from [172.31.0.175] (c-98-202-48-222.hsd1.ut.comcast.net. [98.202.48.222]) by smtp.gmail.com with ESMTPSA id n33sm4108854pgm.55.2021.07.27.08.21.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 27 Jul 2021 08:21:49 -0700 (PDT) Subject: Re: 0001-Don-t-skip-prologue-instructions-as-it-could-affect-.patch To: "Bin.Cheng" Cc: "bin.cheng" , GCC Patches References: <54c74204-38f5-690a-6f0c-6b9fb5e96756@gmail.com> From: Jeff Law Message-ID: <5f831e1d-0e52-1d93-dd32-1ae0053ef46a@gmail.com> Date: Tue, 27 Jul 2021 09:21:45 -0600 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, 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 15:21:52 -0000 On 7/27/2021 2:50 AM, Bin.Cheng wrote: > 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? Yes.  Go ahead and commit it. Thanks for your patience, Jeff