From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x236.google.com (mail-oi1-x236.google.com [IPv6:2607:f8b0:4864:20::236]) by sourceware.org (Postfix) with ESMTPS id B6571385483E for ; Tue, 29 Jun 2021 19:02:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B6571385483E Received: by mail-oi1-x236.google.com with SMTP id d19so27417295oic.7 for ; Tue, 29 Jun 2021 12:02: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:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=TZnCT6LKHmcM9diIFuHm9/g+RSKMM4H3skttBu8PMY0=; b=CfBm1WRHh7MUryHkZTHAinaVjse7T72u6EPzUgwA/o/nhJmtI+Y7MS5o197aWe8Rph iGlnt5q8TUydvcgkJQp9HgmJ/cRCdtE6Eb8dI9Tee7T/URbB3o/flqeHKATktZrmT5uE rDT9AzpB1a933yVGOG1PtHTKBhRnvshMW0lmSKEMznvKdleqfzTkTf8//3V7MN6fLoas xgj5ZWtMjptYIvRHgWatCMolAr4TmYFkhvgzwLCpADFDYGGk9I1472u3zHQOFz1YKw9m 91OYsRoExmUB5izfapNC+7nNI/FITLvRLmzcCDMGl1x+79AxJTzOfNs+WkghD29/LcQA adzQ== X-Gm-Message-State: AOAM533wH0WNqbSrStpDXvQDrY3MM3nRUAZI2R2WfZ7vSTwO4WmaNeQ6 171s/7/nh3I4R+UL+1VlHdYcpf95Gq5kZg== X-Google-Smtp-Source: ABdhPJw++Qz+fwGYUXusFdZJrMHBXQiNBAom38k0mTtHLlSEvVwpv2W/yKDyVYO1A+EZShzhBOv3/g== X-Received: by 2002:a05:6808:683:: with SMTP id k3mr19891919oig.171.1624993340946; Tue, 29 Jun 2021 12:02:20 -0700 (PDT) Received: from [192.168.0.41] (97-118-105-195.hlrn.qwest.net. [97.118.105.195]) by smtp.gmail.com with ESMTPSA id y61sm4269351ota.31.2021.06.29.12.02.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 29 Jun 2021 12:02:20 -0700 (PDT) Subject: Re: [PATCH 1/4] Duplicate the range information of the phi onto the new ssa_name To: apinski@marvell.com, gcc-patches@gcc.gnu.org References: <1624836300-23553-1-git-send-email-apinski@marvell.com> <1624836300-23553-2-git-send-email-apinski@marvell.com> From: Martin Sebor Message-ID: <7625f706-25f6-7935-1c3e-24f94ee3655f@gmail.com> Date: Tue, 29 Jun 2021 13:02:18 -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: <1624836300-23553-2-git-send-email-apinski@marvell.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.1 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.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: Tue, 29 Jun 2021 19:02:23 -0000 On 6/27/21 5:24 PM, apinski--- via Gcc-patches wrote: > From: Andrew Pinski > > Since match_simplify_replacement uses gimple_simplify, there is a new > ssa name created sometimes and then we go and replace the phi edge with > this new ssa name, the range information on the phi is lost. > Placing this in replace_phi_edge_with_variable is the best option instead > of doing it in each time replace_phi_edge_with_variable is called which is > what is done today. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > gcc/ChangeLog: > > * tree-ssa-phiopt.c (replace_phi_edge_with_variable): Duplicate range > info if we're the only things setting the target PHI. > (value_replacement): Don't duplicate range here. > (minmax_replacement): Likewise. > --- > gcc/tree-ssa-phiopt.c | 43 ++++++++++++++++++++++++++----------------- > 1 file changed, 26 insertions(+), 17 deletions(-) > > diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c > index 1777bff2f7c..ab12e85569d 100644 > --- a/gcc/tree-ssa-phiopt.c > +++ b/gcc/tree-ssa-phiopt.c > @@ -391,6 +391,32 @@ replace_phi_edge_with_variable (basic_block cond_block, > basic_block bb = gimple_bb (phi); > basic_block block_to_remove; > gimple_stmt_iterator gsi; > + tree phi_result = PHI_RESULT (phi); > + > + /* Duplicate range info if we're the only things setting the target PHI. I'm not too familiar with the code so the comments are helpful. But I don't understand what you mean by "we're the only things" above. (What's "we" and what might be some of the other "things?") Can you please clarify that comment? > + This is needed as later on, the new_tree will be replacing > + The assignement of the PHI. > + For an example: > + bb1: > + _4 = min > + goto bb2 > + > + range<-INF,255> > + a_3 = PHI<_4(1)> > + bb3: > + > + use(a_3) > + And _4 gets prograted into the use of a_3 and losing the range info. > + This can't be done for more than 2 incoming edges as the progration > + won't happen. */ Presumably you mean propagated and propagation above? Martin > + if (TREE_CODE (new_tree) == SSA_NAME > + && EDGE_COUNT (gimple_bb (phi)->preds) == 2 > + && INTEGRAL_TYPE_P (TREE_TYPE (phi_result)) > + && !SSA_NAME_RANGE_INFO (new_tree) > + && SSA_NAME_RANGE_INFO (phi_result)) > + duplicate_ssa_name_range_info (new_tree, > + SSA_NAME_RANGE_TYPE (phi_result), > + SSA_NAME_RANGE_INFO (phi_result)); > > /* Change the PHI argument to new. */ > SET_USE (PHI_ARG_DEF_PTR (phi, e->dest_idx), new_tree); > @@ -1385,16 +1411,6 @@ value_replacement (basic_block cond_bb, basic_block middle_bb, > : > # u_3 = PHI */ > reset_flow_sensitive_info (lhs); > - if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))) > - { > - /* If available, we can use VR of phi result at least. */ > - tree phires = gimple_phi_result (phi); > - struct range_info_def *phires_range_info > - = SSA_NAME_RANGE_INFO (phires); > - if (phires_range_info) > - duplicate_ssa_name_range_info (lhs, SSA_NAME_RANGE_TYPE (phires), > - phires_range_info); > - } > gimple_stmt_iterator gsi_from; > for (int i = prep_cnt - 1; i >= 0; --i) > { > @@ -1794,13 +1810,6 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, > gimple_seq stmts = NULL; > tree phi_result = PHI_RESULT (phi); > result = gimple_build (&stmts, minmax, TREE_TYPE (phi_result), arg0, arg1); > - /* Duplicate range info if we're the only things setting the target PHI. */ > - if (!gimple_seq_empty_p (stmts) > - && EDGE_COUNT (gimple_bb (phi)->preds) == 2 > - && !POINTER_TYPE_P (TREE_TYPE (phi_result)) > - && SSA_NAME_RANGE_INFO (phi_result)) > - duplicate_ssa_name_range_info (result, SSA_NAME_RANGE_TYPE (phi_result), > - SSA_NAME_RANGE_INFO (phi_result)); > > gsi = gsi_last_bb (cond_bb); > gsi_insert_seq_before (&gsi, stmts, GSI_NEW_STMT); >