[PATCH] Big update to Kaleidoscope tutorials.

Wed Aug 26 14:26:32 PDT 2015

Sorry for coming late to the thread.

I was excited about getting these docs fixed for the 3.7 release. Thanks
Lang for working on this.

If this was just touching files under docs/ I would have merged it already,
but since this is also touching source-code under examples/, I get a bit
more nervous. I don't know if that code actually works today, but we do
generate build targets for it, and it does build (at least on my machine).

Since I don't want to take any code changes unless they're a critical
regression fix, I don't think we should merge this now. I also don't think
the loss is super bad since I hope most folks read the Kaleidoscope
tutorial on tip-of-tree.

On Wed, Aug 26, 2015 at 9:55 AM, Philip Reames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> I don't have a specific objection to this patch, but I'm uncomfortable
> with pushing a patch into a release branch at the last minute.  Yes, it's a
> doc only change, but there's still a chance we need to revise/revert after
> the release has happened.
>
> For the record, given this is now in ToT, some of my discomfort is
> eliminated.  If we could let it bake there for a day or two, I'd be worried
> less.
>
> I'll defer to you to make the actual decision.  I don't care enough to
> actively object.
>
> On 08/25/2015 08:17 PM, Lang Hames wrote:
>
> Hi Philip,
>
> This change is to the Kaleidoscope docs only, and those are fairly
> fundamentally broken in LLVM 3.7 as it stands: The code for chapter 4 is
> convoluted, and doesn't match the docs. The code for chapter 5 and beyond
> doesn't work properly.
>
> While I didn't originally plan for this to go in to 3.7, and I'm not
> personally invested in seeing it make it in, I think it's a big improvement
> for very little risk. I'm currently sanity checking this patch (plus the
> c++11 and clang-format prerequisites) on the release branch. If it passes
> everything I'd be inclined to take it.
>
> Did you have any particular points of concern, or were you just applying
> the usual high bar for late changes to the release branch?
>
> Cheers,
> Lang.
>
> On Tuesday, August 25, 2015, Philip Reames via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> On 08/25/2015 05:39 PM, Andy Somogyi via llvm-commits wrote:
>>
>> I think this should go in the 3.7 release.
>>
>> I disagree.  I'd be fine with a forward reference (e.g. "This document is
>> out of date, we suggest you read the one for ToT."), but this would be a
>> large change to make at the last minute.
>>
>>
>> The kaleidoscope tutorials have been broken for a long time, ever since
>> the removal of the regular JIT, and nearly everyone I know used the
>> Kaleidoscope tutorials to learn LLVM, they are a very good resource and
>> they simply should work.
>>
>> Sent from my iPad
>>
>> On Aug 25, 2015, at 6:35 PM, Eric Christopher via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>> This looks pretty great.
>>
>> Couple of comments:
>>
>> Man, would the renaming have been nice to do before/after :)
>>
>> +simple: The KaleidoscopeJIT has a straigtforward symbol resolution rule
>> that
>>
>> Typo.
>>
>> Any reason not to have the KaleidoscopeJIT file in the tutorial?
>>
>> +      : TM(EngineBuilder().selectTarget()), DL(TM->createDataLayout()),
>>
>> As far as the data layout, this hurts. Can we get it from the module at
>> use time?
>>
>> Otherwise LGTM and hopefully we can just pull this into 3.7 if we have
>> another RC. Otherwise we probably want to hold off?
>>
>> -eric
>>
>>
>> On Tue, Aug 25, 2015 at 2:01 PM Lang Hames <lhames at gmail.com> wrote:
>>
>>> Hi All,
>>>
>>> As some of you have noticed, Kaleidoscope has had some issues since we
>>> removed the legacy JIT last year. The tutorial code had been relying on the
>>> behavior of the legacy JIT, and the switch to MCJIT caused many simple
>>> use-cases to break (e.g. repeat calls to functions). Kaleidoscope Chapter
>>> 4, which introduces JIT support, had been largely fixed by taking some of
>>> the code from Andy Kaylor's Kaleidoscope/MCJIT tutorials, but this added a
>>> lot of engineering detail to what was supposed to be a beginner's tutorial.
>>> The later chapters were never updated.
>>>
>>> The attached patch fixes this situation by updating all of the
>>> Kaleidoscope tutorials to use a new, custom, ORC-based JIT:
>>> KaleidoscopeJIT. By using this instead of MCJIT, all of the original
>>> Kaleidoscope functionality is restored, and we actually do a better job of
>>> behaving like a REPL (for example, functions can be redefined). Assuming
>>> this patch is accepted, in the near future I hope to add one or two new
>>> chapters to Kaleidoscope that involve modifying the KaleidoscopeJIT to
>>> support lazy compilation (This will mostly involve merging code from the
>>> Kaleidoscope/Orc tutorials).
>>>
>>> Most of this patch is concerned with the switch to KaleidoscopeJIT and
>>> corresponding updates to the documentation. In particular, Chapter 4 has
>>> been substantially updated. There is also a little bit of C++11
>>> modernization and general cleanup.
>>>
>>> All comments welcome. If anyone would like to road-test the new
>>> tutorials I'd be particularly interested to hear feedback on that.
>>>
>>> Cheers,
>>> Lang.
>>>
>>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing listllvm-commits at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150826/32438c20/attachment.html>