See this testcase: ------------------------- struct Ball { static const double diameter = 20; void setPosition(double ,double ); double vect_Pos; }; void move (double, double); void Ball::setPosition(double xval,double yval) { vect_Pos=xval; move(xval-(diameter/2),yval-(diameter/2)); } ------------------------- This is from kbilliard, and I only noticed the problem, because a reference to Ball::diameter is left in the generated code, which can't be resolved, as it's defined nowhere. Initially I was tricked by the java like syntax, but then saw PR20098, according to which this is invalid code (for two reasons). So it seems there are two problems: 1) missed optimization, as for instance if I remove the 'vectPos=xval' line I get no linker error, and in fact in the dumps the references to diameter are substituted by the defined value (double)20 2) the invalidness of the code is not diagnosed. It's invalid for two reasons I think, first the missing definition, instead of the declaration. But that can't be diagnosed except by the linker, which indeed it does. But when reading PR20098 I learned that static const members are only allowed to have an initializer when they are of integral type. This is not the case here. If the compiler would have diagnosed it, the root cause of this problem had been more visible.
Yes this is invalid C++ (and is rejected by -pedantic) but we somehow declared this as an extension see PR 11393 which is still open and other threads within a past year. *** This bug has been marked as a duplicate of 11393 ***
Indeed. Okay, but then this really is an optimization regression compared to gcc 3.3.x which compiled this just fine. As it's only rejected with -pedantic (and I think it's a sensible extension), shouldn't we make sure that we can compile this comparatively simple source, i.e. propagate the constant correctly everywhere? I'm not sure what to do, reopening with a new subject, or creating a new bug?
(In reply to comment #2) > Indeed. Okay, but then this really is an optimization regression compared > to gcc 3.3.x which compiled this just fine. As it's only rejected with > -pedantic (and I think it's a sensible extension), shouldn't we make sure > that we can compile this comparatively simple source, i.e. propagate > the constant correctly everywhere? I'm not sure what to do, reopening with > a new subject, or creating a new bug? Oh, in that case I will reopen the bug with a different summary.
Confirmed.
It has nothing to do with members of classes either, see the following code: static const double a = 1.0; static const double b = a+1.0; double c() { return b; } We now longer inline 2.0 into c.
With -O0 we also don't inline 'a'. I thought in the past this already was done in the frontend, so the -O option didn't matter? If yes, this has changed (if not, well, I'm wrong ;-) ).
(In reply to comment #6) > With -O0 we also don't inline 'a'. I thought in the past this already > was done in the frontend, so the -O option didn't matter? If yes, this > has changed (if not, well, I'm wrong ;-) ). The code for 3.4.0 at -O0 for the example in comment #5: .LC0: .long 0 .long 1073741824 .text .align 2 .globl _Z1cv .type _Z1cv, @function _Z1cv: pushl %ebp movl %esp, %ebp fldl .LC0 popl %ebp ret .size _Z1cv, .-_Z1cv .section .rodata .align 8 .type a, @object .size a, 8 a: .long 0 .long 1072693248 .align 8 .type b, @object .size b, 8 b: .long 0 .long 1073741824 so we did inline 2.0 before. The code for 4.0.0 and above is even worse: _Z1cv: pushl %ebp movl %esp, %ebp fldl b popl %ebp ret .size _Z1cv, .-_Z1cv .text .align 2 .type _Z41__static_initialization_and_destruction_0ii, @function _Z41__static_initialization_and_destruction_0ii: pushl %ebp movl %esp, %ebp subl $8, %esp movl %eax, -4(%ebp) movl %edx, -8(%ebp) cmpl $65535, -8(%ebp) jne .L7 cmpl $1, -4(%ebp) jne .L7 fldl a fld1 faddp %st, %st(1) fstpl b .L7: leave ret .size _Z41__static_initialization_and_destruction_0ii, .- _Z41__static_initialization_and_destruction_0ii .text .align 2 .type _GLOBAL__I__Z1cv, @function _GLOBAL__I__Z1cv: pushl %ebp movl %esp, %ebp movl $65535, %edx movl $1, %eax call _Z41__static_initialization_and_destruction_0ii popl %ebp ret We dymanically initialize b too which is what partly PR 20912 is about, Diego filed after seeing eon fail because the front-end was still marking b as constant.
Also note this worked with "4.0.0 2004121", so something after that date changed the problem.
(In reply to comment #8) > Also note this worked with "4.0.0 20041211", so something after that date changed the problem. And it was broken by "4.0.0 20050113". so there is only a month time which it could have changed.
Looks like this was caused by: 2004-12-16 Nathan Sidwell <nathan@codesourcery.com> PR c++/18905 * cp-tree.h (integral_constant_value): Declare. * call.c (null_ptr_cst_p): Use integral_constant_value, not decl_constant_value. (convert_like_real): Likewise. * class.c (check_bitfield_decl): Likewise. * cvt.c (ocp_convert): Likewise. (convert): Remove unnecessary decl_constant_value call. * decl.c (compute_array_index_type): Use integral_constant_value, not decl_constant_value. (build_enumerator): Likewise. * decl2.c (grokfield): Likewise. * init.c (decl_constant_value): Simplify. (integral_constant_value): New. * pt.c (fold_decl_constant_value): Use integral_constant_value, remove subsequent check. (tsubst): Use integral_constant_value, not decl_constant_value. (tsubst_copy, unify): Likewise. * typeck.c (decay_conversion): Likewise. (build_compound_expr): Remove unnecessary decl_constant_value calls. (build_static_cast_1, build_reinterpret_cast_1): (convert_for_assignment): Remove comment about not calling decl_constant_value.
Is this optimization valid? Note that it will change the behavior of this c++ program: ---- #include <stdio.h> static const double X = 1.0; static struct S { S(); } s; static const double Y = X+2.0; S::S() { printf("%f\n", Y); } int main() {} ---- In particular, I think the C++ standard specifies that memory is zero initialized and then ctors (within a translation unit) are run in order. IANALL though. -Chris
(In reply to comment #11) I believe the optimization necessitates the variable's declaration be changed (either explicitly, or by implication): static const double b = a+1.0; => const double b = a+1.0; If it's static-const value can't be pre-computed at compile time. (as opposed to allowing a non-const-literal value to initialize a global static const object). (as with: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20912 )
(In reply to comment #11) > Is this optimization valid? Note that it will change the behavior of this c++ > program: You are correct Chris, this was an invalid optimization. This is a dup of bug 19320. Though we still have a "wrong code" bug with respect with const still being on the decl but that is PR 20912. *** This bug has been marked as a duplicate of 19320 ***
Uhm, wait. Perhaps the optimization would be invalid for your changed example from comment #5, but see below. But it will not be invalid for my initial testcase, where it missed to propagate 20.0 into setPosition. Why I think the transformation is valid _also_ for comment #5: See 3.6.2 #2: ------------------------------- An implementation is permitted to perform the initialization of an object of namespace scope with static storage duration as a static initialization even if such initialization is not required to be done statically, provided that * the dynamic version of the initialization does not change the value of any other object of namespace scope with static storage duration prior to its initialization, and * the static version of the initialization produces the same value in the initialized object as would be produced by the dynamic initialization if all objects not required to be initialized statically were initialized dynamically. ------------------------------------- It then goes on to provide an example which uses an inline function to dynamically initialize something, where comment #5 uses an arithmetic expression. So I think we are permitted to do this optimization, in both, by initial example, and Andrews example from comment #5. Reopening.
the example test case is invalid even with the gnu extension. As with static const int members, you must have a single out-of-class definition of the member EVEN IF the member is initialized in class. [9.4.2/4]
Yes, I determined that already in the initial report; to cite myself: > It's invalid for two reasons I think, first the missing definition, instead > of the declaration. [the second reason being the use of the GNU extension]. But it can be trivially made valid (just provide a definition), and I assumed this to be done for sake of this bugreport. Using the GNU extension this would then be valid, and _then_ the value is still not propagated to the method body. _That_'s what I'm complaining about, the missed optimization.
*** Bug 23975 has been marked as a duplicate of this bug. ***
The optimization question in Comment #11 was answered incorrectly. The C++ standard in fact requires that Y be initialized before the constructor is run; see [basic.start.init].
To clarify: you are saying that the response in comment #12 is wrong? -Chris
Subject: Re: [4.0/4.1 Regression] C++ front-end does not "inline" the static const double sabre at nondot dot org wrote: > ------- Comment #19 from sabre at nondot dot org 2005-10-11 16:58 ------- > To clarify: you are saying that the response in comment #12 is wrong? I don't understand Comment #12. However, Comment #13 says that "You are correct Chris, this was an invalid optimization"; that's incorrect, it is in fact a required optimization.
"required optimization" doesn't make sense to me. To put it more clearly, do you agree that this: "In particular, I think the C++ standard specifies that memory is zero initialized and then ctors (within a translation unit) are run in order." statement is true, and that the code in comment 11 is valid? If so, an attempt to implement this optimization just needs to be careful not the break this sort of case. -Chris
Subject: Re: [4.0/4.1 Regression] C++ front-end does not "inline" the static const double sabre at nondot dot org wrote: > ------- Comment #21 from sabre at nondot dot org 2005-10-11 17:03 ------- > "required optimization" doesn't make sense to me. To put it more clearly, do > you agree that this: > > "In particular, I think the C++ standard specifies that memory is zero > initialized and then ctors (within a translation unit) are run in order." > > statement is true, and that the code in comment 11 is valid? Initializers are not constructors. You'll have to go read the section of the standard I cited. The code in comment #11 is valid -- but it must print 3.0, which may not be what you expected.
Ah, very interesting. Thanks for the clarification. Should I file another PR about cases where this is mishandled? -Chris
At pinskia's request, I filed Bug 24312 to show the miscompilation due to mishandling this. I thought it would be better to keep this PR focused on the missed-optimization aspect. -Chris
Subject: Bug 21089 CVSROOT: /cvs/gcc Module name: gcc Changes by: mmitchel@gcc.gnu.org 2005-10-11 20:58:46 Modified files: gcc/cp : call.c init.c typeck.c ChangeLog gcc/testsuite : ChangeLog Added files: gcc/testsuite/g++.dg/init: float1.C Log message: PR c++/21089 * call.c (convert_like_real): Use decl_constant_value, not integral_constant_value. * init.c (constant_value_1): New function. (integral_constant_value): Use it. (decl_constant_value): Likewise. * typeck.c (decay_conversion): Use decl_constant_value, not integral_constant_value. PR c++/21089 * g++.dg/init/float1.C: New test. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&r1=1.555&r2=1.556 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/init.c.diff?cvsroot=gcc&r1=1.430&r2=1.431 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&r1=1.653&r2=1.654 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4919&r2=1.4920 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/float1.C.diff?cvsroot=gcc&r1=NONE&r2=1.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.6168&r2=1.6169
Subject: Bug 21089 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-4_0-branch Changes by: mmitchel@gcc.gnu.org 2005-10-11 20:59:53 Modified files: gcc/cp : call.c init.c typeck.c ChangeLog gcc/testsuite : ChangeLog Added files: gcc/testsuite/g++.dg/init: float1.C Log message: PR c++/21089 * call.c (convert_like_real): Use decl_constant_value, not integral_constant_value. * init.c (constant_value_1): New function. (integral_constant_value): Use it. (decl_constant_value): Likewise. * typeck.c (decay_conversion): Use decl_constant_value, not integral_constant_value. PR c++/21089 * g++.dg/init/float1.C: New test. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.531.2.9&r2=1.531.2.10 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/init.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.412.2.10&r2=1.412.2.11 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.616.2.19&r2=1.616.2.20 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.4648.2.124&r2=1.4648.2.125 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/float1.C.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.2.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.445&r2=1.5084.2.446
Fixed in 4.0.3.
(In reply to comment #18) > The optimization question in Comment #11 was answered incorrectly. > > The C++ standard in fact requires that Y be initialized before the constructor > is run; see [basic.start.init]. I disagree. In C++03, [basic.start.init] says Objects of POD types (3.9) with static storage duration initialized with constant expressions (5.19) shall be initialized before any dynamic initialization takes place. 5.19 [expr.const] says An arithmetic constant expression shall satisfy the requirements for an integral constant expression, except that — floating literals need not be cast to integral or enumeration type, and — conversions to floating point types are permitted. Note that this does not allow an arithmetic constant expression to involve const variables of floating point type, so "X + 2.0" is not an arithmetic constant expression, so Y is not required to have static initialization. But it is allowed to, as explained in comment #14. I think this distinction is not observable in C++03. But with C++0x constexpr it is; declaring Y as constexpr would be ill-formed unless X is also declared constexpr.
Jason -- I can't argue with that as a literal reading of the standard, but is there any reason why the standard doesn't allow const float variables in (not necessarily integral) constant expressions just as we allow const int variables? -- Mark