[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Preliminary: Porting Poke to the latest Jitter, with sub-package mod
From: |
Jose E. Marchesi |
Subject: |
Re: Preliminary: Porting Poke to the latest Jitter, with sub-package mode and the unified routine API |
Date: |
Sun, 03 Nov 2019 22:50:20 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Hello José.
I have finished cleaning up the big patch, the only remaining task being
writing a ChangeLog entry.
I found a slightly better solution to the problem of breaking the
include dependency cycle in pvm-val.h. Nothing should be particularly
controversial or difficult.
Please tell me if you want something different; otherwise I will send
you the final version tomorrow.
Hi Luca. Thanks for doing this.
Please find my comments below.
-PVM values in PVM programs
+PVM values in PVM routines
~~~~~~~~~~~~~~~~~~~~~~~~~~
-The PVM programs (data structures of type ``pvm_program``) are
-allocated by Jitter, using ``malloc``. Therefore, they are not
-traversed by the GC.
+PVM routines (data structures of type ``pvm_routine``) are
+allocated by Jitter in complicated data structures, internally relying
+on ``malloc``. Their content is therefore not automatically visible to
+the GC.
Thanks for improving my abhorrent English here and in so many other
places! :)
struct pvm_cls
{
- struct jitter_program *program;
- const void *entry_point;
+ pvm_routine routine;
+ pvm_program_point entry_point;
struct pvm_env *env;
};
Hmm, you didn't change pvm_program_point to pvm_routine_point in Jitter?
diff --git a/acinclude.m4 b/acinclude.m4
deleted file mode 100644
index ad0a10c..0000000
--- a/acinclude.m4
+++ /dev/null
So aclinclude.m4 is going away. This is ok.
diff --git a/src/Makefile.am b/src/Makefile.am
index 500c684..13fff4c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -76,17 +76,10 @@ poke_LDFLAGS =
# Integration with jitter.
-if POKE_DEBUG
- poke_CPPFLAGS += $(JITTER_DIRECT_THREADING_CPPFLAGS)
- poke_CFLAGS += $(JITTER_DIRECT_THREADING_CFLAGS)
- poke_LDADD += $(JITTER_DIRECT_THREADING_LDADD)
- poke_LDFLAGS += $(JITTER_DIRECT_THREADING_LDFLAGS)
-else
- poke_CPPFLAGS += $(JITTER_CPPFLAGS)
- poke_CFLAGS += $(JITTER_CFLAGS)
- poke_LDADD += $(JITTER_LDADD)
- poke_LDFLAGS += $(JITTER_LDFLAGS)
-endif
+poke_CPPFLAGS += $(JITTER_CPPFLAGS)
+poke_CFLAGS += $(JITTER_CFLAGS)
+poke_LDADD += $(JITTER_LDADD)
+poke_LDFLAGS += $(JITTER_LDFLAGS)
BUILT_SOURCES += pvm-vm.h pvm-vm1.c pvm-vm2.c
Hm, if --enable-debug no longer controls how the Jitter VM is built, how
can the user force building with direct threading when she wants to
debug something in pvm.jitter?
diff --git a/src/pk-cmd.c b/src/pk-cmd.c
index 9b46349..980efd2 100644
--- a/src/pk-cmd.c
+++ b/src/pk-cmd.c
@@ -353,17 +353,17 @@ pk_cmd_exec_1 (char *str, struct pk_trie *cmds_trie,
char *prefix)
case 'e':
{
/* Compile a poke program. */
- pvm_program prog;
+ pvm_routine routine;
I'm ok with this general rename of `prog' to `routine'.
if (uflags & PK_VM_DIS_F_NAT)
- pvm_disassemble_program (prog, true,
+ pvm_disassemble_routine (routine, true,
JITTER_OBJDUMP, NULL);
else
- pvm_print_program (stdout, prog);
+ pvm_routine_print (stdout, routine);
Just a small note: the API is inconsistent there:
pvm_disassemble_routine vs. pvm_routine_print. I recommend to settle in
one style, like verb first and then subject. Will save us time :)
- pvm_append_label (pasm->program, pasm->level->label1);
+ pvm_routine_append_label (pasm->routine, pasm->level->label1);
I like the fact you normalized the name of the functions in the API.
@@ -266,21 +266,19 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_decl)
{
/* At this point the code for the function specification
INITIAL has been assembled in the current macroassembler.
- Finalize the program and put it in a PVM closure, along
+ Finalize the routine and put it in a PVM closure, along
with the current environment. */
void *pointers;
- pvm_program program = pkl_asm_finish (PKL_GEN_ASM,
+ pvm_routine routine = pkl_asm_finish (PKL_GEN_ASM,
0 /* epilogue */,
&pointers);
pvm_val closure;
PKL_GEN_POP_ASM;
- pvm_specialize_program (program);
- closure = pvm_make_cls (program, pointers);
-
+ closure = pvm_make_cls (routine, pointers);
/*XXX*/
- /* pvm_print_program (stdout, program); */
+ /* pvm_routine_print (stdout, routine); */
Ok so there is no more explicit specialization in this API. Very very
good :)
diff --git a/src/pkl.c b/src/pkl.c
index c5672a5..2b9cdf6 100644
--- a/src/pkl.c
+++ b/src/pkl.c
@@ -47,7 +47,7 @@
#include "poke.h"
#define PKL_COMPILING_EXPRESSION 0
-#define PKL_COMPILING_PROGRAM 1
+#define PKL_COMPILING_ROUTINE 1
#define PKL_COMPILING_STATEMENT 2
Hm, nope. There PKL_COMPILING_PROGRAM refers to the _Poke_ entity that
is being compiled. Unrelated to PVM or jitter. It should stay
PKL_COMPILING_PROGRAM.
struct pkl_compiler
@@ -105,7 +105,7 @@ pkl_free (pkl_compiler compiler)
free (compiler);
}
-static pvm_program
+static pvm_routine
rest_of_compilation (pkl_compiler compiler,
pkl_ast ast,
void **pointers)
@@ -235,7 +235,7 @@ rest_of_compilation (pkl_compiler compiler,
pkl_ast_free (ast);
*pointers = gen_payload.pointers;
- return gen_payload.program;
+ return gen_payload.routine;
error:
pkl_ast_free (ast);
@@ -247,20 +247,20 @@ pkl_compile_buffer (pkl_compiler compiler,
char *buffer, char **end)
{
pkl_ast ast = NULL;
- pvm_program program;
+ pvm_routine routine;
int ret;
pkl_env env = NULL;
/* Note that the sole purpose of `pointers' is to serve as a root
- (in the stack) for the GC, to prevent the boxed values in PROGRAM
+ (in the stack) for the GC, to prevent the boxed values in ROUTINE
to be collected. Ugly as shit, but conservative garbage
collection doesn't really work. */
void *pointers;
- compiler->compiling = PKL_COMPILING_PROGRAM;
+ compiler->compiling = PKL_COMPILING_ROUTINE;
Ditto here.
@@ -397,17 +394,17 @@ pkl_compile_file (pkl_compiler compiler, const char
*fname)
{
int ret;
pkl_ast ast = NULL;
- pvm_program program;
+ pvm_routine routine;
FILE *fd;
pkl_env env = NULL;
/* Note that the sole purpose of `pointers' is to serve as a root
- (in the stack) for the GC, to prevent the boxed values in PROGRAM
+ (in the stack) for the GC, to prevent the boxed values in ROUTINE
to be collected. Ugly as shit, but conservative garbage
collection doesn't really work. */
void *pointers;
- compiler->compiling = PKL_COMPILING_PROGRAM;
+ compiler->compiling = PKL_COMPILING_ROUTINE;
Ditto here.
diff --git a/src/pvm-alloc.c b/src/pvm-alloc.c
index b3c00a0..308d69d 100644
--- a/src/pvm-alloc.c
+++ b/src/pvm-alloc.c
@@ -36,7 +36,7 @@ static void
pvm_alloc_finalize_closure (void *object, void *client_data)
{
pvm_cls cls = (pvm_cls) object;
- pvm_destroy_program (cls->program);
+ pvm_destroy_routine (cls->routine);
}
void *
@@ -54,6 +54,7 @@ pvm_alloc_initialize ()
{
/* Initialize the Boehm Garbage Collector. */
GC_INIT ();
+ GC_disable (); // FIXME: Don't.
}
Please do not include GC_disable() in the patch. The change must work
with the GC switched on! :)
diff --git a/src/pvm-val.h b/src/pvm-val.h
index 1f051ff..7677362 100644
--- a/src/pvm-val.h
+++ b/src/pvm-val.h
@@ -26,6 +26,8 @@
#include <stdint.h>
#include <xalloc.h>
+#include <jitter/jitter-routine.h> /* For Jitter routine types. See
below. */
+
/* The pvm_val opaque type implements values that are native to the
poke virtual machine:
@@ -453,23 +455,31 @@ int pvm_type_equal (pvm_val type1, pvm_val type2);
#define PVM_VAL_CLS(V) (PVM_VAL_BOX_CLS (PVM_VAL_BOX ((V))))
-#define PVM_VAL_CLS_PROGRAM(V) (PVM_VAL_CLS((V))->program)
+#define PVM_VAL_CLS_ROUTINE(V) (PVM_VAL_CLS((V))->routine)
#define PVM_VAL_CLS_ENTRY_POINT(V) (PVM_VAL_CLS((V))->entry_point)
#define PVM_VAL_CLS_ENV(V) (PVM_VAL_CLS((V))->env)
+/* The C header file generated by Jitter for PVM defines pvm_routine as
an alias
+ to a VM-independent type abstract type named jitter_routine. We
cannot the
+ header here without generating a CPP inclusion cycle, so for the time
being
You probably mean "We cannot include the header here"?
+ we resort to using the VM-independent type rather than its VM-specific
+ counterpart. For all intents and purposes they are the same type. */
struct pvm_cls
{
- /* Note we have to use explicit pointers here due to the include
- mess induced by jitter's combined header files :/ */
- struct jitter_program *program;
+ /* Here at the time we use VM-independent Jitter types rather than the
+ corresponding PVM-specific types in order to prevent a CPP inclusion
+ cycle. :/ */
+ jitter_routine routine;
void **pointers;
- const void *entry_point;
+ jitter_program_point entry_point;
struct pvm_env *env;
};
Oh I see. using jitter_routine instead of pvm_routine seems perfectly
ok to me. Certainly better than the previous struct jitter_program
*program.
Other than the points above, the changes look good to me! :)