From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32b.google.com (mail-ot1-x32b.google.com [IPv6:2607:f8b0:4864:20::32b]) by sourceware.org (Postfix) with ESMTPS id 6F31F399C88B for ; Wed, 9 Jun 2021 19:01:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6F31F399C88B Received: by mail-ot1-x32b.google.com with SMTP id o17-20020a9d76510000b02903eabfc221a9so11270503otl.0 for ; Wed, 09 Jun 2021 12:01:21 -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-language :content-transfer-encoding; bh=Wk4MXMIG+3a6SRhtN03HLDQSExYr4JoQz+rSREPg1m0=; b=fHt76XiAMib0/QqtahEa7ipqkeAxVsDZ+R687LK/vcTYaOD5BCvXETaX/c/ktfvSih 7GbupuBEC3Pz3RCxDItH6xgJWogjAQGftpRP+x+Hwq/aY3yjAE60L0qCWOd6Tm0rhiVe pdLO9VWEwhXCtlkvuTaliMrJrsGiZmj/sg1t2sXpSaPn3YwLlpOaV9YydwaxW3199Z7b IJzAETyDwDOf1oT1hxhKgvoJ3+PcQh8HQGRUo8lnIHfH/bfsHZ7YN1059FVE5rh9RaQa 2SgPuPjOJ9TPS8LXvj5qL8KnGWP7t+Kk+s8lI2sI6PpBpf7tEmE0/KHfR25Vi1OgWVPL g6/g== X-Gm-Message-State: AOAM531Ol3t2FfmyyYt5tpAQ2KBBNqtAyJp/mcq2TfBQ/G5d2DKBBIkK KCilGROyn0jua+R2WNcRJOIVk0NHUgs= X-Google-Smtp-Source: ABdhPJwpW4MhGmhl1dsRPb71vjcbi63w7aUPHvgR1Ih8SlHuyMz6/DyJIkwx+vrXQh7l49vIvTmDhg== X-Received: by 2002:a9d:4105:: with SMTP id o5mr760067ote.20.1623265280665; Wed, 09 Jun 2021 12:01:20 -0700 (PDT) Received: from [192.168.0.41] (184-96-226-253.hlrn.qwest.net. [184.96.226.253]) by smtp.gmail.com with ESMTPSA id 2sm158831ota.58.2021.06.09.12.01.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 09 Jun 2021 12:01:20 -0700 (PDT) Subject: Re: [PATCH] Implement a context aware points-to analyzer for use in evrp. To: Aldy Hernandez , Richard Biener Cc: GCC patches References: <20210607101003.615929-1-aldyh@redhat.com> <49261e75-1adc-8603-4ccd-fa61fb0bde6c@redhat.com> <21ea25a3-dfc9-27cb-dfe0-537f5e27b11f@gmail.com> From: Martin Sebor Message-ID: <38ce77fe-6392-4332-55c4-9e9bd3bbf91b@gmail.com> Date: Wed, 9 Jun 2021 13:01:19 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Wed, 09 Jun 2021 19:01:23 -0000 On 6/9/21 12:50 PM, Aldy Hernandez wrote: > > > On 6/9/21 7:10 PM, Martin Sebor wrote: >> On 6/7/21 12:29 PM, Aldy Hernandez via Gcc-patches wrote: >>> > >> Mostly just a question of the type choices in the implementation >> of the ssa_equiv_stack class: m_stack is an auto_vec while >> m_replacements is a plain array.  I'd expect both to be the same >> (auto_vec).  Is there a reason for this choice? >> >> If not, I'd suggest to use auto_vec.  That way the ssa_equiv_stack >> class shouldn't need a dtor (auto_vec doesn't need to be explicitly >> released before it's destroyed), though it should delete its copy >> and assignment. > > TBH, the ssa_equiv_stack was lifted from evrp_range_analyzer (there are > also 2 more copies of the same mechanism in tree-ssa-scopedtables.h). > The code there already used an auto_vec, so I left that.  However, for a > mere array of trees, I thought it'd be simpler to use new/delete.  Which > in retrospect was easy to get wrong, as can be seen by: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100984 > > Would something like the code below work?  It should also fix the PR. > > Thanks. > Aldy > > diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c > index 7e1cf51239a..808d47506be 100644 > --- a/gcc/gimple-ssa-evrp.c > +++ b/gcc/gimple-ssa-evrp.c > @@ -61,19 +61,18 @@ public: > >  private: >    auto_vec> m_stack; > -  tree *m_replacements; > +  auto_vec m_replacements; >    const std::pair m_marker = std::make_pair (NULL, NULL); >  }; > >  ssa_equiv_stack::ssa_equiv_stack () >  { > -  m_replacements = new tree[num_ssa_names] (); > +  m_replacements.safe_grow_cleared (num_ssa_names); >  } > >  ssa_equiv_stack::~ssa_equiv_stack () >  { >    m_stack.release (); > -  delete m_replacements; >  } Yes, this is what I was suggesting, with the additional removal of the dtor above (since release() is called from the auto_vec dtor). (Once auto_vec is safe to copy and assign we'll also get copy and assignment for nothing.) Martin PS Enhancing -Wmismatched-new-delete to detect the bug in PR 100984 is on my to do list.