Bug 51200 - Wrong code sequence to store restrict volatile bitfield
Summary: Wrong code sequence to store restrict volatile bitfield
Status: VERIFIED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.7.0
: P3 major
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-18 02:19 UTC by Joey Ye
Modified: 2012-03-29 02:15 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Joey Ye 2011-11-18 02:19:52 UTC
Trunk 179074 generates wrong code sequence with -fstrict-volatile-bitfields on ARM and x86. 

ARM AAPCS default enable strict volatile bitfields so it is critical on ARM:

/* { dg-do run } */
/* { dg-options "-fstrict-volatile-bitfields" } */

extern void abort(void);
struct thing {
  volatile unsigned short a: 8;
  volatile unsigned short b: 8;
} t = {1,2};

int main()
{
  t.a = 3;
  if (t.a !=3 || t.b !=2) abort();
  return 0;
}
Comment 1 Joey Ye 2011-11-18 02:23:17 UTC
A patch is available at http://gcc.gnu.org/ml/gcc-patches/2010-12/msg00217.html but is pending for about 1 year.

Latest discussion is at http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01623.html
Comment 2 Joey Ye 2011-11-22 03:58:29 UTC
Here is a test case fix.

With this patch, backend part of Bernd's original patch can be skipped. Thus DJ's concern of unnecessary change can be addressed.

Also this test case intends to warn a situation that is incompatible to abi version 1, -fstrict-volatile-bitfields happenly hides the incompatibility. IMHO it is consultable to claim strict volatile bitfields violates version 1. So fixing the test case and make it work as intended is more reasonable to me.


