From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122704 invoked by alias); 20 Aug 2015 11:25:03 -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 122689 invoked by uid 89); 20 Aug 2015 11:25:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.2 required=5.0 tests=AWL,BAYES_99,BAYES_999,FREEMAIL_FROM,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-io0-f170.google.com Received: from mail-io0-f170.google.com (HELO mail-io0-f170.google.com) (209.85.223.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 20 Aug 2015 11:25:01 +0000 Received: by iodt126 with SMTP id t126so42156145iod.2 for ; Thu, 20 Aug 2015 04:24:59 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.107.8.36 with SMTP id 36mr2325955ioi.172.1440069899536; Thu, 20 Aug 2015 04:24:59 -0700 (PDT) Received: by 10.107.32.140 with HTTP; Thu, 20 Aug 2015 04:24:59 -0700 (PDT) In-Reply-To: References: Date: Thu, 20 Aug 2015 11:25:00 -0000 Message-ID: Subject: Re: [PATCH] Only accept BUILT_IN_NORMAL stringops for interesting_stringop_to_profile_p From: Richard Biener To: "Yangfei (Felix)" Cc: "gcc-patches@gcc.gnu.org" , Jiangjiji Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg01197.txt.bz2 On Thu, Aug 20, 2015 at 11:31 AM, Yangfei (Felix) wrote: > Thanks for the comments. Attached please find the updated patch. OK? Ok. Thanks, Richard. > > Index: gcc/value-prof.c > =================================================================== > --- gcc/value-prof.c (revision 141081) > +++ gcc/value-prof.c (working copy) > @@ -209,7 +209,6 @@ gimple_add_histogram_value (struct function *fun, > hist->fun = fun; > } > > - > /* Remove histogram HIST from STMT's histogram list. */ > > void > @@ -234,7 +233,6 @@ gimple_remove_histogram_value (struct function *fu > free (hist); > } > > - > /* Lookup histogram of type TYPE in the STMT. */ > > histogram_value > @@ -389,6 +387,7 @@ stream_out_histogram_value (struct output_block *o > if (hist->hvalue.next) > stream_out_histogram_value (ob, hist->hvalue.next); > } > + > /* Dump information about HIST to DUMP_FILE. */ > > void > @@ -488,7 +487,6 @@ gimple_duplicate_stmt_histograms (struct function > } > } > > - > /* Move all histograms associated with OSTMT to STMT. */ > > void > @@ -529,7 +527,6 @@ visit_hist (void **slot, void *data) > return 1; > } > > - > /* Verify sanity of the histograms. */ > > DEBUG_FUNCTION void > @@ -594,7 +591,6 @@ free_histograms (void) > } > } > > - > /* The overall number of invocations of the counter should match > execution count of basic block. Report it as error rather than > internal error as it might mean that user has misused the profile > @@ -638,7 +634,6 @@ check_counter (gimple stmt, const char * name, > return false; > } > > - > /* GIMPLE based transformations. */ > > bool > @@ -697,7 +692,6 @@ gimple_value_profile_transformations (void) > return changed; > } > > - > /* Generate code for transformation 1 (with parent gimple assignment > STMT and probability of taking the optimal path PROB, which is > equivalent to COUNT/ALL within roundoff error). This generates the > @@ -859,6 +853,7 @@ gimple_divmod_fixed_value_transform (gimple_stmt_i > probability of taking the optimal path PROB, which is equivalent to COUNT/ALL > within roundoff error). This generates the result into a temp and returns > the temp; it does not replace or alter the original STMT. */ > + > static tree > gimple_mod_pow2 (gimple stmt, int prob, gcov_type count, gcov_type all) > { > @@ -938,6 +933,7 @@ gimple_mod_pow2 (gimple stmt, int prob, gcov_type > } > > /* Do transform 2) on INSN if applicable. */ > + > static bool > gimple_mod_pow2_value_transform (gimple_stmt_iterator *si) > { > @@ -1540,15 +1536,15 @@ gimple_ic_transform (gimple_stmt_iterator *gsi) > return true; > } > > -/* Return true if the stringop CALL with FNDECL shall be profiled. > - SIZE_ARG be set to the argument index for the size of the string > - operation. > -*/ > +/* Return true if the stringop CALL shall be profiled. SIZE_ARG be > + set to the argument index for the size of the string operation. */ > + > static bool > -interesting_stringop_to_profile_p (tree fndecl, gimple call, int *size_arg) > +interesting_stringop_to_profile_p (gimple call, int *size_arg) > { > - enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl); > + enum built_in_function fcode; > > + fcode = DECL_FUNCTION_CODE (gimple_call_fndecl (call)); > if (fcode != BUILT_IN_MEMCPY && fcode != BUILT_IN_MEMPCPY > && fcode != BUILT_IN_MEMSET && fcode != BUILT_IN_BZERO) > return false; > @@ -1573,7 +1569,7 @@ static bool > } > } > > -/* Convert stringop (..., vcall_size) > +/* Convert stringop (..., vcall_size) > into > if (vcall_size == icall_size) > stringop (..., icall_size); > @@ -1590,11 +1586,9 @@ gimple_stringop_fixed_value (gimple vcall_stmt, tr > basic_block cond_bb, icall_bb, vcall_bb, join_bb; > edge e_ci, e_cv, e_iv, e_ij, e_vj; > gimple_stmt_iterator gsi; > - tree fndecl; > int size_arg; > > - fndecl = gimple_call_fndecl (vcall_stmt); > - if (!interesting_stringop_to_profile_p (fndecl, vcall_stmt, &size_arg)) > + if (!interesting_stringop_to_profile_p (vcall_stmt, &size_arg)) > gcc_unreachable (); > > cond_bb = gimple_bb (vcall_stmt); > @@ -1673,11 +1667,11 @@ gimple_stringop_fixed_value (gimple vcall_stmt, tr > > /* Find values inside STMT for that we want to measure histograms for > division/modulo optimization. */ > + > static bool > gimple_stringops_transform (gimple_stmt_iterator *gsi) > { > gimple stmt = gsi_stmt (*gsi); > - tree fndecl; > tree blck_size; > enum built_in_function fcode; > histogram_value histogram; > @@ -1688,14 +1682,11 @@ gimple_stringops_transform (gimple_stmt_iterator * > tree tree_val; > int size_arg; > > - if (gimple_code (stmt) != GIMPLE_CALL) > + if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) > return false; > - fndecl = gimple_call_fndecl (stmt); > - if (!fndecl) > + > + if (!interesting_stringop_to_profile_p (stmt, &size_arg)) > return false; > - fcode = DECL_FUNCTION_CODE (fndecl); > - if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg)) > - return false; > > blck_size = gimple_call_arg (stmt, size_arg); > if (TREE_CODE (blck_size) == INTEGER_CST) > @@ -1704,10 +1695,12 @@ gimple_stringops_transform (gimple_stmt_iterator * > histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_SINGLE_VALUE); > if (!histogram) > return false; > + > val = histogram->hvalue.counters[0]; > count = histogram->hvalue.counters[1]; > all = histogram->hvalue.counters[2]; > gimple_remove_histogram_value (cfun, stmt, histogram); > + > /* We require that count is at least half of all; this means > that for the transformation to fire the value must be constant > at least 80% of time. */ > @@ -1719,8 +1712,10 @@ gimple_stringops_transform (gimple_stmt_iterator * > prob = GCOV_COMPUTE_SCALE (count, all); > else > prob = 0; > + > dest = gimple_call_arg (stmt, 0); > dest_align = get_pointer_alignment (dest); > + fcode = DECL_FUNCTION_CODE (gimple_call_fndecl (stmt)); > switch (fcode) > { > case BUILT_IN_MEMCPY: > @@ -1811,6 +1806,7 @@ stringop_block_profile (gimple stmt, unsigned int > > > /* Find values inside STMT for that we want to measure histograms for > division/modulo optimization. */ > + > static void > gimple_divmod_values_to_profile (gimple stmt, histogram_values *values) > { > @@ -1891,21 +1887,18 @@ gimple_indirect_call_to_profile (gimple stmt, hist > > /* Find values inside STMT for that we want to measure histograms for > string operations. */ > + > static void > gimple_stringops_values_to_profile (gimple stmt, histogram_values *values) > { > - tree fndecl; > - tree blck_size; > tree dest; > + tree blck_size; > int size_arg; > > - if (gimple_code (stmt) != GIMPLE_CALL) > + if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) > return; > - fndecl = gimple_call_fndecl (stmt); > - if (!fndecl) > - return; > > - if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg)) > + if (!interesting_stringop_to_profile_p (stmt, &size_arg)) > return; > > dest = gimple_call_arg (stmt, 0); > @@ -1919,6 +1912,7 @@ gimple_stringops_values_to_profile (gimple stmt, h > values->safe_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_AVERAGE, > stmt, blck_size)); > } > + > if (TREE_CODE (blck_size) != INTEGER_CST) > values->safe_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_IOR, > stmt, dest)); > Index: gcc/ChangeLog > =================================================================== > --- gcc/ChangeLog (revision 141081) > +++ gcc/ChangeLog (working copy) > @@ -1,3 +1,12 @@ > +2014-08-20 Felix Yang > + Jiji Jiang > + > + * value-prof.c (interesting_stringop_to_profile_p): Removed FNDECL argument > + and get builtin function code directly from CALL. > + (gimple_stringop_fixed_value): Modified accordingly. > + (gimple_stringops_transform, gimple_stringops_values_to_profile): Modified > + accordingly and only accept BUILT_IN_NORMAL string operations. > + > 2015-08-18 Segher Boessenkool > > Backport from mainline: > > > >> >> On Thu, Aug 20, 2015 at 5:17 AM, Yangfei (Felix) >> wrote: >> > Hi, >> > >> > As DECL_FUNCTION_CODE is overloaded for builtin functions in different >> classes, so need to check builtin class before using fcode. >> > Patch posted below. Bootstrapped on x86_64-suse-linux, OK for trunk? >> > Thanks. >> >> Ugh. The code in the callers already looks like it could have some TLC, like >> instead of >> >> fndecl = gimple_call_fndecl (stmt); >> if (!fndecl) >> return false; >> fcode = DECL_FUNCTION_CODE (fndecl); >> if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg)) >> return false; >> >> simply do >> >> if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) >> return false; >> if (!interesting_stringop_to_profile_p (gimple_call_fndecl (stmt), ....)) >> >> similar for the other caller. interesting_stringop_to_profile_p can also get >> function-code directly from stmt, removing the redundant first argument or even >> do the gimple_call_builtin_p call itself. >> >> Mind reworking the patch accordingly? >> >> Thanks, >> Richard. >