rjmccall added inline comments.

================
Comment at: clang/test/CodeGenCXX/attr-loader-uninitialized.cpp:23
+// CHECK: @nominally_value_init = global i32 undef
+int nominally_value_init  [[clang::loader_uninitialized]] = 4;
+
----------------
Quuxplusone wrote:
> rjmccall wrote:
> > JonChesterfield wrote:
> > > Quuxplusone wrote:
> > > > This test case is identical to line 36 of 
> > > > clang/test/Sema/attr-loader-uninitialized.cpp, where you say you don't 
> > > > want it to compile at all.
> > > > 
> > > > I think you need a clearer idea of how this interacts with 
> > > > initializers. Is it merely supposed to eliminate the 
> > > > //zero-initialization// that happens before the user-specified 
> > > > construction/initialization, or is it supposed to compete with the 
> > > > user-specified construction/initialization?
> > > > 
> > > > That is, for
> > > > 
> > > >     nontrivial unt [[clang::loader_uninitialized]];
> > > > 
> > > > is it merely supposed to call `unt::unt()` on a chunk of undef memory 
> > > > (instead of the usual chunk of zeroed memory), or is it supposed to 
> > > > skip the constructor entirely? And for
> > > > 
> > > >     int x [[clang::loader_uninitialized]] = foo();
> > > > 
> > > > is it merely supposed to call `foo()` and assign the result to a chunk 
> > > > of undef memory (instead of the usual chunk of zeroed memory), or is it 
> > > > supposed to skip the initialization entirely?
> > > I think you commented while the first working piece of sema landed. My 
> > > thinking is relatively clear but my understanding of clang's semantic 
> > > analysis is a work in progress!
> > > 
> > > Initializers (`= foo()`) are straightforward. Error on the basis that the 
> > > attribute effectively means `= undef`, and one should not have two 
> > > initializers. A test case is now added for that (and now passes).
> > > 
> > > The codegen I want for a default constructed global marked with this 
> > > variable is:
> > > - global variable allocated, with undef as the original value
> > > - default constructor call synthesized
> > > - said default constructor set up for invocation from crt, before main, 
> > > writing over the undef value
> > > 
> > > Where the default constructor can be optimized as usual, e.g. if it 
> > > always writes a constant, we can init with that constant instead of the 
> > > undef and elide the constructor.
> > > 
> > > I don't have that actually working yet - the constructor call is not 
> > > being emitted, so we just have the undef global.
> > > 
> > > I think it's important to distinguish between the values of the bits when 
> > > the program is loaded and whether constructor/destructors are called, as 
> > > one could want any combination of the two.
> > I think Arthur is suggesting that it would be useful to allow the attribute 
> > to be used in conjunction with an initializer in C++, since if the 
> > initializer has to be run dynamically, we can still meaningfully suppress 
> > the static zero-initialization.   That is, we've accepted that it's useful 
> > to do this when *default-initializing* a global, but it's actually useful 
> > when doing *any* kind of dynamic initialization.
> > 
> > Maybe we can leave it as an error in C++ when the initializer is a constant 
> > expression.  Although that might be unnecessarily brittle if e.g. the 
> > constructor is `constexpr` in one library version but not another.
> No, that's exctly what I mean. You seem to be holding two contradictory ideas 
> simultaneously:
> 
>     [[loader_uninitialized]] X x = X{};  // two initializers, therefore error
> 
>     [[loader_uninitialized]] X x {}; // one initializer plus one constructor, 
> therefore fine
> 
> In C++, these two declarations have identical semantics. It doesn't make 
> sense to say that one of them "calls a constructor" and the other one "has an 
> initializer." They're literally the same thing.
> 
> Similarly in both C99 and C++ with plain old ints:
> 
>     [[loader_uninitialized]] int x = foo();
> 
> This means "call foo and dynamically initialize x with the result," just as 
> surely as
> 
>     [[loader_uninitialized]] X x = X();
> 
> means "call X::X and dynamically initialize x with the result." Having one 
> rule for dynamic initializers of primitive types and a separate rule for 
> dynamic initializers of class types doesn't work.
> 
> Furthermore, "dynamic initialization" can be promoted to compile-time:
> 
>     [[loader_uninitialized]] int x = 42;
>     [[loader_uninitialized]] std::string_view x = "hello world";
> 
> It wouldn't make semantic sense to say that one of these has "two 
> initializers" and the other has "one initializer," because both of the 
> initializations end up happening at compile time and getting put into .data.
> Similarly in both C99 and C++ with plain old ints:

C does not allow non-constant initialization of global variables.

Let  me try to explain what we're thinking here.  If the variable provides both 
an initializer and this attribute, and the compiler can successfully perform 
constant initialization (as it's required to do in C, and as it often does in 
C++), then the attribute has probably become meaningless because we're not 
going to suppress that constant initialization.  The compiler strongly prefers 
to diagnose meaningless attributes in some way, because they often indirectly 
indicate a bug.  For example, maybe the initializer isn't supposed to be there 
but it's hidden behind a huge mess of macros; or maybe somebody added a 
constant initializer because they changed the variable schema to make that 
possible, and so the programmer should now remove the attribute and the dynamic 
initialization code that went along with it.  Compared to that, the C++ use 
case of "allow a dynamic initializer but suppress static zero-initialization" 
is pretty marginal; that doesn't mean we absolutely shouldn't support it, but 
it does suggest that we shouldn't prioritize it over other aspects of the 
feature, like catching that possible bug.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74361/new/

https://reviews.llvm.org/D74361



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to