aprantl added a comment.
It would be awesome if you could also add an end-to-end test to the
debuginfo-tests repository so we can verify that this actually works in LLDB
and GDB.
================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2358
+ if (auto *SizeNode = getVLASizeExpressionForType(EltTy))
+ Subscripts.push_back(DBuilder.getOrCreateSubrange(0, SizeNode));
+ else
----------------
perhaps write sizeNode/Count to a variable, so you don't have to duplicate the
expression?
================
Comment at: lib/CodeGen/CGDebugInfo.cpp:3473
CGBuilderTy &Builder) {
+ llvm::Optional<llvm::Metadata *> Optional;
+ EmitDeclare(VD, Storage, ArgNo, Optional, Builder);
----------------
Could this function be replaced by a default argument instead?
================
Comment at: lib/CodeGen/CGDebugInfo.h:86
+ /// represented by instantiated Metadata nodes.
+ llvm::SmallDenseMap<QualType, llvm::Metadata *> SizeExprCache;
+
----------------
I'm expecting VLA's to not be very common, should we use a regular DenseMap
instead?
================
Comment at: lib/CodeGen/CGDebugInfo.h:317
+ llvm::Metadata *getVLASizeExpressionForType(QualType Ty) {
+ if (SizeExprCache.count(Ty))
+ return SizeExprCache[Ty];
----------------
This is performing the lookup twice. If you use .find() instead it will be more
efficient. We also don't use accessor functions like this for any of the other
caches. If you think that they help, perhaps make this more generic and useful
for all caches?
================
Comment at: lib/CodeGen/CGDebugInfo.h:404
+ void EmitDeclareOfAutoVariable(const VarDecl *Decl, llvm::Value *AI,
+ llvm::Optional<llvm::Metadata *>
&MetadataDecl,
+ CGBuilderTy &Builder);
----------------
do we need the qualifier on Optional?
================
Comment at: lib/CodeGen/CGDecl.cpp:1125
+ // If we have debug info enabled, describe the VLA dimensions properly.
+ if (EmitDebugInfo) {
+ QualType Type1D = Ty;
----------------
Please move this code into a member function of CGDebugInfo.
================
Comment at: lib/CodeGen/CGDecl.cpp:1137
+ // Allocate memory for the address of the vla expression
+ // We can use this for debugging purposes
+ auto SizeExprAddr = CreateDefaultAlignTempAlloca(
----------------
`.`
================
Comment at: lib/CodeGen/CodeGenFunction.cpp:1963
+ assert(VlaSize->getType() == SizeTy);
+ return std::pair<llvm::Value *, QualType>(VlaSize, Vla->getElementType());
+}
----------------
`return {VlaSize, Vla->getElementType()};`
https://reviews.llvm.org/D41698
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
