rjmccall added a comment. Minor comment request, then LGTM.
================ Comment at: lib/CodeGen/CGDecl.cpp:1102 + // Form a simple per-variable LRU cache of these values in case we find we + // want to reuse them. + llvm::GlobalVariable *&CacheEntry = InitializerConstants[&D]; ---------------- I don't think this cache is LRU in any meaningful sense; it just grows unbounded. There's this thing about requiring the initializer to match, but that's not LRU. ================ Comment at: lib/CodeGen/CGExpr.cpp:1429 +/// for instance if a block or lambda or a member of a local class uses a +/// const int variable or constexpr variable from an enclosing function. CodeGenFunction::ConstantEmission ---------------- rsmith wrote: > rjmccall wrote: > > Isn't the old comment correct here? This is mandatory because of the > > enclosing-local-scope issues; that might be an "optimization" in the > > language, but it's not an optimization at the IRGen level because Sema > > literally is forcing us to do it. > In the absence of this code, we'd emit the variable as a global constant from > `EmitDeclRefLValue` in the enclosing-local-scope case (as we now do in the > more-complex cases), so this should never be necessary for correct code > generation any more. Hmm, alright. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63157/new/ https://reviews.llvm.org/D63157 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
