[PATCH] Add DIBuilder functions to build RAUWable DIVariables and DIFunctions.

David Blaikie dblaikie at gmail.com
Mon Sep 15 11:55:32 PDT 2014


================
Comment at: lib/IR/DIBuilder.cpp:1173
@@ -1141,9 +1172,3 @@
 
-/// createFunction - Create a new descriptor for the specified function.
-DISubprogram DIBuilder::createFunction(DIDescriptor Context, StringRef Name,
-                                       StringRef LinkageName, DIFile File,
-                                       unsigned LineNo, DICompositeType Ty,
-                                       bool isLocalToUnit, bool isDefinition,
-                                       unsigned ScopeLine, unsigned Flags,
-                                       bool isOptimized, Function *Fn,
-                                       MDNode *TParams, MDNode *Decl) {
+static DISubprogram
+createFunctionHelper(LLVMContext &VMContext, DIDescriptor Context, StringRef Name,
----------------
Since this is a static function, you can make it a template and avoid the std::function dynamic overhead - but I suppose that's a binary size V execution time tradeoff... I don't feel too strongly about it either way, though I'd probably have gone with a template rather than std::function, personally.

================
Comment at: lib/IR/DIBuilder.cpp:1248
@@ +1247,3 @@
+                              Flags, isOptimized, Fn, TParams, Decl,
+                              [this] (ArrayRef<Value *> Elts) {
+                                return MDNode::getTemporary(VMContext, Elts);
----------------
Not sure what the style guide says, but I usually just capture by "[&]" rather than listing the me explicitly (goes for here and above)

================
Comment at: lib/IR/DIBuilder.cpp:1072
@@ +1071,3 @@
+    Node = MDNode::get(VMContext, Elts);
+    // Do not put temporary nodes in the GV list. We'll add the
+    // necessary ones at the finalize step.
----------------
friss wrote:
> dblaikie wrote:
> > How's this work? What will we do at the finalize step?
> I realise I hadn't replied to this. In followup patches, in CGDebugInfo::finalize(), forward declarations which have seen definitions are RAUWed and dropped, the other ones are placed into the corresponding retention list. They need to be in retention lists, not because thy would be lost otherwise, but simply because these lists are emitted first in DwarfUnit. And the imported_declaration stuff expects that all the referenced DIEs are already emitted at the time they are processed.
Cna we just fix the imported_declaration stuff so it doesn't expect all the referenced DIEs to be already emitted (it can use the usual "getOrCreate" routines that lazily construct stuff if it's not already been constructed).

Also you said: " forward declarations which have seen definitions are RAUWed and dropped, the other ones are placed into the corresponding retention list." - they'll still need to be RAUW'd with themselves to promote them from temporary to non-temporary. I imagine this should look much the same as the code we already have for handling temporary type forward declarations, no? I'd expect to see it pretty much exactly the same/symmetric - is that not going to be the case?

http://reviews.llvm.org/D5328






More information about the llvm-commits mailing list