[PATCH] Big update to Kaleidoscope tutorials.

Wed Aug 26 14:33:11 PDT 2015

Hi Hans,

Ok - that's fair enough.

Andy - Sorry this won't make it in to 3.7. People will have to work from
top-of-tree for now (luckily I think this is fairly common, as Hans hopes).

Cheers,
Lang.

On Wed, Aug 26, 2015 at 2:26 PM, Hans Wennborg <hans at chromium.org> wrote:

> 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/9d490a2a/attachment.html>