[Bug c++/31809] New: [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[Bug c++/31809] New: [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[pinskia at gcc dot gnu dot org]

Testcase which should not abort but does currently at -O1 and above because SRA
thinks the decl is constant so it using its DECL_INITIAL but that is NULL so we
get a zero instead of the corect value:
struct sc
{
 unsigned v;
 static inline sc f(unsigned a);
};
inline sc sc::f(unsigned a)
{
 static sc t = { a };
 return t;
}
const static sc RC = sc::f( 1 );
extern "C" void abort (void);
int main()
{
  sc RC1 = RC;
 if (! RC1.v ) abort ();
 return 0;
}


-- 
           Summary: [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is
                    still set for non read only variables causing wrong code
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pinskia at gcc dot gnu dot org
GCC target triplet: powerpc-linux-gnu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.0.2 4.1.1 4.2.0 4.3.0
      Known to work|                            |3.4.0
   Target Milestone|---                         |4.1.3


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2007-05-04 10:33 -------
Confirmed.  Should be easy to work around.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-05-04 10:33:41
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mmitchel at gcc dot gnu dot org]

-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[jakub at gcc dot gnu dot org]

------- Comment #2 from jakub at gcc dot gnu dot org  2007-05-25 12:39 -------
I'm not sure we want to work around here though.  Static const variable
initialized during __static_initialization_and_destruction shouldn't have
TREE_READONLY set, because the variable isn't really unchanging.
This is similar to PR20073, but as the var's type doesn't need constructing,
we don't know in cp_apply_type_quals_to_decl that the initialization needs to
be done at runtime.  We won't know that I think until cp_finish_decl.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at codesourcery dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mmitchel at gcc dot gnu dot org]

------- Comment #3 from mmitchel at gcc dot gnu dot org  2007-05-28 01:11 -------
I agree.  I think that in cp_finish_decl, we should unset TREE_READONLY for
variables with a dynamic initializers.  Please let me know if you need help
working on a patch for that.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |jakub at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2007-05-04 10:33:41         |2007-05-30 17:57:58
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[jakub at gcc dot gnu dot org]

------- Comment #4 from jakub at gcc dot gnu dot org  2007-05-30 21:24 -------
Subject: Bug 31809

Author: jakub
Date: Wed May 30 21:24:24 2007
New Revision: 125201

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125201
Log:
        PR c++/31809
        * decl.c (cp_finish_decl): Clear TREE_READONLY flag on TREE_STATIC
        variables that need runtime initialization.

        * g++.dg/opt/static5.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/opt/static5.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mueller at gcc dot gnu dot org]

------- Comment #5 from mueller at gcc dot gnu dot org  2007-05-30 22:46 -------
is it okay that was_readonly will eventually turn on TREE_READONLY()
afterwards?


-- 

mueller at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mueller at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mark at codesourcery dot com]

------- Comment #6 from mark at codesourcery dot com  2007-05-30 23:02 -------
Subject: Re:  [4.1/4.2/4.3 Regression] sometimes TREE_READONLY
 is still set for non read only variables causing wrong code

mueller at gcc dot gnu dot org wrote:
> ------- Comment #5 from mueller at gcc dot gnu dot org  2007-05-30 22:46 -------
> is it okay that was_readonly will eventually turn on TREE_READONLY()
> afterwards?

I wondered about this too, but was_readonly is only set for
REFERENCE_TYPEs.  I'm not sure what happens with something like:

  int& f() { return *new int; }
  int& i = f();

Jakub, does the reference end up TREE_READONLY in that case?  If so, do
we have a problem there too?  I think we might, for something like:

  int& f() { static int x; return x; }
  int& i = f();
  int& j = i;

The compiler must not reorder this to:

  j = i;
  i = &x;

which I guess it might, since it might think that "i" never changes, and
therefore "j" can access its value at any point?

Of course, it's sad that we lose TREE_READONLY on references or "const"
variables, as we know they cannot change after initialization.  There's
no question that Jakub's change is going to make use generate inferior
code in some cases.

In fact, for the reference case, we know that "i" does not change in the
scope of any function except the static initialization function that
assigns the return value of "f" to "i".

One possible way to get some of this performance back would be to avoid
marking references as clobbered by calls -- except to the static
initialization function, which is only called from .ctors, so we can
probably ignore that.  In other words, even though "i" is not
TREE_READONLY, treat it as such when computing what can clobber it.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mueller at gcc dot gnu dot org]

------- Comment #7 from mueller at gcc dot gnu dot org  2007-05-31 09:06 -------
PR 31806 is not fixed by this, and it contains a reference type. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[jakub at gcc dot gnu dot org]

