[LLVMdev] Function calls only being JIT'd once by Kaleidoscope with MCJIT?

[Lang Hames]

Oh - I know what this is. You were running this on Linux, right?

On MacOS I think the symbol is getting double mangled while going through
MCJIT::getSymbolAddress, hence the failure: The IR level foo function gets
compiled to "_foo" in the object file, and then "_foo" gets mangled to
"__foo" when we look it up. Linux doesn't do assembly level name-mangling,
so this bug doesn't show up there.

Since applying this fixes Linux, and leaves MacOS no more broken than
before, I'll put it in. Hopefully I can figure out a fix for MacOS soon.

Thanks again Charlie.

- Lang.


On Thu, Jan 15, 2015 at 4:34 PM, Lang Hames <lhames at gmail.com> wrote:

> Hi Charlie,
>
> Thanks for working on this. Sorry it took me so long to get around to
> looking at it.
>
> I've just tested the patch out on Top-of-tree and I see:
>
> ready> def foo(x y) x+y;
> ready> Read function definition:
> define double @foo(double %x, double %y) {
> entry:
>   %addtmp = fadd double %x, %y
>   ret double %addtmp
> }
>
> ready> foo(1,2);
> ready> Evaluated to 3.000000
> ready> foo(3,4);
> ready> LLVM ERROR: Program used extern function '_foo' which could not be
> resolved!
>
> Have you seen the same thing? If not, can you describe your system config
> and I'll dig in and try to see what's causing this to behave differently
> for me.
>
> - Lang.
>
> On Fri, Dec 26, 2014 at 9:09 AM, Charlie Turner <
> charlesturner7c5 at gmail.com> wrote:
>
>> Hi all,
>>
>> Starting from Chapter 4 of the Kaleidoscope tutorial (where the JIT
>> support is added), there's some strange behaviour,
>>
>> ready> def foo(x y) x+y;
>> ready> Read function definition:
>> define double @foo(double %x, double %y) {
>> entry:
>>   %addtmp = fadd double %x, %y
>>   ret double %addtmp
>> }
>>
>> ready> foo(1, 2);
>> ready> Evaluated to 3.000000
>> ready> foo(3, 4);
>> ready> Evaluated to 3.000000
>>
>> You can see that foo(3, 4) is not being computed correctly. Well
>> actually, it appears to not be getting compiled. It seems like the
>> Kaleidoscope tutorial has slipped a bit with the recent JIT changes in
>> LLVM.
>>
>> I have attached a patch which gets the above working. I found a blog
>> post about getting Kaleidoscope working with MCJIT
>> (http://blog.llvm.org/2013/07/using-mcjit-with-kaleidoscope-tutorial.html
>> )
>> which most of the code comes from. Unfortunately that didn't
>> completely work, it was using a deprecated API
>> "RTDyldMemoryManager::getPointerToNamedFunction" which was failing to
>> find function symbols for reasons I didn't have time to track down. I
>> changed it to use the recommended getSymbolAddress, and now it works.
>>
>> If the attached seems reasonable, there's more to do,
>>
>>   * Updating the Kaleidoscope tutorial to explain all this. Andy
>> Kaylor's blog post [1] is good place to start. There are other methods
>> of using MCJIT in Kaleidoscope explained on that blog, but the
>> attached method is perhaps the most straightforward one.
>>   * Update all future chapter code to fix this bug.
>>   * Updating the Kaleidoscope/MCJIT example code. This code doesn't
>> get built when you request examples to be built, so it has suffered
>> bit rot (doesn't compile at the moment, I have local patches for this
>> but can submit those in the future). Should we be building this code
>> more regularly?
>>   * .... Bonus: Add unit tests to Kaleidoscope so it doesn't slip like
>> this in the future?
>>
>> Am I on the right track with these fixes? If so I'll try and get to
>> the points above, or at least raise a PR for it. Maybe in the meantime
>> a note should be added to the tutorial that this is a known bug?
>>
>> Thanks for your time!
>> Charlie.
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150115/ad971870/attachment.html>