From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53981 invoked by alias); 17 Aug 2016 13:44:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 53968 invoked by uid 89); 17 Aug 2016 13:44:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wm0-f45.google.com Received: from mail-wm0-f45.google.com (HELO mail-wm0-f45.google.com) (74.125.82.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 17 Aug 2016 13:44:47 +0000 Received: by mail-wm0-f45.google.com with SMTP id i5so233367103wmg.0 for ; Wed, 17 Aug 2016 06:44:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=h2FP7ZH02La60nfDaAPiAFZspE88MOfRMJbF3UV8v4Q=; b=J/0PyI0jWvvAhpokyfvOQ4UX4Qj6Rgm6kNPtL9te3t9cCzJQig0M61hHY94qvDU3Xh S2m9RjHQWjS8c9+SCz4G3E8uyHek5Yx3o4Fi/Xixz1vmTdHvqgal3nWkQcpYH82xWuab tNCZvZcOfFqIfKh+vP3KF9qXU0vc37C0wYqcsRsN5fDdq7774155oAlgXCHh2zXDJHgJ T/kgryl83a2Et5UJr6lHlNG1NCOVNtzwWwtGsLXtW7Fi0DbXNsHkDdHQTnWrelwdUncS 1kDW88O8XOgffTFc9QRAWiVY8Fn7x1oT7YQwmfCHPp3IscPqGuZ54L/xI1nvKygp7COM YA7Q== X-Gm-Message-State: AEkooutTSQxauPEYHK++leTTmO30u4mHU2tEJpJSZPXJ2c9n2LgNviOfahK77eUmx3IR7kt736FpKTU3vzSE9Q== X-Received: by 10.28.27.211 with SMTP id b202mr28146442wmb.12.1471441484866; Wed, 17 Aug 2016 06:44:44 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.137.202 with HTTP; Wed, 17 Aug 2016 06:44:44 -0700 (PDT) In-Reply-To: <98dd7d66-d220-a7f0-46c4-4e5ca0af9d28@linaro.org> References: <57886949.8010300@linaro.org> <57886A71.6030103@linaro.org> <57888BD6.6070200@linaro.org> <578891C8.7090609@linaro.org> <19ff8188-aed7-0f9e-cc0b-0603698787ff@linaro.org> <48e42d0c-057c-312a-4e41-cd78c8b38b5e@linaro.org> <3a581e5d-921a-18ef-5c3d-e1d1158ec40c@linaro.org> <98dd7d66-d220-a7f0-46c4-4e5ca0af9d28@linaro.org> From: Richard Biener Date: Wed, 17 Aug 2016 13:44:00 -0000 Message-ID: Subject: Re: [RFC][IPA-VRP] splits out the update_value_range calls from vrp_visit_stmt To: kugan Cc: Andrew Pinski , "gcc-patches@gcc.gnu.org" , Jan Hubicka , Martin Jambor Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-08/txt/msg01267.txt.bz2 On Wed, Aug 17, 2016 at 4:27 AM, kugan wrote: > Hi, > > > On 16/08/16 20:58, Richard Biener wrote: >> >> On Tue, Aug 16, 2016 at 9:39 AM, kugan >> wrote: >>> >>> Hi, >>> >>>> as said the refactoring that would be appreciated is to split out the >>>> update_value_range calls >>>> from the worker functions so you can call the respective functions >>>> from the DOM implementations. >>>> That they are globbed in vrp_visit_stmt currently is due to the API of >>>> the SSA propagator. >>> >>> >>> >>> Here is a patch that just splits out the update_value_range calls >>> visit_stmts. Bootstrapped and regression tested on x86_64-linux with no >>> new >>> regressions. >>> >>> I also verified few random fdump-tree-vrp1-details from stage2 to make >>> sure >>> they are same. >>> >>> Is this OK for trunk? >> >> >> For vrp_visit_assignment_or_call please defer the question whether the >> update >> is interesting (for the internal call stuff) to the caller and always >> return new_vr. >> >> Also do not perform the "not handled stmt" handling here but make the >> return >> value reflect whether we handled the stmt or not and put >> >> /* Every other statement produces no useful ranges. */ >> FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF) >> set_value_range_to_varying (get_value_range (def)); >> >> into the caller (as that's also a lattice-updating thing). >> >> +static enum ssa_prop_result >> +vrp_visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p) >> +{ >> + value_range vr = VR_INITIALIZER; >> + tree lhs = gimple_get_lhs (stmt); >> + bool vr_found = vrp_visit_stmt_worker (stmt, taken_edge_p, >> + output_p, &vr); >> + >> + if (lhs) >> + { >> + if (vr_found >> + && update_value_range (lhs, &vr)) >> + { >> + *output_p = lhs; >> >> I think rather than computing LHS here you should use *output_p. >> >> Otherwise this looks good though I'd rename the _worker variants >> to extract_range_from_phi_node, extract_range_from_stmt and >> extract_range_from_assignment_or_call. >> > > Please find the patch attached which addresses the comments above. Bootstrap > and regression testing is ongoing. Is this of if there is no new > regressions? Yes, this looks good to me. Thanks, Richard. > Thanks, > Kugan > > > gcc/ChangeLog: > > 2016-08-17 Kugan Vivekanandarajah > > * tree-vrp.c (vrp_visit_assignment_or_call): Changed to Return VR. > (vrp_visit_cond_stmt): Just sets TAKEN_EDGE_P. > (vrp_visit_switch_stmt): Likewise. > (extract_range_from_stmt): Factored out from vrp_visit_stmt. > (extract_range_from_phi_node): Factored out from vrp_visit_phi_stmt. > (vrp_visit_stmt): Use extract_range_from_stmt. > (vrp_visit_phi_node): Use extract_range_from_phi_node. >