------- Comment #8 from jakub at gcc dot gnu dot org  2007-05-31 10:00 -------
I saw was_readonly as well and wondered if it might be a problem or not, then
incorrectly assumed REFERENCE_TYPEs can't be an issue, but as PR31806 shows,
they apparently are.

2007-05-31  Jakub Jelinek  <jakub@redhat.com>

        PR c++/31806
        * decl.c (cp_finish_decl): Also clear was_readonly if a static var
        needs runtime initialization.

--- gcc/cp/decl.c.jj    2007-05-30 21:03:07.000000000 +0200
+++ gcc/cp/decl.c       2007-05-31 11:32:48.000000000 +0200
@@ -5361,8 +5361,12 @@ cp_finish_decl (tree decl, tree init, bo
              /* If a TREE_READONLY variable needs initialization
                 at runtime, it is no longer readonly and we need to
                 avoid MEM_READONLY_P being set on RTL created for it.  */
-             if (init && TREE_READONLY (decl))
-               TREE_READONLY (decl) = 0;
+             if (init)
+               {
+                 if (TREE_READONLY (decl))
+                   TREE_READONLY (decl) = 0;
+                 was_readonly = 0;
+               }
              expand_static_init (decl, init);
            }
        }

should cure this.

Regarding the performance degradation, to cure this, we'd either have to
revert to RTX_UNCHANGING_P (which we know just leads to tremendous amount
of bugs), or introduce a way to say "this tree resp. this rtl is readonly
in all functions but <this set of fndecls>".  For filescope C++ vars that need
runtime the set is certainly static initialization and destruction, all
functions inlined into it and then depends on how aggresive IPA optimizations
want to be, if one day they choose to clone some constructor of a initialized
once, then read-only variable and the optimizers would assume that var is
readonly, we'd again miscompile things.  For function-scope static vars we'd
need to put the containing function into the set, so it probably won't help
there at all.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2/4.3 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mark at codesourcery dot com]

------- Comment #9 from mark at codesourcery dot com  2007-05-31 16:32 -------
Subject: Re:  [4.1/4.2/4.3 Regression] sometimes TREE_READONLY
 is still set for non read only variables causing wrong code

jakub at gcc dot gnu dot org wrote:

> 2007-05-31  Jakub Jelinek  <jakub@redhat.com>
> 
>         PR c++/31806
>         * decl.c (cp_finish_decl): Also clear was_readonly if a static var
>         needs runtime initialization.

This patch is OK.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1/4.2 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mueller at gcc dot gnu dot org]

------- Comment #10 from mueller at gcc dot gnu dot org  2007-06-08 21:48 -------
Subject: Bug 31809

Author: mueller
Date: Fri Jun  8 21:48:34 2007
New Revision: 125580

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125580
Log:
2007-06-08  Dirk Mueller  <dmueller@suse.de>

       PR c++/31809
       PR c++/31806
       Backport from mainline:
       2007-05-31  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Also clear was_readonly if a static var
       needs runtime initialization.

       2007-05-30  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Clear TREE_READONLY flag on TREE_STATIC
       variables that need runtime initialization.

       PR c++/31809
       Backport from mainline:
       2007-05-30  Jakub Jelinek  <jakub@redhat.com>

       * g++.dg/opt/static5.C: New test.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/g++.dg/opt/static5.C
      - copied unchanged from r125201, trunk/gcc/testsuite/g++.dg/opt/static5.C
Modified:
    branches/gcc-4_2-branch/gcc/cp/ChangeLog
    branches/gcc-4_2-branch/gcc/cp/decl.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mueller at gcc dot gnu dot org]

------- Comment #11 from mueller at gcc dot gnu dot org  2007-06-20 16:27 -------
Subject: Bug 31809

Author: mueller
Date: Wed Jun 20 16:27:23 2007
New Revision: 125887

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125887
Log:
2007-06-20  Dirk Mueller  <dmueller@suse.de>

       PR c++/31809
       PR c++/31806
       Backport from mainline:
       2007-05-31  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Also clear was_readonly if a static var
       needs runtime initialization.

       2007-05-30  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Clear TREE_READONLY flag on TREE_STATIC
       variables that need runtime initialization.

       * g++.dg/opt/static5.C: New testcase
       * g++.dg/opt/static6.C: Likewise


Added:
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/opt/static5.C
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/opt/static6.C
Modified:
    branches/gcc-4_1-branch/gcc/cp/ChangeLog
    branches/gcc-4_1-branch/gcc/cp/decl.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809

[Bug c++/31809] [4.1 Regression] sometimes TREE_READONLY is still set for non read only variables causing wrong code

[mueller at gcc dot gnu dot org]

------- Comment #12 from mueller at gcc dot gnu dot org  2007-06-20 16:28 -------
Fixed.


-- 

mueller at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31809