Hi David, On 19/01/19 01:36, David Malcolm wrote:
On Fri, 2019-01-18 at 19:25 +0000, Andrea Corallo wrote: > Hi all, > this patch add gcc_jit_context_add_driver_option to the libgccjit ABI > and a testcase for it. > > Using this interface is now possible to pass options affecting > assembler and linker. > > Does not introduce any new regression running make check-jit. > > Bests > > Andrea > > > gcc/jit/ChangeLog > 2019-01-16 Andrea Corallo andrea.cora...@arm.com > > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_11): New ABI tag. > * docs/topics/contexts.rst (Additional driver options): New > section. > * jit-playback.c (invoke_driver): Add call to append_driver_options. > * jit-recording.c: Within namespace gcc::jit... > (recording::context::~context): Free the optnames within > m_driver_options. > (recording::context::add_driver_option): New method. > (recording::context::append_driver_options): New method. > (recording::context::dump_reproducer_to_file): Add driver > options. > * jit-recording.h: Within namespace gcc::jit... > (recording::context::add_driver_option): New method. > (recording::context::append_driver_options): New method. > (recording::context::m_driver_options): New field. > * libgccjit++.h (gccjit::context::add_driver_option): New > method. > * libgccjit.c (gcc_jit_context_add_driver_option): New API > entrypoint. > * libgccjit.h (gcc_jit_context_add_driver_option): New API > entrypoint. > (LIBGCCJIT_HAVE_gcc_jit_context_add_driver_option): New > macro. > * libgccjit.map (LIBGCCJIT_ABI_11): New ABI tag. > > > gcc/testsuite/ChangeLog > 2019-01-16 Andrea Corallo andrea.cora...@arm.com > > * jit.dg/add-driver-options-testlib.c: Add support file for > test-add-driver-options.c testcase. > * jit.dg/all-non-failing-tests.h: Add test-add-driver-options.c > * jit.dg/jit.exp (jit-dg-test): Update to support > add-driver-options-testlib.c compilation. > * jit.dg/test-add-driver-options.c: New testcase.Thanks for this patch. One nit: [...snip...] > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h > index bf02e12..9f816b4 100644 > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h > @@ -251,6 +251,13 @@ > #undef create_code > #undef verify_code > > +/* test-add-driver-options.c */ > +#define create_code create_code_add_driver_options > +#define verify_code verify_code_add_driver_options > +#include "test-add-driver-options.c" > +#undef create_code > +#undef verify_code > + > /* Now expose the individual testcases as instances of this struct. */ > > struct testcase The purpose of the above file is to allow for copies of tests to be built into test-combination.c and test-threads.c (to shake out state- handling bugs). If you're going to embed the test into those, then they'd also need to be added to the "testcases" array towards the end of that header. But given that the new test adds options that affect the whole context, it's probably best not to embed it into those combined tests. Instead, add a comment to all-non-failing-tests.h, similar to the one that reads: /* test-extra-options.c: We don't use this one, since the extra options affect the whole context. */ (changing the filename, of course). Other than that the patch looks good. Do you have your copyright assignment paperwork in place?
I believe Andrea's copyright assignment should be covered by the copyright assignment from Arm. Thanks, Kyrill
Also, we're currently in stage 4 of development for gcc 9, so adding a feature to libgccjit probably requires Release Manager approval. (Given the recent discussion on the jit mailing list, this might not be the only late-breaking jit patch). Dave
