Static inline variable and global are initialized in incorrect order with optimization on

[semushin-s]

Please take a look at the following example:

struct A {
	A() {
        a = 1;
    }

    int a;
};

struct B {
	B() : _a(_ca) {}

	A _a;
	static inline A _ca;
};

B b;

int main() {
	return b._a.a;
}

Currently it returns 0 with -O2 using clang:
https://gcc.godbolt.org/z/66hGqoaj8
and returns 1 with -O0. Compiled by all the other compilers, it results in 1 as well.

It seems initialization of static inline A _ca should be done earlier than initialization of B.
According to [basic.start.dynamic]:
if V has partially-ordered initialization, W does not have unordered initialization, and for every definition E of W there exists a definition D of V such that D is appearance-ordered before E ... V is sequenced before the initialization of W;

Let's say static inline A _ca is definition of V, B b is definition of W, seems by appearance order _ca should have been initialized before b, however with optimization a=1 initialization fails to happen before initializing B b apparently. Please correct me if my interpretation of the standard is wrong in this case.

[semushin-s]

Another interesting aspect of this issue, coming from how it was discovered originally. Initialization order may be affected by adding irrelevant global constants to original example.
Basically it comes to the following:

• Adding const std::string global constant on top makes initialization work expectedly in clang trunk but only with C++17. Link to godbolt
• On clang 17, it seems to behave expectedly with both C++17 and 20 after the same addition. Link to godbolt

Might not be that relevant since it might be just optimization noise, but makes it more surprising when encountering this in the wild.

[etl]

Adding any other struct member or increasing data size adds runtime initialization of _ca and returned value is correct:

struct A {
	A() {
        a = 1;
    }

    int a;
    int b;  // fixes issues
};

struct B {
	B() : _a(_ca) {}

	A _a;
	static inline A _ca;
};

B b;

int main() {
	return b._a.a;
}```

[etl]

@EugeneZelenko any chance this could be fixed?

[EugeneZelenko]

@etl: Sorry, I only help with issues handling.

[efriedma-quic]

This is mostly clang frontend issue. See #107154 for explanation. CC @rnk

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: Сергей Семушин (semushin-s)

Please take a look at the following example: ```cpp struct A { A() { a = 1; }

int a;

};

struct B {
B() : _a(_ca) {}

A _a;
static inline A _ca;

};

B b;

int main() {
return b._a.a;
}

Currently it returns 0 with `-O2` using clang:
https://gcc.godbolt.org/z/66hGqoaj8
and returns 1 with `-O0`. Compiled by all the other compilers, it results in 1 as well.

It seems initialization of `static inline A _ca` should be done earlier than initialization of `B`.
According to `[basic.start.dynamic]`:
`if V has partially-ordered initialization, W does not have unordered initialization, and for every definition E of W there exists a definition D of V such that D is appearance-ordered before E ... V is sequenced before the initialization of W;`

Let's say `static inline A _ca` is definition of `V`, `B b` is definition of `W`, seems by appearance order `_ca` should have been initialized before `b`, however with optimization `a=1` initialization fails to happen before initializing `B b` apparently. Please correct me if my interpretation of the standard is wrong in this case.


</details>

[rnk]

I think this is a globalopt bug or an llvm.global_ctors design flaw. Apparently globalopt considers it legal valid to emulate the load from B::_ca, see the debug logs here:

FULLY EVALUATED GLOBAL CTOR FUNCTION '_GLOBAL__sub_I_example.cpp' to 1 stores.
GLOBAL DEAD: ; Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(readwrite, argmem: none, inaccessiblemem: none) uwtable
define internal void @_GLOBAL__sub_I_example.cpp() #3 section ".text.startup" !dbg !58 {
entry:
    #dbg_value(ptr @b, !61, !DIExpression(), !65)
  %0 = load i32, ptr @_ZN1B3_caE, align 4, !dbg !69, !tbaa !70
  store i32 %0, ptr @b, align 4, !dbg !69, !tbaa !70
  ret void
}

I think the object file implementation efficiency benefits of merging redundant initializers for inline global variables by using separate llvm.global_ctors entries significantly outweigh the benefits of globalopt, especially in a C++ ecosystem where constinit exists.

If our options are:

1. stop globalopt from loading from mutable globals not previously initialized in the current pass attempt
2. make clang combine all inline variable initializers in every TU into a single combined initializer function

I would way that option 1 is better. It should result in smaller binaries with faster startup time and less initialization code.

[efriedma-quic]

I think we'd have to completely turn off evaluating functions after the first one we fail to evaluate. If we forbid reordering, there's no realistic way to tell if a reordering has a user-visible impact.

Maybe that doesn't have much practical impact for modern C++.