This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
[Patch] Tentative fix for PR51374: combine.c reorders volatile memory accesses
- From: Georg-Johann Lay <avr at gjlay dot de>
- To: gcc at gcc dot gnu dot org
- Cc: Eric Weddington <eric dot weddington at atmel dot com>
- Date: Fri, 13 Jan 2012 16:56:28 +0100
- Subject: [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