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

Frederic Riss friss at apple.com
Mon Sep 15 01:58:58 PDT 2014


================
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.
----------------
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.

================
Comment at: lib/IR/DIBuilder.cpp:1240
@@ +1239,3 @@
+
+  if (Temporary)
+    Node = MDNode::getTemporary(VMContext, Elts);
----------------
dblaikie wrote:
> friss wrote:
> > dblaikie wrote:
> > > Wonder if there's a nicer way to do this - templated & pass in a lambda?
> > > 
> > > But it's nothing drastic either way.
> > I had a /simpler/ way, but I'm not sure you'll find it nicer (In fact I changed it because I thought you'd not like it). In the first incarnation of the patch, I just checked the isDefinition value and decided upon temporary or not based on that. I audited all users and if I didn't miss anything, none passes false for isDefinition today. Thus only the ones I will introduce  would need to care about the temporary aspect.
> > 
> > And similarly for the variables (where I introduced a new argument for isDefinition that I lost in this version of the patch).
> > 
> Yeah - the only function declarations we create currently are member functions for which we use "createMethod" - though that's a subtle distinction that's probably not ideal to rely on (one day we might decide that createMethod can go away & we can just create declarations - declarations that would never be replaced with definitions).
> 
> What about passing in a function pointer? (&MDNode::getTemporary or &MDNode::get) Avoids a boolean argument (which is always a bit unclear at a call site without a comment) except we need to do other work (AllSubprograms and AllGVs)...
> 
> I guess we could rely on the fact that we only emit declarations when there's something that refers to them (so they don't necessarily need to be in retention lists), but that's still pretty subtle.
I agree regarding the general meaning of a boolean at the call site. The function pointer would be nice if we didn't do any special processing depending on the value, but I thing testing for a function pointer of a certain value would look odd.

And regarding not putting the declarations in the retention lists, I replied above.

What about your first suggestion of a lambda? It would be pretty explicit.

http://reviews.llvm.org/D5328






More information about the llvm-commits mailing list