From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5826 invoked by alias); 17 Aug 2012 19:59:13 -0000 Received: (qmail 5817 invoked by uid 22791); 17 Aug 2012 19:59:12 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_NONE,RCVD_IN_HOSTKARMA_NO,RP_MATCHES_RCVD,TW_ZJ,UNPARSEABLE_RELAY X-Spam-Check-By: sourceware.org Received: from mailout03.t-online.de (HELO mailout03.t-online.de) (194.25.134.81) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 17 Aug 2012 19:58:56 +0000 Received: from fwd00.aul.t-online.de (fwd00.aul.t-online.de ) by mailout03.t-online.de with smtp id 1T2Sh4-0002gd-Re; Fri, 17 Aug 2012 21:58:54 +0200 Received: from [192.168.0.100] (XVNXorZAZhQ6PSuS19bBhcBCSYlTO6DX0C+fo7l2f3HWJxnFCqAQiDVFdNcNKTJwOn@[93.218.185.166]) by fwd00.t-online.de with esmtp id 1T2Sh0-1ThXCy0; Fri, 17 Aug 2012 21:58:50 +0200 Message-ID: <1345233529.2144.24.camel@yam-132-YW-E178-FTW> Subject: Re: [PATCH, rtl-optimization]: Fix PR46829, ICE in spill_failure with -fschedule-insns [was: Fixing instability of -fschedule-insns for x86] From: Oleg Endo To: Uros Bizjak Cc: gcc-patches@gcc.gnu.org, Jakub Jelinek , Richard Guenther , Yuri Rumyantsev , Igor Zamyatin Date: Fri, 17 Aug 2012 19:59:00 -0000 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Mime-Version: 1.0 X-IsSubscribed: yes 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 X-SW-Source: 2012-08/txt/msg01210.txt.bz2 Hi, On Fri, 2012-08-17 at 20:34 +0200, Uros Bizjak wrote: > On Fri, Aug 17, 2012 at 6:36 PM, Uros Bizjak wrote: > > > 2012-08-17 Uros Bizjak > > > > PR rtl-optimization/46829 > > * combine.c (recog_for_combine): Check operand constraints > > in case hard registers were propagater into insn pattern. > > > > testsuite/ChangeLog: > > > > 2012-08-17 Uros Bizjak > > > > PR rtl-optimization/46829 > > * gcc.target/i386/pr46829.c: New test. > > > > Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}. > > Oh ... > > This part: > > + && rtx_equal_p (recog_data.operand[j], > + recog_data.operand[op_alt[j].matches])) > > should read: > > + && rtx_equal_p (recog_data.operand[i], > + recog_data.operand[op_alt[j].matches])) > > Note the "j" vs "i" array index difference in the first line. > > Correct patch attached, bootstrapped and re-tested on > x86_64-pc-linux-gnu {,-m32}. > I've briefly checked your patch against rev 190459 for the SH target. The CSiBE result-size for '-m4 -ml -O2 -mpretend-cmove' shows quite some movements and improvements. However, the additional restrictions you've added effectively disable some of the combine patterns I've been adding recently to improve code quality on SH and I see some SH specific failures (haven't run the full test suite though): FAIL: gcc.target/sh/pr49263.c scan-assembler-not and FAIL: gcc.target/sh/pr51244-1.c scan-assembler-not movt|tst|negc|extu FAIL: gcc.target/sh/pr51244-10.c scan-assembler-not shll|subc|and FAIL: gcc.target/sh/pr51244-7.c scan-assembler-not cmp/hi FAIL: gcc.target/sh/pr51244-7.c scan-assembler-not mov\t#0 FAIL: gcc.target/sh/pr51244-8.c scan-assembler-not shad|neg FAIL: gcc.target/sh/pr51244-9.c scan-assembler-not mov\t#0 FAIL: gcc.target/sh/pr52933-1.c scan-assembler-times div0s 25 FAIL: gcc.target/sh/pr52933-2.c scan-assembler-times div0s 25 FAIL: gcc.target/sh/pr54236-1.c scan-assembler-times addc 3 FAIL: gcc.target/sh/pr54236-1.c scan-assembler-times subc 3 FAIL: gcc.target/sh/pr54236-1.c scan-assembler-times sett 4 FAIL: gcc.target/sh/pr54236-1.c scan-assembler-not movt I've not checked every single case, but the first two seem to be the core issues. 1) FAIL: gcc.target/sh/pr49263.c scan-assembler-not and Test function: int test (uint8_t x) { return x & 15 ? -40 : -9; } Pattern in sh.md that would originally match: (define_insn "tstsi_t_zero_extract_eq" [(set (reg:SI T_REG) (eq:SI (zero_extract:SI (match_operand 0 "logical_operand" "z") (match_operand:SI 1 "const_int_operand") (match_operand:SI 2 "const_int_operand")) (const_int 0)))] ...) ("z" is a constraint for the r0 register on SH) Combine now says: Trying 11 -> 12: Successfully matched this instruction: (set (reg:SI 147 t) (eq:SI (zero_extract:SI (reg:SI 4 r4 [ x ]) (const_int 4 [0x4]) (const_int 0 [0])) (const_int 0 [0]))) Operand failed to match constraints: (reg:SI 4 r4 [ x ]) The problem in this case is that on SH (and other targets, too) function args are passed in regs. In this case it's the hard reg r4, which is sort of pre-allocated already. Before your patch, the pattern would match and reload would stuff in a 'mov r4,r0' to satisfy the "z" constraint. 2) FAIL: gcc.target/sh/pr51244-1.c scan-assembler-not movt|tst|negc|extu Test function: int testfunc_01 (int a, int b, int c, int d) { return (a == b || a == d) ? b : c; } Pattern in sh.md that would originally match: (define_insn_and_split "nott" [(set (reg:SI T_REG) (xor:SI (match_operand:SI 0 "t_reg_operand" "") (const_int 1)))] "TARGET_SH1" ...) Combine now says: Trying 14 -> 15: Successfully matched this instruction: (set (reg:SI 147 t) (xor:SI (reg:SI 147 t) (const_int 1 [0x1]))) Operand failed to match constraints: (reg:SI 147 t) In this case the T_REG matching is already in the predicate (similar to the thing that's being done on x86 you've mentioned before). I've started using this kind of matching pattern a while ago to get simpler and better matching combine patterns around SH's T_REG. (T_REG is a fixed 1 bit hard reg on SH, treated as SImode throughout the MD. It is used to hold comparison results, carry/borrow bits etc). I'm not sure what would be the possible solutions for those cases. For 1) maybe pre-allocated regs of args should be moved to pseudos before combine? For 2) I could probably try to come up with some matching that would satisfy the new restrictions in combine and fix up the affected patterns throughout the MD. In the worst case, would it be an option to make this restriction in combine optional (target hook or something)? Cheers, Oleg