On Fri, 2014-11-07 at 12:47 -0700, Jeff Law wrote: > On 11/05/14 12:34, David Malcolm wrote: > > > > > I've added comments throughout the file. > > > > I didn't bother adding __attribute__((cold)), instead simply dropping > > that "TODO". > Fine. > > > > > Attached is the current state of the file gcc/jit/libgccjit.c (on the > > branch) for review. > > > > OK for trunk? (conditional on all the rest being approved, and usual > > bootstrap®rtesting; I've merely verified a non-bootstrap compile and > > successful make check-jit so far). > > > > There were a few other changes relative to what you've approved, which > > I'll post for review shortly. > > > > Dave > > > > > > libgccjit.c > > > > > > /* Implementation of the C API; all wrappers into the internal C++ API > > Copyright (C) 2013-2014 Free Software Foundation, Inc. > > Contributed by David Malcolm<dmalc...@redhat.com>. > This is fine. With the comments, it became a lot clearer this was just > the error checking wrappers and not a whole lot else.
Thanks. That was the last of the review requests, so I believe the jit branch is now approved for merger. I plan to do this early next week, to give time to rebase against latest trunk and retest it. > The one thing this does make me wonder is should we add something about > the error checking may change in significant ways from one release to > the next, much like the ABI/API. > > This seems important as the error checking in many ways specifies the > language for the JIT and I suspect we haven't got all the corner cases > sorted out yet (and probably can't until this gets into wider distribution). I agree that the JIT language is specified by the runtime error-checking behaviour, but it's also specified by the types in the API. We're in slightly better shape here than, say gcc's internal tree API, in that the types in the API make a rigid separation between types vs expressions, and it also captures some lvalue vs rvalue distinctions, so client code is likely to not compile if it gets those things wrong. I've also tried to name the params in such a way as to hint at restrictions, e.g. gcc_jit_context_zero's second param is: gcc_jit_type *numeric_type i.e. the "numeric_type" name of that param describes a requirement. There are probably some under-specified behaviors in the JIT language as it stands - perhaps in ordering of operations? In any case, the current disclaimer reads: "Note that libgccjit is currently of “Alpha” quality; the APIs are not yet set in stone, and they shouldn’t be used in production yet." I'm sure we'll want to reword that at some point. One big potential change might be to create a stable *plugin* API, unified with the JIT API (so we'd allow client code to use the same API for both embedding GCC and being embedded within GCC). Most of the "gcc_jit_*" symbols would become just "gcc_*". I've experimented with that, but I don't see myself getting it done in time for the close of stage1 close (am frantically trying to finish the gimple-classes work), so I'm thinking of that unified jit-and-plugin API as a GCC 6 feature. Dave
