From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13532 invoked by alias); 3 Jan 2013 18:43:05 -0000 Received: (qmail 13453 invoked by uid 22791); 3 Jan 2013 18:43:05 -0000 X-SWARE-Spam-Status: No, hits=-3.5 required=5.0 tests=AWL,BAYES_00,DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_SPAMHAUS_DROP,NML_ADSP_CUSTOM_MED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-wg0-f45.google.com (HELO mail-wg0-f45.google.com) (74.125.82.45) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 03 Jan 2013 18:42:32 +0000 Received: by mail-wg0-f45.google.com with SMTP id dq12so7419774wgb.24 for ; Thu, 03 Jan 2013 10:42:31 -0800 (PST) X-Received: by 10.180.24.198 with SMTP id w6mr69577721wif.27.1357238551336; Thu, 03 Jan 2013 10:42:31 -0800 (PST) Received: from localhost ([2.26.203.77]) by mx.google.com with ESMTPS id hu8sm68654371wib.6.2013.01.03.10.42.28 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 03 Jan 2013 10:42:29 -0800 (PST) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener ,gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: gcc-patches@gcc.gnu.org Subject: Re: RFA: Fix ICE on PARALLEL returns when expand builtins References: <87pq21umqp.fsf@talisman.default> <87obh77d9r.fsf@talisman.default> Date: Thu, 03 Jan 2013 18:43:00 -0000 In-Reply-To: (Richard Biener's message of "Thu, 3 Jan 2013 10:14:21 +0100") Message-ID: <877gnu6rcc.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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: 2013-01/txt/msg00136.txt.bz2 Richard Biener writes: > On Wed, Jan 2, 2013 at 5:36 PM, Richard Sandiford > wrote: >> Richard Biener writes: >>> On Sun, Dec 23, 2012 at 10:43 AM, Richard Sandiford >>> wrote: >>>> Some of the maths builtins can expand to a call followed by a bit >>>> of postprocessing. With 4.8's PARALLEL return optimisations, these >>>> embedded calls might return a PARALLEL of pseudos, but the postprocessing >>>> isn't prepared to deal with that. This leads to an ICE in builtins-53.c >>>> on n32 and n64 mips64-linux-gnu. >>>> >>>> One fix might have been to pass an explicit register target to the >>>> expand routines, but that target's only a hint. This patch instead >>>> adds an avoid_group_rtx function (named after gen_group_rtx) to convert >>>> PARALLELs to pseudos where necessary. >>>> >>>> I wondered whether it was really safe for expand_builtin_int_roundingfn_2 >>>> to pass "target == const0_rtx" as the "ignore" parameter to expand_call, >>>> given that we don't actually ignore the return value ourselves >>>> (even if the caller does). I suppose it is safe though, >>>> since expand_call will always return const0_rtx in that case, >>>> and this code is dealing with integer return values. >>>> >>>> Tested on mips64-linux-gnu. OK to install? Or is there a better way? >>> >>> You didn't add a testcase so I can't check myself >> >> It's gcc.dg/builtins-53.c. >> >>> - but why isn't using force_reg enough here? I can imagine other >>> cases than PARALLELs that are not well handled, no? >> >> Not sure either way TBH. Fortunately expanding your own calls seems >> to be pretty rare. >> >> But yeah, having force_reg (or I suppose force_operand) do it sounds >> good in principle. The problem is that the operation needs the type >> tree, which the force_* routines don't have. > > Hm? force_reg/operand only need a mode. > > Index: builtins.c > =================================================================== > *** builtins.c (revision 194787) > --- builtins.c (working copy) > *************** expand_builtin_int_roundingfn (tree exp, > *** 2760,2765 **** > --- 2760,2766 ---- > > /* Truncate the result of floating point optab to integer > via expand_fix (). */ > + tmp = force_reg (TYPE_MODE (TREE_TYPE (TREE_TYPE (fallback_fndecl))), tmp); > target = gen_reg_rtx (mode); > expand_fix (target, tmp, 0); What I mean is: force_operand doesn't convert PARALLELs of pseudos to single pseudos, so this won't work as-is. And we can't make force_operand do the conversion using the current emit_group_store machinery because emit_group_store needs access to the type (in order to work out the padding), which force_operand doesn't have. Richard