Re: PING: Default to -fstrict-volatile-bitfields for ARM EABI

[Jie Zhang <jie at codesourcery dot com>]

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,
--
Jie Zhang
CodeSourcery

	* 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 ();
 }