From patchwork Sat Feb 2 09:13:12 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 217643 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 564072C008E for ; Sat, 2 Feb 2013 20:13:25 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1360401206; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: From:To:Mail-Followup-To:Cc:Subject:References:Date:In-Reply-To: Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=6hLGoiNARSgtSqlvoBYcG0i5pnI=; b=tPxltZ+xTE0umPXN3pzpcXfox8BP28Z8C/OA2CjrJ+Kv3b4xdSv1pUTkAKua3W buX/5CLlUzHrgS1+VbPRXqCM2+XJPnGZjIJTTBYMsfvhprMuwQSJ2VptZTwBxdST 5ryerw2zRTXX0N7VhHSLMw6wJm5AR+DMqRmMMdV3hedXw= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:X-Received:Received:From:To:Mail-Followup-To:Cc:Subject:References:Date:In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=gGC5KZw3kH17KO7/VNYBnRB++JAhP3/XDyQNJoOocb9CPzgFp6rRacqcekfzS0 8GX6iw3bRTqD90TP7zntUuf36t+Vb7tB1iF1YWkhXugw8FcPtSbUT1sXdJM44j1I t6jZIaMH9FmubYI2AATlwQIXlapn0cuBYuk9FvORktUL0=; Received: (qmail 27072 invoked by alias); 2 Feb 2013 09:13:22 -0000 Received: (qmail 27062 invoked by uid 22791); 2 Feb 2013 09:13:21 -0000 X-SWARE-Spam-Status: No, hits=-3.9 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_SPAMHAUS_DROP, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, TW_TQ, TW_XT X-Spam-Check-By: sourceware.org Received: from mail-wi0-f171.google.com (HELO mail-wi0-f171.google.com) (209.85.212.171) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 02 Feb 2013 09:13:16 +0000 Received: by mail-wi0-f171.google.com with SMTP id hn17so108998wib.16 for ; Sat, 02 Feb 2013 01:13:15 -0800 (PST) X-Received: by 10.194.216.66 with SMTP id oo2mr25896863wjc.4.1359796394932; Sat, 02 Feb 2013 01:13:14 -0800 (PST) Received: from localhost ([2.26.176.154]) by mx.google.com with ESMTPS id s10sm8526379wiw.4.2013.02.02.01.13.13 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 02 Feb 2013 01:13:14 -0800 (PST) From: Richard Sandiford To: Richard Henderson Mail-Followup-To: Richard Henderson , GCC Patches , rdsandiford@googlemail.com Cc: GCC Patches Subject: Re: [PATCH, RFC] PR 55403 + 55391 References: <50ABB04E.5070705@redhat.com> <87txskusct.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <50ABB997.3070400@redhat.com> <87pq38uqr3.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <50ABD908.3020907@redhat.com> <50ABE250.2060705@redhat.com> <87r4nouhai.fsf@talisman.default> Date: Sat, 02 Feb 2013 09:13:12 +0000 In-Reply-To: <87r4nouhai.fsf@talisman.default> (Richard Sandiford's message of "Tue, 20 Nov 2012 20:54:13 +0000") Message-ID: <87sj5fqdsn.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (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 Sorry, I realised when posting the expmed patch earlier today that I hadn't finished this off. Richard Sandiford writes: > Richard Henderson writes: >> On 11/20/2012 11:24 AM, Richard Henderson wrote: >>> On 11/20/2012 09:29 AM, Richard Sandiford wrote: >>>> Gah. How about this patch, currently bootstrapping on x86_64-linux-gnu >>>> as a sanity check? The last instance seems glaringly obvious in >>>> hindsight :-( >>>> >>>> Richard >>>> >>>> >>>> gcc/ >>>> * expmed.c (store_bit_field_1): Use adjust_bitfield_address_size >>>> rather than adjust_bitfield_address to change the mode of a reference. >>>> (extract_bit_field_1): Likewise. >>> >>> That patch does fix my ICE. >>> >>> It looks all good, too. As you say, glaringly obvious even. ;-) >> >> One further point -- get_best_mem_extraction_insn does not work for >> the traditional 'extv' patterns that only accept memories. In >> particular, the QImode memory in the extv pattern never matches up >> at the beginning of get_traditional_extraction_insn, so that first >> "if (mode != struct_mode) return false;" always triggers. > > Sorry for all the mishaps. > >> I audited the existing extv patterns and the affected targets are >> alpha, sh, and vax. All of the others implement extv on registers, >> which appears to work. Test case: >> >> struct S { long y __attribute__((packed)); }; >> long g(struct S *s) { return s->y; } >> >> Before: >> >> ldq_u $0,0($16) >> ldq_u $1,7($16) >> extql $0,$16,$0 >> extqh $1,$16,$16 >> bis $0,$16,$0 >> >> After: >> >> ldbu $5,1($16) >> ldbu $8,0($16) >> ldbu $7,2($16) >> ldbu $6,3($16) >> ldbu $4,4($16) >> ldbu $3,5($16) >> sll $5,8,$5 >> ldbu $2,6($16) >> ldbu $1,7($16) >> sll $7,16,$7 >> sll $6,24,$6 >> bis $5,$8,$5 >> sll $4,32,$4 >> sll $3,40,$3 >> bis $7,$5,$5 >> sll $2,48,$2 >> sll $1,56,$1 >> bis $6,$5,$0 >> bis $4,$0,$0 >> bis $3,$0,$0 >> bis $2,$0,$0 >> bis $1,$0,$0 >> >> I suppose the question is: with only 3 affected targets, is it more >> trouble fiddling the somewhat confused "traditional" path, or to >> just go ahead and update the backends? > > I suppose updating them would be the ideal eventually, but I'd still > like to fix the bug. > > I belatedly did a similar audit and it looks there are no patterns that > use a mixture of field and structure modes for register operations. > They're either equal or not present. I was wrong, i386.md has an extzv that allows SImode extractions of DImode registers. I tested the patch below at the time of the original thread but for some reason didn't get around to posting it. I've just rechecked that it doesn't change the output of gcc .ii files for x86_64-linux-gnu and that it fixes your testcase for SH4A. Retested on x86_64-linux-gnu and mips-sde-elf. OK to install? Richard gcc/ * optabs.c (get_traditional_extraction_insn): Check the field mode rather than the structure mode for memory operations. Index: gcc/optabs.c =================================================================== --- gcc/optabs.c 2013-01-31 09:17:21.551473246 +0000 +++ gcc/optabs.c 2013-01-31 09:27:35.535226561 +0000 @@ -8261,23 +8261,35 @@ get_traditional_extraction_insn (extract { const struct insn_data_d *data = &insn_data[icode]; - enum machine_mode struct_mode = data->operand[struct_op].mode; - if (struct_mode == VOIDmode) - struct_mode = word_mode; - if (mode != struct_mode) - return false; - enum machine_mode field_mode = data->operand[field_op].mode; if (field_mode == VOIDmode) field_mode = word_mode; + enum machine_mode struct_mode; + if (type == ET_unaligned_mem) + { + /* Memory structure operands refer to the first byte of the + bitfield and the true mode is taken from the field operand. */ + struct_mode = byte_mode; + if (mode != field_mode) + return false; + } + else + { + struct_mode = data->operand[struct_op].mode; + if (struct_mode == VOIDmode) + struct_mode = word_mode; + if (mode != struct_mode) + return false; + } + enum machine_mode pos_mode = data->operand[struct_op + 2].mode; if (pos_mode == VOIDmode) pos_mode = word_mode; insn->icode = icode; insn->field_mode = field_mode; - insn->struct_mode = (type == ET_unaligned_mem ? byte_mode : struct_mode); + insn->struct_mode = struct_mode; insn->pos_mode = pos_mode; return true; }