From patchwork Sat Oct 23 00:44:36 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jie Zhang X-Patchwork-Id: 68985 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]) by ozlabs.org (Postfix) with SMTP id E69BDB6F07 for ; Sat, 23 Oct 2010 11:44:54 +1100 (EST) Received: (qmail 12765 invoked by alias); 23 Oct 2010 00:44:52 -0000 Received: (qmail 12747 invoked by uid 22791); 23 Oct 2010 00:44:51 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 23 Oct 2010 00:44:45 +0000 Received: (qmail 8717 invoked from network); 23 Oct 2010 00:44:42 -0000 Received: from unknown (HELO ?192.168.1.106?) (jie@127.0.0.2) by mail.codesourcery.com with ESMTPA; 23 Oct 2010 00:44:42 -0000 Message-ID: <4CC22FF4.2020502@codesourcery.com> Date: Sat, 23 Oct 2010 08:44:36 +0800 From: Jie Zhang User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.9) Gecko/20100918 Icedove/3.1.4 MIME-Version: 1.0 To: Andrey Belevantsev CC: gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com, "Vladimir N. Makarov" Subject: Re: [mips] Return correct value from TARGET_SCHED_REORDER2 (PR rtl-optimization/37360) References: <4CBF02B4.4010207@codesourcery.com> <87vd4wppaj.fsf@firetop.home> <4CBF89A7.2060206@codesourcery.com> <4CBFF751.2090004@ispras.ru> <4CBFF887.7020100@codesourcery.com> In-Reply-To: <4CBFF887.7020100@codesourcery.com> X-IsSubscribed: yes 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 On 10/21/2010 04:23 PM, Jie Zhang wrote: > On 10/21/2010 04:18 PM, Andrey Belevantsev wrote: >> On 21.10.2010 4:30, Jie Zhang wrote: >>> On 10/21/2010 04:44 AM, Richard Sandiford wrote: >>>> Jie Zhang writes: >>>>> Currently TARGET_SCHED_REORDER2 returns mips_issue_rate (), which will >>>>> reset can_issue_more to the issue_rate after scheduling each >>>>> instruction. See haifa-sched.c:sched_block for the details. Because of >>>>> this, issue_rate is not honored when scheduling. This caused PR >>>>> rtl-optimization/37360 since the issue_rate is set to a value smaller >>>>> than the number of instructions that scheduler DFA can issue in one >>>>> cycle for mips sb1. >>>> >>>> The haifa-sched.c part looks good to me, but I can't approve it. >>>> Thanks for tracking down the underlying problem. >>>> >>> Andrey and scheduler maintainers? >>> >> I'm all for the patch, life is easier for the scheduler when nobody >> lies. I cannot approve it though. As I wrote earlier, we just need to be >> on the lookout for possible fallout on other targets which may have the >> same problem. >> > Thanks. Then let's wait for review from a scheduler maintainer. > I finally got an approval privately from Vladimir for the haifa-sched.c part. And I also found a compile warning in my original patch. The attached updated patch is what I have committed. Thanks! PR rtl-optimization/37360 * config/mips/mips.c (cached_can_issue_more): New local variable. (mips_sched_reorder_1): New. (mips_sched_reorder): Use mips_sched_reorder_1. (mips_sched_reorder2): New. (mips_variable_issue): Set cached_can_issue_more. (TARGET_SCHED_REORDER2): Define to mips_sched_reorder2 instead of mips_sched_reorder. Revert 2008-09-09 Andrey Belevantsev PR rtl-optimization/37360 * haifa-sched.c (max_issue): Do not assert that we never issue more insns than issue_rate. Add comment. testsuite/ PR rtl-optimization/37360 * gcc.dg/pr37360.c: New test. Index: testsuite/gcc.dg/pr37360.c =================================================================== --- testsuite/gcc.dg/pr37360.c (revision 0) +++ testsuite/gcc.dg/pr37360.c (revision 0) @@ -0,0 +1,21 @@ +/* PR rtl-optimization/37360 */ +/* { dg-do compile { target fpic } } */ +/* { dg-options "-O3 -fPIC" } */ + +typedef unsigned int UQItype __attribute__ ((mode (QI))); +typedef unsigned int USItype __attribute__ ((mode (SI))); + +extern const UQItype __popcount_tab[256]; +extern int __popcountsi2 (USItype); + +int +__popcountsi2 (USItype x) +{ + int i, ret = 0; + + for (i = 0; i < (4 * 8); i += 8) + ret += __popcount_tab[(x >> i) & 0xff]; + + return ret; +} + Index: haifa-sched.c =================================================================== --- haifa-sched.c (revision 165879) +++ haifa-sched.c (working copy) @@ -2479,14 +2479,7 @@ max_issue (struct ready_list *ready, int /* Init max_points. */ max_points = 0; more_issue = issue_rate - cycle_issued_insns; - - /* ??? We used to assert here that we never issue more insns than issue_rate. - However, some targets (e.g. MIPS/SB1) claim lower issue rate than can be - achieved to get better performance. Until these targets are fixed to use - scheduler hooks to manipulate insns priority instead, the assert should - be disabled. - - gcc_assert (more_issue >= 0); */ + gcc_assert (more_issue >= 0); for (i = 0; i < n_ready; i++) if (!ready_try [i]) Index: config/mips/mips.c =================================================================== --- config/mips/mips.c (revision 165879) +++ config/mips/mips.c (working copy) @@ -589,6 +589,10 @@ static const char *mips_hi_relocs[NUM_SY /* Target state for MIPS16. */ struct target_globals *mips16_globals; +/* Cached value of can_issue_more. This is cached in mips_variable_issue hook + and returned from mips_sched_reorder2. */ +static int cached_can_issue_more; + /* Index R is the smallest register class that contains register R. */ const enum reg_class mips_regno_to_class[FIRST_PSEUDO_REGISTER] = { LEA_REGS, LEA_REGS, M16_REGS, V1_REG, @@ -12436,11 +12440,11 @@ mips_sched_init (FILE *file ATTRIBUTE_UN mips_ls2.falu1_turn_p = true; } -/* Implement TARGET_SCHED_REORDER and TARGET_SCHED_REORDER2. */ +/* Subroutine used by TARGET_SCHED_REORDER and TARGET_SCHED_REORDER2. */ -static int -mips_sched_reorder (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED, - rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED) +static void +mips_sched_reorder_1 (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED, + rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED) { if (!reload_completed && TUNE_MACC_CHAINS @@ -12455,10 +12459,28 @@ mips_sched_reorder (FILE *file ATTRIBUTE if (TUNE_74K) mips_74k_agen_reorder (ready, *nreadyp); +} +/* Implement TARGET_SCHED_REORDER. */ + +static int +mips_sched_reorder (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED, + rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED) +{ + mips_sched_reorder_1 (file, verbose, ready, nreadyp, cycle); return mips_issue_rate (); } +/* Implement TARGET_SCHED_REORDER2. */ + +static int +mips_sched_reorder2 (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED, + rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED) +{ + mips_sched_reorder_1 (file, verbose, ready, nreadyp, cycle); + return cached_can_issue_more; +} + /* Update round-robin counters for ALU1/2 and FALU1/2. */ static void @@ -12516,6 +12538,7 @@ mips_variable_issue (FILE *file ATTRIBUT || recog_memoized (insn) < 0 || get_attr_type (insn) != TYPE_MULTI); + cached_can_issue_more = more; return more; } @@ -16417,7 +16440,7 @@ mips_shift_truncation_mask (enum machine #undef TARGET_SCHED_REORDER #define TARGET_SCHED_REORDER mips_sched_reorder #undef TARGET_SCHED_REORDER2 -#define TARGET_SCHED_REORDER2 mips_sched_reorder +#define TARGET_SCHED_REORDER2 mips_sched_reorder2 #undef TARGET_SCHED_VARIABLE_ISSUE #define TARGET_SCHED_VARIABLE_ISSUE mips_variable_issue #undef TARGET_SCHED_ADJUST_COST