From patchwork Mon Feb 28 15:40:21 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chung-Lin Tang X-Patchwork-Id: 84845 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 62EA0B7063 for ; Tue, 1 Mar 2011 02:40:12 +1100 (EST) Received: (qmail 23489 invoked by alias); 28 Feb 2011 15:40:03 -0000 Received: (qmail 23346 invoked by uid 22791); 28 Feb 2011 15:40:01 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, TW_SV, 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; Mon, 28 Feb 2011 15:39:55 +0000 Received: (qmail 26220 invoked from network); 28 Feb 2011 15:39:53 -0000 Received: from unknown (HELO ?192.168.1.16?) (cltang@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Feb 2011 15:39:53 -0000 Message-ID: <4D6BC1E5.50401@codesourcery.com> Date: Mon, 28 Feb 2011 23:40:21 +0800 From: Chung-Lin Tang User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.13) Gecko/20101207 Lightning/1.0b2 Thunderbird/3.1.7 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org Subject: Re: [patch, ARM] Fix minipool ICE References: <4D4F8CA4.7020601@codesourcery.com> <1298905648.13849.27.camel@e102346-lin.cambridge.arm.com> In-Reply-To: <1298905648.13849.27.camel@e102346-lin.cambridge.arm.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 2011/2/28 11:07 PM, Richard Earnshaw wrote: > > On Mon, 2011-02-07 at 14:09 +0800, Chung-Lin Tang wrote: >> Hi, >> the *arm_zero_extendhisi2[_v6] patterns currently do not have the >> constant pool range attributes specified, causing a minipool ICE case. >> This patch adds the needed pool_range/neg_pool_range settings. >> >> Reported originally from https://bugs.launchpad.net/bugs/711819 , also >> happens to occur when building ffmpeg svn trunk. >> >> Ok for trunk? >> >> Thanks, >> Chung-Lin >> >> 2011-02-07 Chung-Lin Tang >> >> * config/arm/arm.md (*arm_zero_extendhisi2): Set pool_range, >> neg_pool_range attributes for ldrh alternative. >> (*arm_zero_extendhisi2_v6): Same. > > This is wrong. Neither the predicate nor the constraint accept an > immediate, so these should never need to generate mini-pool entries. If > reload is letting these through, then that's a bug in reload IMO. If > it's not reload, then that needs investigating further too. Hi Richard, To re-cap some of the discussion Bernd, Ramana, and I had off-list: we are seeing in some cases, IRA/reload ending up with insns with constant pool references (not generated from ARM reorg): (insn 262 92 94 3 (set (reg:HI 4 r4 [orig:328 iftmp.6 ] [328]) (mem/u/c/i:HI (symbol_ref/u:SI ("*.LC0") [flags 0x2]) [2 S2 A16])) k.c:11 177 {*movhi_insn_arch4} (expr_list:REG_EQUAL (const_int 2047 [0x7ff]) (nil))) It looks like IRA spilled a register which is a constant. Probably because 2047 is not a valid ARM movable constant, it gets turned into a constant pool reference. After postreload, the above insn gets turned into: (insn 262 92 94 3 (set (reg:SI 4 r4) (zero_extend:SI (mem/u/c/i:HI (symbol_ref/u:SI ("*.LC0") [flags 0x2]) [2 S2 A16]))) k.c:11 148 {*arm_zero_extendhisi2_v6} (expr_list:REG_EQUAL (const_int 2047 [0x7ff]) (nil))) Thus arm.c:note_invalid_constants() kicks in, then ICEing later I haven't yet investigated where exactly postreload produces that zero_extend (maybe someone here can be quick to point out where), but if it's correct behavior, then I guess the pool range attributes have to be there... Ramana also mentioned about using MOVW for doing HImode constant moves, which currently do not seem to be supported under ARM mode (only Thumb-2). This could be added, but still the pre-ARMv6T2 case needs solving. Here's the patch with a testcase added. Thanks, Chung-Lin Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 170534) +++ gcc/config/arm/arm.md (working copy) @@ -4202,7 +4202,9 @@ # ldr%(h%)\\t%0, %1" [(set_attr "type" "alu_shift,load_byte") - (set_attr "predicable" "yes")] + (set_attr "predicable" "yes") + (set_attr "pool_range" "*,256") + (set_attr "neg_pool_range" "*,244")] ) (define_insn "*arm_zero_extendhisi2_v6" @@ -4213,7 +4215,9 @@ uxth%?\\t%0, %1 ldr%(h%)\\t%0, %1" [(set_attr "type" "alu_shift,load_byte") - (set_attr "predicable" "yes")] + (set_attr "predicable" "yes") + (set_attr "pool_range" "*,256") + (set_attr "neg_pool_range" "*,244")] ) (define_insn "*arm_zero_extendhisi2addsi" Index: gcc/testsuite/gcc.target/arm/pr47719.c =================================================================== --- gcc/testsuite/gcc.target/arm/pr47719.c (revision 0) +++ gcc/testsuite/gcc.target/arm/pr47719.c (revision 0) @@ -0,0 +1,45 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -march=armv5te -marm" } */ + +static inline short baz (int a) +{ + int x; + __asm__ ("mov %0, %1" : "=r"(x) : "r"(a)); + return x; +} + +static void bar (short *out, unsigned char *in) +{ + int sc = ((unsigned short)*in << 8); + int i, s0, s1, d; + in += 2; + s1 = *in; + for(i=0;i<16;i++) + { + d = in[i]; + d = ((signed char)d >> 4); + s0 = (0x4000*d*sc + 0x7298*s1 - 0x3350*s1)>>14; + s1 = baz (s0); + *out++=s1; + d = in[i]; + d = ((signed char)(d<<4) >> 4); + s0 = (0x4000*d*sc + 0x7298*s1 - 0x3350*s1)>>14; + s1 = baz (s0); + *out++=s1; + } +} + +void foo (int c, void *data, int size) +{ + int i; + short *samples = data, tmp[64]; + while(size-- >= 32) + { + bar (tmp, data); + for (i = 0; i < 32; i++) + { + samples[i*2] = tmp[i]; + samples[i*2+1] = tmp[i+32]; + } + } +}