nickdesaulniers added a comment. So, @rsmith are you ok with a patch like https://reviews.llvm.org/D76169 that removes those fixmes?
It makes sense to me that `const int foo[] = [ ...massive list...];` would take long to validate the entire initializer as all constant expressions, but I'm simply trying to allow: const int foo[] = [ ...massive list...]; int bar = foo[0]; ================ Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) + return Visit(V->getInit(), V->getType()); + return nullptr; ---------------- efriedma wrote: > rsmith wrote: > > efriedma wrote: > > > rsmith wrote: > > > > nickdesaulniers wrote: > > > > > efriedma wrote: > > > > > > You need to be more careful here; we can call ConstExprEmitter for > > > > > > arbitrary expressions. > > > > > "Be more careful" how? > > > > Here are some specific cases in which you need to be more careful > > > > because the code as-is would do the wrong thing: > > > > > > > > * when emitting a global's initializer in C++, where the value of the > > > > object denoted by the DeclRefExpr could have changed between its > > > > initialization and the expression we're currently emitting > > > > * when emitting anything other than a global initializer in C, where > > > > the value of a global could have changed after its emission > > > > * when emitting a reference to a non-global declaration in C (local > > > > variables might change between initialization and use) > > > > > > > > We would need to restrict this to the cases where the variable's value > > > > cannot possibly have changed between initialization and use. > > > > > > > > In C, that's (mostly) the case for a static storage variable referenced > > > > from the initializer of a static storage variable, for a thread storage > > > > variable referenced from the initializer of a static storage variable, > > > > or for a thread storage variable referenced from the initializer of a > > > > thread storage variable. Even then, this isn't strictly correct in the > > > > presence of DSOs, but I think it should be correct if the two variables > > > > are defined in the same translation unit. > > > > > > > > In C++, that's (mostly) the case when the variable is `const` or > > > > `constexpr` and has no mutable subobjects. (There's still the case > > > > where the reference happens outside the lifetime of the object -- for > > > > the most part we can handwave that away by saying it must be UB, but > > > > that's not true in general in the period of construction and period of > > > > destruction.) > > > > > > > > In both cases, the optimization is (arguably) still wrong if there are > > > > any volatile subobjects. > > > And this is why I don't want to duplicate the logic. :) > > > > > > I'd rather not make different assumptions for C and C++; instead, I'd > > > prefer to just use the intersection that's safe in both. I'm concerned > > > that we could come up with weird results for mixed C and C++ code, > > > otherwise. Also, it's easier to define the C++ rules because we can base > > > them on the constexpr rules in the standard. > > I agree we probably want the same outcome as D76169 provides, if either the > > performance is acceptable or we can find a way to avoid the performance > > cost for the cases we already accept. Perhaps we could get that outcome by > > ensuring that we try the CGExprConstant fast-path (at least in C > > compilations, maybe in general) before we consider the complete-but-slow > > evaluation strategy in ExprConstant? > I like the idea of guarding constant evaluation with a "fast path" that > doesn't actually compute the APValues in simple cases. We can just implement > the simple stuff, and fall back to the full logic for complicated stuff. > > My one concern is that we could go to a bunch of effort to emit a variable on > the fast path, then fall off the fast path later because "return a[0];" tries > to constant-fold a big array "a". > the CGExprConstant fast-path Sorry, what is "the CGExprConstant fast-path"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76096/new/ https://reviews.llvm.org/D76096 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
