In the code below the read "(int)ptr->b;" gets optimized away. It appears to be optimized away before the RTL stages, so I guess it must be because of the tree optimalisation. I think that the C standard says C that the "name" of variable becomes an rvalue (the actual value) thus requires that the variable is accessed. Code: struct test { volatile int a; volatile int b; }; int main(int argc, char *argv[]){ struct test *ptr = (struct test *)0x10000000; ptr->b = 10; (int)ptr->b; return 1; }
Created attachment 9059 [details] Preprocessed testcase.
This works on the mainline, it might work on the 4.0.x branch too but I did not check yet.
Still fails in 4.0.1. Confirmed.
Is it posible find out why it is woking in the mainline and generate a patch for 4.0.0?
(In reply to comment #0) > I think that the C standard says C that the "name" of variable > becomes an rvalue (the actual value) thus requires that the variable > is accessed. It actually says that accessing a volatile qualified object through a nonvolatile qualified lvalue (as would be the case if referenced via a non-volatile qualified cast expression) is implementation defined. Personally, it seems more consistent to abide by the semantics specified by a programmer's explicit cast expression, thereby if given: int x, y; const int *z; x = (volatile int)y; // specifies a reference to a volatile int object. z = &(const int)x; // specifies a reference to a const int object. And correspondingly: int x; volatile int y; const int z; x = (int)y; // cast's away volatile (therefore may be optimized away), // therefore if not desired, shouldn't be "cast away". (int)z = x; // cast's away const (although may generate a run-time // exception if referencing a READONLY allocated object), // therefore if not desired, shouldn't be "cast away".
(In reply to comment #5) > (In reply to comment #0) > > I think that the C standard says C that the "name" of variable > > becomes an rvalue (the actual value) thus requires that the variable > > is accessed. > > It actually says that accessing a volatile qualified object through a nonvolatile > qualified lvalue (as would be the case if referenced via a non-volatile qualified > cast expression) is implementation defined. > If you are correct, the proposed testcase might be misleading. Changing the expression for accessing the volatile member of the struct to: "(volatile int)ptr->b;" or just: "ptr->b;" still leads to the access being optimized away. > Personally, it seems more consistent to abide by the semantics specified > by a programmer's explicit cast expression, thereby if given: > > int x, y; > const int *z; > > x = (volatile int)y; // specifies a reference to a volatile int object. > > z = &(const int)x; // specifies a reference to a const int object. > > And correspondingly: > > int x; > volatile int y; > const int z; > > x = (int)y; // cast's away volatile (therefore may be optimized away), > // therefore if not desired, shouldn't be "cast away". > > (int)z = x; // cast's away const (although may generate a run-time > // exception if referencing a READONLY allocated object), > // therefore if not desired, shouldn't be "cast away". > I agree with you on this.
(In reply to comment #6) > Changing the expression for accessing the volatile member > of the struct to: > "(volatile int)ptr->b;" > or just: > "ptr->b;" > still leads to the access being optimized away. Which seems like the real problem, as volatile access sematics should clearly apply in both instances.
This looks like a latent bug on the mainline too, just harder to reproduce and here is a testcase: struct test { struct tt { volatile int a; volatile int b; } t; }; int main(int argc, char *argv[]){ struct test *p = (struct test *)0x10000000; struct tt *ptr = &p->t; ptr->b = 10; (int)ptr->b; return 1; }
I somewhat suspect this hunk of code in c-typeck.c:build_component_ref: if (TREE_THIS_VOLATILE (datum) || TREE_THIS_VOLATILE (subdatum)) TREE_THIS_VOLATILE (ref) = 1; I don't see anything that sets TREE_SIDE_EFFECTS. However, from tree.h: If this bit is set in an expression, so is TREE_SIDE_EFFECTS. */ #define TREE_THIS_VOLATILE(NODE) ((NODE)->common.volatile_flag) However, I'm having a hard time getting good connectivity back to my build machine to experiment with this. Joseph, any ideas about this?
Hmm, no, TREE_SIDE_EFFECTS does seem to be getting set.
Created attachment 9134 [details] Proposed patch This patch seems to fix the bug. I'll kick off a test cycle.
Subject: Re: [4.0/4.1 Regression] Read from volatile member of struct is optimized away On Thu, 23 Jun 2005, mmitchel at gcc dot gnu dot org wrote: > I somewhat suspect this hunk of code in c-typeck.c:build_component_ref: > > if (TREE_THIS_VOLATILE (datum) || TREE_THIS_VOLATILE (subdatum)) > TREE_THIS_VOLATILE (ref) = 1; > > I don't see anything that sets TREE_SIDE_EFFECTS. In general TREE_SIDE_EFFECTS is set by the low-level tree-building code in tree.c (if set for the operands of a tree) so front ends need only change it if the defaults are wrong. > If this bit is set in an expression, so is TREE_SIDE_EFFECTS. */ > #define TREE_THIS_VOLATILE(NODE) ((NODE)->common.volatile_flag) This does seem like something tree checking could usefully be added for.
Subject: Bug 22000 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-4_0-branch Changes by: mmitchel@gcc.gnu.org 2005-06-23 20:15:29 Modified files: gcc : ChangeLog tree-ssa-operands.c Log message: PR 22000 * tree-ssa-operands.c (get_expr_operands): Check the volatility of the FIELD_DECL and set s_ann->has_volatile_ops accordingly. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.290&r2=2.7592.2.291 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-ssa-operands.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.63&r2=2.63.2.1
Fixed in 4.0.1.
Subject: Bug 22000 CVSROOT: /cvs/gcc Module name: gcc Changes by: mmitchel@gcc.gnu.org 2005-06-24 03:38:06 Modified files: gcc : ChangeLog tree-ssa-operands.c Log message: PR 22000 * tree-ssa-operands.c (get_expr_operands): Check the volatility of the FIELD_DECL and set s_ann->has_volatile_ops accordingly. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9215&r2=2.9216 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-ssa-operands.c.diff?cvsroot=gcc&r1=2.87&r2=2.88