From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id D17D03857C50 for ; Thu, 5 May 2022 13:49:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D17D03857C50 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id E8A11282645; Thu, 5 May 2022 15:49:19 +0200 (CEST) Date: Thu, 5 May 2022 15:49:19 +0200 From: Jan Hubicka To: Martin =?iso-8859-2?Q?Li=B9ka?= Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Add operators / and * for profile_{count,probability}. Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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: Thu, 05 May 2022 13:49:26 -0000 Hi, > The patch simplifies usage of the profile_{count,probability} types. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? The reason I intentionally did not add * and / to the original API was to detect situations where values that should be profile_count/profile_probability are stored into integers, since previous code used integers for everything. Having one to add apply_scale made him/her (mostly me :) to think if the value is really just a fixed scale or it it should be better converted to proper data type (count or probability). I guess now we completed the conversion so risk of this creeping in is relatively low and the code indeed looks better. It will make it bit harder for me to backport jump threading profile updating fixes I plan for 12.2 but it should not be hard. > diff --git a/gcc/cfgloopmanip.cc b/gcc/cfgloopmanip.cc > index b4357c03e86..a1ac1146445 100644 > --- a/gcc/cfgloopmanip.cc > +++ b/gcc/cfgloopmanip.cc > @@ -563,8 +563,7 @@ scale_loop_profile (class loop *loop, profile_probability p, > > /* Probability of exit must be 1/iterations. */ > count_delta = e->count (); > - e->probability = profile_probability::always () > - .apply_scale (1, iteration_bound); > + e->probability = profile_probability::always () / iteration_bound; However this is kind of example of the problem. iteration_bound is gcov_type so we can get overflow here. I guess we want to downgrade iteration_bound since it is always either 0 or int. > diff --git a/gcc/tree-switch-conversion.cc b/gcc/tree-switch-conversion.cc > index e14b4e6c94a..cef26a9878e 100644 > --- a/gcc/tree-switch-conversion.cc > +++ b/gcc/tree-switch-conversion.cc > @@ -1782,7 +1782,7 @@ switch_decision_tree::analyze_switch_statement () > tree high = CASE_HIGH (elt); > > profile_probability p > - = case_edge->probability.apply_scale (1, (intptr_t) (case_edge->aux)); > + = case_edge->probability / ((intptr_t) (case_edge->aux)); I think the switch ranges may be also in risk of overflow? We could make operators to accept gcov_type or int64_t. Thanks, Honza