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

Project Infrastructure LLVM Dev List Archives
1

Hi all,

Starting from Chapter 4 of the Kaleidoscope tutorial (where the JIT
support is added), there's some strange behaviour,

def foo(x y) x+y;
Read function definition:

define double @foo(double %x, double %y) {
entry:
  %addtmp = fadd double %x, %y
  ret double %addtmp
}

foo(1, 2);
Evaluated to 3.000000
foo(3, 4);
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
(Using MCJIT with the Kaleidoscope Tutorial - The LLVM Project Blog)
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.

0001-Function-calls-not-getting-compiled-in-Kaleidoscope-.patch (13.1 KB)

2

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:

def foo(x y) x+y;
Read function definition:
define double @foo(double %x, double %y) {
entry:
%addtmp = fadd double %x, %y
ret double %addtmp
}

foo(1,2);
Evaluated to 3.000000
foo(3,4);
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.
3

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.
4

Committed in r226308. Thanks Charlie!

  • Lang.
5

Cheers Lang!

You were right, I was testing this on Linux.

I was planning on committing these changes with the corresponding
changes to the Kaleidoscope tutorial walk-through. Might be a bit of a
surprise to have no explanation of what MCJITHelper and friends is
doing.

I'll try and make time to prepare some patches along these lines, as
well as updating future chapters with the same fix.

Thanks for taking a look :slight_smile:

Charlie.

6

Charlie Turner <charlesturner7c5 <at> gmail.com> writes:

I was planning on committing these changes with the corresponding
changes to the Kaleidoscope tutorial walk-through. Might be a bit of a
surprise to have no explanation of what MCJITHelper and friends is
doing.

I'll try and make time to prepare some patches along these lines, as
well as updating future chapters with the same fix.

Thanks for taking a look :slight_smile:

Any progress on fixing the tutorial?

I am about to dive into LLVM and after some hair-pulling found that the
Kaleidoscope tutorials (from part 5 onwards), are broken.

Obviously this is not a good first impression.

(even a note in the source code, #error "currently does not work", would be
a huge improvement, since it would avoid a lot of wasted time for the beginner.)

PS. On OS X 10.8.5, the compile command for part 4 of the tutorial needs
-fno-rtti, presumably because the code now inherits from a LLVM class.

Regards,

Per Mildner

7

Hi Per,

After feedback from other places, I wasn't sure if committing what is
admittedly a hack to the rest of the tutorial code was a better choice
than waiting for Orc to take a shape, and to see if we can't revamp
the tutorial presentation to use the new APIs. Furthermore, the
"workaround" really needs to be weaved into the tutorial text, to
explain to the reader what this gob of code is even doing there.

I've dropped the ball on chasing this up. I can't commit to fixing
this myself, sorry. I've lost track of the Orc progress.

- Charlie.

8

I could not find a bug for this so I have created https://llvm.org/bugs/show_bug.cgi?id=23302 to track this issue.

Per Mildner
Per.Mildner@sics.se

9

Charlie Turner <charlesturner7c5 <at> gmail.com> writes:

> I was planning on committing these changes with the corresponding
> changes to the Kaleidoscope tutorial walk-through. Might be a bit of a
> surprise to have no explanation of what MCJITHelper and friends is
> doing.
>
> I'll try and make time to prepare some patches along these lines, as
> well as updating future chapters with the same fix.
>
> Thanks for taking a look :slight_smile:

Any progress on fixing the tutorial?

I am about to dive into LLVM and after some hair-pulling found that the
Kaleidoscope tutorials (from part 5 onwards), are broken.

Obviously this is not a good first impression.

(even a note in the source code, #error "currently does not work", would be
a huge improvement, since it would avoid a lot of wasted time for the
beginner.)

PS. On OS X 10.8.5, the compile command for part 4 of the tutorial needs
-fno-rtti, presumably because the code now inherits from a LLVM class.

Lang - recall what the state is here?

10

No progress since Charlie last looked at it. As Per suggested, it may be worth adding an explicit #error, or at least a comment, noting that the later examples are broken.

Going forward, I see three options:

(1) Remove the old tutorials entirely, and encourage people to check out the MCJIT and Orc versions.
(2) Build an Orc-based stack that approximates the behavior of the Old JIT (which the old tutorials assume).
(3) Rewrite the old tutorial to natively use the new APIs.

I much prefer the third option. The first option misses out on a lot of language/API discussion that’s contained in the later chapters of the old tutorials. The second option replicates a JIT stack that wouldn’t be useful for most clients.

If we were going to pursue option three, the simple JIT from the Orc/initial tutorial seems like a good starting point to re-write the old tutorials. Then we’d have an interesting bifurcation part way through the tutorial series: Go one way and add new language features on top of the simple JIT, or go another and add features to the JIT while keeping the language steady. Cross-polination of the two branches (new JIT features + new language features) could be handled in future tutorials, or left as an exercise for the reader.

The tricky part, as usual, is finding the time to do this. Volunteers/patches welcome. :slight_smile:

Cheers,
Lang.

11

No progress since Charlie last looked at it.

Sorry, I meant to refer to the issue Per Mildner mentioned regarding
the tutorials not building due to RTTI - which I recall discussing
with you & Eric, but don't know what came of it.

12

Hi Dave, Per,

Dave - The issue Eric raised was with chapters 7 and 8. I’ve fixed that in 235541.

Per - It’s not obvious to my why Chapter 4 should need RTTI. What error are you seeing?

Cheers,
Lang.

13

