Re: A first attempt to implement jit_embed

[Marc Nieper-Wißkirchen]

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?

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).

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 = "" len);
> +    *(signed int *)data = ""> > +    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])