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

David Blaikie dblaikie at gmail.com
Fri Sep 12 10:00:52 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.
----------------
How's this work? What will we do at the finalize step?

================
Comment at: lib/IR/DIBuilder.cpp:1240
@@ +1239,3 @@
+
+  if (Temporary)
+    Node = MDNode::getTemporary(VMContext, Elts);
----------------
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.

http://reviews.llvm.org/D5328






More information about the llvm-commits mailing list