djtodoro marked 3 inline comments as done. djtodoro added inline comments.
================ Comment at: lib/CodeGen/CGDebugInfo.cpp:4537 + CGM.getLangOpts().Optimize) { + for (auto &SP : DeclCache) { + auto *D = SP.first; ---------------- aprantl wrote: > djtodoro wrote: > > aprantl wrote: > > > Just looking at the type declarations in CGDebugInfo.h: Why not iterate > > > over the `SPCache` directly? Shouldn't that contain all Function > > > declarations only? > > I tried it, but `SPCache` is empty at this point. > Where is it emptied? Just grepping through CGDebugInfo did not make this > obvious to me. The `SPCache` actually gets filled only in the case of `CXXMemberFunction`. In the other cases of `SP` production there is only filling of `DeclCache`. Should we use it like this or ? ================ Comment at: lib/CodeGen/CGDebugInfo.cpp:3885 + if (CGM.getCodeGenOpts().EnableDebugEntryValues && + CGM.getLangOpts().Optimize && ArgNo) { + if (auto *PD = dyn_cast<ParmVarDecl>(VD)) ---------------- aprantl wrote: > We shouldn't query CGM.getLangOpts().Optimize. If we don't want this to > happen at -O0, we shouldn't set EnableDebugEntryValues at a higher level > (Driver, CompilerInvocation, ...) ..Otherwise inevitably someone will query > one but not the other and things will go out of sync. I agree. Thanks! ================ Comment at: lib/CodeGen/CGDebugInfo.cpp:4535 DBuilder.retainType(cast<llvm::DIType>(MD)); + if (CGM.getCodeGenOpts().EnableDebugEntryValues && ---------------- aprantl wrote: > Please add either a top-level comment about what this block is doing or > perhaps factor this out into a descriptively named static function. Sure. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits