From eaeb380a9f91fd9ef907256d5c3981fb4d43eaa0 Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Thu, 16 Jul 2015 06:17:14 +0000 Subject: [PATCH] Make ExecutionEngine owning a DataLayout Summary: This change is part of a series of commits dedicated to have a single DataLayout during compilation by using always the one owned by the module. The ExecutionEngine will act as an exception and will be unsafe to be reused across context. We don't enforce this rule but undefined behavior can occurs if the user tries to do it. Reviewers: lhames Subscribers: echristo, llvm-commits, rafael, yaron.keren Differential Revision: http://reviews.llvm.org/D11110 From: Mehdi Amini git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@242387 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../llvm/ExecutionEngine/ExecutionEngine.h | 15 ++++---- lib/ExecutionEngine/ExecutionEngine.cpp | 36 +++++++++---------- .../ExecutionEngineBindings.cpp | 2 +- lib/ExecutionEngine/Interpreter/Execution.cpp | 10 +++--- .../Interpreter/ExternalFunctions.cpp | 2 +- .../Interpreter/Interpreter.cpp | 5 ++- lib/ExecutionEngine/Interpreter/Interpreter.h | 1 - lib/ExecutionEngine/MCJIT/MCJIT.cpp | 18 ++++++---- lib/ExecutionEngine/Orc/OrcMCJITReplacement.h | 27 +++++++------- 9 files changed, 61 insertions(+), 55 deletions(-) diff --git a/include/llvm/ExecutionEngine/ExecutionEngine.h b/include/llvm/ExecutionEngine/ExecutionEngine.h index 821c0181ce8..dbe99745b2f 100644 --- a/include/llvm/ExecutionEngine/ExecutionEngine.h +++ b/include/llvm/ExecutionEngine/ExecutionEngine.h @@ -104,7 +104,12 @@ class ExecutionEngine { ExecutionEngineState EEState; /// The target data for the platform for which execution is being performed. - const DataLayout *DL; + /// + /// Note: the DataLayout is LLVMContext specific because it has an + /// internal cache based on type pointers. It makes unsafe to reuse the + /// ExecutionEngine across context, we don't enforce this rule but undefined + /// behavior can occurs if the user tries to do it. + const DataLayout DL; /// Whether lazy JIT compilation is enabled. bool CompilingLazily; @@ -126,8 +131,6 @@ protected: /// optimize for the case where there is only one module. SmallVector, 1> Modules; - void setDataLayout(const DataLayout *Val) { DL = Val; } - /// getMemoryforGV - Allocate memory for a global variable. virtual char *getMemoryForGV(const GlobalVariable *GV); @@ -194,7 +197,7 @@ public: //===--------------------------------------------------------------------===// - const DataLayout *getDataLayout() const { return DL; } + const DataLayout &getDataLayout() const { return DL; } /// removeModule - Remove a Module from the list of modules. Returns true if /// M is found. @@ -478,8 +481,8 @@ public: } protected: - ExecutionEngine() {} - explicit ExecutionEngine(std::unique_ptr M); + ExecutionEngine(const DataLayout DL) : DL(std::move(DL)){}; + explicit ExecutionEngine(const DataLayout DL, std::unique_ptr M); void emitGlobals(); diff --git a/lib/ExecutionEngine/ExecutionEngine.cpp b/lib/ExecutionEngine/ExecutionEngine.cpp index c2ff8e27af4..c9558119071 100644 --- a/lib/ExecutionEngine/ExecutionEngine.cpp +++ b/lib/ExecutionEngine/ExecutionEngine.cpp @@ -61,8 +61,8 @@ ExecutionEngine *(*ExecutionEngine::InterpCtor)(std::unique_ptr M, void JITEventListener::anchor() {} -ExecutionEngine::ExecutionEngine(std::unique_ptr M) - : LazyFunctionCreator(nullptr) { +ExecutionEngine::ExecutionEngine(const DataLayout DL, std::unique_ptr M) + : DL(std::move(DL)), LazyFunctionCreator(nullptr) { CompilingLazily = false; GVCompilationDisabled = false; SymbolSearchingDisabled = false; @@ -115,7 +115,7 @@ public: } // anonymous namespace char *ExecutionEngine::getMemoryForGV(const GlobalVariable *GV) { - return GVMemoryBlock::Create(GV, *getDataLayout()); + return GVMemoryBlock::Create(GV, getDataLayout()); } void ExecutionEngine::addObjectFile(std::unique_ptr O) { @@ -326,7 +326,7 @@ void *ArgvArray::reset(LLVMContext &C, ExecutionEngine *EE, const std::vector &InputArgv) { Values.clear(); // Free the old contents. Values.reserve(InputArgv.size()); - unsigned PtrSize = EE->getDataLayout()->getPointerSize(); + unsigned PtrSize = EE->getDataLayout().getPointerSize(); Array = make_unique((InputArgv.size()+1)*PtrSize); DEBUG(dbgs() << "JIT: ARGV = " << (void*)Array.get() << "\n"); @@ -401,7 +401,7 @@ void ExecutionEngine::runStaticConstructorsDestructors(bool isDtors) { #ifndef NDEBUG /// isTargetNullPtr - Return whether the target pointer stored at Loc is null. static bool isTargetNullPtr(ExecutionEngine *EE, void *Loc) { - unsigned PtrSize = EE->getDataLayout()->getPointerSize(); + unsigned PtrSize = EE->getDataLayout().getPointerSize(); for (unsigned i = 0; i < PtrSize; ++i) if (*(i + (uint8_t*)Loc)) return false; @@ -634,8 +634,8 @@ GenericValue ExecutionEngine::getConstantValue(const Constant *C) { case Instruction::GetElementPtr: { // Compute the index GenericValue Result = getConstantValue(Op0); - APInt Offset(DL->getPointerSizeInBits(), 0); - cast(CE)->accumulateConstantOffset(*DL, Offset); + APInt Offset(DL.getPointerSizeInBits(), 0); + cast(CE)->accumulateConstantOffset(DL, Offset); char* tmp = (char*) Result.PointerVal; Result = PTOGV(tmp + Offset.getSExtValue()); @@ -722,16 +722,16 @@ GenericValue ExecutionEngine::getConstantValue(const Constant *C) { } case Instruction::PtrToInt: { GenericValue GV = getConstantValue(Op0); - uint32_t PtrWidth = DL->getTypeSizeInBits(Op0->getType()); + uint32_t PtrWidth = DL.getTypeSizeInBits(Op0->getType()); assert(PtrWidth <= 64 && "Bad pointer width"); GV.IntVal = APInt(PtrWidth, uintptr_t(GV.PointerVal)); - uint32_t IntWidth = DL->getTypeSizeInBits(CE->getType()); + uint32_t IntWidth = DL.getTypeSizeInBits(CE->getType()); GV.IntVal = GV.IntVal.zextOrTrunc(IntWidth); return GV; } case Instruction::IntToPtr: { GenericValue GV = getConstantValue(Op0); - uint32_t PtrWidth = DL->getTypeSizeInBits(CE->getType()); + uint32_t PtrWidth = DL.getTypeSizeInBits(CE->getType()); GV.IntVal = GV.IntVal.zextOrTrunc(PtrWidth); assert(GV.IntVal.getBitWidth() <= 64 && "Bad pointer width"); GV.PointerVal = PointerTy(uintptr_t(GV.IntVal.getZExtValue())); @@ -1033,7 +1033,7 @@ static void StoreIntToMemory(const APInt &IntVal, uint8_t *Dst, void ExecutionEngine::StoreValueToMemory(const GenericValue &Val, GenericValue *Ptr, Type *Ty) { - const unsigned StoreBytes = getDataLayout()->getTypeStoreSize(Ty); + const unsigned StoreBytes = getDataLayout().getTypeStoreSize(Ty); switch (Ty->getTypeID()) { default: @@ -1073,7 +1073,7 @@ void ExecutionEngine::StoreValueToMemory(const GenericValue &Val, break; } - if (sys::IsLittleEndianHost != getDataLayout()->isLittleEndian()) + if (sys::IsLittleEndianHost != getDataLayout().isLittleEndian()) // Host and target are different endian - reverse the stored bytes. std::reverse((uint8_t*)Ptr, StoreBytes + (uint8_t*)Ptr); } @@ -1110,7 +1110,7 @@ static void LoadIntFromMemory(APInt &IntVal, uint8_t *Src, unsigned LoadBytes) { void ExecutionEngine::LoadValueFromMemory(GenericValue &Result, GenericValue *Ptr, Type *Ty) { - const unsigned LoadBytes = getDataLayout()->getTypeStoreSize(Ty); + const unsigned LoadBytes = getDataLayout().getTypeStoreSize(Ty); switch (Ty->getTypeID()) { case Type::IntegerTyID: @@ -1176,20 +1176,20 @@ void ExecutionEngine::InitializeMemory(const Constant *Init, void *Addr) { if (const ConstantVector *CP = dyn_cast(Init)) { unsigned ElementSize = - getDataLayout()->getTypeAllocSize(CP->getType()->getElementType()); + getDataLayout().getTypeAllocSize(CP->getType()->getElementType()); for (unsigned i = 0, e = CP->getNumOperands(); i != e; ++i) InitializeMemory(CP->getOperand(i), (char*)Addr+i*ElementSize); return; } if (isa(Init)) { - memset(Addr, 0, (size_t)getDataLayout()->getTypeAllocSize(Init->getType())); + memset(Addr, 0, (size_t)getDataLayout().getTypeAllocSize(Init->getType())); return; } if (const ConstantArray *CPA = dyn_cast(Init)) { unsigned ElementSize = - getDataLayout()->getTypeAllocSize(CPA->getType()->getElementType()); + getDataLayout().getTypeAllocSize(CPA->getType()->getElementType()); for (unsigned i = 0, e = CPA->getNumOperands(); i != e; ++i) InitializeMemory(CPA->getOperand(i), (char*)Addr+i*ElementSize); return; @@ -1197,7 +1197,7 @@ void ExecutionEngine::InitializeMemory(const Constant *Init, void *Addr) { if (const ConstantStruct *CPS = dyn_cast(Init)) { const StructLayout *SL = - getDataLayout()->getStructLayout(cast(CPS->getType())); + getDataLayout().getStructLayout(cast(CPS->getType())); for (unsigned i = 0, e = CPS->getNumOperands(); i != e; ++i) InitializeMemory(CPS->getOperand(i), (char*)Addr+SL->getElementOffset(i)); return; @@ -1342,7 +1342,7 @@ void ExecutionEngine::EmitGlobalVariable(const GlobalVariable *GV) { InitializeMemory(GV->getInitializer(), GA); Type *ElTy = GV->getType()->getElementType(); - size_t GVSize = (size_t)getDataLayout()->getTypeAllocSize(ElTy); + size_t GVSize = (size_t)getDataLayout().getTypeAllocSize(ElTy); NumInitBytes += (unsigned)GVSize; ++NumGlobals; } diff --git a/lib/ExecutionEngine/ExecutionEngineBindings.cpp b/lib/ExecutionEngine/ExecutionEngineBindings.cpp index 55ab5af2b90..892c941a292 100644 --- a/lib/ExecutionEngine/ExecutionEngineBindings.cpp +++ b/lib/ExecutionEngine/ExecutionEngineBindings.cpp @@ -318,7 +318,7 @@ void *LLVMRecompileAndRelinkFunction(LLVMExecutionEngineRef EE, } LLVMTargetDataRef LLVMGetExecutionEngineTargetData(LLVMExecutionEngineRef EE) { - return wrap(unwrap(EE)->getDataLayout()); + return wrap(&unwrap(EE)->getDataLayout()); } LLVMTargetMachineRef diff --git a/lib/ExecutionEngine/Interpreter/Execution.cpp b/lib/ExecutionEngine/Interpreter/Execution.cpp index dbfa37e2b0d..53beed87a41 100644 --- a/lib/ExecutionEngine/Interpreter/Execution.cpp +++ b/lib/ExecutionEngine/Interpreter/Execution.cpp @@ -968,7 +968,7 @@ void Interpreter::visitAllocaInst(AllocaInst &I) { unsigned NumElements = getOperandValue(I.getOperand(0), SF).IntVal.getZExtValue(); - unsigned TypeSize = (size_t)TD.getTypeAllocSize(Ty); + unsigned TypeSize = (size_t)getDataLayout().getTypeAllocSize(Ty); // Avoid malloc-ing zero bytes, use max()... unsigned MemToAlloc = std::max(1U, NumElements * TypeSize); @@ -1000,7 +1000,7 @@ GenericValue Interpreter::executeGEPOperation(Value *Ptr, gep_type_iterator I, for (; I != E; ++I) { if (StructType *STy = dyn_cast(*I)) { - const StructLayout *SLO = TD.getStructLayout(STy); + const StructLayout *SLO = getDataLayout().getStructLayout(STy); const ConstantInt *CPU = cast(I.getOperand()); unsigned Index = unsigned(CPU->getZExtValue()); @@ -1020,7 +1020,7 @@ GenericValue Interpreter::executeGEPOperation(Value *Ptr, gep_type_iterator I, assert(BitWidth == 64 && "Invalid index type for getelementptr"); Idx = (int64_t)IdxGV.IntVal.getZExtValue(); } - Total += TD.getTypeAllocSize(ST->getElementType())*Idx; + Total += getDataLayout().getTypeAllocSize(ST->getElementType()) * Idx; } } @@ -1477,7 +1477,7 @@ GenericValue Interpreter::executeIntToPtrInst(Value *SrcVal, Type *DstTy, GenericValue Dest, Src = getOperandValue(SrcVal, SF); assert(DstTy->isPointerTy() && "Invalid PtrToInt instruction"); - uint32_t PtrSize = TD.getPointerSizeInBits(); + uint32_t PtrSize = getDataLayout().getPointerSizeInBits(); if (PtrSize != Src.IntVal.getBitWidth()) Src.IntVal = Src.IntVal.zextOrTrunc(PtrSize); @@ -1497,7 +1497,7 @@ GenericValue Interpreter::executeBitCastInst(Value *SrcVal, Type *DstTy, (DstTy->getTypeID() == Type::VectorTyID)) { // vector src bitcast to vector dst or vector src bitcast to scalar dst or // scalar src bitcast to vector dst - bool isLittleEndian = TD.isLittleEndian(); + bool isLittleEndian = getDataLayout().isLittleEndian(); GenericValue TempDst, TempSrc, SrcVec; const Type *SrcElemTy; const Type *DstElemTy; diff --git a/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp b/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp index 9b44042d614..e738a8c65fe 100644 --- a/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp +++ b/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp @@ -368,7 +368,7 @@ static GenericValue lle_X_sprintf(FunctionType *FT, case 'x': case 'X': if (HowLong >= 1) { if (HowLong == 1 && - TheInterpreter->getDataLayout()->getPointerSizeInBits() == 64 && + TheInterpreter->getDataLayout().getPointerSizeInBits() == 64 && sizeof(long) < sizeof(int64_t)) { // Make sure we use %lld with a 64 bit argument because we might be // compiling LLI on a 32 bit compiler. diff --git a/lib/ExecutionEngine/Interpreter/Interpreter.cpp b/lib/ExecutionEngine/Interpreter/Interpreter.cpp index f103c09659a..14ce74fcc14 100644 --- a/lib/ExecutionEngine/Interpreter/Interpreter.cpp +++ b/lib/ExecutionEngine/Interpreter/Interpreter.cpp @@ -49,16 +49,15 @@ ExecutionEngine *Interpreter::create(std::unique_ptr M, // Interpreter ctor - Initialize stuff // Interpreter::Interpreter(std::unique_ptr M) - : ExecutionEngine(std::move(M)), TD(Modules.back().get()) { + : ExecutionEngine(M->getDataLayout(), std::move(M)) { memset(&ExitValue.Untyped, 0, sizeof(ExitValue.Untyped)); - setDataLayout(&TD); // Initialize the "backend" initializeExecutionEngine(); initializeExternalFunctions(); emitGlobals(); - IL = new IntrinsicLowering(TD); + IL = new IntrinsicLowering(getDataLayout()); } Interpreter::~Interpreter() { diff --git a/lib/ExecutionEngine/Interpreter/Interpreter.h b/lib/ExecutionEngine/Interpreter/Interpreter.h index f97664181a8..bd813ac4900 100644 --- a/lib/ExecutionEngine/Interpreter/Interpreter.h +++ b/lib/ExecutionEngine/Interpreter/Interpreter.h @@ -95,7 +95,6 @@ struct ExecutionContext { // class Interpreter : public ExecutionEngine, public InstVisitor { GenericValue ExitValue; // The return value of the called function - DataLayout TD; IntrinsicLowering *IL; // The runtime stack of executing code. The top of the stack is the current diff --git a/lib/ExecutionEngine/MCJIT/MCJIT.cpp b/lib/ExecutionEngine/MCJIT/MCJIT.cpp index a7d67050c7a..5f4641515ce 100644 --- a/lib/ExecutionEngine/MCJIT/MCJIT.cpp +++ b/lib/ExecutionEngine/MCJIT/MCJIT.cpp @@ -65,12 +65,13 @@ MCJIT::createJIT(std::unique_ptr M, std::move(Resolver)); } -MCJIT::MCJIT(std::unique_ptr M, std::unique_ptr tm, +MCJIT::MCJIT(std::unique_ptr M, std::unique_ptr TM, std::shared_ptr MemMgr, std::shared_ptr Resolver) - : ExecutionEngine(std::move(M)), TM(std::move(tm)), Ctx(nullptr), - MemMgr(std::move(MemMgr)), Resolver(*this, std::move(Resolver)), - Dyld(*this->MemMgr, this->Resolver), ObjCache(nullptr) { + : ExecutionEngine(*TM->getDataLayout(), std::move(M)), TM(std::move(TM)), + Ctx(nullptr), MemMgr(std::move(MemMgr)), + Resolver(*this, std::move(Resolver)), Dyld(*this->MemMgr, this->Resolver), + ObjCache(nullptr) { // FIXME: We are managing our modules, so we do not want the base class // ExecutionEngine to manage them as well. To avoid double destruction // of the first (and only) module added in ExecutionEngine constructor @@ -85,7 +86,6 @@ MCJIT::MCJIT(std::unique_ptr M, std::unique_ptr tm, Modules.clear(); OwnedModules.addModule(std::move(First)); - setDataLayout(TM->getDataLayout()); RegisterJITEventListener(JITEventListener::createGDBRegistrationListener()); } @@ -193,7 +193,11 @@ void MCJIT::generateCodeForModule(Module *M) { if (ObjCache) ObjectToLoad = ObjCache->getObject(M); - M->setDataLayout(*TM->getDataLayout()); + if (M->getDataLayout().isDefault()) { + M->setDataLayout(getDataLayout()); + } else { + assert(M->getDataLayout() == getDataLayout() && "DataLayout Mismatch"); + } // If the cache did not contain a suitable object, compile the object if (!ObjectToLoad) { @@ -265,7 +269,7 @@ void MCJIT::finalizeModule(Module *M) { RuntimeDyld::SymbolInfo MCJIT::findExistingSymbol(const std::string &Name) { SmallString<128> FullName; - Mangler::getNameWithPrefix(FullName, Name, *TM->getDataLayout()); + Mangler::getNameWithPrefix(FullName, Name, getDataLayout()); return Dyld.getSymbol(FullName); } diff --git a/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h b/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h index 7dc5164c419..a097fdfe950 100644 --- a/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h +++ b/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h @@ -137,25 +137,26 @@ public: } OrcMCJITReplacement( - std::shared_ptr MemMgr, - std::shared_ptr ClientResolver, - std::unique_ptr TM) - : TM(std::move(TM)), MemMgr(*this, std::move(MemMgr)), - Resolver(*this), ClientResolver(std::move(ClientResolver)), - NotifyObjectLoaded(*this), NotifyFinalized(*this), + std::shared_ptr MemMgr, + std::shared_ptr ClientResolver, + std::unique_ptr TM) + : ExecutionEngine(*TM->getDataLayout()), TM(std::move(TM)), + MemMgr(*this, std::move(MemMgr)), Resolver(*this), + ClientResolver(std::move(ClientResolver)), NotifyObjectLoaded(*this), + NotifyFinalized(*this), ObjectLayer(NotifyObjectLoaded, NotifyFinalized), CompileLayer(ObjectLayer, SimpleCompiler(*this->TM)), - LazyEmitLayer(CompileLayer) { - setDataLayout(this->TM->getDataLayout()); - } + LazyEmitLayer(CompileLayer) {} void addModule(std::unique_ptr M) override { // If this module doesn't have a DataLayout attached then attach the // default. - if (M->getDataLayout().isDefault()) - M->setDataLayout(*getDataLayout()); - + if (M->getDataLayout().isDefault()) { + M->setDataLayout(getDataLayout()); + } else { + assert(M->getDataLayout() == getDataLayout() && "DataLayout Mismatch"); + } Modules.push_back(std::move(M)); std::vector Ms; Ms.push_back(&*Modules.back()); @@ -310,7 +311,7 @@ private: std::string MangledName; { raw_string_ostream MangledNameStream(MangledName); - Mang.getNameWithPrefix(MangledNameStream, Name, *TM->getDataLayout()); + Mang.getNameWithPrefix(MangledNameStream, Name, getDataLayout()); } return MangledName; }