From patchwork Wed Jun 1 19:50:14 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 98254 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 DABFEB6F9C for ; Thu, 2 Jun 2011 05:50:38 +1000 (EST) Received: (qmail 28977 invoked by alias); 1 Jun 2011 19:50:37 -0000 Received: (qmail 28967 invoked by uid 22791); 1 Jun 2011 19:50:35 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RFC_ABUSE_POST, TW_MG X-Spam-Check-By: sourceware.org Received: from mail-wy0-f175.google.com (HELO mail-wy0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 01 Jun 2011 19:50:20 +0000 Received: by wye20 with SMTP id 20so136322wye.20 for ; Wed, 01 Jun 2011 12:50:18 -0700 (PDT) Received: by 10.227.205.12 with SMTP id fo12mr2701755wbb.70.1306957817762; Wed, 01 Jun 2011 12:50:17 -0700 (PDT) Received: from localhost (rsandifo.gotadsl.co.uk [82.133.89.107]) by mx.google.com with ESMTPS id ej7sm965811wbb.53.2011.06.01.12.50.16 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 01 Jun 2011 12:50:16 -0700 (PDT) From: Richard Sandiford To: Bernd Schmidt Mail-Followup-To: Bernd Schmidt , gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: gcc-patches@gcc.gnu.org Subject: Re: Ping^2: PR target/45074: Check targets of multi-word operations References: <87lixmrc5z.fsf@firetop.home> <4DE550B9.30407@codesourcery.com> Date: Wed, 01 Jun 2011 20:50:14 +0100 In-Reply-To: <4DE550B9.30407@codesourcery.com> (Bernd Schmidt's message of "Tue, 31 May 2011 22:34:01 +0200") Message-ID: <87aae1s555.fsf@firetop.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 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 Bernd Schmidt writes: > On 05/31/2011 07:51 PM, Richard Sandiford wrote: >> Ping for: >> >> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01327.html >> >> It fixes the expansion of multiword operations in cases where the >> suggested target is a hard register and where CANNOT_CHANGE_MODE_CLASS >> forbids word-mode subparts. > > Can you call the new function valid_multiword_target_p? In a sense, we > already know it's a multiword target, so the function name is a bit > unfortunate. Yeah, that's better. > I see two copies of this code > > /* If TARGET is the same as one of the operands, the REG_EQUAL note > won't be accurate, so use a new target. */ > - if (target == 0 || target == op0 || target == op1) > > in expand_binop, and you seem to be changing only one? Also, there's > > xtarget = gen_reg_rtx (mode); > > if (target == 0 || !REG_P (target)) > target = xtarget; > > /* Indicate for flow that the entire target reg is being set. */ > if (REG_P (target)) > emit_clobber (xtarget); Good catch. > Ok with these changes (or if there's a good reason not to touch the ones > you left out). Thanks. Here's what I installed after retesting on x86_64-linux-gnu. Richard gcc/ PR target/45074 * optabs.h (valid_multiword_target_p): Declare. * expmed.c (extract_bit_field_1): Check valid_multiword_target_p when doing multi-word operations. * optabs.c (expand_binop): Likewise. (expand_doubleword_bswap): Likewise. (expand_absneg_bit): Likewise. (expand_unop): Likewise. (expand_copysign_bit): Likewise. (multiword_target_p): New function. gcc/testsuite/ PR target/45074 * gcc.target/mips/pr45074.c: New test. Index: gcc/optabs.h =================================================================== --- gcc/optabs.h 2011-06-01 18:53:46.000000000 +0100 +++ gcc/optabs.h 2011-06-01 18:54:09.000000000 +0100 @@ -1059,6 +1059,8 @@ create_integer_operand (struct expand_op create_expand_operand (op, EXPAND_INTEGER, GEN_INT (intval), VOIDmode, false); } +extern bool valid_multiword_target_p (rtx); + extern bool maybe_legitimize_operands (enum insn_code icode, unsigned int opno, unsigned int nops, struct expand_operand *ops); Index: gcc/expmed.c =================================================================== --- gcc/expmed.c 2011-06-01 18:53:46.000000000 +0100 +++ gcc/expmed.c 2011-06-01 18:54:09.000000000 +0100 @@ -1341,7 +1341,7 @@ extract_bit_field_1 (rtx str_rtx, unsign unsigned int nwords = (bitsize + (BITS_PER_WORD - 1)) / BITS_PER_WORD; unsigned int i; - if (target == 0 || !REG_P (target)) + if (target == 0 || !REG_P (target) || !valid_multiword_target_p (target)) target = gen_reg_rtx (mode); /* Indicate for flow that the entire target reg is being set. */ Index: gcc/optabs.c =================================================================== --- gcc/optabs.c 2011-06-01 18:53:46.000000000 +0100 +++ gcc/optabs.c 2011-06-01 18:57:38.000000000 +0100 @@ -1537,7 +1537,10 @@ expand_binop (enum machine_mode mode, op /* If TARGET is the same as one of the operands, the REG_EQUAL note won't be accurate, so use a new target. */ - if (target == 0 || target == op0 || target == op1) + if (target == 0 + || target == op0 + || target == op1 + || !valid_multiword_target_p (target)) target = gen_reg_rtx (mode); start_sequence (); @@ -1605,7 +1608,10 @@ expand_binop (enum machine_mode mode, op /* If TARGET is the same as one of the operands, the REG_EQUAL note won't be accurate, so use a new target. */ - if (target == 0 || target == op0 || target == op1) + if (target == 0 + || target == op0 + || target == op1 + || !valid_multiword_target_p (target)) target = gen_reg_rtx (mode); start_sequence (); @@ -1659,7 +1665,11 @@ expand_binop (enum machine_mode mode, op opportunities, and second because if target and op0 happen to be MEMs designating the same location, we would risk clobbering it too early in the code sequence we generate below. */ - if (target == 0 || target == op0 || target == op1 || ! REG_P (target)) + if (target == 0 + || target == op0 + || target == op1 + || !REG_P (target) + || !valid_multiword_target_p (target)) target = gen_reg_rtx (mode); start_sequence (); @@ -1779,7 +1789,7 @@ expand_binop (enum machine_mode mode, op xtarget = gen_reg_rtx (mode); - if (target == 0 || !REG_P (target)) + if (target == 0 || !REG_P (target) || !valid_multiword_target_p (target)) target = xtarget; /* Indicate for flow that the entire target reg is being set. */ @@ -2481,7 +2491,7 @@ expand_doubleword_bswap (enum machine_mo t0 = expand_unop (word_mode, bswap_optab, operand_subword_force (op, 1, mode), NULL_RTX, true); - if (target == 0) + if (target == 0 || !valid_multiword_target_p (target)) target = gen_reg_rtx (mode); if (REG_P (target)) emit_clobber (target); @@ -2724,7 +2734,9 @@ expand_absneg_bit (enum rtx_code code, e if (code == ABS) mask = double_int_not (mask); - if (target == 0 || target == op0) + if (target == 0 + || target == op0 + || (nwords > 1 && !valid_multiword_target_p (target))) target = gen_reg_rtx (mode); if (nwords > 1) @@ -2915,7 +2927,7 @@ expand_unop (enum machine_mode mode, opt int i; rtx insns; - if (target == 0 || target == op0) + if (target == 0 || target == op0 || !valid_multiword_target_p (target)) target = gen_reg_rtx (mode); start_sequence (); @@ -3386,7 +3398,10 @@ expand_copysign_bit (enum machine_mode m mask = double_int_setbit (double_int_zero, bitpos); - if (target == 0 || target == op0 || target == op1) + if (target == 0 + || target == op0 + || target == op1 + || (nwords > 1 && !valid_multiword_target_p (target))) target = gen_reg_rtx (mode); if (nwords > 1) @@ -7034,6 +7049,23 @@ insn_operand_matches (enum insn_code ico (operand, insn_data[(int) icode].operand[opno].mode))); } +/* TARGET is a target of a multiword operation that we are going to + implement as a series of word-mode operations. Return true if + TARGET is suitable for this purpose. */ + +bool +valid_multiword_target_p (rtx target) +{ + enum machine_mode mode; + int i; + + mode = GET_MODE (target); + for (i = 0; i < GET_MODE_SIZE (mode); i += UNITS_PER_WORD) + if (!validate_subreg (word_mode, mode, target, i)) + return false; + return true; +} + /* Like maybe_legitimize_operand, but do not change the code of the current rtx value. */ Index: gcc/testsuite/gcc.target/mips/pr45074.c =================================================================== --- /dev/null 2011-06-01 19:41:05.354325326 +0100 +++ gcc/testsuite/gcc.target/mips/pr45074.c 2011-06-01 18:54:09.000000000 +0100 @@ -0,0 +1,8 @@ +/* { dg-options "-mhard-float -mgp32 -O" } */ +register double g __asm__("$f20"); + +NOMIPS16 void +test (double a) +{ + g = -a; +}