[PATCH] Big update to Kaleidoscope tutorials.

[Philip Reames via llvm-commits]

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 <mailto: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
>>     <javascript:_e(%7B%7D,'cvml','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
>>>     <javascript:_e(%7B%7D,'cvml','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
>>>     <javascript:_e(%7B%7D,'cvml','llvm-commits at lists.llvm.org');>
>>>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>     _______________________________________________
>>     llvm-commits mailing list
>>     llvm-commits at lists.llvm.org  <javascript:_e(%7B%7D,'cvml','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/52a6c082/attachment.html>