[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: A first attempt to implement jit_embed
|
From: |
Paulo César Pereira de Andrade |
|
Subject: |
Re: A first attempt to implement jit_embed |
|
Date: |
Wed, 31 Aug 2022 15:10:43 -0300 |
Em ter., 30 de ago. de 2022 às 18:29, Marc Nieper-Wißkirchen
<marc.nieper+gnu@gmail.com> escreveu:
>
> I am currently trying to wrap my head around GNU lightning's size
> calculations. I have never paid them any attention.
>
> Where does the magic number 144 in jit_get_max_instr in lib/jit_size.c come
> from?
I do not remember all the details. I believe it was some special value for
x86 prolog or arm embedded constants. The commit does not say much:
""
...
Avoid problems if JIT_INSTR_MAX is miscalculated
* lib/jit_size.c: Preventively use at least 144 bytes
if JIT_INSTR_MAX is less than it. The logic is not
guaranteed to be 100% precise, it is mostly heuristics
to allocate a buffer with as close as possible size,
but a wrong value may cause code generation to write
past the end of the buffer.
...
diff --git a/lib/jit_size.c b/lib/jit_size.c
index ae4c633..612417e 100644
--- a/lib/jit_size.c
+++ b/lib/jit_size.c
@@ -110,7 +110,7 @@ _jit_get_size(jit_state_t *_jit)
jit_word_t
jit_get_max_instr(void)
{
- return (JIT_INSTR_MAX);
+ return (JIT_INSTR_MAX >= 144 ? JIT_INSTR_MAX : 144);
}
...
"""
> In lib/jit_x86-sz.c, the size of the align statement is 6 for x86_64. This
> seems to be off by one given the current limitation of jit_align (at most a
> word size).
It just collects the size generated by instructions during a "make check"
when built with "--enable-devel-get-jit-size" and run "make get_jit_size"
and chooses the largest number of bytes used for the specific lightning
instruction.
> Marc
>
> Am Di., 16. Aug. 2022 um 20:35 Uhr schrieb Paulo César Pereira de Andrade
> <paulo.cesar.pereira.de.andrade@gmail.com>:
>>
>> Em dom., 14 de ago. de 2022 às 17:35, Marc Nieper-Wißkirchen
>> <marc.nieper+gnu@gmail.com> escreveu:
>>
>> Hi,
>>
>> > With the patch below, I implemented
>> >
>> > jit_embed(jit_pointer_t data, jit_uint32_t length)
>> >
>> > At the moment, the implementation is for x86, but porting to other CPUs is
>> > simple.
>> >
>> > Criticism is welcome.
>>
>> It appears you forgot to add the embed.tst file to the patch.
>> It could be useful to add multiple data types, so one could even add
>> assembly code in some different way, like encoded bytes.
>>
>> It should also be possible to add embed data larger than jit_instr_max.
>> Could change lib/jit_size.c:_jit_get_size() to check for jit_embed and
>> update the guessed size as appropriate. BTW, jit_get_size() likely
>> overestimates the size. It just plays safe, to not risk writing out of
>> bounds.
>> jit_code_embed should be handled in a special way, so that the jit_*sz.c
>> files would not add the largest found value when computing the size.
>> The patch also did not update the jit_x86-sz.c file, nor any other files,
>> so, it will compute the guessed buffer size incorrectly.
>> Probably no need to have jit_cc_a1_len, otherwise, could have a
>> generic way to print integers. Currently it only prints hexadecimal values
>> for any immediate value. Having it print signed integers for offsets would
>> be useful.
>> Could also use the word (w) field, no need for an extra uint32 field.
>> I suggested a prototype with a int32 field because I do not expect one
>> to embed more than 2G of data in a code buffer :)
>>
>> > Thanks,
>> >
>> > Marc
>> >
>> > diff --git a/bootstrap.conf b/bootstrap.conf
>> > index 423491b..c257814 100644
>> > --- a/bootstrap.conf
>> > +++ b/bootstrap.conf
>> > @@ -18,6 +18,7 @@
>> >
>> > # gnulib modules used by this package.
>> > gnulib_modules="
>> > + obstack
>> > "
>> >
>> > # gnulib library name.
>> > diff --git a/check/Makefile.am b/check/Makefile.am
>> > index fc9f232..48b6f41 100644
>> > --- a/check/Makefile.am
>> > +++ b/check/Makefile.am
>> > @@ -49,6 +49,7 @@ EXTRA_DIST = \
>> > 3to2.tst 3to2.ok \
>> > add.tst add.ok \
>> > align.tst align.ok \
>> > + embed.tst embed.ok \
>> > allocai.tst allocai.ok \
>> > allocar.tst allocar.ok \
>> > bp.tst bp.ok \
>> > @@ -113,7 +114,7 @@ EXTRA_DIST = \
>> > run-test all.tst
>> >
>> > base_TESTS = \
>> > - 3to2 add align allocai \
>> > + 3to2 add align embed allocai \
>> > allocar bp divi fib rpn \
>> > ldstr ldsti \
>> > ldstxr ldstxi \
>> > @@ -192,7 +193,8 @@ endif
>> > if test_arm_arm
>> > #arm_TESTS = $(addsuffix .arm, $(base_TESTS))
>> > arm_TESTS = \
>> > - 3to2.arm add.arm align.arm allocai.arm \
>> > + 3to2.arm add.arm align.arm embed.arm \
>> > + allocai.arm \
>> > allocar.arm bp.arm divi.arm fib.arm \
>> > rpn.arm ldstr.arm ldsti.arm \
>> > ldstxr.arm ldstxi.arm \
>> > diff --git a/check/lightning.c b/check/lightning.c
>> > index 3cf3e70..0a4b5ca 100644
>> > --- a/check/lightning.c
>> > +++ b/check/lightning.c
>> > @@ -26,6 +26,7 @@
>> > #else
>> > # include <unistd.h>
>> > #endif
>> > +#include <obstack.h>
>> > #include <stdio.h>
>> > #include <stdarg.h>
>> > #include <lightning.h>
>> > @@ -529,6 +530,7 @@ static label_t *get_label(skip_t skip);
>> > static token_t regname(void);
>> > static token_t identifier(int ch);
>> > static void get_data(type_t type);
>> > +static void get_embedded_data(type_t type);
>> > static void dot(void);
>> > static token_t number(int ch);
>> > static int escape(int ch);
>> > @@ -585,6 +587,7 @@ static int symbol_offset;
>> > static hash_t *instrs;
>> > static char *data;
>> > static size_t data_offset, data_length;
>> > +static struct obstack obstack[1];
>> > static instr_t instr_vector[] = {
>> > #define entry(value) { NULL, #value, value }
>> > #define entry2(name, function) { NULL, name, function }
>> > @@ -833,6 +836,12 @@ static instr_t instr_vector[] = {
>> > #undef entry
>> > };
>> >
>> > +/*
>> > + * Obstack allocation
>> > + */
>> > +#define obstack_chunk_alloc xmalloc
>> > +#define obstack_chunk_free free
>> > +
>> > /*
>> > * Implementation
>> > */
>> > @@ -2176,6 +2185,9 @@ get_data(type_t type)
>> > token_t token;
>> > char *test = data;
>> >
>> > + if (parser.parsing == PARSING_CODE)
>> > + return get_embedded_data(type);
>> > +
>> > for (;;) {
>> > switch (type) {
>> > case type_c:
>> > @@ -2239,6 +2251,32 @@ get_data(type_t type)
>> > }
>> > }
>> >
>> > +static void
>> > +get_embedded_data(type_t type)
>> > +{
>> > + void *data;
>> > + int ch;
>> > + jit_uint32_t len;
>> > +
>> > + for (;;) {
>> > + switch (type) {
>> > + case type_i:
>> > + len = sizeof(signed int);
>> > + data = obstack_alloc(obstack, len);
>> > + *(signed int *)data = get_int(skip_ws);
>> > + jit_embed(data, len);
>> > + break;
>> > + /* FIXME **more types** */
>> > + default:
>> > + abort();
>> > + }
>> > + ch = skipws();
>> > + if (ch == '\n' || ch == ';' || ch == EOF)
>> > + break;
>> > + ungetch(ch);
>> > + }
>> > +}
>> > +
>> > static void
>> > dot(void)
>> > {
>> > @@ -4086,6 +4124,8 @@ main(int argc, char *argv[])
>> > optind = 1;
>> > #endif
>> >
>> > + obstack_init (obstack);
>> > +
>> > progname = argv[0];
>> >
>> > init_jit(progname);
>> > @@ -4344,5 +4384,7 @@ main(int argc, char *argv[])
>> >
>> > finish_jit();
>> >
>> > + obstack_free (obstack, NULL);
>> > +
>> > return (0);
>> > }
>> > diff --git a/gnulib-lib/.gitignore b/gnulib-lib/.gitignore
>> > index d9f5394..9d61cde 100644
>> > --- a/gnulib-lib/.gitignore
>> > +++ b/gnulib-lib/.gitignore
>> > @@ -1,2 +1,18 @@
>> > /Makefile.am
>> > -/dummy.c
>> > +/_Noreturn.h
>> > +/alignof.h
>> > +/arg-nonnull.h
>> > +/c++defs.h
>> > +/exitfail.c
>> > +/exitfail.h
>> > +/gettext.h
>> > +/limits.in.h
>> > +/obstack.c
>> > +/obstack.h
>> > +/stddef.in.h
>> > +/stdint.in.h
>> > +/stdlib.in.h
>> > +/sys_types.in.h
>> > +/unistd.c
>> > +/unistd.in.h
>> > +/warn-on-use.h
>> > diff --git a/include/lightning.h.in b/include/lightning.h.in
>> > index 887a951..a7186a7 100644
>> > --- a/include/lightning.h.in
>> > +++ b/include/lightning.h.in
>> > @@ -187,6 +187,8 @@ typedef enum {
>> > #define jit_align(u) jit_new_node_w(jit_code_align, u)
>> > jit_code_live, jit_code_align,
>> > jit_code_save, jit_code_load,
>> > +#define jit_embed(u,v) jit_new_node_pl(jit_code_embed,u,v)
>> > + jit_code_embed,
>> > #define jit_name(u) _jit_name(_jit,u)
>> > jit_code_name,
>> > #define jit_note(u, v) _jit_note(_jit, u, v)
>> > @@ -1096,6 +1098,9 @@ extern jit_node_t *_jit_new_node_pwf(jit_state_t*,
>> > jit_code_t,
>> > #define jit_new_node_pwd(c,u,v,w) _jit_new_node_pwd(_jit,c,u,v,w)
>> > extern jit_node_t *_jit_new_node_pwd(jit_state_t*, jit_code_t,
>> > jit_pointer_t, jit_word_t, jit_float64_t);
>> > +#define jit_new_node_pl(c,u,v) _jit_new_node_pl(_jit,c,u,v)
>> > +extern jit_node_t *_jit_new_node_pl(jit_state_t*, jit_code_t,
>> > + jit_pointer_t, jit_uint32_t);
>> >
>> > #define jit_arg_register_p(u) _jit_arg_register_p(_jit,u)
>> > extern jit_bool_t _jit_arg_register_p(jit_state_t*, jit_node_t*);
>> > diff --git a/include/lightning/jit_private.h
>> > b/include/lightning/jit_private.h
>> > index 0af24cb..97822e2 100644
>> > --- a/include/lightning/jit_private.h
>> > +++ b/include/lightning/jit_private.h
>> > @@ -276,6 +276,7 @@ extern jit_node_t *_jit_data(jit_state_t*, const void*,
>> > #define jit_cc_a2_int 0x00100000 /* arg2 is immediate word */
>> > #define jit_cc_a2_flt 0x00200000 /* arg2 is immediate float */
>> > #define jit_cc_a2_dbl 0x00400000 /* arg2 is immediate double */
>> > +#define jit_cc_a1_len 0x00800000 /* arg1 is an immediate uint
>> > length */
>> >
>> > #if __ia64__ || (__sparc__ && __WORDSIZE == 64)
>> > extern void
>> > @@ -381,6 +382,7 @@ union jit_data {
>> > jit_float64_t d;
>> > jit_pointer_t p;
>> > jit_node_t *n;
>> > + jit_uint32_t l;
>> > };
>> >
>> > struct jit_note {
>> > diff --git a/lib/jit_names.c b/lib/jit_names.c
>> > index ebd3d56..69ae274 100644
>> > --- a/lib/jit_names.c
>> > +++ b/lib/jit_names.c
>> > @@ -21,6 +21,7 @@ static char *code_name[] = {
>> > "data",
>> > "live", "align",
>> > "save", "load",
>> > + "embed",
>> > "#name", "#note",
>> > "label",
>> > "prolog",
>> > diff --git a/lib/jit_print.c b/lib/jit_print.c
>> > index 61d9650..b9b3ece 100644
>> > --- a/lib/jit_print.c
>> > +++ b/lib/jit_print.c
>> > @@ -107,7 +107,7 @@ _jit_print_node(jit_state_t *_jit, jit_node_t *node)
>> > (jit_cc_a0_int|jit_cc_a0_flt|jit_cc_a0_dbl|jit_cc_a0_jmp|
>> > jit_cc_a0_reg|jit_cc_a0_rlh|jit_cc_a0_arg|
>> > jit_cc_a1_reg|jit_cc_a1_int|jit_cc_a1_flt|jit_cc_a1_dbl|jit_cc_a1_arg|
>> > - jit_cc_a2_reg|jit_cc_a2_int|jit_cc_a2_flt|jit_cc_a2_dbl);
>> > + jit_cc_a2_reg|jit_cc_a2_int|jit_cc_a2_flt|jit_cc_a2_dbl|jit_cc_a1_len);
>> > if (!(node->flag & jit_flag_synth) && ((value & jit_cc_a0_jmp) ||
>> > node->code == jit_code_finishr ||
>> > node->code == jit_code_finishi))
>> > @@ -289,6 +289,10 @@ _jit_print_node(jit_state_t *_jit, jit_node_t *node)
>> > else
>> > print_flt(node->w.d);
>> > return;
>> > + l:
>> > + print_chr(' '); print_ptr(node->u.p);
>> > + print_chr(' '); print_dec(node->v.l);
>> > + return;
>> > case jit_code_name:
>> > print_chr(' ');
>> > if (node->v.p && _jitc->emit)
>> > @@ -371,7 +375,9 @@ _jit_print_node(jit_state_t *_jit, jit_node_t *node)
>> > goto n_r_f;
>> > case jit_cc_a0_jmp|jit_cc_a1_reg|jit_cc_a2_dbl:
>> > goto n_r_d;
>> > - default:
>> > + case jit_cc_a1_len:
>> > + goto l;
>> > + default:
>> > abort();
>> > }
>> > break;
>> > diff --git a/lib/jit_x86.c b/lib/jit_x86.c
>> > index e3e1383..b650f7f 100644
>> > --- a/lib/jit_x86.c
>> > +++ b/lib/jit_x86.c
>> > @@ -1598,6 +1598,11 @@ _emit_code(jit_state_t *_jit)
>> > if ((word = _jit->pc.w & (node->u.w - 1)))
>> > nop(node->u.w - word);
>> > break;
>> > + case jit_code_embed:
>> > + assert(node->v.l <= jit_get_max_instr());
>> > + jit_memcpy(_jit->pc.uc, node->u.p, node->v.l);
>> > + _jit->pc.uc += node->v.l;
>> > + break;
>> > case jit_code_note: case jit_code_name:
>> > node->u.w = _jit->pc.w;
>> > break;
>> > diff --git a/lib/lightning.c b/lib/lightning.c
>> > index b78bd07..374aa32 100644
>> > --- a/lib/lightning.c
>> > +++ b/lib/lightning.c
>> > @@ -1198,6 +1198,17 @@ _jit_new_node_pwd(jit_state_t *_jit, jit_code_t
>> > code,
>> > return (link_node(node));
>> > }
>> >
>> > +jit_node_t *
>> > +_jit_new_node_pl(jit_state_t *_jit, jit_code_t code,
>> > + jit_pointer_t u, jit_uint32_t v)
>> > +{
>> > + jit_node_t *node = new_node(code);
>> > + assert(!_jitc->realize);
>> > + node->u.p = u;
>> > + node->v.l = v;
>> > + return (link_node(node));
>> > +}
>> > +
>> > jit_node_t *
>> > _jit_label(jit_state_t *_jit)
>> > {
>> > @@ -1316,6 +1327,9 @@ _jit_classify(jit_state_t *_jit, jit_code_t code)
>> > case jit_code_finishi: /* synthesized will set jit_cc_a0_jmp */
>> > mask = jit_cc_a0_int;
>> > break;
>> > + case jit_code_embed:
>> > + mask = jit_cc_a1_len;
>> > + break;
>> > case jit_code_reti_f: case jit_code_pushargi_f:
>> > mask = jit_cc_a0_flt;
>> > break;
>> > diff --git a/m4/.gitignore b/m4/.gitignore
>> > index 24e2f3f..870b024 100644
>> > --- a/m4/.gitignore
>> > +++ b/m4/.gitignore
>> > @@ -8,3 +8,21 @@
>> > /gnulib-comp.m4
>> > /gnulib-tool.m4
>> > /zzgnulib.m4
>> > +/absolute-header.m4
>> > +/extensions.m4
>> > +/extern-inline.m4
>> > +/include_next.m4
>> > +/limits-h.m4
>> > +/multiarch.m4
>> > +/obstack.m4
>> > +/off_t.m4
>> > +/pid_t.m4
>> > +/ssize_t.m4
>> > +/stddef_h.m4
>> > +/stdint.m4
>> > +/stdlib_h.m4
>> > +/sys_types_h.m4
>> > +/unistd_h.m4
>> > +/warn-on-use.m4
>> > +/wchar_t.m4
>> > +/wint_t.m4
>> > diff --git a/m4/gnulib-cache.m4 b/m4/gnulib-cache.m4
>> > index 45be7ba..2a2d48b 100644
>> > --- a/m4/gnulib-cache.m4
>> > +++ b/m4/gnulib-cache.m4
>> > @@ -36,12 +36,13 @@
>> > # --aux-dir=build-aux \
>> > # --no-conditional-dependencies \
>> > # --libtool \
>> > -# --macro-prefix=gl
>> > +# --macro-prefix=gl \
>> > +# obstack
>> >
>> > # Specification in the form of a few gnulib-tool.m4 macro invocations:
>> > gl_LOCAL_DIR([gl])
>> > gl_MODULES([
>> > -
>> > + obstack
>> > ])
>> > gl_AVOID([])
>> > gl_SOURCE_BASE([gnulib-lib])