From patchwork Mon Jun 24 16:44:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dehao Chen X-Patchwork-Id: 253909 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id B46672C0079 for ; Tue, 25 Jun 2013 02:44:59 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=dKQZnCxbz5bbt02FXT KMjmH0BgrtaVOlDFkOUHlEuUs1IxLOudOQBJg+ESkxgil1yiTkt32C2V9mdBYpZa /lpXQLlmYRtvVeESQl/NDbSmgShR4zQx2nkp4pmSsRQCTx17lJjOzJxpvpcsGsjP 2b14R/oLaXdBY7djFwo3Bkw9o= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=2p7cU/gR5elptF6MRFuFhOAk G2M=; b=t1F6R86ZN0n4oSOgDlAFE+4RBzXFN+1KjfTIJC9C0AAyrusbBWYyBi3V FgZ2YDXSKBo6qXX2GgRWiBPgBZuYo3RGRYAWRZQmTBmo0a1YgPgRfUYFpULHXAJM qOvOoxORR7oMcmuUBwpaJhVcY5Y6QEvC5eyXyBURjcpeOyWWv7k= Received: (qmail 11428 invoked by alias); 24 Jun 2013 16:44:53 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 11419 invoked by uid 89); 24 Jun 2013 16:44:52 -0000 X-Spam-SWARE-Status: No, score=-4.6 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.1 Received: from mail-ea0-f180.google.com (HELO mail-ea0-f180.google.com) (209.85.215.180) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 24 Jun 2013 16:44:51 +0000 Received: by mail-ea0-f180.google.com with SMTP id k10so6240670eaj.25 for ; Mon, 24 Jun 2013 09:44:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-gm-message-state; bh=7b2a7664oiGRaL52tsZlpvvLjjUHzpoj4F/GyS2J5DI=; b=Crqv3iIjRjCci1xYzsAJO4k66xNb4dhHJqOlZNWJhYlN0cMWCm29+tTQgMScXul1Tx gnYJWa0RhTwWqk2/scEsi4qE7QxEdFmgf998f0A7rKbeAwfYLMCzjreh1hEqYaFHOEwA EgVXoM7XyIicDYWyYRT9jdy6+yt9AIOcHlXyemrrOL6GtigXj7cIa678isus2a74pWgV xTfjNTEmk8eaJxs313tPF5oCq25zILW9JRcaY3BFD7TcRVTO6PgCR52aIYEPhJ4YsMDQ t/9V9/pb7zE/N6WwfmaKyT0KstAskyfNXx2qxagZO7lLZciZaBL+mrbO4cttlJq5wie2 kQxQ== MIME-Version: 1.0 X-Received: by 10.15.23.73 with SMTP id g49mr25892295eeu.8.1372092289094; Mon, 24 Jun 2013 09:44:49 -0700 (PDT) Received: by 10.14.210.69 with HTTP; Mon, 24 Jun 2013 09:44:48 -0700 (PDT) In-Reply-To: References: Date: Mon, 24 Jun 2013 09:44:48 -0700 Message-ID: Subject: Re: [PATCH] Change the badness computation to ensure no integer-underflow From: Dehao Chen To: Richard Biener Cc: GCC Patches X-Gm-Message-State: ALoCoQn1wcgcrypRUzYjlJ9WF21wdFTVV8DUZWWcBll7VkhwUVXk1KXJkcj19ktT8Ak18KEacbqf2U45ftgkqc91liW3St2ol42obCmfN0euMaTV4ZUG3ZTl119EL6gxcO/MvlAUuUW21r/wPuNxnHSVoGp9FsJ5LGkbGhMMKzjJISWgAooxYQyIvK1BR/2evu5xQPaC9PY6koDmL0rKf1i34kQmtRq/8g== X-Virus-Found: No Hi, Richard, I've updated the patch (as attached) to use sreal to compute badness. Thanks, Dehao On Mon, Jun 24, 2013 at 5:41 AM, Richard Biener wrote: > On Sat, Jun 22, 2013 at 2:59 AM, Dehao Chen wrote: >> This patch prevents integer-underflow of badness computation in ipa-inline. >> >> Bootstrapped and passed regression tests. >> >> OK for trunk? >> >> Thanks, >> Dehao >> >> gcc/ChangeLog: >> 2013-06-21 Dehao Chen >> >> * ipa-inline.c (edge_badness): Fix integer underflow. >> >> Index: gcc/ipa-inline.c >> =================================================================== >> --- gcc/ipa-inline.c (revision 200326) >> +++ gcc/ipa-inline.c (working copy) >> @@ -888,11 +888,9 @@ edge_badness (struct cgraph_edge *edge, bool dump) >> else if (max_count) >> { >> int relbenefit = relative_time_benefit (callee_info, edge, edge_time); >> - badness = >> - ((int) >> - ((double) edge->count * INT_MIN / 2 / max_count / >> RELATIVE_TIME_BENEFIT_RANGE) * >> - relbenefit) / growth; >> - >> + badness = ((int)((double) edge->count / max_count >> + * relbenefit / RELATIVE_TIME_BENEFIT_RANGE * INT_MIN / 2)) / growth; >> + > > FP operations on the host are frowned upon if code generation depends > on their outcome. They all should use sreal_* as predict already does. > Other than that I wonder why the final division is int (so we get truncation > and not rounding)? That increases the effect of doing the multiply by > relbenefit in double. > > Richard. > >> /* Be sure that insanity of the profile won't lead to increasing counts >> in the scalling and thus to overflow in the computation above. */ >> gcc_assert (max_count >= edge->count); Index: gcc/ipa-inline.c =================================================================== --- gcc/ipa-inline.c (revision 200326) +++ gcc/ipa-inline.c (working copy) @@ -113,10 +113,12 @@ along with GCC; see the file COPYING3. If not see #include "target.h" #include "ipa-inline.h" #include "ipa-utils.h" +#include "sreal.h" /* Statistics we collect about inlining algorithm. */ static int overall_size; static gcov_type max_count; +static sreal max_count_real, max_relbenefit_real, half_int_min_real; /* Return false when inlining edge E would lead to violating limits on function unit growth or stack usage growth. @@ -887,12 +889,26 @@ edge_badness (struct cgraph_edge *edge, bool dump) else if (max_count) { + sreal tmp, relbenefit_real, growth_real; int relbenefit = relative_time_benefit (callee_info, edge, edge_time); - badness = - ((int) - ((double) edge->count * INT_MIN / 2 / max_count / RELATIVE_TIME_BENEFIT_RANGE) * - relbenefit) / growth; - + + sreal_init(&relbenefit_real, relbenefit, 0); + sreal_init(&growth_real, growth, 0); + + /* relative_edge_count. */ + sreal_init (&tmp, edge->count, 0); + sreal_div (&tmp, &tmp, &max_count_real); + + /* relative_time_benefit. */ + sreal_mul (&tmp, &tmp, &relbenefit_real); + sreal_div (&tmp, &tmp, &max_relbenefit_real); + + /* growth_f_caller. */ + sreal_mul (&tmp, &tmp, &half_int_min_real); + sreal_div (&tmp, &tmp, &growth_real); + + badness = sreal_to_int (&tmp); + /* Be sure that insanity of the profile won't lead to increasing counts in the scalling and thus to overflow in the computation above. */ gcc_assert (max_count >= edge->count); @@ -1448,6 +1464,9 @@ inline_small_functions (void) if (max_count < edge->count) max_count = edge->count; } + sreal_init (&max_count_real, max_count, 0); + sreal_init (&max_relbenefit_real, RELATIVE_TIME_BENEFIT_RANGE, 0); + sreal_init (&half_int_min_real, INT_MIN / 2, 0); ipa_free_postorder_info (); initialize_growth_caches (); Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 200326) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2013-06-21 Dehao Chen + + * ipa-inline.c (edge_badness): Fix integer underflow. + 2013-06-21 Andi Kleen * doc/extend.texi: Dont use __atomic_clear in HLE Index: gcc/Makefile.in =================================================================== --- gcc/Makefile.in (revision 200326) +++ gcc/Makefile.in (working copy) @@ -2947,7 +2947,7 @@ ipa-inline.o : ipa-inline.c $(CONFIG_H) $(SYSTEM_H $(DIAGNOSTIC_H) $(FIBHEAP_H) $(PARAMS_H) $(TREE_PASS_H) \ $(COVERAGE_H) $(GGC_H) $(TREE_FLOW_H) $(RTL_H) $(IPA_PROP_H) \ $(EXCEPT_H) $(GIMPLE_PRETTY_PRINT_H) $(IPA_INLINE_H) $(TARGET_H) \ - $(IPA_UTILS_H) + $(IPA_UTILS_H) sreal.h ipa-inline-analysis.o : ipa-inline-analysis.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ $(TREE_H) langhooks.h $(TREE_INLINE_H) $(FLAGS_H) $(CGRAPH_H) intl.h \ $(DIAGNOSTIC_H) $(PARAMS_H) $(TREE_PASS_H) $(CFGLOOP_H) \