From patchwork Sat Nov 3 11:21:16 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 196822 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 1DD5B2C00DC for ; Sat, 3 Nov 2012 22:21:28 +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=1352546489; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:From:To:Mail-Followup-To: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=TmuKnp55bD8+9zbVDgEG zRWkJDs=; b=OqJ+zkD5nziDlFcXdSxO7lrFR9jOkv/w6OACi0K6Bnna+QumlIKV rmTx+ataFFVt8nMCk1oxCmJxa5Z0Khipb65JpVKqTuAtDNPBpPrh6AZfVqExzplF iLe82gq5Z+lr4xb/I9cgQGAOBh/qi9WcsC9h/NizKkFdr0Zo/1AOx2g= 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:Received:Received:From:To:Mail-Followup-To: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=P3R+kYxXLhsI/z+HcDwJSgd5Iamh0PfPTksHpgrtTX0Eb3hn2pQsP2lqD75rgw EDs/w7bH3svYS51ievy5bd7htnyvD+iSqYH+8jJ2rDBMTDd0opXTL/JvSzszNMKo bh4QYzSHGLZgTS0sqPhH4U5WTYRbq5xxZ2CjIG0ls2RyI=; Received: (qmail 4370 invoked by alias); 3 Nov 2012 11:21:23 -0000 Received: (qmail 4356 invoked by uid 22791); 3 Nov 2012 11:21:22 -0000 X-SWARE-Spam-Status: No, hits=-3.6 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, TW_EG X-Spam-Check-By: sourceware.org Received: from mail-we0-f175.google.com (HELO mail-we0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 03 Nov 2012 11:21:14 +0000 Received: by mail-we0-f175.google.com with SMTP id t44so2048092wey.20 for ; Sat, 03 Nov 2012 04:21:13 -0700 (PDT) Received: by 10.180.80.33 with SMTP id o1mr6075314wix.14.1351941672833; Sat, 03 Nov 2012 04:21:12 -0700 (PDT) Received: from localhost ([2.26.192.222]) by mx.google.com with ESMTPS id gm7sm2297843wib.10.2012.11.03.04.21.11 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 03 Nov 2012 04:21:12 -0700 (PDT) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Subject: [4/8] Add bit_field_mode_iterator References: <87k3u3eybu.fsf@talisman.home> Date: Sat, 03 Nov 2012 11:21:16 +0000 In-Reply-To: <87k3u3eybu.fsf@talisman.home> (Richard Sandiford's message of "Sat, 03 Nov 2012 11:10:45 +0000") Message-ID: <87390rexub.fsf@talisman.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (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 get_best_mode has various checks to decide what counts as an acceptable bitfield mode. It actually has two copies of them, with slightly different alignment checks: MIN (unit, BIGGEST_ALIGNMENT) > align vs. unit <= MIN (align, BIGGEST_ALIGNMENT) The second looks more correct, since we can't necessarily guarantee larger alignments than BIGGEST_ALIGNMENT in all cases. This patch adds a new iterator class that can be used to walk through the modes, and rewrites get_best_mode to use it. I kept the existing checks with two changes: - bitregion_start is now tested independently of bitregion_end - MAX_FIXED_MODE_SIZE is used as a limit even if a bitregion is defined It shouldn't make any difference in practice, but both changes felt more in keeping with the documentation of bitregion_start and MAX_FIXED_MODE_SIZE, and the next patch wants the bitregion_end test to be separate from bitregion_start. The behaviour of the Sequent i386 compiler probably isn't the issue it once was, but that's also dealt with in the next patch. Tested as described in the covering note. OK to install? Richard gcc/ * machmode.h (bit_field_mode_iterator): New class. (get_best_mode): Change final parameter to bool. * stor-layout.c (bit_field_mode_iterator::bit_field_mode_iterator) (bit_field_mode_iterator::next_mode): New functions, split out from... (get_best_mode): ...here. Change final parameter to bool. Use bit_field_mode_iterator. Index: gcc/machmode.h =================================================================== --- gcc/machmode.h 2012-11-02 22:43:15.633373159 +0000 +++ gcc/machmode.h 2012-11-02 22:47:03.291372600 +0000 @@ -259,13 +259,36 @@ extern enum machine_mode int_mode_for_mo extern enum machine_mode mode_for_vector (enum machine_mode, unsigned); +/* A class for iterating through possible bitfield modes. */ +class bit_field_mode_iterator +{ +public: + bit_field_mode_iterator (HOST_WIDE_INT, HOST_WIDE_INT, + HOST_WIDE_INT, HOST_WIDE_INT, + unsigned int, bool); + bool next_mode (enum machine_mode *); + bool prefer_smaller_modes (); + +private: + enum machine_mode mode_; + /* We use signed values here because the bit position can be negative + for invalid input such as gcc.dg/pr48335-8.c. */ + HOST_WIDE_INT bitsize_; + HOST_WIDE_INT bitpos_; + HOST_WIDE_INT bitregion_start_; + HOST_WIDE_INT bitregion_end_; + unsigned int align_; + bool volatilep_; + int count_; +}; + /* Find the best mode to use to access a bit field. */ extern enum machine_mode get_best_mode (int, int, unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT, unsigned int, - enum machine_mode, int); + enum machine_mode, bool); /* Determine alignment, 1<=result<=BIGGEST_ALIGNMENT. */ Index: gcc/stor-layout.c =================================================================== --- gcc/stor-layout.c 2012-11-02 22:43:15.633373159 +0000 +++ gcc/stor-layout.c 2012-11-02 22:47:11.367372581 +0000 @@ -2624,6 +2624,98 @@ fixup_unsigned_type (tree type) layout_type (type); } +/* Construct an iterator for a bitfield that spans BITSIZE bits, + starting at BITPOS. + + BITREGION_START is the bit position of the first bit in this + sequence of bit fields. BITREGION_END is the last bit in this + sequence. If these two fields are non-zero, we should restrict the + memory access to a maximum sized chunk of + BITREGION_END - BITREGION_START + 1. Otherwise, we are allowed to touch + any adjacent non bit-fields. + + ALIGN is the alignment of the underlying object in bits. + VOLATILEP says whether the bitfield is volatile. */ + +bit_field_mode_iterator +::bit_field_mode_iterator (HOST_WIDE_INT bitsize, HOST_WIDE_INT bitpos, + HOST_WIDE_INT bitregion_start, + HOST_WIDE_INT bitregion_end, + unsigned int align, bool volatilep) +: mode_ (GET_CLASS_NARROWEST_MODE (MODE_INT)), bitsize_ (bitsize), + bitpos_ (bitpos), bitregion_start_ (bitregion_start), + bitregion_end_ (bitregion_end), align_ (MIN (align, BIGGEST_ALIGNMENT)), + volatilep_ (volatilep), count_ (0) +{ + if (bitregion_end_) + bitregion_end_ += 1; +} + +/* Calls to this function return successively larger modes that can be used + to represent the bitfield. Return true if another bitfield mode is + available, storing it in *OUT_MODE if so. */ + +bool bit_field_mode_iterator::next_mode (enum machine_mode *out_mode) +{ + for (; mode_ != VOIDmode; mode_ = GET_MODE_WIDER_MODE (mode_)) + { + unsigned int unit = GET_MODE_BITSIZE (mode_); + + /* Skip modes that don't have full precision. */ + if (unit != GET_MODE_PRECISION (mode_)) + continue; + + /* Skip modes that are too small. */ + if ((bitpos_ % unit) + bitsize_ > unit) + continue; + + /* Stop if the mode is too wide to handle efficiently. */ + if (unit > MAX_FIXED_MODE_SIZE) + break; + + /* Don't deliver more than one multiword mode; the smallest one + should be used. */ + if (count_ > 0 && unit > BITS_PER_WORD) + break; + + /* Stop if the mode is wider than the alignment of the containing + object. + + It is tempting to omit the following line unless STRICT_ALIGNMENT + is true. But that is incorrect, since if the bitfield uses part + of 3 bytes and we use a 4-byte mode, we could get a spurious segv + if the extra 4th byte is past the end of memory. + (Though at least one Unix compiler ignores this problem: + that on the Sequent 386 machine. */ + if (unit > align_) + break; + + /* Stop if the mode goes outside the bitregion. */ + HOST_WIDE_INT start = bitpos_ - (bitpos_ % unit); + if (bitregion_start_ && start < bitregion_start_) + break; + if (bitregion_end_ && start + unit > bitregion_end_) + break; + + *out_mode = mode_; + mode_ = GET_MODE_WIDER_MODE (mode_); + count_++; + return true; + } + return false; +} + +/* Return true if smaller modes are generally preferred for this kind + of bitfield. */ + +bool +bit_field_mode_iterator::prefer_smaller_modes () +{ + return (volatilep_ + ? targetm.narrow_volatile_bitfield () + : !SLOW_BYTE_ACCESS); +} + /* Find the best machine mode to use when referencing a bit field of length BITSIZE bits starting at BITPOS. @@ -2655,69 +2747,21 @@ get_best_mode (int bitsize, int bitpos, unsigned HOST_WIDE_INT bitregion_start, unsigned HOST_WIDE_INT bitregion_end, unsigned int align, - enum machine_mode largest_mode, int volatilep) + enum machine_mode largest_mode, bool volatilep) { + bit_field_mode_iterator iter (bitsize, bitpos, bitregion_start, + bitregion_end, align, volatilep); + enum machine_mode widest_mode = VOIDmode; enum machine_mode mode; - unsigned int unit = 0; - unsigned HOST_WIDE_INT maxbits; - - /* If unset, no restriction. */ - if (!bitregion_end) - maxbits = MAX_FIXED_MODE_SIZE; - else - maxbits = bitregion_end - bitregion_start + 1; - - /* Find the narrowest integer mode that contains the bit field. */ - for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT); mode != VOIDmode; - mode = GET_MODE_WIDER_MODE (mode)) + while (iter.next_mode (&mode) + && (largest_mode == VOIDmode + || GET_MODE_SIZE (mode) <= GET_MODE_SIZE (largest_mode))) { - unit = GET_MODE_BITSIZE (mode); - if (unit == GET_MODE_PRECISION (mode) - && (bitpos % unit) + bitsize <= unit) + widest_mode = mode; + if (iter.prefer_smaller_modes ()) break; } - - if (mode == VOIDmode - /* It is tempting to omit the following line - if STRICT_ALIGNMENT is true. - But that is incorrect, since if the bitfield uses part of 3 bytes - and we use a 4-byte mode, we could get a spurious segv - if the extra 4th byte is past the end of memory. - (Though at least one Unix compiler ignores this problem: - that on the Sequent 386 machine. */ - || MIN (unit, BIGGEST_ALIGNMENT) > align - || (largest_mode != VOIDmode && unit > GET_MODE_BITSIZE (largest_mode)) - || unit > maxbits - || (bitregion_end - && bitpos - (bitpos % unit) + unit > bitregion_end + 1)) - return VOIDmode; - - if ((SLOW_BYTE_ACCESS && ! volatilep) - || (volatilep && !targetm.narrow_volatile_bitfield ())) - { - enum machine_mode wide_mode = VOIDmode, tmode; - - for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT); tmode != VOIDmode; - tmode = GET_MODE_WIDER_MODE (tmode)) - { - unit = GET_MODE_BITSIZE (tmode); - if (unit == GET_MODE_PRECISION (tmode) - && bitpos / unit == (bitpos + bitsize - 1) / unit - && unit <= BITS_PER_WORD - && unit <= MIN (align, BIGGEST_ALIGNMENT) - && unit <= maxbits - && (largest_mode == VOIDmode - || unit <= GET_MODE_BITSIZE (largest_mode)) - && (bitregion_end == 0 - || bitpos - (bitpos % unit) + unit <= bitregion_end + 1)) - wide_mode = tmode; - } - - if (wide_mode != VOIDmode) - return wide_mode; - } - - return mode; + return widest_mode; } /* Gets minimal and maximal values for MODE (signed or unsigned depending on