[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: |
Mon, 04 Nov 2019 00:22:01 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
> Hmm, you didn't change pvm_program_point to pvm_routine_point in Jitter?
No, and I like it this way. "Program point" is a standard phrase,
identifying a source location as related to the dynamic semantics of a
program.
Ok.
A routine is a compilation unit for Jitter. Execution will always
start from some program point in a known routine but then it may branch
to some other program point belonging to a different routine. Soon it
will be possible to have multiple entry points per routine: I have an
important use case for that, related to tail calls.
The mapping between program points and routines will become even more
complex with stepping; imagine being able to run a VM instruction and
then returning to C, with the program point being saved to be able to
branch back to the next instruction. Which routine is the current one
can be computed if needed, but after the code is compiled it is not even
particularly important, except for debugging.
Of course being very undisciplined in the mapping between closures,
routines and program points might make routine finalization more
difficult when routines are garbage collected, and I do not recommend
it. I would still say that the name is the right one.
> Hm, if --enable-debug no longer controls how the Jitter VM is built,
With my proposed change --enable-debug no longer does anything, in fact.
We could remove it, but you may want to reuse the option for some other
purpose in the future.
> how can the user force building with direct threading when she wants
> to debug something in pvm.jitter?
You can pass options to the sub-package configure from the super-package
configure: in particular you can, for example, configure *Poke* with
--enable-dispatch-no-threading. You can pass any combination of options
manually enabling or disabling specific dispatches, if you want. The
default is meant to always remain safe and correct, even as Jitter
evolves.
Right now there is no obvious way of *computing* options for the
sub-package configure script from the super-package configure script,
even if it would be nice. The internal implementation in Autoconf makes
it difficult.
Oh, that's unfortunate.
Well, then please update the "Building with Debuggin Support" section in
HACKING documenting --enable-dispatch-no-threading.
If you really like the old behavior of --enable-debug I can reinstate
it. Otherwise you can now force minimal-threading and no-threading to
be disabled by configuring Poke with
--disable-dispatch-minimal-threading --disable-dispatch-no-threading
This is almost equivalent to the old --enable-debug, but a little
better: with a compiler only supporting switch dispatching Poke would
use switch dispatching instead of failing.
At the present time even passing
--disable-dispatch-minimal-threading --disable-dispatch-no-threading
is pointless, however, as minimal-threading and no-threading are already
disabled by default. They will be enabled when I fix defective
instructions in a reliable way.
> 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 :)
You are right, that part is ugly -- I would also like to simplify the
options for disassembling, without exposing the objdump command line at
run time, after it is already resolved at Jitter configuration time.
Since the API will change again because of libtextstyle I think I will
seize that occasion to clean it up, once and for all.
It will require another change in Poke, but it will be easy. Since
integrating libtextstyle turned out not to be completely trivial I
decided to deal with Poke first.
> #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.
This escaped me, sorry. I caught the same nuance in the AST but not
here. Undoing that change.
Ok thanks.
> Please do not include GC_disable() in the patch. The change must work
> with the GC switched on! :)
Thanks for noticing that. Undoing it.
> +/* 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"?
Fixing that, and improving the comment.
The next iteration is attached here. The test suite passes. I have not
tried to pull the most recent changes yet, but they should not interfere
too much.
Points to be settled:
* --enable-debug (remove it, reinstate the old behavior, leave it as a
no-op for the future)?
Let's remove --enable-debug from Poke. I don't like having NOP
interfaces around. We can always reinstate it if we find we actually
need it.
* ... ?
A little note: since this series does not include the jitter
subpackaging mode, you will also need to update HACKING to reflect a
recent enough jitter git version that supports the new routines
interface.
Other than that I'm good. This new version LGTM.