From ff8adf7b45121aada0b4f4403b0f45a6e2dfe475 Mon Sep 17 00:00:00 2001 From: Yong He Date: Mon, 19 Feb 2018 13:00:51 -0500 Subject: Fix IR memory leaks. 1, make IRModule class own a memory pool for all IR object allocations 2. For now, we allow IR objects to own other (externally) heap allocated objects, such as String, List and RefPtrs by tracking all IR objects that has been allocated for the IRModule in a list named `IRModule::irObjectsToFree`. and call destructor for all these objects upon the destruction of the IRModule. In the long term, we should eliminate the use of all these externally allocated types in IR system and get rid of this tracking and explicit destructor calls. 3. remove non-generic `createValueImpl` functions and retain only generic versions in IRBulider so we can properly call the constructor of the IR types to set up virtual tables correctly for destructor dispatching. 4. add `MemoryPool` class for allocation of the IR objects. 5. Make sure we are disposing IRSpecContexts when we are done with the specialized IR module. 6. Add `_CrtDumpMemoryLeaks()` calls to check memory leaks upon destruction of a Slang session. If we are to support multiple sessions at a time, this call should probably be replaced with the more advanced MemoryState versions of the memory leak checker. --- source/slang/emit.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index 672798359..b8fbc350e 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -8158,6 +8158,7 @@ String emitEntryPoint( // TODO: do we want to emit directly from IR, or translate the // IR back into AST for emission? visitor.emitIRModule(&context, irModule); + destroyIRSpecializationState(irSpecializationState); } String code = sharedContext.sb.ProduceString(); @@ -8189,7 +8190,7 @@ String emitEntryPoint( finalResultBuilder << code; String finalResult = finalResultBuilder.ProduceString(); - + return finalResult; } -- cgit v1.2.3 From 5de62bbe4dddc64895ddb17c4eb3572c3c9be248 Mon Sep 17 00:00:00 2001 From: Yong He Date: Mon, 19 Feb 2018 19:53:45 -0500 Subject: more to fixing memory leaks 1. reorder destruction order of several key classes to avoid using deleted IR objects when destroying Types 2. remove Session::canonicalTypes and make each Type own a RefPtr to the canonicalType, to allow types to be destroyed along with each IRModule it belongs to. --- source/core/smart-pointer.h | 14 ++++++++++++++ source/slang/compiler.h | 2 +- source/slang/emit.cpp | 16 ++++++++-------- source/slang/ir-insts.h | 27 ++++++++++++++++++++++++++ source/slang/ir.cpp | 15 +++++++++++++-- source/slang/ir.h | 20 ++++++++++++++++++-- source/slang/slang.cpp | 13 +++++++++++++ source/slang/syntax-base-defs.h | 4 ++-- source/slang/syntax.cpp | 42 ++++++++++++++++++++--------------------- source/slang/type-defs.h | 24 +++++++++++------------ 10 files changed, 128 insertions(+), 49 deletions(-) (limited to 'source/slang/emit.cpp') diff --git a/source/core/smart-pointer.h b/source/core/smart-pointer.h index 17d6caaa4..bc1683a5b 100644 --- a/source/core/smart-pointer.h +++ b/source/core/smart-pointer.h @@ -35,6 +35,11 @@ namespace Slang referenceCount++; } + void decreaseReference() + { + --referenceCount; + } + void releaseReference() { SLANG_ASSERT(referenceCount != 0); @@ -192,6 +197,15 @@ namespace Slang return pointer; } + T* detach() + { + if (pointer) + dynamic_cast(pointer)->decreaseReference(); + auto rs = pointer; + pointer = nullptr; + return rs; + } + private: T* pointer; diff --git a/source/slang/compiler.h b/source/slang/compiler.h index 7ead5d07e..b1cebc2a1 100644 --- a/source/slang/compiler.h +++ b/source/slang/compiler.h @@ -450,7 +450,6 @@ namespace Slang Dictionary> builtinTypes; Dictionary magicDecls; - List> canonicalTypes; void initializeTypes(); @@ -505,6 +504,7 @@ namespace Slang RefPtr const& scope, String const& path, String const& source); + ~Session(); }; } diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index b8fbc350e..46c0b20a9 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -8084,17 +8084,17 @@ String emitEntryPoint( EmitVisitor visitor(&context); + // We are going to create a fresh IR module that we will use to + // clone any code needed by the user's entry point. + IRSpecializationState* irSpecializationState = createIRSpecializationState( + entryPoint, + programLayout, + target, + targetRequest); { TypeLegalizationContext typeLegalizationContext; typeLegalizationContext.session = entryPoint->compileRequest->mSession; - // We are going to create a fresh IR module that we will use to - // clone any code needed by the user's entry point. - IRSpecializationState* irSpecializationState = createIRSpecializationState( - entryPoint, - programLayout, - target, - targetRequest); IRModule* irModule = getIRModule(irSpecializationState); typeLegalizationContext.irModule = irModule; @@ -8158,8 +8158,8 @@ String emitEntryPoint( // TODO: do we want to emit directly from IR, or translate the // IR back into AST for emission? visitor.emitIRModule(&context, irModule); - destroyIRSpecializationState(irSpecializationState); } + destroyIRSpecializationState(irSpecializationState); String code = sharedContext.sb.ProduceString(); sharedContext.sb.Clear(); diff --git a/source/slang/ir-insts.h b/source/slang/ir-insts.h index 1cfe83b97..aaebb9973 100644 --- a/source/slang/ir-insts.h +++ b/source/slang/ir-insts.h @@ -33,6 +33,11 @@ struct IRLayoutDecoration : IRDecoration enum { kDecorationOp = kIRDecorationOp_Layout }; RefPtr layout; + virtual void dispose() override + { + IRDecoration::dispose(); + layout = nullptr; + } }; enum IRLoopControl @@ -52,6 +57,11 @@ struct IRTargetSpecificDecoration : IRDecoration { // TODO: have a more structured representation of target specifiers String targetName; + virtual void dispose()override + { + IRDecoration::dispose(); + targetName = String(); + } }; struct IRTargetDecoration : IRTargetSpecificDecoration @@ -64,6 +74,11 @@ struct IRTargetIntrinsicDecoration : IRTargetSpecificDecoration enum { kDecorationOp = kIRDecorationOp_TargetIntrinsic }; String definition; + virtual void dispose()override + { + IRTargetSpecificDecoration::dispose(); + definition = String(); + } }; // @@ -73,6 +88,11 @@ struct IRTargetIntrinsicDecoration : IRTargetSpecificDecoration struct IRDeclRef : IRValue { DeclRef declRef; + virtual void dispose() override + { + IRValue::dispose(); + declRef = decltype(declRef)(); + } }; // An instruction that specializes another IR value @@ -333,6 +353,13 @@ struct IRWitnessTable : IRGlobalValue RefPtr genericDecl; DeclRef subTypeDeclRef, supTypeDeclRef; IRValueList entries; + virtual void dispose() override + { + IRGlobalValue::dispose(); + genericDecl = decltype(genericDecl)(); + subTypeDeclRef = decltype(subTypeDeclRef)(); + supTypeDeclRef = decltype(supTypeDeclRef)(); + } }; // An instruction that yields an undefined value. diff --git a/source/slang/ir.cpp b/source/slang/ir.cpp index 8667a67bd..65f792577 100644 --- a/source/slang/ir.cpp +++ b/source/slang/ir.cpp @@ -2648,6 +2648,12 @@ namespace Slang #endif } + void IRValue::dispose() + { + IRObject::dispose(); + type = decltype(type)(); + } + // Insert this instruction into the same basic block // as `other`, right before it. void IRInst::insertBefore(IRInst* other) @@ -4739,9 +4745,9 @@ namespace Slang if( !module ) { module = builder->createModule(); - sharedBuilder->module = module; } + sharedBuilder->module = module; sharedContext->module = module; sharedContext->originalModule = originalModule; sharedContext->target = target; @@ -4777,6 +4783,12 @@ namespace Slang IRSharedSpecContext* getSharedContext() { return &sharedContextStorage; } IRSpecContext* getContext() { return &contextStorage; } + ~IRSpecializationState() + { + newProgramLayout = nullptr; + contextStorage = IRSpecContext(); + sharedContextStorage = IRSharedSpecContext(); + } }; IRSpecializationState* createIRSpecializationState( @@ -4816,7 +4828,6 @@ namespace Slang auto context = state->getContext(); context->shared = sharedContext; context->builder = &sharedContext->builderStorage; - // Create the GlobalGenericParamSubstitution for substituting global generic types // into user-provided type arguments auto globalParamSubst = createGlobalGenericParamSubstitution(entryPointRequest, programLayout, context, originalIRModule); diff --git a/source/slang/ir.h b/source/slang/ir.h index 4cdd1f1de..fdca73a83 100644 --- a/source/slang/ir.h +++ b/source/slang/ir.h @@ -119,6 +119,10 @@ enum IRDecorationOp : uint16_t struct IRObject { bool isDestroyed = false; + virtual void dispose() + { + isDestroyed = true; + } virtual ~IRObject() { isDestroyed = true; @@ -134,7 +138,6 @@ struct IRDecoration : public IRObject IRDecoration* next; IRDecorationOp op; - virtual ~IRDecoration() = default; }; // Use AST-level types directly to represent the @@ -181,6 +184,8 @@ struct IRValue : public IRObject // Free a value (which needs to have been removed // from its parent, had its uses eliminated, etc.) void deallocate(); + + virtual void dispose() override; }; // Values that are contained in a doubly-linked @@ -492,6 +497,11 @@ struct IRGlobalValue : IRValue void removeFromParent(); void moveToEnd(); + virtual void dispose() override + { + IRValue::dispose(); + mangledName = String(); + } }; /// @brief A global value that potentially holds executable code. @@ -541,6 +551,12 @@ struct IRFunc : IRGlobalValueWithCode // which are actually the parameters of the first // block. IRParam* getFirstParam(); + + virtual void dispose() override + { + IRGlobalValueWithCode::dispose(); + genericDecls = decltype(genericDecls)(); + } }; // A module is a parent to functions, global variables, types, etc. @@ -563,7 +579,7 @@ struct IRModule : RefObject { for (auto val : irObjectsToFree) if (!val->isDestroyed) - val->~IRObject(); + val->dispose(); irObjectsToFree = List(); } }; diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp index 0f5d8e17a..b65ec5615 100644 --- a/source/slang/slang.cpp +++ b/source/slang/slang.cpp @@ -734,6 +734,19 @@ void Session::addBuiltinSource( loadedModuleCode.Add(syntax); } +Session::~Session() +{ + // free all built-in types first + errorType = nullptr; + initializerListType = nullptr; + overloadedType = nullptr; + irBasicBlockType = nullptr; + + builtinTypes = decltype(builtinTypes)(); + // destroy modules next + loadedModuleCode = decltype(loadedModuleCode)(); +} + } // implementation of C interface diff --git a/source/slang/syntax-base-defs.h b/source/slang/syntax-base-defs.h index 2eb130863..2c15f7215 100644 --- a/source/slang/syntax-base-defs.h +++ b/source/slang/syntax-base-defs.h @@ -119,7 +119,7 @@ public: protected: virtual bool EqualsImpl(Type * type) = 0; - virtual Type* CreateCanonicalType() = 0; + virtual RefPtr CreateCanonicalType() = 0; Type* canonicalType = nullptr; RefPtr canonicalTypeRefPtr; @@ -227,7 +227,7 @@ RAW( return rs; } typedef List, RefPtr>> WitnessTableLookupTable; -) + ) // The witness tables for each interface this actual type implements SYNTAX_FIELD(WitnessTableLookupTable, witnessTables) END_SYNTAX_CLASS() diff --git a/source/slang/syntax.cpp b/source/slang/syntax.cpp index e050fc977..62e467a44 100644 --- a/source/slang/syntax.cpp +++ b/source/slang/syntax.cpp @@ -18,7 +18,7 @@ namespace Slang return basicType->baseType == this->baseType; } - Type* BasicExpressionType::CreateCanonicalType() + RefPtr BasicExpressionType::CreateCanonicalType() { // A basic type is already canonical, in our setup return this; @@ -194,9 +194,12 @@ void Type::accept(IValVisitor* visitor, void* extra) if (!et->canonicalType) { // TODO(tfoley): worry about thread safety here? - et->canonicalType = et->CreateCanonicalType(); + auto canType = et->CreateCanonicalType(); + et->canonicalType = canType; if (dynamic_cast(et->canonicalType) != this) - et->canonicalTypeRefPtr = et->canonicalType; + et->canonicalTypeRefPtr = canType; + else + canType.detach(); SLANG_ASSERT(et->canonicalType); } return et->canonicalType; @@ -318,10 +321,10 @@ void Type::accept(IValVisitor* visitor, void* extra) substitutions->args.Add(valueType); auto declRef = DeclRef(typeDecl.Ptr(), substitutions); - - return DeclRefType::Create( + auto rsType = DeclRefType::Create( this, - declRef)->As(); + declRef); + return rsType->As(); } RefPtr Session::getArrayType( @@ -381,13 +384,12 @@ void Type::accept(IValVisitor* visitor, void* extra) return this; } - Type* ArrayExpressionType::CreateCanonicalType() + RefPtr ArrayExpressionType::CreateCanonicalType() { auto canonicalElementType = baseType->GetCanonicalType(); auto canonicalArrayType = getArrayType( canonicalElementType, ArrayLength); - session->canonicalTypes.Add(canonicalArrayType); return canonicalArrayType; } int ArrayExpressionType::GetHashCode() @@ -420,11 +422,10 @@ void Type::accept(IValVisitor* visitor, void* extra) return valueType->Equals(t->valueType); } - Type* GroupSharedType::CreateCanonicalType() + RefPtr GroupSharedType::CreateCanonicalType() { auto canonicalValueType = valueType->GetCanonicalType(); auto canonicalGroupSharedType = getSession()->getGroupSharedType(canonicalValueType); - session->canonicalTypes.Add(canonicalGroupSharedType); return canonicalGroupSharedType; } @@ -456,7 +457,7 @@ void Type::accept(IValVisitor* visitor, void* extra) return false; } - Type* DeclRefType::CreateCanonicalType() + RefPtr DeclRefType::CreateCanonicalType() { // A declaration reference is already canonical return this; @@ -792,7 +793,7 @@ void Type::accept(IValVisitor* visitor, void* extra) return false; } - Type* OverloadGroupType::CreateCanonicalType() + RefPtr OverloadGroupType::CreateCanonicalType() { return this; } @@ -814,7 +815,7 @@ void Type::accept(IValVisitor* visitor, void* extra) return false; } - Type* IRBasicBlockType::CreateCanonicalType() + RefPtr IRBasicBlockType::CreateCanonicalType() { return this; } @@ -836,7 +837,7 @@ void Type::accept(IValVisitor* visitor, void* extra) return false; } - Type* InitializerListType::CreateCanonicalType() + RefPtr InitializerListType::CreateCanonicalType() { return this; } @@ -860,7 +861,7 @@ void Type::accept(IValVisitor* visitor, void* extra) return false; } - Type* ErrorType::CreateCanonicalType() + RefPtr ErrorType::CreateCanonicalType() { return this; } @@ -889,7 +890,7 @@ void Type::accept(IValVisitor* visitor, void* extra) UNREACHABLE_RETURN(false); } - Type* NamedExpressionType::CreateCanonicalType() + RefPtr NamedExpressionType::CreateCanonicalType() { if (!innerType) innerType = GetType(declRef); @@ -981,7 +982,7 @@ void Type::accept(IValVisitor* visitor, void* extra) return substType; } - Type* FuncType::CreateCanonicalType() + RefPtr FuncType::CreateCanonicalType() { // result type RefPtr canResultType = resultType->GetCanonicalType(); @@ -998,8 +999,6 @@ void Type::accept(IValVisitor* visitor, void* extra) canType->resultType = resultType; canType->paramTypes = canParamTypes; - session->canonicalTypes.Add(canType); - return canType; } @@ -1035,10 +1034,9 @@ void Type::accept(IValVisitor* visitor, void* extra) return false; } - Type* TypeType::CreateCanonicalType() + RefPtr TypeType::CreateCanonicalType() { auto canType = getTypeType(type->GetCanonicalType()); - session->canonicalTypes.Add(canType); return canType; } @@ -1070,7 +1068,7 @@ void Type::accept(IValVisitor* visitor, void* extra) return declRef.GetHashCode(); } - Type* GenericDeclRefType::CreateCanonicalType() + RefPtr GenericDeclRefType::CreateCanonicalType() { return this; } diff --git a/source/slang/type-defs.h b/source/slang/type-defs.h index a5b9cd659..14e99caf9 100644 --- a/source/slang/type-defs.h +++ b/source/slang/type-defs.h @@ -10,7 +10,7 @@ public: protected: virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) END_SYNTAX_CLASS() @@ -23,7 +23,7 @@ RAW( protected: virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) END_SYNTAX_CLASS() @@ -37,7 +37,7 @@ public: protected: virtual bool EqualsImpl(Type * type) override; virtual RefPtr SubstituteImpl(SubstitutionSet subst, int* ioDiff) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) END_SYNTAX_CLASS() @@ -51,7 +51,7 @@ public: protected: virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) END_SYNTAX_CLASS() @@ -77,7 +77,7 @@ RAW( protected: virtual int GetHashCode() override; virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; ) END_SYNTAX_CLASS() @@ -103,7 +103,7 @@ RAW( protected: virtual BasicExpressionType* GetScalarType() override; virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; ) END_SYNTAX_CLASS() @@ -312,7 +312,7 @@ RAW( protected: virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual RefPtr SubstituteImpl(SubstitutionSet subst, int* ioDiff) override; virtual int GetHashCode() override; ) @@ -331,7 +331,7 @@ RAW( protected: virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) @@ -356,7 +356,7 @@ public: protected: virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) END_SYNTAX_CLASS() @@ -440,7 +440,7 @@ RAW( protected: virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) END_SYNTAX_CLASS() @@ -469,7 +469,7 @@ RAW( protected: virtual RefPtr SubstituteImpl(SubstitutionSet subst, int* ioDiff) override; virtual bool EqualsImpl(Type * type) override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; virtual int GetHashCode() override; ) END_SYNTAX_CLASS() @@ -495,6 +495,6 @@ SYNTAX_CLASS(GenericDeclRefType, Type) protected: virtual bool EqualsImpl(Type * type) override; virtual int GetHashCode() override; - virtual Type* CreateCanonicalType() override; + virtual RefPtr CreateCanonicalType() override; ) END_SYNTAX_CLASS() -- cgit v1.2.3 From 61a6d18c4870eb55b804d36a30608a34c55e801d Mon Sep 17 00:00:00 2001 From: Yong He Date: Tue, 20 Feb 2018 16:34:59 -0500 Subject: make CompileRequest retain specailized IR module. This is to workaround with the issue that the Types returned in ProgramLayout may reference to IRWitnessTables via GlobalGenericParamSubstitution. --- source/slang/compiler.h | 2 ++ source/slang/emit.cpp | 6 ++++-- source/slang/slang.cpp | 8 +++++++- 3 files changed, 13 insertions(+), 3 deletions(-) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/compiler.h b/source/slang/compiler.h index b1cebc2a1..845de5c81 100644 --- a/source/slang/compiler.h +++ b/source/slang/compiler.h @@ -313,6 +313,8 @@ namespace Slang // Map from the logical name of a module to its definition Dictionary> mapNameToLoadedModules; + // The resulting specialized IR module for each entry point request + List> compiledModules; CompileRequest(Session* session); diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index 46c0b20a9..d2ed1c7c3 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -8158,6 +8158,10 @@ String emitEntryPoint( // TODO: do we want to emit directly from IR, or translate the // IR back into AST for emission? visitor.emitIRModule(&context, irModule); + + // retain the specialized ir module, because the current + // GlobalGenericParamSubstitution implementation may reference ir objects + targetRequest->compileRequest->compiledModules.Add(irModule); } destroyIRSpecializationState(irSpecializationState); @@ -8167,8 +8171,6 @@ String emitEntryPoint( // Now that we've emitted the code for all the declaratiosn in the file, // it is time to stich together the final output. - - // There may be global-scope modifiers that we should emit now visitor.emitGLSLPreprocessorDirectives(translationUnitSyntax); String prefix = sharedContext.sb.ProduceString(); diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp index b65ec5615..42e412438 100644 --- a/source/slang/slang.cpp +++ b/source/slang/slang.cpp @@ -123,7 +123,13 @@ CompileRequest::CompileRequest(Session* session) } CompileRequest::~CompileRequest() -{} +{ + // delete things that may reference IR objects first + targets = decltype(targets)(); + translationUnits = decltype(translationUnits)(); + entryPoints = decltype(entryPoints)(); + types = decltype(types)(); +} RefPtr CompileRequest::parseTypeString(TranslationUnitRequest * translationUnit, String typeStr, RefPtr scope) -- cgit v1.2.3