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

login
register
mail settings
Submitter Jie Zhang
Date Oct. 22, 2010, 6:03 p.m.
Message ID <4CC1D201.7000906@codesourcery.com>
Download mbox | patch
Permalink /patch/68894/
State New
Headers show

Comments

Jie Zhang - Oct. 22, 2010, 6:03 p.m.
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,
Paul Brook - Oct. 22, 2010, 8:15 p.m.
> 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.

> +/* { dg-do compile { target arm*-*-eabi* } } */

Should probably be /* { dg-require-effective-target arm_eabi } */
(several occurrences).

Other than that, OK.

Paul

Patch


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