From patchwork Thu Oct 31 00:46:09 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sandra Loosemore X-Patchwork-Id: 287370 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]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 424382C03BD for ; Thu, 31 Oct 2013 11:47:01 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; q=dns; s=default; b=aJvDzu+5Wa1GwBu1V qiRhd9XLgbxTDzZzrlMhOdQ8MWci26g7lkEhux2AMgAvE25t38qoGLGGiqmfvmBm vefzkEs3Jeuc32DZyH1PLuQYxxbEJGTsiFiqlEUccOepjtlZTlONTD1iGO9MRaXB buILi4iZZrvPsQrfbUNneAcqjk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; s=default; bh=XbgMzYW6IX9lhbRMUH8pi/S gcko=; b=Fj8Edrffe3ufjDFEnqEjeO7OQyQPY6RyXFivddgoSLwb84vkYudZWQi IOLcqcm8qP4LgxM4dvrFYEx7fGRAUcvCzgG0pE4YsBApZAdesienSmPnlII4D6+n FGuAm6zFK434dHAspAiMgHcO+jYdGCK2kETwTVbdB83KqDYzTrb0= Received: (qmail 25796 invoked by alias); 31 Oct 2013 00:46:54 -0000 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 Received: (qmail 25783 invoked by uid 89); 31 Oct 2013 00:46:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 31 Oct 2013 00:46:51 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1VbgPP-0007JC-CN from Sandra_Loosemore@mentor.com ; Wed, 30 Oct 2013 17:46:47 -0700 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Wed, 30 Oct 2013 17:46:47 -0700 Received: from [IPv6:::1] (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.2.247.3; Wed, 30 Oct 2013 17:46:47 -0700 Message-ID: <5271A851.6000808@codesourcery.com> Date: Wed, 30 Oct 2013 18:46:09 -0600 From: Sandra Loosemore User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Bernd Edlinger , Richard Biener , GCC Patches Subject: Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 1/2 References: <52463CA4.7060303@codesourcery.com> <526161EC.5070200@codesourcery.com> <325c359b-22e1-43dc-8050-5a11deb66e95@email.android.com>, <52649035.6000802@codesourcery.com> , <526F2B94.8020907@codesourcery.com> In-Reply-To: On 10/29/2013 02:51 AM, Bernd Edlinger wrote: > > On Mon, 28 Oct 2013 21:29:24, Sandra Loosemore wrote: >> On 10/28/2013 03:20 AM, Bernd Edlinger wrote: >>> I have attached an update to your patch, that should >>> a) fix the recursion problem. >>> b) restrict the -fstrict-volatile-bitfields to not violate the C++ memory model. Here's a new version of the update patch. >> Alternatively, if strict_volatile_bitfield_p returns false but >> flag_strict_volatile_bitfields> 0, then always force to word_mode and >> change the -fstrict-volatile-bitfields documentation to indicate that's >> the fallback if the insertion/extraction cannot be done in the declared >> mode, rather than claiming that it tries to do the same thing as if >> -fstrict-volatile-bitfields were not enabled at all. I decided that this approach was more expedient, after all. I've tested this patch (in conjunction with my already-approved but not-yet-applied patch) on mainline for arm-none-eabi, x86_64-linux-gnu, and mips-linux gnu. I also backported the entire series to GCC 4.8 and tested there on arm-none-eabi and x86_64-linux-gnu. OK to apply? -Sandra diff -u gcc/doc/invoke.texi gcc/doc/invoke.texi --- gcc/doc/invoke.texi (working copy) +++ gcc/doc/invoke.texi (working copy) @@ -21659,7 +21659,8 @@ In some cases, such as when the @code{packed} attribute is applied to a structure field, it may not be possible to access the field with a single read or write that is correctly aligned for the target machine. In this -case GCC falls back to generating multiple accesses rather than code that +case GCC falls back to generating either multiple accesses +or an access in a larger mode, rather than code that will fault or truncate the result at run time. The default value of this option is determined by the application binary diff -u gcc/testsuite/gcc.dg/pr23623.c gcc/testsuite/gcc.dg/pr23623.c --- gcc/testsuite/gcc.dg/pr23623.c (revision 0) +++ gcc/testsuite/gcc.dg/pr23623.c (revision 0) @@ -8,16 +8,19 @@ extern struct { unsigned int b : 1; + unsigned int : 31; } bf1; extern volatile struct { unsigned int b : 1; + unsigned int : 31; } bf2; extern struct { volatile unsigned int b : 1; + volatile unsigned int : 31; } bf3; void writeb(void) diff -u gcc/expmed.c gcc/expmed.c --- gcc/expmed.c (working copy) +++ gcc/expmed.c (working copy) @@ -416,12 +416,17 @@ } /* Return true if -fstrict-volatile-bitfields applies an access of OP0 - containing BITSIZE bits starting at BITNUM, with field mode FIELDMODE. */ + containing BITSIZE bits starting at BITNUM, with field mode FIELDMODE. + Return false if the access would touch memory outside the range + BITREGION_START to BITREGION_END for conformance to the C++ memory + model. */ static bool strict_volatile_bitfield_p (rtx op0, unsigned HOST_WIDE_INT bitsize, unsigned HOST_WIDE_INT bitnum, - enum machine_mode fieldmode) + enum machine_mode fieldmode, + unsigned HOST_WIDE_INT bitregion_start, + unsigned HOST_WIDE_INT bitregion_end) { unsigned HOST_WIDE_INT modesize = GET_MODE_BITSIZE (fieldmode); @@ -448,6 +453,12 @@ && bitnum % GET_MODE_ALIGNMENT (fieldmode) + bitsize > modesize)) return false; + /* Check for cases where the C++ memory model applies. */ + if (bitregion_end != 0 + && (bitnum - bitnum % modesize < bitregion_start + || bitnum - bitnum % modesize + modesize > bitregion_end)) + return false; + return true; } @@ -904,7 +915,8 @@ rtx value) { /* Handle -fstrict-volatile-bitfields in the cases where it applies. */ - if (strict_volatile_bitfield_p (str_rtx, bitsize, bitnum, fieldmode)) + if (strict_volatile_bitfield_p (str_rtx, bitsize, bitnum, fieldmode, + bitregion_start, bitregion_end)) { /* Storing any naturally aligned field can be done with a simple @@ -923,6 +935,14 @@ store_fixed_bit_field (str_rtx, bitsize, bitnum, 0, 0, value); return; } + else if (MEM_P (str_rtx) + && MEM_VOLATILE_P (str_rtx) + && flag_strict_volatile_bitfields > 0) + /* This is a case where -fstrict-volatile-bitfields doesn't apply + because we can't do a single access in the declared mode of the field. + Since the incoming STR_RTX has already been adjusted to that mode, + fall back to word mode for subsequent logic. */ + str_rtx = adjust_address (str_rtx, word_mode, 0); /* Under the C++0x memory model, we must not touch bits outside the bit region. Adjust the address to start at the beginning of the @@ -1695,7 +1715,7 @@ else mode1 = tmode; - if (strict_volatile_bitfield_p (str_rtx, bitsize, bitnum, mode1)) + if (strict_volatile_bitfield_p (str_rtx, bitsize, bitnum, mode1, 0, 0)) { rtx result; @@ -1709,6 +1729,14 @@ target, unsignedp); return convert_extracted_bit_field (result, mode, tmode, unsignedp); } + else if (MEM_P (str_rtx) + && MEM_VOLATILE_P (str_rtx) + && flag_strict_volatile_bitfields > 0) + /* This is a case where -fstrict-volatile-bitfields doesn't apply + because we can't do a single access in the declared mode of the field. + Since the incoming STR_RTX has already been adjusted to that mode, + fall back to word mode for subsequent logic. */ + str_rtx = adjust_address (str_rtx, word_mode, 0); return extract_bit_field_1 (str_rtx, bitsize, bitnum, unsignedp, target, mode, tmode, true);