This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 1/2


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
2013-10-30  Bernd Edlinger <bernd.edlinger@hotmail.de>
	    Sandra Loosemore  <sandra@codesourcery.com>

	PR middle-end/23623
	PR middle-end/48784
	PR middle-end/56341
	PR middle-end/56997

	gcc/
	* expmed.c (strict_volatile_bitfield_p): Add bitregion_start
	and bitregion_end parameters.  Test for compliance with C++
	memory model.
	(store_bit_field): Adjust call to strict_volatile_bitfield_p.
	Add fallback logic for cases where -fstrict-volatile-bitfields
	is supposed to apply, but cannot.
	(extract_bit-field): Likewise.
	* doc/invoke.texi (Code Gen Options): Better document fallback for
	-fstrict-volatile-bitfields.

	gcc/testsuite/
	* gcc.dg/pr23623.c: Update to test interaction with C++
	memory model.
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);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]