[llvm] r243589 - [MCJIT] Fix PR20656 by teaching MCJIT to honor ExecutionEngine's global mapping.

[Lang Hames]

Hi Aaron,

Thanks for spotting this. I've committed a fix in 243891.

Cheers,
Lang.


On Fri, Jul 31, 2015 at 1:13 PM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Wed, Jul 29, 2015 at 7:12 PM, Lang Hames <lhames at gmail.com> wrote:
> > Author: lhames
> > Date: Wed Jul 29 18:12:33 2015
> > New Revision: 243589
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=243589&view=rev
> > Log:
> > [MCJIT] Fix PR20656 by teaching MCJIT to honor ExecutionEngine's global
> mapping.
> >
> > This is important for users of the C API who can't supply custom symbol
> > resolvers yet.
> >
> >
> > Modified:
> >     llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp
> >     llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
> >     llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
> >
> > Modified: llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp?rev=243589&r1=243588&r2=243589&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp (original)
> > +++ llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp Wed Jul 29
> 18:12:33 2015
> > @@ -189,10 +189,17 @@ uint64_t ExecutionEngineState::RemoveMap
> >  }
> >
> >  std::string ExecutionEngine::getMangledName(const GlobalValue *GV) {
> > +  assert(GV->hasName() && "Global must have name.");
> > +
> >    MutexGuard locked(lock);
> > -  Mangler Mang;
> >    SmallString<128> FullName;
> > -  Mang.getNameWithPrefix(FullName, GV, false);
> > +
> > +  const DataLayout &DL =
> > +    GV->getParent()->getDataLayout().isDefault()
> > +      ? getDataLayout()
> > +      : GV->getParent()->getDataLayout();
> > +
> > +  Mangler::getNameWithPrefix(FullName, GV->getName(), DL);
> >    return FullName.str();
> >  }
> >
> >
> > Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp?rev=243589&r1=243588&r2=243589&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp (original)
> > +++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp Wed Jul 29 18:12:33
> 2015
> > @@ -270,6 +270,12 @@ void MCJIT::finalizeModule(Module *M) {
> >  RuntimeDyld::SymbolInfo MCJIT::findExistingSymbol(const std::string
> &Name) {
> >    SmallString<128> FullName;
> >    Mangler::getNameWithPrefix(FullName, Name, getDataLayout());
> > +
> > +  if (void *Addr = getPointerToGlobalIfAvailable(FullName))
> > +    return RuntimeDyld::SymbolInfo(static_cast<uint64_t>(
> > +                                     reinterpret_cast<uintptr_t>(Addr)),
> > +                                   JITSymbolFlags::Exported);
> > +
> >    return Dyld.getSymbol(FullName);
> >  }
> >
> >
> > Modified: llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp?rev=243589&r1=243588&r2=243589&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
> (original)
> > +++ llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp Wed Jul
> 29 18:12:33 2015
> > @@ -488,3 +488,36 @@ TEST_F(MCJITCAPITest, yield) {
> >    EXPECT_TRUE(didCallYield);
> >  }
> >
> > +static int localTestFunc() {
> > +  return 42;
> > +}
> > +
> > +TEST_F(MCJITCAPITest, addGlobalMapping) {
> > +  SKIP_UNSUPPORTED_PLATFORM;
> > +
> > +  Module = LLVMModuleCreateWithName("testModule");
> > +  LLVMTypeRef FunctionType = LLVMFunctionType(LLVMInt32Type(), NULL, 0,
> 0);
> > +  LLVMValueRef MappedFn = LLVMAddFunction(Module, "mapped_fn",
> FunctionType);
> > +
> > +  Function = LLVMAddFunction(Module, "test_fn", FunctionType);
> > +  LLVMBasicBlockRef Entry = LLVMAppendBasicBlock(Function, "");
> > +  LLVMBuilderRef Builder = LLVMCreateBuilder();
> > +  LLVMPositionBuilderAtEnd(Builder, Entry);
> > +  LLVMValueRef RetVal = LLVMBuildCall(Builder, MappedFn, NULL, 0, "");
> > +  LLVMBuildRet(Builder, RetVal);
> > +
> > +  LLVMVerifyModule(Module, LLVMAbortProcessAction, &Error);
> > +  LLVMDisposeMessage(Error);
> > +
> > +  buildMCJITOptions();
> > +  buildMCJITEngine();
> > +
> > +  LLVMAddGlobalMapping(Engine, MappedFn,
> reinterpret_cast<void*>(&localTestFunc));
>
> This is currently causing a warning:
>
>
> /opt/llvm/build-llvm/src/llvm/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp:
> In member function ‘virtual void
> MCJITCAPITest_addGlobalMapping_Test::TestBody()’:
>
> /opt/llvm/build-llvm/src/llvm/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp:517:80:
> warning: ISO C++ forbids casting between pointer-to-function and
> pointer-to-object [enabled by default]
>
> Any chance you can take care of this one?
>
> Thanks!
>
> ~Aaron
>
> > +
> > +  buildAndRunPasses();
> > +
> > +  uint64_t raw = LLVMGetFunctionAddress(Engine, "test_fn");
> > +  int (*usable)() = (int (*)()) raw;
> > +
> > +  EXPECT_EQ(42, usable());
> > +}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150803/011a4b78/attachment.html>