[llvm] r241308 - DIBuilder: Now that DICompileUnit is distinct, stop using temporary nodes
Adrian Prantl
aprantl at apple.com
Mon Jul 6 09:38:40 PDT 2015
> On Jul 2, 2015, at 6:17 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>>
>> On 2015-Jul-02, at 15:32, Adrian Prantl <aprantl at apple.com> wrote:
>>
>> Author: adrian
>> Date: Thu Jul 2 17:32:52 2015
>> New Revision: 241308
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=241308&view=rev
>> Log:
>> DIBuilder: Now that DICompileUnit is distinct, stop using temporary nodes
>> for the arrays.
>
> Great!
>
>>
>> Modified:
>> llvm/trunk/include/llvm/IR/DIBuilder.h
>> llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
>> llvm/trunk/lib/IR/DIBuilder.cpp
>>
>> Modified: llvm/trunk/include/llvm/IR/DIBuilder.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=241308&r1=241307&r2=241308&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/DIBuilder.h (original)
>> +++ llvm/trunk/include/llvm/IR/DIBuilder.h Thu Jul 2 17:32:52 2015
>> @@ -36,14 +36,9 @@ namespace llvm {
>> Module &M;
>> LLVMContext &VMContext;
>>
>> - TempMDTuple TempEnumTypes;
>> - TempMDTuple TempRetainTypes;
>> - TempMDTuple TempSubprograms;
>> - TempMDTuple TempGVs;
>> - TempMDTuple TempImportedModules;
>> -
>> - Function *DeclareFn; // llvm.dbg.declare
>> - Function *ValueFn; // llvm.dbg.value
>> + DICompileUnit *CUNode; ///< The one compile unit created by this DIBuiler.
>> + Function *DeclareFn; ///< llvm.dbg.declare
>> + Function *ValueFn; ///< llvm.dbg.value
>>
>> SmallVector<Metadata *, 4> AllEnumTypes;
>> /// Track the RetainTypes, since they can be updated later on.
>>
>> Modified: llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfoMetadata.h?rev=241308&r1=241307&r2=241308&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/DebugInfoMetadata.h (original)
>> +++ llvm/trunk/include/llvm/IR/DebugInfoMetadata.h Thu Jul 2 17:32:52 2015
>> @@ -1085,12 +1085,21 @@ public:
>> /// deleted on a uniquing collision. In practice, uniquing collisions on \a
>> /// DICompileUnit should be fairly rare.
>> /// @{
>> + void replaceEnumTypes(DISubprogramArray N) {
>> + replaceOperandWith(4, N.get());
>> + }
>> + void replaceRetainedTypes(DISubprogramArray N) {
>> + replaceOperandWith(5, N.get());
>> + }
>> void replaceSubprograms(DISubprogramArray N) {
>> replaceOperandWith(6, N.get());
>> }
>> void replaceGlobalVariables(DIGlobalVariableArray N) {
>> replaceOperandWith(7, N.get());
>> }
>> + void replaceImportedEntities(DIGlobalVariableArray N) {
>> + replaceOperandWith(8, N.get());
>> + }
>> /// @}
>>
>> static bool classof(const Metadata *MD) {
>>
>> Modified: llvm/trunk/lib/IR/DIBuilder.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=241308&r1=241307&r2=241308&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
>> +++ llvm/trunk/lib/IR/DIBuilder.cpp Thu Jul 2 17:32:52 2015
>> @@ -58,8 +58,7 @@ public:
>> }
>>
>> DIBuilder::DIBuilder(Module &m, bool AllowUnresolvedNodes)
>> - : M(m), VMContext(M.getContext()), TempEnumTypes(nullptr),
>> - TempRetainTypes(nullptr), TempSubprograms(nullptr), TempGVs(nullptr),
>> + : M(m), VMContext(M.getContext()), CUNode(nullptr),
>> DeclareFn(nullptr), ValueFn(nullptr),
>> AllowUnresolvedNodes(AllowUnresolvedNodes) {}
>>
>> @@ -74,35 +73,37 @@ void DIBuilder::trackIfUnresolved(MDNode
>> }
>>
>> void DIBuilder::finalize() {
>> - TempEnumTypes->replaceAllUsesWith(MDTuple::get(VMContext, AllEnumTypes));
>> + if (CUNode) {
>
> This extra indentation is unfortunate. Could we shove all this CUNode
> stuff in a function? Or can we return early here (even better)?
>
r241465
>> + CUNode->replaceEnumTypes(MDTuple::get(VMContext, AllEnumTypes));
>
> Since we're changing this, can we be more aggressive? IMO, there's no
> value in having an empty array; it would be better to leave the argument
> "null". That'll clean up the assembly, too.
>
r241470
thanks,
adrian
>>
>> - SmallVector<Metadata *, 16> RetainValues;
>> - // Declarations and definitions of the same type may be retained. Some
>> - // clients RAUW these pairs, leaving duplicates in the retained types
>> - // list. Use a set to remove the duplicates while we transform the
>> - // TrackingVHs back into Values.
>> - SmallPtrSet<Metadata *, 16> RetainSet;
>> - for (unsigned I = 0, E = AllRetainTypes.size(); I < E; I++)
>> - if (RetainSet.insert(AllRetainTypes[I]).second)
>> - RetainValues.push_back(AllRetainTypes[I]);
>> - TempRetainTypes->replaceAllUsesWith(MDTuple::get(VMContext, RetainValues));
>> -
>> - DISubprogramArray SPs = MDTuple::get(VMContext, AllSubprograms);
>> - TempSubprograms->replaceAllUsesWith(SPs.get());
>> - for (auto *SP : SPs) {
>> - if (MDTuple *Temp = SP->getVariables().get()) {
>> - const auto &PV = PreservedVariables.lookup(SP);
>> - SmallVector<Metadata *, 4> Variables(PV.begin(), PV.end());
>> - DINodeArray AV = getOrCreateArray(Variables);
>> - TempMDTuple(Temp)->replaceAllUsesWith(AV.get());
>> + SmallVector<Metadata *, 16> RetainValues;
>> + // Declarations and definitions of the same type may be retained. Some
>> + // clients RAUW these pairs, leaving duplicates in the retained types
>> + // list. Use a set to remove the duplicates while we transform the
>> + // TrackingVHs back into Values.
>> + SmallPtrSet<Metadata *, 16> RetainSet;
>> + for (unsigned I = 0, E = AllRetainTypes.size(); I < E; I++)
>> + if (RetainSet.insert(AllRetainTypes[I]).second)
>> + RetainValues.push_back(AllRetainTypes[I]);
>> + CUNode->replaceRetainedTypes(MDTuple::get(VMContext, RetainValues));
>
> (Same comment for this and the remaining arrays.)
>
>> +
>> + DISubprogramArray SPs = MDTuple::get(VMContext, AllSubprograms);
>> + CUNode->replaceSubprograms(SPs.get());
>> + for (auto *SP : SPs) {
>> + if (MDTuple *Temp = SP->getVariables().get()) {
>> + const auto &PV = PreservedVariables.lookup(SP);
>> + SmallVector<Metadata *, 4> Variables(PV.begin(), PV.end());
>> + DINodeArray AV = getOrCreateArray(Variables);
>> + TempMDTuple(Temp)->replaceAllUsesWith(AV.get());
>> + }
>> }
>> - }
>>
>> - TempGVs->replaceAllUsesWith(MDTuple::get(VMContext, AllGVs));
>> + CUNode->replaceGlobalVariables(MDTuple::get(VMContext, AllGVs));
>>
>> - TempImportedModules->replaceAllUsesWith(MDTuple::get(
>> - VMContext, SmallVector<Metadata *, 16>(AllImportedModules.begin(),
>> - AllImportedModules.end())));
>> + CUNode->replaceImportedEntities(MDTuple::get(
>> + VMContext, SmallVector<Metadata *, 16>(AllImportedModules.begin(),
>> + AllImportedModules.end())));
>> + }
>>
>> // Now that all temp nodes have been replaced or deleted, resolve remaining
>> // cycles.
>> @@ -133,19 +134,11 @@ DICompileUnit *DIBuilder::createCompileU
>> assert(!Filename.empty() &&
>> "Unable to create compile unit without filename");
>>
>> - // TODO: Once we make DICompileUnit distinct, stop using temporaries here
>> - // (just start with operands assigned to nullptr).
>> - TempEnumTypes = MDTuple::getTemporary(VMContext, None);
>> - TempRetainTypes = MDTuple::getTemporary(VMContext, None);
>> - TempSubprograms = MDTuple::getTemporary(VMContext, None);
>> - TempGVs = MDTuple::getTemporary(VMContext, None);
>> - TempImportedModules = MDTuple::getTemporary(VMContext, None);
>> -
>> - DICompileUnit *CUNode = DICompileUnit::getDistinct(
>> + assert(!CUNode && "Can only make one compile unit per DIBuilder instance");
>> + CUNode = DICompileUnit::getDistinct(
>> VMContext, Lang, DIFile::get(VMContext, Filename, Directory), Producer,
>> - isOptimized, Flags, RunTimeVer, SplitName, Kind, TempEnumTypes.get(),
>> - TempRetainTypes.get(), TempSubprograms.get(), TempGVs.get(),
>> - TempImportedModules.get(), DWOId);
>> + isOptimized, Flags, RunTimeVer, SplitName, Kind, nullptr,
>> + nullptr, nullptr, nullptr, nullptr, DWOId);
>>
>> // Create a named metadata so that it is easier to find cu in a module.
>> // Note that we only generate this when the caller wants to actually
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list