This is the mail archive of the gcc@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]

[Patch] Tentative fix for PR51374: combine.c reorders volatile memory accesses


This is a tentative patch to fix combine.c so that it no more reorders volatile
memory accesses.

Even reading volatile  memory can change other (volatile) memory as explained
in the patch. This also applies to volatile memory that is not wired to
hardware that changes by magic when reading: An ISR is similar example that
volatile memory accesses must not be exchanged.

Moreover, the patch handles (set (zero_extract (mem)) ...) because there is
hardware that is able to change some bits of memory only by special
load-modify-store instructions.

The patch also fixed situations like memory barriers that are typically
implemented by means of UNSPEC_VOLATIE.

See also discussion in this topic in

http://gcc.gnu.org/cgi-bin/search.cgi?q=volatile+correctness&cmd=Search!&form=extended&m=all&ps=50&fmt=long&wm=wrd&sp=1&sy=1&wf=2221&type=&GroupBySite=no&ul=%2Fml%2Fgcc%2F2011-1%25

The patch looks reasonable to me but I am not familiar with that piece of
compiler so it would be great if someone working in that area would take care
of these lines and integrate them into the compiler.

Johann

	* combine.c (record_dead_and_set_regs_1): Update mem_last_set
	when reading from volatile memory or writing to mem via zero
	extract.
Index: combine.c
===================================================================
--- combine.c	(revision 183150)
+++ combine.c	(working copy)
@@ -12365,7 +12365,24 @@ record_dead_and_set_regs_1 (rtx dest, co
   else if (MEM_P (dest)
 	   /* Ignore pushes, they clobber nothing.  */
 	   && ! push_operand (dest, GET_MODE (dest)))
-    mem_last_set = DF_INSN_LUID (record_dead_insn);
+    {
+      mem_last_set = DF_INSN_LUID (record_dead_insn);
+    }
+  else if (ZERO_EXTRACT == GET_CODE (dest)
+           && MEM_P (XEXP (dest, 0)))
+    {
+      mem_last_set = DF_INSN_LUID (record_dead_insn);
+    }
+
+  /* Even reading a volatile memory location might change memory.
+     One example is reading an SFR that contains a latch or that belong to
+     a part of a communication unit where reading a FIFO register sets some
+     status or buisy bits located elsewhere.  */
+
+  if (volatile_refs_p (PATTERN (record_dead_insn)))
+    {
+      mem_last_set = DF_INSN_LUID (record_dead_insn);
+    }
 }
 
 /* Update the records of when each REG was most recently set or killed

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