rsmith added inline comments.
================ Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) + return Visit(V->getInit(), V->getType()); + return nullptr; ---------------- 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? 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