--- a/gcc/testsuite/g++.dg/abi/bitfield12.C
+++ b/gcc/testsuite/g++.dg/abi/bitfield12.C
@@ -1,4 +1,4 @@
-// { dg-options "-Wabi -fabi-version=1" }
+// { dg-options "-Wabi -fabi-version=1 -fno-strict-volatile-bitfields" }
 
 struct S { // { dg-warning "ABI" }
   char c : 1024; // { dg-warning "width" }

 struct S { // { dg-warning "ABI" }
   char c : 1024; // { dg-warning
Comment 3 Bernd Schmidt 2011-12-21 01:22:08 UTC
Fixed, I think?
Comment 4 Joey Ye 2011-12-21 04:34:57 UTC
Fixed in trunk 182545
Comment 5 jye2 2011-12-26 08:43:51 UTC
Author: jye2
Date: Mon Dec 26 08:43:48 2011
New Revision: 182685

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182685
Log:
2011-12-26  Joey Ye  <joey.ye@arm.com>

	PR middle-end/51200
	* gcc.dg/volatile-bitfields-2.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/volatile-bitfields-2.c
Modified:
    trunk/gcc/testsuite/ChangeLog
Comment 6 jye2 2011-12-27 02:27:01 UTC
Author: jye2
Date: Tue Dec 27 02:26:57 2011
New Revision: 182691

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182691
Log:
2011-12-26  Joey Ye  <joey.ye@arm.com>
	Revert original fix and backport r182545, 182649 from mainline

	Revert:
	2011-11-18  Joey Ye  <joey.ye@arm.com>

	Port Bernd's fix to volatile bitfields
	2010-12-02  Bernd Schmidt  <bernds_cb1@t-online.de>

	* expr.c (store_field): Avoid a direct store if the mode is larger
	than the size of the bit field.
	* stor-layout.c (layout_decl): If flag_strict_volatile_bitfields,
	treat non-volatile bit fields like volatile ones.
	* toplev.c (process_options): Disallow combination of
	-fstrict-volatile-bitfields and ABI versions less than 2.
	* config/arm/arm.c (arm_option_override): Don't enable
	flag_strict_volatile_bitfields if the ABI version is less than 2.

	Backport:
	2011-12-20  Bernd Schmidt  <bernds@codesourcery.com>

	PR middle-end/51200
	* expr.c (store_field): Avoid a direct store if the mode is larger
	than the size of the bit field.
	* stor-layout.c (layout_decl): If flag_strict_volatile_bitfields,
	treat non-volatile bit fields like volatile ones.
	* toplev.c (process_options): Disallow combination of
	-fstrict-volatile-bitfields and ABI versions less than 2.
	* config/arm/arm.c (arm_option_override): Don't enable
	flag_strict_volatile_bitfields if the ABI version is less than 2.
	* config/h8300/h8300.c (h8300_option_override): Likewise.
	* config/rx/rx.c (rx_option_override): Likewise.
	* config/m32c/m32c.c (m32c_option_override): Likewise.
	* config/sh/sh.c (sh_option_override): Likewise.

	2011-12-22  Joey Ye  <joey.ye@arm.com>

	* toplev.c (process_options): Fix typo.

testsute
2011-12-26  Joey Ye  <joey.ye@arm.com>

	Revert original fix and backport r182545, r182649 from mainline

	Revert:
	2011-11-23  Joey Ye  <joey.ye@arm.com>

	Apply restrict volatile bitfield test case.
	2011-11-23  Joey Ye  <joey.ye@arm.com>
	* g++.dg/abi/bitfield12.C: Add option -fno-strict-volatile-bitfields.

	Backport:
	2011-12-20  Bernd Schmidt  <bernds@codesourcery.com>

	PR middle-end/51200
	* gcc.target/arm/volatile-bitfields-4.c: New test.
	* c-c++-common/abi-bf.c: New test.

	2011-12-26  Joey Ye  <joey.ye@arm.com>

	PR middle-end/51200
	* gcc.dg/volatile-bitfields-2.c: New test.


Added:
    branches/ARM/embedded-4_6-branch/gcc/testsuite/c-c++-common/abi-bf.c
    branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.dg/volatile-bitfields-2.c
    branches/ARM/embedded-4_6-branch/gcc/testsuite/gcc.target/arm/volatile-bitfields-4.c
Modified:
    branches/ARM/embedded-4_6-branch/gcc/ChangeLog.arm
    branches/ARM/embedded-4_6-branch/gcc/config/arm/arm.c
    branches/ARM/embedded-4_6-branch/gcc/config/h8300/h8300.c
    branches/ARM/embedded-4_6-branch/gcc/config/m32c/m32c.c
    branches/ARM/embedded-4_6-branch/gcc/config/rx/rx.c
    branches/ARM/embedded-4_6-branch/gcc/config/sh/sh.c
    branches/ARM/embedded-4_6-branch/gcc/testsuite/ChangeLog.arm
    branches/ARM/embedded-4_6-branch/gcc/testsuite/g++.dg/abi/bitfield12.C
    branches/ARM/embedded-4_6-branch/gcc/toplev.c
Comment 7 jye2 2012-03-29 02:15:37 UTC
Author: jye2
Date: Thu Mar 29 02:15:29 2012
New Revision: 185944

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185944
Log:
2012-03-28  Joey Ye  <joey.ye@arm.com>

	Backported from mainline
	2011-12-20  Bernd Schmidt  <bernds@codesourcery.com>

	PR middle-end/51200
	* expr.c (store_field): Avoid a direct store if the mode is larger
	than the size of the bit field.
	* stor-layout.c (layout_decl): If flag_strict_volatile_bitfields,
	treat non-volatile bit fields like volatile ones.
	* toplev.c (process_options): Disallow combination of
	-fstrict-volatile-bitfields and ABI versions less than 2.
	* config/arm/arm.c (arm_option_override): Don't enable
	flag_strict_volatile_bitfields if the ABI version is less than 2.
	* config/h8300/h8300.c (h8300_option_override): Likewise.
	* config/rx/rx.c (rx_option_override): Likewise.
	* config/m32c/m32c.c (m32c_option_override): Likewise.
	* config/sh/sh.c (sh_option_override): Likewise.

	2011-12-22  Joey Ye  <joey.ye@arm.com>

	* toplev.c (process_options): Fix typo.

testcases:

	Backported from mainline
	2011-12-20  Bernd Schmidt  <bernds@codesourcery.com>

	PR middle-end/51200
	* gcc.target/arm/volatile-bitfields-4.c: New test.
	* c-c++-common/abi-bf.c: New test.

	2011-12-26  Joey Ye  <joey.ye@arm.com>

	PR middle-end/51200
	* gcc.dg/volatile-bitfields-2.c: New test.


Added:
    branches/gcc-4_6-branch/gcc/testsuite/c-c++-common/abi-bf.c
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/volatile-bitfields-2.c
    branches/gcc-4_6-branch/gcc/testsuite/gcc.target/arm/volatile-bitfields-4.c
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/arm/arm.c
    branches/gcc-4_6-branch/gcc/config/h8300/h8300.c
    branches/gcc-4_6-branch/gcc/config/m32c/m32c.c
    branches/gcc-4_6-branch/gcc/config/rx/rx.c
    branches/gcc-4_6-branch/gcc/config/sh/sh.c
    branches/gcc-4_6-branch/gcc/expr.c
    branches/gcc-4_6-branch/gcc/stor-layout.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_6-branch/gcc/toplev.c