[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #27 from jason at gcc dot gnu dot org 2009-02-19 01:12 --- I reverted the mistaken checkins a few seconds later. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #26 from ebotcazou at gcc dot gnu dot org 2009-02-18 22:11 --- > Fixed. The C++ static/extern issue has been added as PR 39236. You have installed a lot more things than what's described in the ChangeLog. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #25 from jason at gcc dot gnu dot org 2009-02-18 21:09 --- Fixed. The C++ static/extern issue has been added as PR 39236. -- jason at gcc dot gnu dot org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #24 from jason at gcc dot gnu dot org 2009-02-18 21:01 --- Subject: Bug 39179 Author: jason Date: Wed Feb 18 21:01:03 2009 New Revision: 144270 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144270 Log: PR target/39179 * tree-ssa-ccp.c (get_symbol_constant_value): Don't assume zero value if DECL_EXTERNAL. * tree-sra.c (sra_walk_gimple_assign): Likewise. * target.h (gcc_target::binds_local_p): Clarify module. * tree.h (TREE_PUBLIC): Clarify module. Added: trunk/gcc/testsuite/g++.dg/opt/const6.C Modified: trunk/gcc/ChangeLog trunk/gcc/cp/ChangeLog trunk/gcc/cp/cfns.h trunk/gcc/cp/ptree.c trunk/gcc/target.h trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/g++.dg/init/const7.C trunk/gcc/tree-pretty-print.c trunk/gcc/tree-sra.c trunk/gcc/tree-ssa-ccp.c trunk/gcc/tree.h trunk/gcc/varasm.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #23 from jason at gcc dot gnu dot org 2009-02-18 18:31 --- ...and then of course there's the actual documentation: TARGET_BINDS_LOCAL_P (tree exp) Returns true if exp names an object for which name resolution rules must resolve to the current ``module'' (dynamic shared library or executable image). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
-- jason at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |jason at gcc dot gnu dot org |dot org | Status|NEW |ASSIGNED Last reconfirmed|2009-02-14 16:55:18 |2009-02-18 18:10:48 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #22 from jason at gcc dot gnu dot org 2009-02-18 18:06 --- Seems like we already had this discussion last year, starting at http://gcc.gnu.org/ml/gcc/2007-06/msg00848.html The conclusion there was that binds_local_p means "binds to this executable/shared library", and the PE definition is correct under that meaning. The conclusion in that discussion was that users need to check DECL_EXTERNAL as well if they want to check whether something will bind to the current translation unit. Which leads me back to my patch attached above. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #21 from jason at gcc dot gnu dot org 2009-02-18 17:51 --- OTOH, the use of visibility in default_binds_local_p is also wrong under this interpretation... -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #20 from jason at gcc dot gnu dot org 2009-02-18 17:47 --- (In reply to comment #19) > I suppose it's a question of what "module" means. "module" is used in a lot of different ways, but this usage definitely refers to the current translation unit: /* In a VAR_DECL, FUNCTION_DECL, NAMESPACE_DECL or TYPE_DECL, nonzero means name is to be accessible from outside this module. [...] #define TREE_PUBLIC(NODE) ((NODE)->base.public_flag) Looking at more uses of binds_local_p and the RTL SYMBOL_FLAG_LOCAL, it does seem to be primarily used for references to the current translation unit. Which means that the i386 back end is using it incorrectly somewhere; we don't need a GOT lookup for any reference to a symbol in another translation unit. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #19 from jason at gcc dot gnu dot org 2009-02-18 02:03 --- I suppose it's a question of what "module" means. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #18 from jakub at gcc dot gnu dot org 2009-02-17 23:40 --- /* True if EXP names an object for which name resolution must resolve to the current module. */ If something uses it to find if it binds to current TU, then it is using it incorrectly. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #17 from jason at gcc dot gnu dot org 2009-02-17 23:01 --- I will note that the C++ front end has been setting TREE_STATIC and DECL_EXTERNAL on the same variables for many, many years, so changing that isn't going to be simple. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #16 from jason at gcc dot gnu dot org 2009-02-17 22:56 --- (In reply to comment #12) > Well, certainly binds_local_p is used for exactly this semantic. Perhaps I'm wrong then, and it does mean "binds to a definition in this translation unit." The default definition does check DECL_EXTERNAL, I notice; I guess a quick fix would be to add that to the PE version of the hook. I'll look at fixing the C++ front end. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #15 from ebotcazou at gcc dot gnu dot org 2009-02-17 18:56 --- TREE_STATIC && DECL_EXTERNAL doesn't make sense for a VAR_DECL as per tree.h: /* In a VAR_DECL, nonzero means allocate static storage. #define TREE_STATIC(NODE) ((NODE)->base.static_flag) * In a VAR_DECL or FUNCTION_DECL, nonzero means external reference: do not allocate storage, and refer to a definition elsewhere. #define DECL_EXTERNAL(NODE) (DECL_COMMON_CHECK (NODE)->decl_common.decl_flag_2) That's why the Ada FE has simply stopped to generate this combination. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #14 from rguenth at gcc dot gnu dot org 2009-02-17 18:54 --- It won't "fix" PR35501, as if there is a DECL_INITIAL we do not care about visibility or binding in that case in get_symbol_constant_value. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #13 from hjl dot tools at gmail dot com 2009-02-17 18:51 --- binds_local_p is very weak and fragile. See PR 35513. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #12 from rguenth at gcc dot gnu dot org 2009-02-17 18:48 --- Well, certainly binds_local_p is used for exactly this semantic. Maybe mis-used. A grep over the tree optimizers is in oder to check that. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #11 from hjl dot tools at gmail dot com 2009-02-17 18:29 --- Will this also fix PR 35501? -- hjl dot tools at gmail dot com changed: What|Removed |Added CC||hjl dot tools at gmail dot ||com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
[Bug target/39179] [4.4 Regression] Wrong code in c++ for const members initialized in external file
--- Comment #10 from jason at gcc dot gnu dot org 2009-02-17 18:23 --- Created an attachment (id=17313) --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17313&action=view) fix to get_symbol_constant_value (untested) The bug is not in binds_local_p: binds_local_p returns true if the reference can bind to a different shared object, which is not the case in this example. A reference to another translation unit in the same module is still a local binding. The problem is that get_symbol_constant_value assumes that TREE_STATIC means that the variable is defined in the current translation unit, which is not currently the case for C++. Apparently this wasn't the case for Ada either, but that has been corrected. It seems to me that the obvious fix for 4.4 is to make get_symbol_constant_value check !DECL_EXTERNAL. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39179
