From patchwork Fri Oct 22 18:03:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jie Zhang X-Patchwork-Id: 68894 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 28FBFB70A5 for ; Sat, 23 Oct 2010 05:04:02 +1100 (EST) Received: (qmail 22989 invoked by alias); 22 Oct 2010 18:04:00 -0000 Received: (qmail 22978 invoked by uid 22791); 22 Oct 2010 18:03:59 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 22 Oct 2010 18:03:54 +0000 Received: (qmail 11283 invoked from network); 22 Oct 2010 18:03:52 -0000 Received: from unknown (HELO ?192.168.1.106?) (jie@127.0.0.2) by mail.codesourcery.com with ESMTPA; 22 Oct 2010 18:03:52 -0000 Message-ID: <4CC1D201.7000906@codesourcery.com> Date: Sat, 23 Oct 2010 02:03:45 +0800 From: Jie Zhang User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.9) Gecko/20100918 Icedove/3.1.4 MIME-Version: 1.0 To: Richard Guenther CC: gcc-patches@gcc.gnu.org, Richard Earnshaw , Paul Brook , nickc@redhat.com Subject: Re: PING: Default to -fstrict-volatile-bitfields for ARM EABI References: <4CC0EA53.4070100@codesourcery.com> In-Reply-To: X-IsSubscribed: yes 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 On 10/22/2010 05:40 PM, Richard Guenther wrote: > On Fri, 22 Oct 2010, Jie Zhang wrote: > >> Since the regression has been fixed now, it's time to reconsider this patch: >> >> http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00155.html >> >> Regression tested on latest trunk for arm-none-eabi with c,lto,c++. >> >> ARM maintainers: is the ARM port part OK? >> >> Richard G: Could you help me review the stor-layout.c part? > > Why is the stor-layout.c part necessary? Why does it not possibly > cause layout changes? I'm not very familiar with the stor-layout.c > code, and given that questions I think I'm not the right person > to review this patch. > Richard, Thanks for your questions! It's not necessary indeed. I just retested the patch with the stor-layout.c change dropped. It has no regressions on arm-none-eabi with c,c++,lto and it passes all three new tests. I don't remember the exact details why I changed stor-layout.c at that time. So now the patch is only for ARM port. The updated patch is attached. Regards, * config/arm/arm.c (arm_override_options): Default to -fstrict-volatile-bitfields. testsuite/ * gcc.target/arm/volatile-bitfields-1.c: New test. * gcc.target/arm/volatile-bitfields-2.c: New test. * gcc.target/arm/volatile-bitfields-3.c: New test. Index: testsuite/gcc.target/arm/volatile-bitfields-1.c =================================================================== --- testsuite/gcc.target/arm/volatile-bitfields-1.c (revision 0) +++ testsuite/gcc.target/arm/volatile-bitfields-1.c (revision 0) @@ -0,0 +1,17 @@ +/* { dg-do compile { target arm*-*-eabi* } } */ +/* { dg-options "-O2" } */ + +typedef struct { + char a:1; + char b:7; + int c; +} BitStruct; + +volatile BitStruct bits; + +int foo () +{ + return bits.b; +} + +/* { dg-final { scan-assembler "ldrb\[\\t \]+.*,\[\\t \]*\\\[.*\\\]" } } */ Index: testsuite/gcc.target/arm/volatile-bitfields-2.c =================================================================== --- testsuite/gcc.target/arm/volatile-bitfields-2.c (revision 0) +++ testsuite/gcc.target/arm/volatile-bitfields-2.c (revision 0) @@ -0,0 +1,17 @@ +/* { dg-do compile { target arm*-*-eabi* } } */ +/* { dg-options "-O2" } */ + +typedef struct { + volatile unsigned long a:8; + volatile unsigned long b:8; + volatile unsigned long c:16; +} BitStruct; + +BitStruct bits; + +unsigned long foo () +{ + return bits.b; +} + +/* { dg-final { scan-assembler "ldr\[\\t \]+.*,\[\\t \]*\\\[.*\\\]" } } */ Index: testsuite/gcc.target/arm/volatile-bitfields-3.c =================================================================== --- testsuite/gcc.target/arm/volatile-bitfields-3.c (revision 0) +++ testsuite/gcc.target/arm/volatile-bitfields-3.c (revision 0) @@ -0,0 +1,17 @@ +/* { dg-do compile { target arm*-*-eabi* } } */ +/* { dg-options "-O2" } */ + +typedef struct { + volatile unsigned long a:8; + volatile unsigned long b:8; + volatile unsigned long c:16; +} BitStruct; + +BitStruct bits; + +unsigned long foo () +{ + return bits.c; +} + +/* { dg-final { scan-assembler "ldr\[\\t \]+.*,\[\\t \]*\\\[.*\\\]" } } */ Index: config/arm/arm.c =================================================================== --- config/arm/arm.c (revision 163756) +++ config/arm/arm.c (working copy) @@ -1916,6 +1916,10 @@ arm_override_options (void) calculation, which is 2 instructions. */ set_param_value ("gcse-unrestricted-cost", 2); + /* ARM EABI defaults to strict volatile bitfields. */ + if (TARGET_AAPCS_BASED && flag_strict_volatile_bitfields < 0) + flag_strict_volatile_bitfields = 1; + /* Register global variables with the garbage collector. */ arm_add_gc_roots (); }