Re: lto1: internal compiler error: original not compressed with zstd
On Sun, 2024-03-17 at 18:36 -0500, StrawberryTea wrote: > Hello GCC libjit people, Hi StrawberryTea I'm the author/maintainer of libgccjit. I confess that I don't think I've ever tried using LTO with libgccjit, but rustc_codegen_gcc appears to, given: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=8415bceea9d3ca86adc00ae8ad92deaec0457dd1 > > Whenever I try to compile Elisp packages with GCC libjit and -flto, I > get "lto1: internal compiler error: original not compressed with > zstd". FWIW that message appears in the sources in gcc/lto-compress.cc's lto_uncompression_zstd here: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/lto-compress.cc;#l162 but I'm unfamiliar with that part of the compiler. > I have an Emacs bug report on > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69689 and a Gentoo bug > report on https://bugs.gentoo.org/926953 but nobody has been able to > resolve this. There are hints that this might be related to the way > Gentoo builds GCC, but I'm not sure. I suspect that debugging this further would be very hard, due to it involving a large amount of non-trivial code and interactions between gcc, the linker, emacs C, the emacs AOT compilation code, and how gentoo packages all of these. You'd need to turn on debug logging from libgccjit which I don't know how to do from Emacs [2], and that might give clues as to what's going wrong (or might not!) Do you know if the Emacs native compilation project has tried turning on LTO? [1] What does that actually mean? (e.g. LTO between compiled Emacs lisp modules? Or between a compiled Emacs lisp module and Emacs C code? Are both built using the same GCC version? etc) The simplest fix is probably to turn off LTO. Sorry not to be of more help Dave [1] https://www.emacswiki.org/emacs/GccEmacs [2] FWIW, from C you'd do it with gcc_jit_context_set_logfile: https://gcc.gnu.org/onlinedocs/jit/topics/contexts.html#c.gcc_jit_context_set_logfile
Re: [PATCH] Allow `gcc_jit_type_get_size` to work with pointers
On Thu, 2024-03-28 at 23:47 +0100, Guillaume Gomez wrote: > Hi, > > Here's a little fix to allow the `gcc_jit_type_get_size` function to > work on pointer types as well. > Thanks, looks good to me. Are you able to push this, or do you want me to? Dave
Building libgccjit with -fno-semantic-interposition? ( was Re: 1.76% performance loss in VRP due to inlining)
On Tue, 2024-04-30 at 21:15 +0200, Richard Biener via Gcc wrote: > > > > Am 30.04.2024 um 21:11 schrieb Jason Merrill via Gcc > > : > > > > On Fri, Apr 26, 2024 at 5:44 AM Aldy Hernandez via Gcc > > wrote: > > > > > > In implementing prange (pointer ranges), I have found a 1.74% > > > slowdown > > > in VRP, even without any code path actually using the code. I > > > have > > > tracked this down to irange::get_bitmask() being compiled > > > differently > > > with and without the bare bones patch. With the patch, > > > irange::get_bitmask() has a lot of code inlined into it, > > > particularly > > > get_bitmask_from_range() and consequently the wide_int_storage > > > code. > > ... > > > +static irange_bitmask > > > +get_bitmask_from_range (tree type, > > > + const wide_int &min, const wide_int &max) > > ... > > > -irange_bitmask > > > -irange::get_bitmask_from_range () const > > > > My guess is that this is the relevant change: the old function has > > external linkage, and is therefore interposable, which inhibits > > inlining. The new function has internal linkage, which allows > > inlining. > > > > Relatedly, I wonder if we want to build GCC with -fno-semantic- > > interposition? > > I guess that’s a good idea, though it’s already implied when doing > LTO bootstrap and building cc1 and friends? (But not for libgccjit?) [CCing jit mailing list] FWIW I've no idea if any libgccjit users are using semantic interposition; I suspect the answer is "no one is using it". Antoyo, Andrea [also CCed]: are either of you using semantic interposition of symbols within libgccjit? If not, we *might* get a slightly faster libgccjit by building it with -fno-semantic-interposition. Or maybe not... Dave > > Richard > > > > > Jason > > >
Re: Debug info on macOS
On Fri, 2024-05-10 at 15:46 +0200, Gerd Möllmann wrote: > Hi, > > I'm developing Emacs using its native compilation on macOS, which is > based on libgccjit. > > In this context, I'm currently failing to get .eln files (= .so, > .dylib, > .dll depending on the platform) with debug info. This has probably > its > roots in the special handling of DWARF under macOS, a long-winded > story > leading to dSYM bundles... > > My question is: can I somehow configure libgccjit in a way that keeps > the .o file that was used to produce the resulting .dylib, like it > keeps/writes the various intermediate files that one can get? > > If that's possible, my hope would be to extract the debug info from > the > .o using dsymutil and use that with the .dylib. [CCing Andrea Corallo for his Emacs expertise] I confess I've not done any compilation on macOS and am not familiar with the tools, formats and "special handling of DWARF under macOS" you mention. As I understand it, Emacs is using libgccjit to do ahead-of-time compilation, presumably compiling the optimized ELisp code to machine code as a shared library. Is Emacs using gcc_jit_context_compile_to_file with GCC_JIT_OUTPUT_KIND_DYNAMIC_LIBRARY, or is it doing something more complicated? If that's what it's doing, you might want to take a look at playback::context::compile in gcc/jit-playback.cc (and try stepping through it in the debugger). In particular, the playback::compile_to_file::postprocess implementation of the playback::context::postprocess vfunc is responsible for taking a .s in a tempdir and turning it into the desired output. Perhaps there's some macOS-specific special-casing needed there? You might also find it useful to look at the overview of how libgccjit's internals here: https://gcc.gnu.org/onlinedocs/jit/internals/index.html#overview-of-code-structure Hope this is helpful Dave
Re: [PATCH 10/52] jit: Replace uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE
On Sun, 2024-06-02 at 22:01 -0500, Kewen Lin wrote: > Joseph pointed out "floating types should have their mode, > not a poorly defined precision value" in the discussion[1], > as he and Richi suggested, the existing macros > {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a > hook mode_for_floating_type. Unlike the other FEs, for the > uses in recording::memento_of_get_type::get_size, since > {float,{,long_}double}_type_node haven't been initialized > yet, this is to replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE > with calling hook targetm.c.mode_for_floating_type. > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html > > gcc/jit/ChangeLog: > > * jit-recording.cc > (recording::memento_of_get_type::get_size): Update > macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE by calling > targetm.c.mode_for_floating_type with > TI_{FLOAT,DOUBLE,LONG_DOUBLE}_TYPE. > --- > gcc/jit/jit-recording.cc | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc > index 68a2e860c1f..7719b898e57 100644 > --- a/gcc/jit/jit-recording.cc > +++ b/gcc/jit/jit-recording.cc > @@ -21,7 +21,7 @@ along with GCC; see the file COPYING3. If not see > #include "config.h" > #include "system.h" > #include "coretypes.h" > -#include "tm.h" > +#include "target.h" > #include "pretty-print.h" > #include "toplev.h" > > @@ -2353,6 +2353,7 @@ size_t > recording::memento_of_get_type::get_size () > { > int size; > + machine_mode m; > switch (m_kind) > { > case GCC_JIT_TYPE_VOID: > @@ -2399,13 +2400,16 @@ recording::memento_of_get_type::get_size () > size = 128; > break; > case GCC_JIT_TYPE_FLOAT: > - size = FLOAT_TYPE_SIZE; > + m = targetm.c.mode_for_floating_type (TI_FLOAT_TYPE); > + size = GET_MODE_PRECISION (m).to_constant (); > break; > case GCC_JIT_TYPE_DOUBLE: > - size = DOUBLE_TYPE_SIZE; > + m = targetm.c.mode_for_floating_type (TI_DOUBLE_TYPE); > + size = GET_MODE_PRECISION (m).to_constant (); > break; > case GCC_JIT_TYPE_LONG_DOUBLE: > - size = LONG_DOUBLE_TYPE_SIZE; > + m = targetm.c.mode_for_floating_type (TI_LONG_DOUBLE_TYPE); > + size = GET_MODE_PRECISION (m).to_constant (); > break; > case GCC_JIT_TYPE_SIZE_T: > size = MAX_BITS_PER_WORD; [CCing jit mailing list] Thanks for the patch; sorry for the delay in responding. Did your testing include jit? Note that --enable-languages=all does *not* include it (due to it needing --enable-host-shared). The jit::recording code runs *very* early - before toplev::main. For example, a call to gcc_jit_type_get_size can trigger the above code path before toplev::main has run. target.h says each target should have a: struct gcc_target targetm = TARGET_INITIALIZER; Has targetm.c.mode_for_floating_type been initialized enough by that static initialization? Could the mode_for_floating_type hook be relying on some target-specific dynamic initialization that hasn't run yet? (e.g. taking account of command-line options?) Dave
Re: [PATCH 10/52] jit: Replace uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE
On Fri, 2024-06-14 at 10:16 +0800, Kewen.Lin wrote: > Hi David, > > on 2024/6/13 21:44, David Malcolm wrote: > > On Sun, 2024-06-02 at 22:01 -0500, Kewen Lin wrote: > > > Joseph pointed out "floating types should have their mode, > > > not a poorly defined precision value" in the discussion[1], > > > as he and Richi suggested, the existing macros > > > {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a > > > hook mode_for_floating_type. Unlike the other FEs, for the > > > uses in recording::memento_of_get_type::get_size, since > > > {float,{,long_}double}_type_node haven't been initialized > > > yet, this is to replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE > > > with calling hook targetm.c.mode_for_floating_type. > > > > > > [1] > > > https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html > > > > > > gcc/jit/ChangeLog: > > > > > > * jit-recording.cc > > > (recording::memento_of_get_type::get_size): Update > > > macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE by calling > > > targetm.c.mode_for_floating_type with > > > TI_{FLOAT,DOUBLE,LONG_DOUBLE}_TYPE. > > > --- > > > gcc/jit/jit-recording.cc | 12 > > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > > > diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc > > > index 68a2e860c1f..7719b898e57 100644 > > > --- a/gcc/jit/jit-recording.cc > > > +++ b/gcc/jit/jit-recording.cc > > > @@ -21,7 +21,7 @@ along with GCC; see the file COPYING3. If not > > > see > > > #include "config.h" > > > #include "system.h" > > > #include "coretypes.h" > > > -#include "tm.h" > > > +#include "target.h" > > > #include "pretty-print.h" > > > #include "toplev.h" > > > > > > @@ -2353,6 +2353,7 @@ size_t > > > recording::memento_of_get_type::get_size () > > > { > > > int size; > > > + machine_mode m; > > > switch (m_kind) > > > { > > > case GCC_JIT_TYPE_VOID: > > > @@ -2399,13 +2400,16 @@ recording::memento_of_get_type::get_size > > > () > > > size = 128; > > > break; > > > case GCC_JIT_TYPE_FLOAT: > > > - size = FLOAT_TYPE_SIZE; > > > + m = targetm.c.mode_for_floating_type (TI_FLOAT_TYPE); > > > + size = GET_MODE_PRECISION (m).to_constant (); > > > break; > > > case GCC_JIT_TYPE_DOUBLE: > > > - size = DOUBLE_TYPE_SIZE; > > > + m = targetm.c.mode_for_floating_type (TI_DOUBLE_TYPE); > > > + size = GET_MODE_PRECISION (m).to_constant (); > > > break; > > > case GCC_JIT_TYPE_LONG_DOUBLE: > > > - size = LONG_DOUBLE_TYPE_SIZE; > > > + m = targetm.c.mode_for_floating_type > > > (TI_LONG_DOUBLE_TYPE); > > > + size = GET_MODE_PRECISION (m).to_constant (); > > > break; > > > case GCC_JIT_TYPE_SIZE_T: > > > size = MAX_BITS_PER_WORD; > > > > [CCing jit mailing list] > > > > Thanks for the patch; sorry for the delay in responding. > > > > Did your testing include jit? Note that --enable-languages=all > > does > > *not* include it (due to it needing --enable-host-shared). > > Thanks for the hints! Yes, as noted in the cover letter, I did test > jit. > Initially I used TYPE_PRECISION ({float,{long_,}double_type_node) to > replace these just like what I proposed for the other FE changes, but > the > testing showed some failures on test-combination.c etc., by looking > into > them, I realized that this call > recording::memento_of_get_type::get_size > can happen before when we set up those type nodes. Then I had to use > the > current approach with the new hook, it made all failures gone (no > regressions). btw, test result comparison showed some more lines > with > "NA->PASS: test-threads.c.exe", since it's positive, I didn't look > into > it. > > > > > The jit::recording code runs *very* early - before toplev::main. > > For > > example, a call to gcc_jit_type_get_size can trigger the above code > > path before toplev::main has run. > > > > target.h says each target should have a: > > > > struct gcc_target targetm = TARGET_INITIALIZER; > > > > Has targetm.c.mode_for_floating_type be
Re: [PATCH] Add rvalue::get_name method (and its C equivalent)
On Mon, 2024-04-22 at 19:56 +0200, Guillaume Gomez wrote: > `param` is also inheriting from `lvalue`. I don't think adding this > check is a good idea > because it will not evolve nicely if more changes are done in > libgccjit. Sorry for not responding earlier. I think I agree with Guillaume here. Looking at the checklist at: https://gcc.gnu.org/onlinedocs/jit/internals/index.html#submitting-patches the patch is missing: - a feature macro in libgccjit.h such as #define LIBGCCJIT_HAVE_gcc_jit_lvalue_get_name - documentation for the new C entrypoint - documentation for the new ABI tag (see topics/compatibility.rst). Other than that, the patch looks reasonable to me. Dave > > Le lun. 22 avr. 2024 à 17:19, Antoni Boucher a > écrit : > > > > For your new API endpoint, please add a check like: > > > > RETURN_IF_FAIL (lvalue->is_global () || lvalue->is_local (), > > NULL, > > NULL, > > "lvalue should be a variable"); > > > > > > Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit : > > > Good point! > > > > > > New patch attached. > > > > > > Le lun. 22 avr. 2024 à 15:13, Antoni Boucher a > > > écrit : > > > > > > > > Please move the function to be on lvalue since there are no > > > > rvalue types > > > > that are not lvalues that have a name. > > > > > > > > Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit : > > > > > Hey Arthur :) > > > > > > > > > > > Is there any reason for that getter to return a mutable > > > > > > pointer to the > > > > > > name? Would something like this work instead if you're just > > > > > > looking at > > > > > > getting the name? > > > > > > > > > > > > + virtual string * get_name () const { return NULL; } > > > > > > > > > > > > With of course adequate modifications to the inheriting > > > > > > classes. > > > > > > > > > > Good catch, thanks! > > > > > > > > > > Updated the patch and attached the new version to this email. > > > > > > > > > > Cordially. > > > > > > > > > > Le lun. 22 avr. 2024 à 11:51, Arthur Cohen > > > > > a écrit : > > > > > > > > > > > > Hey Guillaume :) > > > > > > > > > > > > On 4/20/24 01:05, Guillaume Gomez wrote: > > > > > > > Hi, > > > > > > > > > > > > > > I just encountered the need to retrieve the name of an > > > > > > > `rvalue` (if > > > > > > > there is one) while working on the Rust GCC backend. > > > > > > > > > > > > > > This patch adds a getter to retrieve the information. > > > > > > > > > > > > > > Cordially. > > > > > > > > > > > > > virtual bool get_wide_int (wide_int *) const { > > > > > > > return false; } > > > > > > > > > > > > > > + virtual string * get_name () { return NULL; } > > > > > > > + > > > > > > > private: > > > > > > > virtual enum precedence get_precedence () const = 0; > > > > > > > > > > > > Is there any reason for that getter to return a mutable > > > > > > pointer to the > > > > > > name? Would something like this work instead if you're just > > > > > > looking at > > > > > > getting the name? > > > > > > > > > > > > + virtual string * get_name () const { return NULL; } > > > > > > > > > > > > With of course adequate modifications to the inheriting > > > > > > classes. > > > > > > > > > > > > Best, > > > > > > > > > > > > Arthur >
Re: [PATCH] libgccjit: Add ability to get the alignment of a type
On Thu, 2024-04-04 at 18:59 -0400, Antoni Boucher wrote: > Hi. > This patch adds a new API to produce an rvalue representing the > alignment of a type. > Thanks for the review. Patch looks good to me (but may need the usual ABI version updates when merging). Thanks; sorry for the delay in reviewing. Dave
Re: [PATCH] libgccjit: Make new_array_type take unsigned long
On Fri, 2024-02-23 at 09:55 -0500, Antoni Boucher wrote: > I had forgotten to add the doc since there is now a new API. > Here it is. Sorry for the delay; the updated patch looks good to me (but may need usual ABI tag changes when pushing). Thanks Dave > > On Wed, 2024-02-21 at 19:45 -0500, Antoni Boucher wrote: > > Thanks for the review. > > > > Here's the updated patch. > > > > On Thu, 2023-12-07 at 20:04 -0500, David Malcolm wrote: > > > On Thu, 2023-12-07 at 17:29 -0500, Antoni Boucher wrote: > > > > Hi. > > > > This patches update gcc_jit_context_new_array_type to take the > > > > size > > > > as > > > > an unsigned long instead of a int, to allow creating bigger > > > > array > > > > types. > > > > > > > > I haven't written the ChangeLog yet because I wasn't sure it's > > > > allowed > > > > to change the type of a function like that. > > > > If it isn't, what would you suggest? > > > > > > We've kept ABI compatibility all the way back to the version in > > > GCC > > > 5, > > > so it seems a shame to break ABI. > > > > > > How about a new API entrypoint: > > > gcc_jit_context_new_array_type_unsigned_long > > > whilst keeping the old one. > > > > > > Then everything internally can use "unsigned long"; we just keep > > > the > > > old entrypoint accepting int (which internally promotes the arg > > > to > > > unsigned long, if positive, sharing all the implementation). > > > > > > Alternatively, I think there may be a way to do this with symbol > > > versioning: > > > https://gcc.gnu.org/wiki/SymbolVersioning > > > see e.g. Section 3.7 of Ulrich Drepper's "How To Write Shared > > > Libraries", but I'm a bit wary of cross-platform compatibility > > > with > > > that. > > > > > > Dave > > > > > > > > >
Re: [PATCH] libgccjit: Allow comparing array types
On Thu, 2024-01-25 at 07:52 -0500, Antoni Boucher wrote: > Thanks. > Can we please agree on some wording to use so I know when the patch > can > be pushed. Especially since we're now in stage 4, it would help me if > you say something like "you can push to master". Sorry about the ambiguity. Yes, you can push this one. Thanks Dave > Regards. > > On Wed, 2024-01-24 at 12:14 -0500, David Malcolm wrote: > > On Fri, 2024-01-19 at 16:55 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch allows comparing different instances of array types as > > > equal. > > > Thanks for the review. > > > > Thanks; the patch looks good to me. > > > > Dave > > >
Re: [PATCH] libgccjit: Add support for the type bfloat16
On Wed, 2024-02-21 at 10:56 -0500, Antoni Boucher wrote: > Thanks for the review. > Here's the updated patch. Thanks for the update patch; sorry for the delay in reviewing. The updated patch looks good for trunk. Dave > > On Fri, 2023-12-01 at 12:45 -0500, David Malcolm wrote: > > On Thu, 2023-11-16 at 17:20 -0500, Antoni Boucher wrote: > > > I forgot to attach the patch. > > > > > > On Thu, 2023-11-16 at 17:19 -0500, Antoni Boucher wrote: > > > > Hi. > > > > This patch adds the support for the type bfloat16 (bug 112574). > > > > > > > > This was asked to be splitted from a another patch sent here: > > > > https://gcc.gnu.org/pipermail/jit/2023q1/001607.html > > > > > > > > Thanks for the review. > > > > > > > Thanks for the patch. > > > > > diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc > > > index 18cc4da25b8..7e1c97a4638 100644 > > > --- a/gcc/jit/jit-playback.cc > > > +++ b/gcc/jit/jit-playback.cc > > > @@ -280,6 +280,8 @@ get_tree_node_for_type (enum gcc_jit_types > > > type_) > > > > > > case GCC_JIT_TYPE_FLOAT: > > > return float_type_node; > > > + case GCC_JIT_TYPE_BFLOAT16: > > > + return bfloat16_type_node; > > > > The code to create bfloat16_type_node (in build_common_tree_nodes) > > is > > guarded by #ifdef HAVE_BFmode, so we should probably have a test > > for > > this in case GCC_JIT_TYPE_BFLOAT16 to at least add an error message > > when it's NULL_TREE, rather than silently returning NULL_TREE and > > crashing. > > > > [...] > > > > > diff --git a/gcc/testsuite/jit.dg/test-bfloat16.c > > > b/gcc/testsuite/jit.dg/test-bfloat16.c > > > new file mode 100644 > > > index 000..6aed3920351 > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-bfloat16.c > > > @@ -0,0 +1,37 @@ > > > +/* { dg-do compile { target x86_64-*-* } } */ > > > + > > > +#include > > > +#include > > > + > > > +#include "libgccjit.h" > > > + > > > +/* We don't want set_options() in harness.h to set -O3 so our > > > little local > > > + is optimized away. */ > > > +#define TEST_ESCHEWS_SET_OPTIONS > > > +static void set_options (gcc_jit_context *ctxt, const char > > > *argv0) > > > +{ > > > +} > > > > > > Please add a comment to all-non-failing-tests.h noting the > > exclusion > > of > > this test case from the array. > > > > [...] > > > > > diff --git a/gcc/testsuite/jit.dg/test-types.c > > > b/gcc/testsuite/jit.dg/test-types.c > > > index a01944e35fa..9e7c4f3e046 100644 > > > --- a/gcc/testsuite/jit.dg/test-types.c > > > +++ b/gcc/testsuite/jit.dg/test-types.c > > > @@ -1,3 +1,4 @@ > > > +#include > > > #include > > > #include > > > #include > > > @@ -492,4 +493,5 @@ verify_code (gcc_jit_context *ctxt, > > > gcc_jit_result *result) > > > > > > CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type > > > (ctxt, GCC_JIT_TYPE_FLOAT)), sizeof (float)); > > > CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type > > > (ctxt, GCC_JIT_TYPE_DOUBLE)), sizeof (double)); > > > + CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type > > > (ctxt, GCC_JIT_TYPE_BFLOAT16)), sizeof (__bfloat16)); > > > > > > This is only going to work on targets which #ifdef HAVE_BFmode, so > > this > > CHECK_VALUE needs to be conditionalized somehow, to avoid having > > this, > > test-combination, and test-threads from bailing out early on > > targets > > without BFmode. > > > > Dave > > >
Re: Frontend access to target features (was Re: [PATCH] libgccjit: Add ability to get CPU features)
On Sun, 2024-03-10 at 12:05 +0100, Iain Buclaw wrote: > Excerpts from David Malcolm's message of März 5, 2024 4:09 pm: > > On Thu, 2023-11-09 at 19:33 -0500, Antoni Boucher wrote: > > > Hi. > > > See answers below. > > > > > > On Thu, 2023-11-09 at 18:04 -0500, David Malcolm wrote: > > > > On Thu, 2023-11-09 at 17:27 -0500, Antoni Boucher wrote: > > > > > Hi. > > > > > This patch adds support for getting the CPU features in > > > > > libgccjit > > > > > (bug > > > > > 112466) > > > > > > > > > > There's a TODO in the test: > > > > > I'm not sure how to test that gcc_jit_target_info_arch > > > > > returns > > > > > the > > > > > correct value since it is dependant on the CPU. > > > > > Any idea on how to improve this? > > > > > > > > > > Also, I created a CStringHash to be able to have a > > > > > std::unordered_set. Is there any built-in way > > > > > of > > > > > doing > > > > > this? > > > > > > > > Thanks for the patch. > > > > > > > > Some high-level questions: > > > > > > > > Is this specifically about detecting capabilities of the host > > > > that > > > > libgccjit is currently running on? or how the target was > > > > configured > > > > when libgccjit was built? > > > > > > I'm less sure about this part. I'll need to do more tests. > > > > > > > > > > > One of the benefits of libgccjit is that, in theory, we support > > > > all > > > > of > > > > the targets that GCC already supports. Does this patch change > > > > that, > > > > or > > > > is this more about giving client code the ability to determine > > > > capabilities of the specific host being compiled for? > > > > > > This should not change that. If it does, this is a bug. > > > > > > > > > > > I'm nervous about having per-target jit code. Presumably > > > > there's a > > > > reason that we can't reuse existing target logic here - can you > > > > please > > > > describe what the problem is. I see that the ChangeLog has: > > > > > > > > > * config/i386/i386-jit.cc: New file. > > > > > > > > where i386-jit.cc has almost 200 lines of nontrivial code. > > > > Where > > > > did > > > > this come from? Did you base it on existing code in our source > > > > tree, > > > > making modifications to fit the new internal API, or did you > > > > write > > > > it > > > > from scratch? In either case, how onerous would this be for > > > > other > > > > targets? > > > > > > This was mostly copied from the same code done for the Rust and D > > > frontends. > > > See this commit and the following: > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b1c06fd9723453dd2b2ec306684cb806dc2b4fbb > > > The equivalent to i386-jit.cc is there: > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=22e3557e2d52f129f2bbfdc98688b945dba28dc9 > > > > [CCing Iain and Arthur re those patches; for reference, the patch > > being > > discussed is attached to : > > https://gcc.gnu.org/pipermail/jit/2024q1/001792.html ] > > > > One of my concerns about this patch is that we seem to be gaining > > code > > that's per-(frontend x config) which seems to be copied and pasted > > with > > a search and replace, which could lead to an M*N explosion. > > > > That's certainly the case with the configure/make rules. Itself I > think > is copied originally from the {cpu_type}-protos.h machinery. > > It might be worth pointing out that the c-family of front-ends don't > have separate headers because their per-target macros are defined in > {cpu_type}.h directly - for better or worse. > > > Is there any real difference between the per-config code for the > > different frontends, or should there be a general "enumerate all > > features of the target" hook that's independent of the frontend? > > (but > > perhaps calls into it). > > > > As far as I understand, the configure parts should all be identical > between tm_p, tm_d, tm_rust, ..., so would benefit f
Re: [PATCH] libgccjit: Fix get_size of size_t
On Wed, 2024-02-21 at 14:16 -0500, Antoni Boucher wrote: > On Thu, 2023-12-07 at 19:57 -0500, David Malcolm wrote: > > On Thu, 2023-12-07 at 17:26 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch fixes getting the size of size_t (bug 112910). > > > > > > There's one issue with this patch: like every other feature that > > > checks > > > for target-specific stuff, it requires a compilation before > > > actually > > > fetching the size of the type. > > > Which means that getting the size before a compilation might be > > > wrong > > > (and I actually believe is wrong on x86-64). > > > > > > I was wondering if we should always implicitely do the first > > > compilation to gather the correct info: this would fix this issue > > > and > > > all the others that we have due to that. > > > I'm not sure what would be the performance implication. > > > > Maybe introduce a new class target_info which contains all the > > information we might want to find via a compilation, and have the > > top- > > level recording::context have a pointer to it, which starts as > > nullptr, > > but can be populated on-demand the first time something needs it? > > That would mean that we'll need to populate it for every top-level > context, right? Would the idea be that we should then use child > contexts to have the proper information filled? > If so, how is this different than just compiling two contexts like > what > I currently do? > This would also mean that we'll do an implicit compilation whenever > we > use an API that needs this info, right? Wouldn't that be unexpected? I was thinking a compilation with an empty playback::context to lazily capture the target data. My hope was that this would make things easier for users. But you're the one using this API, so if you're more comfortable with the explicit initial compilation approach, let's go with that. If so, this is OK for trunk - but we might want to add a note to the documentation about the double-compilation workaround. Dave > > Thanks for the idea. > > > > > > > > > Another solution that I have been thinking about for a while now > > > would > > > be to have another frontend libgccaot (I don't like that name), > > > which > > > is like libgccjit but removes the JIT part so that we get access > > > to > > > the > > > target stuff directly and would remove the need for having a > > > seperation > > > between recording and playback as far as I understand. > > > That's a long-term solution, but I wanted to share the idea now > > > and > > > gather your thoughts on that. > > > > FWIW the initial version of libgccjit didn't have a split between > > recording and playback; instead the client code had to pass in a > > callback to call into the various API functions (creating tree > > nodes). > > See: > > https://gcc.gnu.org/legacy-ml/gcc-patches/2013-10/msg00228.html > > > > Dave > > >
Re: [PATCH] libgccjit: Add support for machine-dependent builtins
On Thu, 2023-11-23 at 17:17 -0500, Antoni Boucher wrote: > Hi. > I did split the patch and sent one for the bfloat16 support and > another > one for the vector support. > > Here's the updated patch for the machine-dependent builtins. > Thanks for the patch; sorry about the long delay in reviewing it. CCing Jan and Uros re the i386 part of that patch; for reference the patch being discussed is here: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638027.html > From e025f95f4790ae861e709caf23cbc0723c1a3804 Mon Sep 17 00:00:00 2001 > From: Antoni Boucher > Date: Mon, 23 Jan 2023 17:21:15 -0500 > Subject: [PATCH] libgccjit: Add support for machine-dependent builtins [...snip...] > diff --git a/gcc/config/i386/i386-builtins.cc > b/gcc/config/i386/i386-builtins.cc > index 42fc3751676..5cc1d6f4d2e 100644 > --- a/gcc/config/i386/i386-builtins.cc > +++ b/gcc/config/i386/i386-builtins.cc > @@ -225,6 +225,22 @@ static GTY(()) tree ix86_builtins[(int) > IX86_BUILTIN_MAX]; > > struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX]; > > +static void > +clear_builtin_types (void) > +{ > + for (int i = 0 ; i < IX86_BT_LAST_CPTR + 1 ; i++) > +ix86_builtin_type_tab[i] = NULL; > + > + for (int i = 0 ; i < IX86_BUILTIN_MAX ; i++) > + { > +ix86_builtins[i] = NULL; > +ix86_builtins_isa[i].set_and_not_built_p = true; > + } > + > + for (int i = 0 ; i < IX86_BT_LAST_ALIAS + 1 ; i++) > +ix86_builtin_func_type_tab[i] = NULL; > +} > + > tree get_ix86_builtin (enum ix86_builtins c) > { >return ix86_builtins[c]; > @@ -1483,6 +1499,8 @@ ix86_init_builtins (void) > { >tree ftype, decl; > > + clear_builtin_types (); > + >ix86_init_builtin_types (); > >/* Builtins to get CPU type and features. */ Please can one of the i386 maintainers check this? (CCing Jan and Uros: this is for the case where the compiler code runs multiple times in-process due to being linked into libgccjit.so. We want to restore state within i386-builtins.cc to an initial state, and ensure that no GC-managed objects persist from previous in-memory compiles). > diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst > index ebede440ee4..764de23341e 100644 > --- a/gcc/jit/docs/topics/compatibility.rst > +++ b/gcc/jit/docs/topics/compatibility.rst > @@ -378,3 +378,12 @@ alignment of a variable: > > ``LIBGCCJIT_ABI_25`` covers the addition of > :func:`gcc_jit_type_get_restrict` > + > +.. _LIBGCCJIT_ABI_26: > + > +``LIBGCCJIT_ABI_26`` > + > + > +``LIBGCCJIT_ABI_26`` covers the addition of a function to get target > builtins: > + > + * :func:`gcc_jit_context_get_target_builtin_function` > diff --git a/gcc/jit/docs/topics/functions.rst > b/gcc/jit/docs/topics/functions.rst > index cf5cb716daf..e9b77fdb892 100644 > --- a/gcc/jit/docs/topics/functions.rst > +++ b/gcc/jit/docs/topics/functions.rst > @@ -140,6 +140,25 @@ Functions >uses such a parameter will lead to an error being emitted within >the context. > > +.. function:: gcc_jit_function *\ > + gcc_jit_context_get_target_builtin_function (gcc_jit_context > *ctxt,\ > +const char *name) > + > + Get the :type:`gcc_jit_function` for the built-in function with the > + given name. For example: Might be nice to add the "(sometimes called intrinsic functions)" text you have in the header here. [...snip] > diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc > index a729086bafb..3ca9702d429 100644 > --- a/gcc/jit/dummy-frontend.cc > +++ b/gcc/jit/dummy-frontend.cc [...] > @@ -29,8 +30,14 @@ along with GCC; see the file COPYING3. If not see > #include "options.h" > #include "stringpool.h" > #include "attribs.h" > +#include "jit-recording.h" > +#include "print-tree.h" > > #include > +#include > +#include > + > +using namespace gcc::jit; > > /* Attribute handling. */ > > @@ -86,6 +93,11 @@ static const struct attribute_spec::exclusions > attr_const_pure_exclusions[] = >ATTR_EXCL (NULL, false, false, false) > }; > > +hash_map target_builtins{}; I was wondering if this needs a GTY marker, but I don't think it does: presumably it's only used within jit_langhook_parse_file where no GC can happen - unless jit_langhook_write_globals makes use of it? > +std::unordered_map target_function_types > +{}; > +recording::context target_builtins_ctxt{NULL}; Please add a comment to target_builtins_ctxt saying what it's for. As far as I can tell, it's for getting at recording::types from tree_type_to_jit_type; we then use a new "copy" mechanism to copy objects from target_builtins_ctxt for use with the real recording::context. This feels ugly, but maybe it's the only way to make it work. Could tree_type_to_jit_type take a recording::context as a param? The only non-recursive uses of tree_type_to_jit_type seem to be in jit_langhook_builtin_funct
Re: Frontend access to target features (was Re: [PATCH] libgccjit: Add ability to get CPU features)
On Tue, 2024-10-29 at 07:59 -0400, Antoni Boucher wrote: > David: Arthur reviewed the gccrs patch and would be OK with it. > > Could you please take a look and review it? https://github.com/Rust-GCC/gccrs/pull/3195 looks good to me; thanks! Dave > > Le 2024-10-17 à 11 h 38, Antoni Boucher a écrit : > > Hi. > > Thanks for the review, David! > > > > I talked to Arthur and he's OK with having a file to include in > > both > > gccrs and libgccjit. > > > > I sent the patch to gccrs to move the code in a new file that we > > can > > include in both frontends: > > https://github.com/Rust-GCC/gccrs/pull/3195 > > > > I also renamed gcc_jit_target_info_supports_128bit_int to > > gcc_jit_target_info_supports_target_dependent_type because a > > subsequent > > patch will allow to check if other types are supported like > > _Float16 and > > _Float128. > > > > Here's the patch for libgccjit updated to include this file. > > > > Thanks. > > > > Le 2024-06-26 à 17 h 55, David Malcolm a écrit : > > > On Sun, 2024-03-10 at 12:05 +0100, Iain Buclaw wrote: > > > > Excerpts from David Malcolm's message of März 5, 2024 4:09 pm: > > > > > On Thu, 2023-11-09 at 19:33 -0500, Antoni Boucher wrote: > > > > > > Hi. > > > > > > See answers below. > > > > > > > > > > > > On Thu, 2023-11-09 at 18:04 -0500, David Malcolm wrote: > > > > > > > On Thu, 2023-11-09 at 17:27 -0500, Antoni Boucher wrote: > > > > > > > > Hi. > > > > > > > > This patch adds support for getting the CPU features in > > > > > > > > libgccjit > > > > > > > > (bug > > > > > > > > 112466) > > > > > > > > > > > > > > > > There's a TODO in the test: > > > > > > > > I'm not sure how to test that gcc_jit_target_info_arch > > > > > > > > returns > > > > > > > > the > > > > > > > > correct value since it is dependant on the CPU. > > > > > > > > Any idea on how to improve this? > > > > > > > > > > > > > > > > Also, I created a CStringHash to be able to have a > > > > > > > > std::unordered_set. Is there any built-in > > > > > > > > way > > > > > > > > of > > > > > > > > doing > > > > > > > > this? > > > > > > > > > > > > > > Thanks for the patch. > > > > > > > > > > > > > > Some high-level questions: > > > > > > > > > > > > > > Is this specifically about detecting capabilities of the > > > > > > > host > > > > > > > that > > > > > > > libgccjit is currently running on? or how the target was > > > > > > > configured > > > > > > > when libgccjit was built? > > > > > > > > > > > > I'm less sure about this part. I'll need to do more tests. > > > > > > > > > > > > > > > > > > > > One of the benefits of libgccjit is that, in theory, we > > > > > > > support > > > > > > > all > > > > > > > of > > > > > > > the targets that GCC already supports. Does this patch > > > > > > > change > > > > > > > that, > > > > > > > or > > > > > > > is this more about giving client code the ability to > > > > > > > determine > > > > > > > capabilities of the specific host being compiled for? > > > > > > > > > > > > This should not change that. If it does, this is a bug. > > > > > > > > > > > > > > > > > > > > I'm nervous about having per-target jit code. Presumably > > > > > > > there's a > > > > > > > reason that we can't reuse existing target logic here - > > > > > > > can you > > > > > > > please > > > > > > > describe what the problem is. I see that the ChangeLog > > > > > > > has: > > > > > > > > > > > > &g
jit backports to GCC 14
I've backported the following patches from trunk to releases/gcc-14 testsuite, jit: fix test-error-pr63969-missing-driver.c https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665552.html Trunk: r15-4360-gf8dcb559e615db. GCC 14: r14-10854-g771873f0a95162 jit: reset state in varasm.cc [PR117275] https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666263.html Trunk: r15-4580-g779c0390e3b57d. GCC 14: r14-10855-g70f911bf547326 jit: fix leak of pending_assemble_externals_set [PR117275] https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666732.html Trunk: r15-4739-g7f41203f08b994. GCC 14: r14-10856-gacc0b9ff9cf1bc Dave
[pushed: r15-4580] jit: reset state in varasm.cc [PR117275]
PR jit/117275 reports various jit test failures seen on powerpc64le-unknown-linux-gnu due to hitting this assertion in varasm.cc on the 2nd compilation in a process: #2 0x763e67d0 in assemble_external_libcall (fun=0x72a4b1d8) at ../../src/gcc/varasm.cc:2650 2650 gcc_assert (!pending_assemble_externals_processed); (gdb) p pending_assemble_externals_processed $1 = true We're not properly resetting state in varasm.cc after a compile for libgccjit. Fixed thusly. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Lightly tested on powerpc64le-unknown-linux-gnu (cfarm120). Pushed to trunk as r15-4580-g779c0390e3b57d. gcc/ChangeLog: PR jit/117275 * toplev.cc (toplev::finalize): Call varasm_cc_finalize. * varasm.cc (varasm_cc_finalize): New. * varasm.h (varasm_cc_finalize): New decl. Signed-off-by: David Malcolm --- gcc/toplev.cc | 1 + gcc/varasm.cc | 53 +++ gcc/varasm.h | 2 ++ 3 files changed, 56 insertions(+) diff --git a/gcc/toplev.cc b/gcc/toplev.cc index 5df59b79c803..62034c32b4af 100644 --- a/gcc/toplev.cc +++ b/gcc/toplev.cc @@ -2433,6 +2433,7 @@ toplev::finalize (void) ira_costs_cc_finalize (); tree_cc_finalize (); reginfo_cc_finalize (); + varasm_cc_finalize (); /* save_decoded_options uses opts_obstack, so these must be cleaned up together. */ diff --git a/gcc/varasm.cc b/gcc/varasm.cc index 92b105a4089a..b380963596ef 100644 --- a/gcc/varasm.cc +++ b/gcc/varasm.cc @@ -8849,4 +8849,57 @@ handle_vtv_comdat_section (section *sect, const_tree decl ATTRIBUTE_UNUSED) switch_to_comdat_section(sect, DECL_NAME (decl)); } +void +varasm_cc_finalize () +{ + first_global_object_name = nullptr; + weak_global_object_name = nullptr; + + const_labelno = 0; + size_directive_output = 0; + + last_assemble_variable_decl = NULL_TREE; + first_function_block_is_cold = false; + saw_no_split_stack = false; + text_section = nullptr; + data_section = nullptr; + readonly_data_section = nullptr; + sdata_section = nullptr; + ctors_section = nullptr; + dtors_section = nullptr; + bss_section = nullptr; + sbss_section = nullptr; + tls_comm_section = nullptr; + comm_section = nullptr; + lcomm_section = nullptr; + bss_noswitch_section = nullptr; + exception_section = nullptr; + eh_frame_section = nullptr; + in_section = nullptr; + in_cold_section_p = false; + cold_function_name = NULL_TREE; + unnamed_sections = nullptr; + section_htab = nullptr; + object_block_htab = nullptr; + anchor_labelno = 0; + shared_constant_pool = nullptr; + pending_assemble_externals = NULL_TREE; + pending_libcall_symbols = nullptr; + +#ifdef ASM_OUTPUT_EXTERNAL + pending_assemble_externals_processed = false; + pending_assemble_externals_set = nullptr; +#endif + + weak_decls = NULL_TREE; + initial_trampoline = nullptr; + const_desc_htab = nullptr; + weakref_targets = NULL_TREE; + alias_pairs = nullptr; + tm_clone_hash = nullptr; + trampolines_created = 0; + elf_init_array_section = nullptr; + elf_fini_array_section = nullptr; +} + #include "gt-varasm.h" diff --git a/gcc/varasm.h b/gcc/varasm.h index d9311dc370bb..f00ac7f3e5c9 100644 --- a/gcc/varasm.h +++ b/gcc/varasm.h @@ -81,4 +81,6 @@ extern rtx assemble_trampoline_template (void); extern void switch_to_comdat_section (section *, tree); +extern void varasm_cc_finalize (); + #endif // GCC_VARASM_H -- 2.26.3
Re: [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
On Thu, 2023-12-21 at 08:36 -0500, Antoni Boucher wrote: > Hi. > Here's the updated patch. > Thanks. Sorry for the delay in responding. The updated patch is good for trunk - thanks! Dave > > On Thu, 2023-12-07 at 20:15 -0500, David Malcolm wrote: > > On Thu, 2023-12-07 at 17:34 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch adds checks gcc_jit_block_add_assignment_op to make > > > sure > > > it > > > is only ever called on numeric types. > > > > > > With the previous patch, this might require a change to also > > > allow > > > vector types here. > > > > > > Thanks for the review. > > > > Thanks for the patch. > > > > [...snip...] > > > > > @@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op > > > (gcc_jit_block *block, > > > lvalue->get_type ()->get_debug_string (), > > > rvalue->get_debug_string (), > > > rvalue->get_type ()->get_debug_string ()); > > > + // TODO: check if it is a numeric vector? > > > + RETURN_IF_FAIL_PRINTF3 ( > > > + lvalue->get_type ()->is_numeric () && rvalue->get_type ()- > > > > is_numeric (), ctxt, loc, > > > + "gcc_jit_block_add_assignment_op %s has non-numeric lvalue > > > %s > > > (type: %s)", > > > + gcc::jit::binary_op_reproducer_strings[op], > > > + lvalue->get_debug_string (), lvalue->get_type ()- > > > > get_debug_string ()); > > > > The condition being tested here should probably just be: > > > > lvalue->get_type ()->is_numeric () > > > > since otherwise if the lvalue's type is numeric and the rvalue's > > type > > fails to be, then the user would incorrectly get a message about > > the > > lvalue. > > > > > + RETURN_IF_FAIL_PRINTF3 ( > > > + rvalue->get_type ()->is_numeric () && rvalue->get_type ()- > > > > is_numeric (), ctxt, loc, > > > + "gcc_jit_block_add_assignment_op %s has non-numeric rvalue > > > %s > > > (type: %s)", > > > + gcc::jit::binary_op_reproducer_strings[op], > > > + rvalue->get_debug_string (), rvalue->get_type ()- > > > > get_debug_string ()); > > > > The condition being tested here seems to have a redundant repeated: > > && rvalue->get_type ()->is_numeric () > > > > Am I missing something, or is that a typo? > > > > [...snip...] > > > > The patch is OK otherwise. > > > > Thanks > > Dave > > > > > > >
Re: [PATCH] libgccjit: Fix float playback for cross-compilation
On Thu, 2024-01-25 at 16:04 -0500, Antoni Boucher wrote: > Thanks for the review! > On Wed, 2024-01-24 at 13:10 -0500, David Malcolm wrote: > > On Thu, 2024-01-11 at 18:42 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch fixes the bug 113343. > > > I'm wondering if there's a better solution than using mpfr. > > > The only other solution I found is real_from_string, but that > > > seems > > > overkill to convert the number to a string. > > > I could not find a better way to create a real value from a host > > > double. > > > > I took a look, and I don't see a better way; it seems weird to go > > through a string stage. Ideally there would be a > > real_from_host_double, but I don't see one. Sorry for the delay in responding > > > > Is there a cross-platform way to directly access the representation > > of > > a host double? > > I have no idea. Neither do I, but presumably this patch is fixing a real problem (no pun intended)... > > > > > > If there's no solution, do we lose some precision by using mpfr? > > > Running Rust's core library tests, there was a difference of one > > > decimal, so I'm wondering if there's some lost precision, or if > > > it's > > > just because those tests don't work on m68k which was my test > > > target. > > > > Sorry, can you clarify what you mean by "a difference of one > > decimal" > > above? > > Let's say the Rust core tests expected the value "1.23456789", it > instead got the value "1.2345678" (e.g. without the last decimal). > Not sure if this is expected. > Everything works fine for x86-64; this just happened for m68k which > is > not well supported for now in Rust, so that might just be that the > test > doesn't work on this platform. > > > > > > Also, I'm not sure how to write a test this fix. Any ideas? > > > > I think we don't need cross-compilation-specific tests, we should > > just > > use and/or extend the existing coverage for > > gcc_jit_context_new_rvalue_from_double e.g. in test-constants.c and > > test-types.c > > > > We probably should have test coverage for "awkward" values; we > > already > > have coverage for DBL_MIN and DBL_MAX, but we don't yet have test > > coverage for: > > * quiet/signaling NaN > > * +ve/-ve inf > > * -ve zero ...and I'm guessing you've tested this code on all of the various configurations you're targeting. Assuming that, this patch is good for trunk. > > Is this something you would want for this patch? No, that's just for bonus points :) Thanks Dave
Re: [PATCH] libgccjit: Add vector permutation and vector access operations
On Thu, 2023-11-30 at 17:16 -0500, Antoni Boucher wrote: > All of these are fixed in this new patch. > Thanks for the review. Thanks for the updated patch. I had said "OK with those fixed" on the older version and it looks like you have indeed fixed the issues I noticed, so this updated patch is OK for trunk. Sorry for not being clarifying earlier Dave > > On Mon, 2023-11-20 at 18:05 -0500, David Malcolm wrote: > > On Fri, 2023-11-17 at 17:36 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch adds a vector permutation and vector access operations > > > (bug > > > 112602). > > > > > > This was split from this patch: > > > https://gcc.gnu.org/pipermail/jit/2023q1/001606.html > > > > > > > Thanks for the patch. > > > > Overall, looks good, but 3 minor nitpicks: > > > > [...snip...] > > > > > diff --git a/gcc/jit/docs/topics/compatibility.rst > > > b/gcc/jit/docs/topics/compatibility.rst > > > index ebede440ee4..a764e3968d1 100644 > > > --- a/gcc/jit/docs/topics/compatibility.rst > > > +++ b/gcc/jit/docs/topics/compatibility.rst > > > @@ -378,3 +378,13 @@ alignment of a variable: > > > > > > ``LIBGCCJIT_ABI_25`` covers the addition of > > > :func:`gcc_jit_type_get_restrict` > > > + > > > + > > > +.. _LIBGCCJIT_ABI_26: > > > + > > > +``LIBGCCJIT_ABI_26`` > > > + > > > +``LIBGCCJIT_ABI_26`` covers the addition of functions to > > > manipulate vectors: > > > + > > > + * :func:`gcc_jit_context_new_rvalue_vector_perm` > > > + * :func:`gcc_jit_context_new_vector_access` > > > diff --git a/gcc/jit/docs/topics/expressions.rst > > > b/gcc/jit/docs/topics/expressions.rst > > > index 42cfee36302..4a45aa13f5c 100644 > > > --- a/gcc/jit/docs/topics/expressions.rst > > > +++ b/gcc/jit/docs/topics/expressions.rst > > > @@ -295,6 +295,35 @@ Vector expressions > > > > > > #ifdef > > > LIBGCCJIT_HAVE_gcc_jit_context_new_rvalue_from_vector > > > > > > +.. function:: gcc_jit_rvalue * \ > > > + gcc_jit_context_new_rvalue_vector_perm > > > (gcc_jit_context *ctxt, \ > > > + > > > gcc_jit_location *loc, \ > > > + > > > gcc_jit_rvalue *elements1, \ > > > + > > > gcc_jit_rvalue *elements2, \ > > > + > > > gcc_jit_rvalue *mask); > > > + > > > + Build a permutation of two vectors. > > > + > > > + "elements1" and "elements2" should have the same type. > > > + The length of "mask" and "elements1" should be the same. > > > + The element type of "mask" should be integral. > > > + The size of the element type of "mask" and "elements1" should > > > be the same. > > > + > > > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_25`; you can > > > test for > > ^^ > > Should be 26 > > > > [...snip...] > > > > > Unary Operations > > > > > > > > > @@ -1020,3 +1049,27 @@ Field access is provided separately for > > > both > > > lvalues and rvalues. > > > PTR[INDEX] > > > > > > in C (or, indeed, to ``PTR + INDEX``). > > > + > > > +.. function:: gcc_jit_lvalue *\ > > > + gcc_jit_context_new_vector_access (gcc_jit_context > > > *ctxt,\ > > > + > > > gcc_jit_location > > > *loc,\ > > > + gcc_jit_rvalue > > > *vector,\ > > > + gcc_jit_rvalue > > > *index) > > > + > > > + Given an rvalue of vector type ``T __attribute__ > > > ((__vector_size__ (SIZE)))``, get the element `T` at > > > + the given index. > > > + > > > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_25`; you can > > > test for > > ^^ > > > > Likewise here. > > > > [...snip...] > > > > > @@ -4071,6
Re: [PATCH] libgccjit: Add support for machine-dependent builtins
On Tue, 2024-11-19 at 21:45 +0100, Mark Wielaard wrote: > Hi, > > Random request... > > On Tue, Nov 19, 2024 at 11:14:38AM -0500, David Malcolm wrote: > > > Here's the updated patch and answers below. > > > > > > (GitHub link if you find it easier for review: > > > https://github.com/antoyo/libgccjit/pull/5) > > > > > > Thanks. > > > > Thanks; I looked over the patch via the above link and it looks > > good to > > me for trunk. > > Since we now have an experimental forge at > https://forge.sourceware.org > would it be an idea to use that for such reviews? > > We would love to get feedback on the forge idea (but ideally one > based > on Free Software and under community control). > > See for some more background: > https://gcc.gnu.org/wiki/ForgeExperiment > > You could sign up with your gcc ids (antoyo@gcc... or > dmalcolm@gcc...). > > Please sent requests for help, feedback (good or bad) to the forge > mailinglist: https://sourceware.org/mailman/listinfo/forge (You don't > need to subscribe unless you want to be part of the forge community.) Thanks Mark. I'm finding it a *lot* easier to manage patch reviews using github rather than mailing list threads: specifically: the github web UI has: (a) convenient metadata tags that make it clear "who has the ball" for each patch, integrated with: (b) really nice UX for viewing and commenting on patches which the mailing-list plus patchwork approach is far inferior to IMHO. I hope the forge instance has similar capabilities for both (a) and (b). One downside of the https://github.com/antoyo/libgccjit/pull workflow for (a) is that I can't edit the labels when I review things (though maybe Antoni can give me access to that?) ...but yeah, it's not ideal to be using a closed source site for this. That said, Antoni and I have things split between two places already: the mailing lists and his github. I'd be up for doing it in the forge instance instead, but arguably we'd then be splitting things into *three* places... gahhh AIUI Antoni is already using github for interacting with the rustc project, so it might be considerably easier for him to stick to github; I'm feeling guilty about my crappiness at patch review so I feel I don't have much of a moral high ground here to push for a non- proprietary tool. Dave
Re: [PATCH] libgccjit: Allow comparing aligned int types
On Thu, 2024-02-22 at 12:40 -0500, Antoni Boucher wrote: > Thanks for the review. > Here's the updated patch. Thanks; the updated patch is good for trunk. Dave > > On Wed, 2024-01-24 at 12:18 -0500, David Malcolm wrote: > > On Thu, 2023-12-21 at 08:33 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch allows comparing aligned integer types as equal. > > > There's a TODO in the code about whether we should check that the > > > alignment is equal. > > > What are your thoughts on this? > > > > I think we should check for equal alignment. > > > > [...snip...] > > > > > diff --git a/gcc/testsuite/jit.dg/test-types.c > > > b/gcc/testsuite/jit.dg/test-types.c > > > index a01944e35fa..c2f4d2bcb3d 100644 > > > --- a/gcc/testsuite/jit.dg/test-types.c > > > +++ b/gcc/testsuite/jit.dg/test-types.c > > > @@ -485,11 +485,15 @@ verify_code (gcc_jit_context *ctxt, > > > gcc_jit_result *result) > > > > > > CHECK_VALUE (z.m_FILE_ptr, stderr); > > > > > > + gcc_jit_type *long_type = gcc_jit_context_get_type (ctxt, > > > GCC_JIT_TYPE_LONG); > > > + gcc_jit_type *int64_type = gcc_jit_context_get_type (ctxt, > > > GCC_JIT_TYPE_INT64_T); > > > if (sizeof(long) == 8) > > > - CHECK (gcc_jit_compatible_types ( > > > - gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_LONG), > > > - gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT64_T))); > > > + CHECK (gcc_jit_compatible_types (long_type, int64_type)); > > > > > > CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type > > > (ctxt, GCC_JIT_TYPE_FLOAT)), sizeof (float)); > > > CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type > > > (ctxt, GCC_JIT_TYPE_DOUBLE)), sizeof (double)); > > > + > > > + gcc_jit_type *aligned_long = gcc_jit_type_get_aligned > > > (long_type, 4); > > > + gcc_jit_type *aligned_int64 = gcc_jit_type_get_aligned > > > (int64_type, 4); > > > + CHECK (gcc_jit_compatible_types (aligned_long, > > > aligned_int64)); > > > > This CHECK should be guarded on sizeof(long) == 8 like the check > > above. > > > > > > Dave > > >
Re: [PATCH] libgccjit: Allow sending a const pointer as argument
On Thu, 2023-12-21 at 11:59 -0500, Antoni Boucher wrote: > Hi. > This patch adds the ability to send const pointer as argument to a > function. > Thanks for the review. Sorry for the long delay in responding to this. I'm a bit worried that this might break some type-safety within libgccjit, or that you might have a bug in your client code. In the testcase you have: gcc_jit_type *int_type = gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); gcc_jit_type *int_ptr_type = gcc_jit_type_get_pointer (int_type); gcc_jit_type *const_ptr_type = gcc_jit_type_get_const (int_ptr_type); /* Build the test_ptr. */ gcc_jit_param *param = gcc_jit_context_new_param (ctxt, NULL, const_ptr_type, "value"); which is creating the equivalent of int *const value i.e. a const pointer to a non-const int whereas in the comment you have: /* Let's try to inject the equivalent of: int test_ptr(const int* value) where "const int *" is a non-const pointer to a const int. So is the get_const and the get_pointer the wrong way round somewhere in your code? Or am I missing something here? Dave
Re: [PATCH] libgccjit: Support signed char flag
On Thu, 2024-02-22 at 15:29 -0500, Antoni Boucher wrote: > Thanks for the review and idea. Thanks for the updated patch; sorry about the delay in reviewing. > > Here's the updated patch. I added a test, but I could not set - > fsigned- > char as this is not an option accepted by the jit frontend. > However, the test still works in the sense that it fails without this > patch and passes with it. > I'm just wondering if it would pass on all targets or if I should add > a > target filtering directive to only execute on some target. > What do you think? The test looks good to me, I don't think it needs a target filtering directive since presumably any target on which it already passed without the patch will still pass with the patch. The patch is OK for trunk; thanks. Dave > > On Tue, 2024-01-09 at 11:01 -0500, David Malcolm wrote: > > On Thu, 2023-12-21 at 08:42 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch adds support for the -fsigned-char flag. > > > > Thanks. The patch looks correct to me. > > > > > I'm not sure how to test it since I stumbled upon this bug when I > > > found > > > this other bug > > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107863) > > > which is now fixed. > > > Any idea how I could test this patch? > > > > We already document that GCC_JIT_TYPE_CHAR has "some signedness". > > The > > bug being fixed here is that gcc_jit_context compilations were > > always > > treating "char" as unsigned, regardless of the value of -fsigned- > > char > > (either from the target's default, or as a context option), when it > > makes more sense to follow the C frontend's behavior. > > > > So perhaps jit-written code with a context that has -fsigned-char > > as > > an > > option (via gcc_jit_context_add_command_line_option), and which > > promotes a negative char to a signed int, and then returns the > > result > > as an int? Presumably if we're erroneously forcing "char" to be > > unsigned, the int will be in the range 0x80 to 0xff, rather that > > being > > negative. > > > > Dave > > >
Re: [PATCH] libgccjit: Add support for machine-dependent builtins
On Thu, 2024-11-14 at 15:27 -0500, Antoni Boucher wrote: > It seems we don't need to do the cleanup in i386-builtins.cc anymore, > so > I removed it. > David: Is it possible that your recent fixes for the GC within > libgccjit > also fixed the issue here? > > Here's the updated patch and answers below. > > (GitHub link if you find it easier for review: > https://github.com/antoyo/libgccjit/pull/5) > > Thanks. Thanks; I looked over the patch via the above link and it looks good to me for trunk. Dave > > Le 2024-06-26 à 18 h 49, David Malcolm a écrit : > > On Thu, 2023-11-23 at 17:17 -0500, Antoni Boucher wrote: > > > Hi. > > > I did split the patch and sent one for the bfloat16 support and > > > another > > > one for the vector support. > > > > > > Here's the updated patch for the machine-dependent builtins. > > > > > > > Thanks for the patch; sorry about the long delay in reviewing it. > > > > CCing Jan and Uros re the i386 part of that patch; for reference > > the > > patch being discussed is here: > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638027.html > > > > > From e025f95f4790ae861e709caf23cbc0723c1a3804 Mon Sep 17 > > > 00:00:00 2001 > > > From: Antoni Boucher > > > Date: Mon, 23 Jan 2023 17:21:15 -0500 > > > Subject: [PATCH] libgccjit: Add support for machine-dependent > > > builtins > > > > [...snip...] > > > > > diff --git a/gcc/config/i386/i386-builtins.cc > > > b/gcc/config/i386/i386-builtins.cc > > > index 42fc3751676..5cc1d6f4d2e 100644 > > > --- a/gcc/config/i386/i386-builtins.cc > > > +++ b/gcc/config/i386/i386-builtins.cc > > > @@ -225,6 +225,22 @@ static GTY(()) tree ix86_builtins[(int) > > > IX86_BUILTIN_MAX]; > > > > > > struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX]; > > > > > > +static void > > > +clear_builtin_types (void) > > > +{ > > > + for (int i = 0 ; i < IX86_BT_LAST_CPTR + 1 ; i++) > > > + ix86_builtin_type_tab[i] = NULL; > > > + > > > + for (int i = 0 ; i < IX86_BUILTIN_MAX ; i++) > > > + { > > > + ix86_builtins[i] = NULL; > > > + ix86_builtins_isa[i].set_and_not_built_p = true; > > > + } > > > + > > > + for (int i = 0 ; i < IX86_BT_LAST_ALIAS + 1 ; i++) > > > + ix86_builtin_func_type_tab[i] = NULL; > > > +} > > > + > > > tree get_ix86_builtin (enum ix86_builtins c) > > > { > > > return ix86_builtins[c]; > > > @@ -1483,6 +1499,8 @@ ix86_init_builtins (void) > > > { > > > tree ftype, decl; > > > > > > + clear_builtin_types (); > > > + > > > ix86_init_builtin_types (); > > > > > > /* Builtins to get CPU type and features. */ > > > > Please can one of the i386 maintainers check this? > > (CCing Jan and Uros: this is for the case where the compiler code > > runs > > multiple times in-process due to being linked into libgccjit.so. > > We > > want to restore state within i386-builtins.cc to an initial state, > > and > > ensure that no GC-managed objects persist from previous in-memory > > compiles). > > > > > diff --git a/gcc/jit/docs/topics/compatibility.rst > > b/gcc/jit/docs/topics/compatibility.rst > > > index ebede440ee4..764de23341e 100644 > > > --- a/gcc/jit/docs/topics/compatibility.rst > > > +++ b/gcc/jit/docs/topics/compatibility.rst > > > @@ -378,3 +378,12 @@ alignment of a variable: > > > > > > ``LIBGCCJIT_ABI_25`` covers the addition of > > > :func:`gcc_jit_type_get_restrict` > > > + > > > +.. _LIBGCCJIT_ABI_26: > > > + > > > +``LIBGCCJIT_ABI_26`` > > > + > > > + > > > +``LIBGCCJIT_ABI_26`` covers the addition of a function to get > > > target builtins: > > > + > > > + * :func:`gcc_jit_context_get_target_builtin_function` > > > diff --git a/gcc/jit/docs/topics/functions.rst > > > b/gcc/jit/docs/topics/functions.rst > > > index cf5cb716daf..e9b77fdb892 100644 > > > --- a/gcc/jit/docs/topics/functions.rst > > > +++ b/gcc/jit/docs/topics/functions.rst > > > @@ -140,6 +140,25 @@ Functions > > > uses such a parameter will lead to an error being emitted > > > within > > > the context. > > &
Re: GCC 14 jit API in C++ for creating union types?
On Fri, 2024-12-13 at 11:24 +0100, Basile Starynkevitch wrote: > On Fri, 2024-12-13 at 11:03 +0100, Basile Starynkevitch wrote: > > Hello all, > > > > On GNU Linux/Ubuntu/x86_64 I am using libgccjit++ (in > > https://github.com/RefPerSys/RefPerSys/ - a GPLv3+ inference engine > > project) but I cannot understand how to create in C++ a union type. > > > > > > Today (Dec 13, 2024) the file > > https://gcc.gnu.org/onlinedocs/jit/cp/topics/types.html > > don't explain how to create a GCCJIT union type. > > > > Is there a simple C++ example creating a union for libgccjit 14? > > > > Thanks > > > > It seems that gcc_jit_context_new_union_type exists as a C callable > function in /usr/lib/gcc/x86_64-linux-gnu/14/include/libgccjit.h but > has not yet a C++ wrapping function? That's quite likely. I wrote libgccjit++.h a decade ago for a particular project that I've abandoned [1], and it's become something of a second-class part of libgccjit as the C API has moved forward - sorry. I don't know of anyone else using libgccjit++.h, so you're likely to run into gaps like this - but any fixes would (I hope) be relatively simple, if you don't mind working on patches for libgccjit++.h. Dave [1] https://savannah.gnu.org/patch/index.php?8395 (a port of GNU Octave's JIT compiler to libgccjit)
Re: Running Compiled Guile Objects
On Sat, 2024-12-14 at 18:11 +0100, Basile Starynkevitch wrote: > On Sat, 2024-12-14 at 09:15 +0900, Nala Ginrut wrote: > > Hi Hakan! > > The current Guile is not AOT yet. Although the object file is ELF, > > it's > > just bytecode wrapped ELF header. So you can't run it as a regular > > executable file. > > > [Sorry if I'm missing context here, I'm only seeing the part of the thread where Basile CCed the jit@gcc.gnu.org mailing list] > Those willing to contribute a proper ahead-of-time compiler to GNU > guile could use the GNU CC libgccjit library which is part of the GCC > compiler. > https://gcc.gnu.org/onlinedocs/jit/ ...and https://gcc.gnu.org/wiki/JIT Indeed, it turns out that everyone using libgccjit is using it for ahead-of-time compilation, rather than jit-compilation. Sorry about picking a bad name :) Probably of most interest to guile developers might be the gccemacs project here: https://akrl.sdf.org/gccemacs.html since AFAIK that has successfully used libgccjit from within a lisp- like language to get noticeable speedups. But guile developers probably know more about this than I do; I confess I don't know much about the lisp/scheme world. > > This libgccjit layer of the GCC compiler is stable and maintained C > API > and has some obsolete C++ API (which seems unmaintained in december > 2024). Most of the libgccjit code is internally coded (under GPL > license) in C++, but the stable API is for C. Indeed, it is gcc's code generation source code (which is C++), wrapped in a C API and header (it looks like a frontend to the rest of gcc, and like an API to the client code). Hope this is constructive Dave > > I am using the C API of libgccjit in the RefPerSys open source > inference engine project (GPLv3+ licensed) on > https://github.com/RefPerSys/RefPerSys/ > > Both libgccjit and GNU lightning (see > https://www.gnu.org/software/lightning/ ...) could be a basis for > adding a genuine compilation layer to GNU guile. And RefPerSys uses > both. > > I guess a significant issue would be to use libgccjit (or GNU > lightning) with GUILE's garbage collector (which seems to be Boehm > conservative GC). > > The RefPerSys project has a precise garbage collector and some > persistence (to textual files). Since it is GPLv3+ licensed, its code > could be reused in a future GUILE major version. RefPerSys is mostly > coded in C++. > > I did contribute to GCC long time ago and hope that RefPerSys could > become a GNU project (but don't know how to make that happen) > > Regards from near Paris in France. >
Re: Running Compiled Guile Objects
On Sun, 2024-12-15 at 00:43 +0100, Maxime Devos wrote: > > > Those willing to contribute a proper ahead-of-time compiler to > > > GNU > > > guile could use the GNU CC libgccjit library which is part of the > > > GCC > > > compiler. > > > https://gcc.gnu.org/onlinedocs/jit/ > > > > ...and https://gcc.gnu.org/wiki/JIT > > > > Indeed, it turns out that everyone using libgccjit is using it for > > ahead-of-time compilation, rather than jit-compilation. Sorry > > about > > picking a bad name :) > > Are we talking about implementing a ‘to machine code’ compiler for > Guile, or about implementing an ‘AOT to machine code’? I confess I have no idea what the conversation was about; I was just responding to Basile's email to the "jit" mailing list :) > Guile already has the former – it has a JIT (bytecode -> machine > code) for some systems. > > For what it’s worth -- I never worked with libgccjit or with the JIT > code of Guile: > > I imagine a basic (POC) AOT approach for Guile would be to let it > compile AOT – with the JIT implementation, except adjusted to be > relocatable and to add relocation information. As far as I can tell, > libgccjit does not seem to support relocations and doesn’t say > anything about whether the results are position-independent or not > (so not suitable fo AOT), though presumably there are ways around > that given the existence of gccemacs. FWIW libgccjit builds position independent code, and can be used to build dynamic libraries (which is what I believe gccemacs is doing). Dave
[pushed: r15-7126] jit: fix startup on aarch64
libgccjit fails on startup on aarch64 (and probably other archs). The issues are that (a) within jit_langhook_init the call to targetm.init_builtins can use types that aren't representable via jit::recording::type, and (b) targetm.init_builtins can call lang_hooks.decls.pushdecl, which although a no-op for libgccjit has a gcc_unreachable. Fixed thusly. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r15-7126-g27470f9a818538. gcc/jit/ChangeLog: * dummy-frontend.cc (tree_type_to_jit_type): For POINTER_TYPE, bail out if the inner call to tree_type_to_jit_type fails. Don't abort on unknown types. (jit_langhook_pushdecl): Replace gcc_unreachable with return of NULL_TREE. Signed-off-by: David Malcolm --- gcc/jit/dummy-frontend.cc | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc index 574851696311..1d0080d6fecb 100644 --- a/gcc/jit/dummy-frontend.cc +++ b/gcc/jit/dummy-frontend.cc @@ -1278,6 +1278,8 @@ recording::type* tree_type_to_jit_type (tree type) { tree inner_type = TREE_TYPE (type); recording::type* element_type = tree_type_to_jit_type (inner_type); +if (!element_type) + return nullptr; return element_type->get_pointer (); } else @@ -1299,10 +1301,6 @@ recording::type* tree_type_to_jit_type (tree type) } } } - -fprintf (stderr, "Unknown type:\n"); -debug_tree (type); -abort (); } return NULL; @@ -1372,7 +1370,7 @@ jit_langhook_global_bindings_p (void) static tree jit_langhook_pushdecl (tree decl ATTRIBUTE_UNUSED) { - gcc_unreachable (); + return NULL_TREE; } static tree -- 2.26.3
Re: [pushed: r15-7126] jit: fix startup on aarch64
On Wed, 2025-01-22 at 08:47 -0500, Antoni Boucher wrote: > Hi David. Hi Antoni I went ahead and pushed this patch since without it even simple unit tests like test-factorial.c were failing for me on aarch64; in particular, the build of emacs was failing in Fedora's mass rebuild with gcc 15, and the patch seems to be a minimal way to fix this. > I had a patch for this here: > https://github.com/antoyo/libgccjit/pull/20 Sorry for this getting stuck. I see you marked that as "waiting for review", but it looks like the patch there hasn't changed since my last review within the github web UI; was that to signify that you were hoping for an answer to the questions you asked on how to better implement the fix? > > The fact that you removed the debug_tree (and abort) will make it > harder > to figure out what the missing types to handle are. > This will also probably make it hard for people to understand why > they > get a type error when calling a builtin function with an unsupported > type. > And as you can see in my PR, at least a few types were missing that > you > didn't add in your patch. > > Do you have a better solution for this? > > I just thought about this potential solution: perhaps if we get an > unsupported type, we could add the builtin to an array instead of the > hashmap: this way, we could tell the user that this builtin is not > currently supported. > What are your thoughts on this? I'll take another look at PR 117886 now and see if I can implement something. Dave > > Thanks. > > Le 2025-01-22 à 08 h 38, David Malcolm a écrit : > > libgccjit fails on startup on aarch64 (and probably other archs). > > > > The issues are that > > > > (a) within jit_langhook_init the call to > > targetm.init_builtins can use types that aren't representable > > via jit::recording::type, and > > > > (b) targetm.init_builtins can call lang_hooks.decls.pushdecl, which > > although a no-op for libgccjit has a gcc_unreachable. > > > > Fixed thusly. > > > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > Pushed to trunk as r15-7126-g27470f9a818538. > > > > gcc/jit/ChangeLog: > > * dummy-frontend.cc (tree_type_to_jit_type): For > > POINTER_TYPE, > > bail out if the inner call to tree_type_to_jit_type fails. > > Don't abort on unknown types. > > (jit_langhook_pushdecl): Replace gcc_unreachable with > > return of > > NULL_TREE. > > > > Signed-off-by: David Malcolm > > --- > > gcc/jit/dummy-frontend.cc | 8 +++- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc > > index 574851696311..1d0080d6fecb 100644 > > --- a/gcc/jit/dummy-frontend.cc > > +++ b/gcc/jit/dummy-frontend.cc > > @@ -1278,6 +1278,8 @@ recording::type* tree_type_to_jit_type (tree > > type) > > { > > tree inner_type = TREE_TYPE (type); > > recording::type* element_type = tree_type_to_jit_type > > (inner_type); > > + if (!element_type) > > + return nullptr; > > return element_type->get_pointer (); > > } > > else > > @@ -1299,10 +1301,6 @@ recording::type* tree_type_to_jit_type (tree > > type) > > } > > } > > } > > - > > - fprintf (stderr, "Unknown type:\n"); > > - debug_tree (type); > > - abort (); > > } > > > > return NULL; > > @@ -1372,7 +1370,7 @@ jit_langhook_global_bindings_p (void) > > static tree > > jit_langhook_pushdecl (tree decl ATTRIBUTE_UNUSED) > > { > > - gcc_unreachable (); > > + return NULL_TREE; > > } > > > > static tree >
[pushed: r15-7179] jit: fix for write_reproducer [PR117886]
The original generated .c reproducer for PR jit/117886 did not compile, with: main.c: In function ‘create_code’: main.c:600:9: error: initialization of ‘gcc_jit_rvalue *’ from incompatible pointer type ‘gcc_jit_lvalue *’ [-Wincompatible-pointer-types] 600 | local__1, | ^~~~ The issue is that recording::ctor::write_reproducer was missing creation of casts to gcc_jit_rvalue * for gcc_jit_context_new_array_constructor and gcc_jit_context_new_struct_constructor. Fixed thusly. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r15-7179-g4d18acf8023ba0. gcc/jit/ChangeLog: PR jit/117886 * jit-recording.cc (reproducer::get_identifier_as_rvalue): Handle null memento. (reproducer::get_identifier_as_lvalue): Likewise. (reproducer::get_identifier_as_type): Likewise. (recording::ctor::write_reproducer): Use get_identifier_as_rvalue rather than get_identifier when writing out gcc_jit_rvalue * expressions. gcc/testsuite/ChangeLog: PR jit/117886 * jit.dg/all-non-failing-tests.h: Add test-pr117886-write-reproducer.c. * jit.dg/test-pr117886-write-reproducer.c: New test. Signed-off-by: David Malcolm --- gcc/jit/jit-recording.cc | 10 +- gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 ++ .../jit.dg/test-pr117886-write-reproducer.c | 103 ++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/jit.dg/test-pr117886-write-reproducer.c diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc index e6fef5bca076..8da3cb059156 100644 --- a/gcc/jit/jit-recording.cc +++ b/gcc/jit/jit-recording.cc @@ -406,6 +406,8 @@ reproducer::get_identifier (recording::memento *m) const char * reproducer::get_identifier_as_rvalue (recording::rvalue *m) { + if (!m) +return "NULL"; return m->access_as_rvalue (*this); } @@ -415,6 +417,8 @@ reproducer::get_identifier_as_rvalue (recording::rvalue *m) const char * reproducer::get_identifier_as_lvalue (recording::lvalue *m) { + if (!m) +return "NULL"; return m->access_as_lvalue (*this); } @@ -424,6 +428,8 @@ reproducer::get_identifier_as_lvalue (recording::lvalue *m) const char * reproducer::get_identifier_as_type (recording::type *m) { + if (!m) +return "NULL"; return m->access_as_type (*this); } @@ -6041,7 +6047,7 @@ recording::ctor::write_reproducer (reproducer &r) r.write ("gcc_jit_rvalue *value = NULL;\n"); else r.write ("gcc_jit_rvalue *value = %s;\n", -r.get_identifier (m_values[0])); +r.get_identifier_as_rvalue (m_values[0])); if (m_fields.length () == 0) r.write ("gcc_jit_field *field = NULL;\n"); @@ -6058,7 +6064,7 @@ recording::ctor::write_reproducer (reproducer &r) { r.write ("gcc_jit_rvalue *values[] = {\n"); for (size_t i = 0; i < m_values.length (); i++) - r.write ("%s,\n", r.get_identifier (m_values[i])); + r.write ("%s,\n", r.get_identifier_as_rvalue (m_values[i])); r.write (" };\n"); } /* Write the array of fields. */ diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h index b566925fd3dd..add5619aebd4 100644 --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h @@ -344,6 +344,13 @@ #undef create_code #undef verify_code +/* test-pr117886-write-reproducer.c. */ +#define create_code create_code_pr117886_write_reproducer +#define verify_code verify_code_pr117886_write_reproducer +#include "test-pr117886-write-reproducer.c" +#undef create_code +#undef verify_code + /* test-pure-attribute.c: This can't be in the testcases array as it needs the `-O3` flag. */ @@ -603,6 +610,9 @@ const struct testcase testcases[] = { {"pr95314_rvalue_reuse", create_code_pr95314_rvalue_reuse, verify_code_pr95314_rvalue_reuse}, + {"pr117886_write_reproducer", + create_code_pr117886_write_reproducer, + verify_code_pr117886_write_reproducer}, {"reading_struct ", create_code_reading_struct , verify_code_reading_struct }, diff --git a/gcc/testsuite/jit.dg/test-pr117886-write-reproducer.c b/gcc/testsuite/jit.dg/test-pr117886-write-reproducer.c new file mode 100644 index ..5018b0ea3eda --- /dev/null +++ b/gcc/testsuite/jit.dg/test-pr117886-write-reproducer.c @@ -0,0 +1,103 @@ +/* Verify that we can generate and compile reproducers for + gcc_jit_context_new_array_constructor + when the values are lvalues */ + +#include +#include + +#include "libgccjit.h" +#include "harness.h" + +/* + int foo[3]; + + void t
Re: Allow to build libgccjit with a soname bound to the GCC major version
On Sat, 2025-02-08 at 10:33 +0100, Matthias Klose wrote: > When configuring GCC with --program-suffix=-$(BASE_VERSION) to allow > installation multiple GCC versions in parallel, the executable of the > driver (gcc-$(BASE_VERSION)) gets recorded in the libgccjit.so.0 > library. Assuming, that you only install the libgccjit.so.0 library > from the newest GCC, you have a libgccjit installed, which always > calls > back to the newest installed version of GCC. I'm not saying that the > ABI is changing, but I'd like to see the libgccjit calling out to the > corresponding compiler, and therefore installing a libgccjit with a > soname that matches the GCC major version. > > The downside is having to rebuild packages built against libgccjit > with > each major GCC version, but looking at the reverse dependencies, at > least for package builds, only emacs is using libgccjit. > > My plan to use this feature is to build a libgccjit0 using the > default > GCC (e.g. gcc-14), and a libgccjit15, when building a newer GCC. When > changing the GCC default to 15, building a libgccjit0 from gcc-15, > and a > libgccjit14 from gcc-14. > > When configuring without --enable-versioned-jit, the behavior is > unchanged. > > Ok for the trunk? Thanks; LGTM. Dave
[pushed: r15-7515] jit: add "final override" to diagnostic sink [PR116613]
I added class jit_diagnostic_listener in r15-4760-g0b73e9382ab51c but forgot to annotate one of the vfuncs with "override". Fixed thusly. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r15-7515-g6ac313525a1fae. gcc/jit/ChangeLog: PR other/116613 * dummy-frontend.cc (jit_diagnostic_listener::on_report_diagnostic): Add "final override". Signed-off-by: David Malcolm --- gcc/jit/dummy-frontend.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc index 1d0080d6fec..88784ec9e92 100644 --- a/gcc/jit/dummy-frontend.cc +++ b/gcc/jit/dummy-frontend.cc @@ -1017,7 +1017,7 @@ public: } void on_report_diagnostic (const diagnostic_info &info, -diagnostic_t orig_diag_kind) +diagnostic_t orig_diag_kind) final override { JIT_LOG_SCOPE (gcc::jit::active_playback_ctxt->get_logger ()); -- 2.26.3
Re: Re: Using libgccjit as an AOT compiler backend forcross-compilation (second attempt)
On Mon, 2025-06-16 at 16:43 +0200, m...@rochus-keller.ch wrote: > > Thanks for your respones. > > @David: > > > I can imagine something that implements the libgccjit API and then > > > proxies the calls to different underlying target support libraries, > > but > > > that doesn't exist yet, and wouldn't you save any space on disk or > > > build times, alas. > > I will load the library via dlopen/dlsym to avoid static dependencies > anyway > and I assume by this very mechanism I would also be able to deal with > different > versions of the library without implementing a proxy. That sounds like it should work, but see the caveat below... > > > > Not without a huge amount of work, as noted above. > > I understand, but at least building libgccjit so that the gcc > specific dependencies > like libgmp, libmpfr, libmpc can be avoided. Does build with -static > work for > libgccjit? To be honest, I'm not sure. You need to configure gcc with --enable- host-shared when enabling "jit", but maybe those flags are compatible? > Ideally I would the users offer a simple tar.gz download for each > architecture > and host system which they would reference by path from the IDE. > What do you think? Note that inasmuch as libgccjit.so implements the "code generation", it implements the code that goes from a high-level representation down to optimized assembler (analogous to, say, the cc1 binary in normal GCC usage), and has the code that "knows" how to implement the assembler and linker (analogous to the "gcc" "driver" binary in normal GCC usage) to go beyond justthe .s text-based version of the code. So to get object files or executables/libraries you'll also need an assembler and linker. This works for the single target case in that libgccjit.so does the same thing as "gcc" and invokes e.g. GNU as and ld with appropriate flags) - but if you're hoping to provide something per- target that users can download you're probably going to have to think about this too (sorry!) IIRC binutils supports all of its targets in one configuration/build, but the gcc "driver" logic might not have the smarts to do the right thing when invoking them (I'm not sure). So there *might* be some extra work there. If you haven't seen it yet, see: https://gcc.gnu.org/onlinedocs/jit/internals/index.html#overview-of-code-structure which has some diagrams that attempt to explain how it all fits together (for the jit use-case, at least). > > > @Basile > > Please not that I will use it for AOT, not for jit. Ironically, everyone using libgccjit is using it for AOT, rather than jit use-cases. Sorry (again) about the name... Hope this is constructive Dave
Re: Using libgccjit as an AOT compiler backend for cross-compilation (second attempt)
On Mon, 2025-06-16 at 15:29 +0200, m...@rochus-keller.ch wrote: > (second attempt, this time text-only, sorry) Hi R.K. > I consider using libgccjit as a backend for my forthcoming Micron > compiler. The compiler is supposed to be able to generate code for > different bare-metal targets like Raspi Pico, Raspi Zero, ESP32 > Xtensa or Risc-V etc. > > From what I understand so far I would have to build a separate > instance of libgccjit for each target. This results in quite a lot of > large binaries with a lot of redundancy. You are correct. Unfortunately gcc's code assumes a specific target chosen when gcc is configured. There have been some attempts at fixing this, see e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44566 but it would be a huge amount of work and isn't something that's going to be doable any time soon; sorry. > > Is it possible to build libgccjit with all the mentioned targets > included and then select via API for which target code should be > generated? Assume there is already a separate assembler and linker > für each target available. I can imagine something that implements the libgccjit API and then proxies the calls to different underlying target support libraries, but that doesn't exist yet, and wouldn't you save any space on disk or build times, alas. > > Is it possible to build a version of libgccjit which includes all > required GCC libraries so that only one shared library has to be > deployed? Not without a huge amount of work, as noted above. > > Are there already projects where libgccjit is used like this? Not to my knowledge. Sorry about this; hope this answer is helpful. Dave
Re: Make recording::memento::get_debug_string return the ownership of strings
On Fri, 2025-06-20 at 15:04 -0400, Antoni Boucher wrote: > Hi. > In rustc_codegen_gcc, we rely on recording::memento::get_debug_string > even though this function should only be used for debugging (because > there are no ways for now to compare rvalues, but this is another, > bigger story) and we faced huge memory usage (+100 GB) because > recording::memento::get_debug_string create memento strings, which > means > they will only be freed at the end of the compilation process. > > Would it be OK to change this so that the responsibility of freeing > the > strings is moved to the caller of gcc_jit_object_get_debug_string? > That might require to remove the caching that is done in > recording::memento::get_debug_string. > > What are your thoughts on this? I don't like this idea, since it would change the meaning of the API, creating a memory leak in any existing code that uses gcc_jit_object_get_debug_string. I suppose we could add a new entrypoint that creates a client-owned debug string, but it sounds like what you really want is a way to compare rvalues. What sort of comparison do you need on rvalues, and if we provided it, would it solve your memory issues? Hope this is constructive Dave