[PATCH] D19072: [CodeGen] Split CGDebugInfo into CGDebugInfo and CGDebugDwarf

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 18 14:06:28 PDT 2016


rnk marked an inline comment as done.

================
Comment at: lib/CodeGen/CGDebugInfo.h:97
@@ -273,3 +96,3 @@
   /// Set the main CU's DwoId field to \p Signature.
-  void setDwoId(uint64_t Signature);
+  virtual void setDwoId(uint64_t Signature) = 0;
 
----------------
aprantl wrote:
> Why does this need to be virtual?
I guess it adjusts the CU, which is shared. OK, I'll move it back to CGDebugInfo. It just felt fairly dwarf-specific.

================
Comment at: lib/CodeGen/CGDebugInfo.h:298
@@ +297,3 @@
+  virtual llvm::DISubroutineType *
+  getOrCreateFunctionType(const Decl *D, QualType FnType, llvm::DIFile *F) = 0;
+
----------------
aprantl wrote:
> As you said, this isn't very clean. Can you outline how this can be improved later? Will CodeView still need some form of DISubroutineTypes?
I think the parameters are right for CodeView and DWARF, but the return type will have to be DITypeRef so that I can get a DITypeIndex out of here. My next patch changes this and a few other return types.

================
Comment at: lib/CodeGen/CGDebugInfo.h:348
@@ -493,1 +347,3 @@
+  virtual llvm::DIType *EmitTypeForVarWithBlocksAttr(const VarDecl *VD,
+                                                     uint64_t *OffSet) = 0;
 
----------------
aprantl wrote:
> Should this be part of CGDebugInfoDwarf?
The implementation directly uses DIBuilder to make a DICompositeType. If we wanted to make this logic common, we would have to first build a synthetic clang AST RecordDecl and then send that through getOrCreateType.

================
Comment at: lib/CodeGen/CGDebugInfo.h:350
@@ -493,4 +349,3 @@
 
-  /// Get the vtable name for the given class.
-  StringRef getVTableName(const CXXRecordDecl *Decl);
+  virtual void retainType(void *OpaqueDecl) {};
 
----------------
aprantl wrote:
> 1. Why does retainType need to be virtual?
> 2. I assume that the list of retained types is meaningless for CodeView? Could this be moved to CGDebugInfoDwarf instead?
1. Yes, I intend to keep retainType as a no-op for CodeView. If a type makes it to the stream, it's up to the linker to decide whether it keeps it or not.
2. Are you asking if I can sink all the callers of this method down into CGDebugDwarf? Possibly, but EmitGlobalVariable directly references it, and I would expect that to be part of CGDebugInfo. We could come up with a different level of indirection, such as getOrCreateGlobalVariableType, similar to getOrCreateFunctionType and sink that logic to DWARF.


http://reviews.llvm.org/D19072





More information about the cfe-commits mailing list