<div dir="ltr">Hi Hans,<div><br></div><div>Ok - that's fair enough.</div><div><br></div><div>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).</div><div><br></div><div>Cheers,</div><div>Lang.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 26, 2015 at 2:26 PM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sorry for coming late to the thread.<div><br></div><div>I was excited about getting these docs fixed for the 3.7 release. Thanks Lang for working on this.</div><div><br></div><div>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).</div><div><br></div><div>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.</div><div><div class="h5"><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 26, 2015 at 9:55 AM, Philip Reames via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>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. <br>
<br>
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. <br>
<br>
I'll defer to you to make the actual decision. I don't care
enough to actively object. <br><div><div>
<br>
On 08/25/2015 08:17 PM, Lang Hames wrote:<br>
</div></div></div><div><div>
<blockquote type="cite">
<div><span></span></div>
<div>Hi Philip,
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Did you have any particular points of concern, or were you
just applying the usual high bar for late changes to the
release branch? </div>
<div><br>
</div>
<div>Cheers,</div>
<div>Lang.</div>
<div><br>
On Tuesday, August 25, 2015, Philip Reames via llvm-commits
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>On 08/25/2015 05:39 PM, Andy Somogyi via llvm-commits
wrote:<br>
</div>
<blockquote type="cite">
<div>I think this should go in the 3.7 release.</div>
</blockquote>
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. <br>
<blockquote type="cite">
<div><br>
</div>
<div>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. <br>
<br>
Sent from my iPad
<div><br>
On Aug 25, 2015, at 6:35 PM, Eric Christopher via
llvm-commits <<a>llvm-commits@lists.llvm.org</a>>
wrote:<br>
<br>
</div>
<blockquote type="cite">
<div>
<div dir="ltr">This looks pretty great.
<div><br>
</div>
<div>Couple of comments:</div>
<div><br>
</div>
<div>Man, would the renaming have been nice to
do before/after :)</div>
<div><br>
</div>
<div>
<div>+simple: The KaleidoscopeJIT has a
straigtforward symbol resolution rule that</div>
</div>
<div><br>
</div>
<div>Typo.</div>
<div><br>
</div>
<div>Any reason not to have the KaleidoscopeJIT
file in the tutorial?</div>
<div><br>
</div>
<div>
<div>+ :
TM(EngineBuilder().selectTarget()),
DL(TM->createDataLayout()),</div>
</div>
<div><br>
</div>
<div>As far as the data layout, this hurts. Can
we get it from the module at use time?</div>
<div><br>
</div>
<div>Otherwise LGTM and hopefully we can just
pull this into 3.7 if we have another RC.
Otherwise we probably want to hold off?</div>
<div><br>
</div>
<div>-eric</div>
<div><br>
</div>
<div><br>
</div>
<div>
<div>
<div class="gmail_quote">
<div dir="ltr">On Tue, Aug 25, 2015 at
2:01 PM Lang Hames <<a>lhames@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Hi All,
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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).</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>All comments welcome. If anyone
would like to road-test the new
tutorials I'd be particularly
interested to hear feedback on
that. </div>
<div><br>
</div>
<div>Cheers,</div>
<div>Lang.</div>
<div><br>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<blockquote type="cite">
<div><span>_______________________________________________</span><br>
<span>llvm-commits mailing list</span><br>
<span><a>llvm-commits@lists.llvm.org</a></span><br>
<span><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></span><br>
</div>
</blockquote>
<br>
<fieldset></fieldset>
<br>
<pre>_______________________________________________
llvm-commits mailing list
<a>llvm-commits@lists.llvm.org</a>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a>
</pre>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</div></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div></div></div></div>
</blockquote></div><br></div>