* [Bug rtl-optimization/45235] const volatile read moved out of order
2010-08-09 11:56 [Bug rtl-optimization/45235] New: " bigotp at acm dot org
@ 2010-08-09 11:57 ` bigotp at acm dot org
2010-08-09 13:56 ` rguenth at gcc dot gnu dot org
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: bigotp at acm dot org @ 2010-08-09 11:57 UTC (permalink / raw)
To: gcc-bugs
------- Comment #1 from bigotp at acm dot org 2010-08-09 11:57 -------
Created an attachment (id=21441)
--> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21441&action=view)
fixes for assumption that readonly means constant
The problem is caused by the same sort of test as was fixed in a different
situation in #35729. In the attached patch, it is the change to
rtanal.c:rtx_varies_p that fixes the problem. The remaining changes are
plausible, but I don't know whether they're necessary. There are additional
uses of MEM_READONLY_P that are also questionable, that I didn't get around
to trying. I suggest a thorough review by somebody who, unlike me, knows GCC
internals.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45235
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug rtl-optimization/45235] const volatile read moved out of order
2010-08-09 11:56 [Bug rtl-optimization/45235] New: " bigotp at acm dot org
2010-08-09 11:57 ` [Bug rtl-optimization/45235] " bigotp at acm dot org
@ 2010-08-09 13:56 ` rguenth at gcc dot gnu dot org
2010-08-11 21:41 ` pinskia at gcc dot gnu dot org
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2010-08-09 13:56 UTC (permalink / raw)
To: gcc-bugs
------- Comment #2 from rguenth at gcc dot gnu dot org 2010-08-09 13:56 -------
Probably easier to not set TREE_READONLY or MEM_READONY_P here.
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c (revision 163030)
+++ gcc/emit-rtl.c (working copy)
@@ -1669,7 +1670,8 @@ set_mem_attributes_minus_bitpos (rtx ref
base = get_base_address (base);
if (base && DECL_P (base)
&& TREE_READONLY (base)
- && (TREE_STATIC (base) || DECL_EXTERNAL (base)))
+ && (TREE_STATIC (base) || DECL_EXTERNAL (base))
+ && !TREE_THIS_VOLATILE (base))
MEM_READONLY_P (ref) = 1;
/* If this expression uses it's parent's alias set, mark it such
--
rguenth at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Ever Confirmed|0 |1
Keywords| |wrong-code
Last reconfirmed|0000-00-00 00:00:00 |2010-08-09 13:56:06
date| |
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45235
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug rtl-optimization/45235] const volatile read moved out of order
2010-08-09 11:56 [Bug rtl-optimization/45235] New: " bigotp at acm dot org
2010-08-09 11:57 ` [Bug rtl-optimization/45235] " bigotp at acm dot org
2010-08-09 13:56 ` rguenth at gcc dot gnu dot org
@ 2010-08-11 21:41 ` pinskia at gcc dot gnu dot org
2010-08-11 22:55 ` bigotp at acm dot org
2010-08-11 23:11 ` rguenth at gcc dot gnu dot org
4 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu dot org @ 2010-08-11 21:41 UTC (permalink / raw)
To: gcc-bugs
------- Comment #3 from pinskia at gcc dot gnu dot org 2010-08-11 21:41 -------
Hmm, I don't think this is correct as const volatile is a bit weird. It means
a read must happen but it does not say order compared to other volatile
variables (or at least I think).
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45235
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug rtl-optimization/45235] const volatile read moved out of order
2010-08-09 11:56 [Bug rtl-optimization/45235] New: " bigotp at acm dot org
` (2 preceding siblings ...)
2010-08-11 21:41 ` pinskia at gcc dot gnu dot org
@ 2010-08-11 22:55 ` bigotp at acm dot org
2010-08-11 23:11 ` rguenth at gcc dot gnu dot org
4 siblings, 0 replies; 9+ messages in thread
From: bigotp at acm dot org @ 2010-08-11 22:55 UTC (permalink / raw)
To: gcc-bugs
------- Comment #4 from bigotp at acm dot org 2010-08-11 22:54 -------
I don't see that the const qualifier should be relevant: doesn't it simply
indicate that the code is not permitted to write through that lvalue? (FWIW,
the real code uses a memory mapped address and the const qualifier was placed
on the address declaration by the chip manufacturer. I assume that a write to
the location may be interpreted as a command to the hardware to self-destruct.)
Given that, the existing behavior appears to be a clear violation of the
requirement that volatile accesses not be re-ordered across sequence points.
In concept, I like rguenth's solution (which I haven't tested), as it fits with
the documented semantics of MEM_READONLY_P indicating "not modified during the
lifetime of the program", something that certainly can't be said of any
readable volatile object. That the RTL flag is stored in a field named
"unchanging" is a lot more clear than "READONLY". I think that approach
eliminates the need to muck with any of those other places where the same
problem might be occurring.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45235
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug rtl-optimization/45235] const volatile read moved out of order
2010-08-09 11:56 [Bug rtl-optimization/45235] New: " bigotp at acm dot org
` (3 preceding siblings ...)
2010-08-11 22:55 ` bigotp at acm dot org
@ 2010-08-11 23:11 ` rguenth at gcc dot gnu dot org
4 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2010-08-11 23:11 UTC (permalink / raw)
To: gcc-bugs
------- Comment #5 from rguenth at gcc dot gnu dot org 2010-08-11 23:11 -------
(In reply to comment #4)
> I don't see that the const qualifier should be relevant: doesn't it simply
> indicate that the code is not permitted to write through that lvalue?
That's true which is why I think this bug is valid.
> (FWIW,
> the real code uses a memory mapped address and the const qualifier was placed
> on the address declaration by the chip manufacturer. I assume that a write to
> the location may be interpreted as a command to the hardware to self-destruct.)
>
> Given that, the existing behavior appears to be a clear violation of the
> requirement that volatile accesses not be re-ordered across sequence points.
>
> In concept, I like rguenth's solution (which I haven't tested), as it fits with
> the documented semantics of MEM_READONLY_P indicating "not modified during the
> lifetime of the program", something that certainly can't be said of any
> readable volatile object. That the RTL flag is stored in a field named
> "unchanging" is a lot more clear than "READONLY". I think that approach
> eliminates the need to muck with any of those other places where the same
> problem might be occurring.
Indeed.
--
rguenth at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
AssignedTo|unassigned at gcc dot gnu |rguenth at gcc dot gnu dot
|dot org |org
Status|NEW |ASSIGNED
Last reconfirmed|2010-08-09 13:56:06 |2010-08-11 23:11:05
date| |
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45235
^ permalink raw reply [flat|nested] 9+ messages in thread