[PATCH] Debug info: Support fragmented variables.

Chandler Carruth chandlerc at gmail.com
Mon Dec 22 12:55:41 PST 2014


LGTM with a minor change below, thanks for all the work explaining things. =]


================
Comment at: lib/Transforms/Scalar/SROA.cpp:3728
@@ -3703,1 +3727,3 @@
+
+  DIB.reset(new DIBuilder(*F.getParent(), /*AllowUnresolved*/ false));
 
----------------
aprantl wrote:
> chandlerc wrote:
> > Is there a particular cost to constructiog DIBuilder objects? I ask because, were this an IRBuilder, I would have said to just construct it directly in each place we need it, because there is no real cast associated with it. If the same is true for DIBuilder, then I'd suggest the same change. If DIBuilder works differently, I'd like to know how to better use it. =]
> Compared to an IRBuilder it looks as if DIBuilder has to initialize quite a few members (that will never be used). But I'm unsure whether there actually is a cost to initializing DenseMaps and SmallVectors? Otherwise I'd be happy to instantiate the DIBuilder on the fly. Alternatively we could factor out a less capable DIBuilder base class that does not have all these members.
>   
> ```
>     Module &M;
>     LLVMContext &VMContext;
> 
>     MDNode *TempEnumTypes;
>     MDNode *TempRetainTypes;
>     MDNode *TempSubprograms;
>     MDNode *TempGVs;
>     MDNode *TempImportedModules;
> 
>     Function *DeclareFn;     // llvm.dbg.declare
>     Function *ValueFn;       // llvm.dbg.value
> 
>     SmallVector<Metadata *, 4> AllEnumTypes;
>     /// Track the RetainTypes, since they can be updated later on.
>     SmallVector<TrackingMDNodeRef, 4> AllRetainTypes;
>     SmallVector<Metadata *, 4> AllSubprograms;
>     SmallVector<Metadata *, 4> AllGVs;
>     SmallVector<TrackingMDNodeRef, 4> AllImportedModules;
> 
>     /// \brief Track nodes that may be unresolved.
>     SmallVector<TrackingMDNodeRef, 4> UnresolvedNodes;
>     bool AllowUnresolvedNodes;
> 
>     /// Each subprogram's preserved local variables.
>     DenseMap<MDNode *, std::vector<TrackingMDNodeRef>> PreservedVariables;
> ```
Interesting.

I suggest you just build them on the fly here, and if these members ever end up being a problem, we can go in and avoid their cost much as you describe. No need to do that until we see a problem, but it seems nicer than having a heap-allocated builder.

http://reviews.llvm.org/D2680

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list