From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533]) by sourceware.org (Postfix) with ESMTPS id 35224397B813 for ; Tue, 20 Jul 2021 11:20:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 35224397B813 Received: by mail-ed1-x533.google.com with SMTP id l1so27934404edr.11 for ; Tue, 20 Jul 2021 04:20:00 -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=nDkkUUvKlksIlOg7UQJMScNPTu36DKKmmrqrklp6pj0=; b=nCmRgkDtr8uwGojP+65AftyDgmUL74sXsecrWT5aML9rbfexq/Ury3UPEEvg/WTRNW G7jlkrPCwcAIQhkOK806fdXMcpGhYE7rlnSFQ8gFw1c+4102tjxjh1k08Z2/Ucvw9hrO UryHGQazAZ7rXyTZJNcv28orldeCVdV2wx0U+zrCDP84/k5o9kvkPVGRbN7cN8+HLdNj yuwu5+42ptuqDgSgR8jcZrDY+QJQmrn4DPcmzUKOIJTW7HkD1raFZ5hnDbB/kSsPHhFZ Gb2Acg4SG3YKQZOEY5RZ9+Jx38UpdF/fI8OpIfNPI7+OZfmy4FqhcE/L/ydqH8p4DSqE zTeA== X-Gm-Message-State: AOAM5305InMU4uT+19YxrlgkThc3nLB6FMEJDNsIttq/tII7F7rARQRe SmoTiJcBaVL8Arbh5hbk7tYm9XxXMFpfS+vEKcQ= X-Google-Smtp-Source: ABdhPJzfXyBUlvyS2IpyWcwF8W0UAN/6D/OSsuw0FUqHxjKXF6ZawePODq0iZt9cWC/XY5lmbCyM5rpm8bmm4Pkfd0c= X-Received: by 2002:aa7:c808:: with SMTP id a8mr40552228edt.245.1626779999294; Tue, 20 Jul 2021 04:19:59 -0700 (PDT) MIME-Version: 1.0 References: <6f695498-7f72-fe2f-b88b-4240f0d4569a@linux.ibm.com> <2b1c6017-08b4-e96e-cc0d-b229e912ea0e@linux.ibm.com> <90d0d676-3f65-78b4-b8d5-072020459345@linux.ibm.com> In-Reply-To: <90d0d676-3f65-78b4-b8d5-072020459345@linux.ibm.com> From: Richard Biener Date: Tue, 20 Jul 2021 13:19:48 +0200 Message-ID: Subject: Re: [PATCH] predcom: Refactor more using auto_vec To: "Kewen.Lin" Cc: Martin Sebor , Bill Schmidt , GCC Patches , Segher Boessenkool Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, 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, 20 Jul 2021 11:20:01 -0000 On Mon, Jul 19, 2021 at 8:29 AM Kewen.Lin wrote: > > Hi Martin & Richard, > > >> A further improvement worth considering (if you're so inclined :) > >> is replacing the pcom_worker vec members with auto_vec (obviating > >> having to explicitly release them) and for the same reason also > >> replacing the comp_ptrs bare pointer members with auto_vecs. > >> There may be other opportunities to do the same in individual > >> functions (I'd look to get rid of as many calls to functions > >> like XNEW()/XNEWVEC() and free() use auto_vec instead). > >> > >> An unrelated but worthwhile change is to replace the FOR_EACH_ > >> loops with C++ 11 range loops, analogously to: > >> https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572315.html > >> > >> Finally, the only loosely followed naming convention for member > >> variables is to start them with the m_ prefix. > >> > >> These just suggestions that could be done in a followup, not > >> something I would consider prerequisite for accepting the patch > >> as is if I were in a position to make such a decision. > >> > > Sorry for the late update, this patch follows your previous > advices to refactor it more by: > - Adding m_ prefix for class pcom_worker member variables. > - Using auto_vec instead of vec among class pcom_worker, > chain, component and comp_ptrs. > > btw, the changes in tree-data-ref.[ch] is required, without > it the destruction of auto_vec instance could try to double > free the memory pointed by m_vec. > > The suggestion on range loops is addressed by one separated > patch: https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575536.html > > Bootstrapped and regtested on powerpc64le-linux-gnu P9, > x86_64-redhat-linux and aarch64-linux-gnu, also > bootstrapped on ppc64le P9 with bootstrap-O3 config. > > Is it ok for trunk? OK. Thanks, Richard. > BR, > Kewen > ----- > gcc/ChangeLog: > > * tree-data-ref.c (free_dependence_relations): Adjust to pass vec by > reference. > (free_data_refs): Likewise. > * tree-data-ref.h (free_dependence_relations): Likewise. > (free_data_refs): Likewise. > * tree-predcom.c (struct chain): Use auto_vec instead of vec for > members. > (struct component): Likewise. > (pcom_worker::pcom_worker): Adjust for auto_vec and renaming changes. > (pcom_worker::~pcom_worker): Likewise. > (pcom_worker::release_chain): Adjust as auto_vec changes. > (pcom_worker::loop): Rename to ... > (pcom_worker::m_loop): ... this. > (pcom_worker::datarefs): Rename to ... > (pcom_worker::m_datarefs): ... this. Use auto_vec instead of vec. > (pcom_worker::dependences): Rename to ... > (pcom_worker::m_dependences): ... this. Use auto_vec instead of vec. > (pcom_worker::chains): Rename to ... > (pcom_worker::m_chains): ... this. Use auto_vec instead of vec. > (pcom_worker::looparound_phis): Rename to ... > (pcom_worker::m_looparound_phis): ... this. Use auto_vec instead of > vec. > (pcom_worker::cache): Rename to ... > (pcom_worker::m_cache): ... this. Use auto_vec instead of vec. > (pcom_worker::release_chain): Adjust for auto_vec changes. > (pcom_worker::release_chains): Adjust for auto_vec and renaming > changes. > (release_component): Remove. > (release_components): Adjust for release_component removal. > (component_of): Adjust to use vec. > (merge_comps): Likewise. > (pcom_worker::aff_combination_dr_offset): Adjust for renaming changes. > (pcom_worker::determine_offset): Likewise. > (class comp_ptrs): Remove. > (pcom_worker::split_data_refs_to_components): Adjust for renaming > changes, for comp_ptrs removal with auto_vec. > (pcom_worker::suitable_component_p): Adjust for renaming changes. > (pcom_worker::filter_suitable_components): Adjust for release_component > removal. > (pcom_worker::valid_initializer_p): Adjust for renaming changes. > (pcom_worker::find_looparound_phi): Likewise. > (pcom_worker::add_looparound_copies): Likewise. > (pcom_worker::determine_roots_comp): Likewise. > (pcom_worker::single_nonlooparound_use): Likewise. > (pcom_worker::execute_pred_commoning_chain): Likewise. > (pcom_worker::execute_pred_commoning): Likewise. > (pcom_worker::try_combine_chains): Likewise. > (pcom_worker::prepare_initializers_chain): Likewise. > (pcom_worker::prepare_initializers): Likewise. > (pcom_worker::prepare_finalizers_chain): Likewise. > (pcom_worker::prepare_finalizers): Likewise. > (pcom_worker::tree_predictive_commoning_loop): Likewise.