Hi Dave, Per,

Dave - The issue Eric raised was with chapters 7 and 8. I've fixed that in 235541.

Per - It's not obvious to my why Chapter 4 should need RTTI. What error are you seeing?

The Chapter 4 code has a class, HelpingMemoryManager, that inherits from the LLVM class SectionMemoryManager.

It was my understanding that LLBM is built without RTTI by default, but clang++ enables RTTI by default, and that it is not possible for code using RTTI (toy.cpp) to inherit from a class compiled without RTTI.

Transcript:

bash$ uname -a
Darwin dhcpu02.sics.se 12.6.0 Darwin Kernel Version 12.6.0: Wed Mar 18 16:23:48 PDT 2015; root:xnu-2050.48.19~1/RELEASE_X86_64 x86_64
bash$ /usr/bin/clang++ --version
Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)
Target: x86_64-apple-darwin12.6.0
Thread model: posix
bash$ llvm-config --version
3.7.0svn
bash$ llvm-config --cxxflags --ldflags --system-libs --libs core mcjit native
-I/Users/perm/bin/llvm-git/include -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -std=c++11 -fcolor-diagnostics -g -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-L/Users/perm/bin/llvm-git/lib -Wl,-search_paths_first -Wl,-headerpad_max_install_names
-lLLVMX86Disassembler -lLLVMX86AsmParser -lLLVMX86CodeGen -lLLVMSelectionDAG -lLLVMAsmPrinter -lLLVMCodeGen -lLLVMScalarOpts -lLLVMProfileData -lLLVMInstCombine -lLLVMTransformUtils -lLLVMipa -lLLVMX86Desc -lLLVMMCDisassembler -lLLVMX86Info -lLLVMX86AsmPrinter -lLLVMX86Utils -lLLVMMCJIT -lLLVMTarget -lLLVMAnalysis -lLLVMExecutionEngine -lLLVMRuntimeDyld -lLLVMObject -lLLVMMCParser -lLLVMBitReader -lLLVMMC -lLLVMCore -lLLVMSupport
-lcurses -lpthread -lz -lm
bash$ /usr/bin/clang++ -g toy4.cpp `llvm-config --cxxflags --ldflags --system-libs --libs core mcjit native` -O3 -o toy4
Undefined symbols for architecture x86_64:
  "typeinfo for llvm::SectionMemoryManager", referenced from:
      typeinfo for HelpingMemoryManager in toy4-e7e3d4.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
bash$ /usr/bin/clang++ -fno-rtti -g toy4.cpp `llvm-config --cxxflags --ldflags --system-libs --libs core mcjit native` -O3 -o toy4
bash$ ./toy4

Cheers,
Lang.

> No progress since Charlie last looked at it.

Sorry, I meant to refer to the issue Per Mildner mentioned regarding
the tutorials not building due to RTTI - which I recall discussing
with you & Eric, but don't know what came of it.

> As Per suggested, it may be
> worth adding an explicit #error, or at least a comment, noting that the
> later examples are broken.
>
> Going forward, I see three options:
>
> (1) Remove the old tutorials entirely, and encourage people to check out the
> MCJIT and Orc versions.
> (2) Build an Orc-based stack that approximates the behavior of the Old JIT
> (which the old tutorials assume).
> (3) Rewrite the old tutorial to natively use the new APIs.
>
> I much prefer the third option. The first option misses out on a lot of
> language/API discussion that's contained in the later chapters of the old
> tutorials. The second option replicates a JIT stack that wouldn't be useful
> for most clients.
>
> If we were going to pursue option three, the simple JIT from the Orc/initial
> tutorial seems like a good starting point to re-write the old tutorials.
> Then we'd have an interesting bifurcation part way through the tutorial
> series: Go one way and add new language features on top of the simple JIT,
> or go another and add features to the JIT while keeping the language steady.
> Cross-polination of the two branches (new JIT features + new language
> features) could be handled in future tutorials, or left as an exercise for
> the reader.
>
> The tricky part, as usual, is finding the time to do this.
> Volunteers/patches welcome. :slight_smile:
>
> Cheers,
> Lang.
>
>
>>
>>
>>
>>>
>>> Charlie Turner <charlesturner7c5 <at> gmail.com> writes:
>>>
>>> > I was planning on committing these changes with the corresponding
>>> > changes to the Kaleidoscope tutorial walk-through. Might be a bit of a
>>> > surprise to have no explanation of what MCJITHelper and friends is
>>> > doing.
>>> >
>>> > I'll try and make time to prepare some patches along these lines, as
>>> > well as updating future chapters with the same fix.
>>> >
>>> > Thanks for taking a look :slight_smile:
>>>
>>>
>>> Any progress on fixing the tutorial?
>>>
>>> I am about to dive into LLVM and after some hair-pulling found that the
>>> Kaleidoscope tutorials (from part 5 onwards), are broken.
>>>
>>> Obviously this is not a good first impression.
>>>
>>> (even a note in the source code, #error "currently does not work", would
>>> be
>>> a huge improvement, since it would avoid a lot of wasted time for the
>>> beginner.)
>>>
>>> PS. On OS X 10.8.5, the compile command for part 4 of the tutorial needs
>>> -fno-rtti, presumably because the code now inherits from a LLVM class.
>>
>>
>> Lang - recall what the state is here?
>>
>>>
>>>
>>> Regards,
>>>
>>> Per Mildner
>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> LLVMdev@cs.uiuc.edu http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>>
>

Per Mildner
Per.Mildner@sics.se