From: Bob Rogers <[EMAIL PROTECTED]>
Date: Mon, 21 Apr 2008 18:54:19 -0400
From: "Patrick R. Michaud" <[EMAIL PROTECTED]>
Date: Mon, 21 Apr 2008 15:42:27 -0500
. . . If it looks like having a separate stack for bsr/ret is
workable then I'll go at it that way.
Fair enough. FWIW, I'm now working on a simple hack that should show
how much speedup we can expect.
The hack is attached; it isn't headerized, has some coding standards
failures, and leaks memory, so it's incomplete. Disappointingly, it
produces an overall speedup of less than 4% in building gen_actions.pir
in r27087 (which has chromatic's ref-counting speedup).
And it fails t/pmc/continuation.t test 4 ("continuations preserve
bsr/ret state"), as expected. I don't think a 4% speedup is worth it,
so I'm giving up on it. Maybe somebody else will find a way to improve
it.
A warning, though: Do "make clean" after applying the patch, or the
new "parrot" will consume all memory. I suspect there is some code that
uses Parrot_Context that doesn't have the right dependencies, so is not
recompiled when interpreter.h is changed.
-- Bob
[P.S. Oops.]
*** Give bsr/ret a stack of their own ***
* include/parrot/interpreter.h:
+ Add bsr_stack, bsr_allocated, and bsr_stack_size members to
Parrot_Context.
* src/gc/register.c:
+ (init_context): Initialize bsr_stack.
* src/stacks.c:
+ (Parrot_bsr_stack_push, Parrot_bsr_stack_pop): New. (Probably
need a better permanent home for them.)
* src/ops/core.ops:
+ (bsr, ret, jsr): Use Parrot_bsr_stack_push/Parrot_bsr_stack_pop.
Diffs between last version checked in and current workfile(s):
Index: include/parrot/interpreter.h
===================================================================
--- include/parrot/interpreter.h (revision 27087)
+++ include/parrot/interpreter.h (working copy)
@@ -218,6 +218,10 @@
struct Stack_Chunk *user_stack; /* Base of the scratch stack */
PMC *lex_pad; /* LexPad PMC */
struct Parrot_Context *outer_ctx; /* outer context, if a closure */
+ opcode_t **bsr_stack; /* pointer to bsr/ret addresses, lazily
allocated
+ by Parrot_bsr_stack_push. */
+ UINTVAL bsr_allocated; /* allocated bsr_stack size */
+ UINTVAL bsr_stack_size; /* used bsr_stack size */
UINTVAL warns; /* Keeps track of what warnings
* have been activated */
UINTVAL errors; /* fatals that can be turned off */
Index: src/gc/register.c
===================================================================
--- src/gc/register.c (revision 27087)
+++ src/gc/register.c (working copy)
@@ -297,6 +297,9 @@
ctx->current_cont = NULL;
ctx->current_object = NULL; /* RT#46181 who clears it? */
ctx->current_HLL = 0;
+ /* We don't need to initialize bsr_allocated or bsr_stack_size, because
they
+ won't be examined if bsr_stack is NULL. */
+ ctx->bsr_stack = NULL;
if (old) {
/* some items should better be COW copied */
Index: src/stacks.c
===================================================================
--- src/stacks.c (revision 27087)
+++ src/stacks.c (working copy)
@@ -484,6 +484,49 @@
height, dynamic_env, dynamic_env->name);
}
+/*** bsr/ret stack support ***/
+
+PARROT_API
+void
+Parrot_bsr_stack_push(PARROT_INTERP, ARGIN(opcode_t *return_addr))
+{
+ parrot_context_t *context = CONTEXT(interp);
+
+ if (! context->bsr_stack) {
+ /* Need a new stack. */
+ UINTVAL size = 100;
+ /* fprintf(stderr, "[first alloc for %p]\n", context); */
+ context->bsr_allocated = size;
+ context->bsr_stack_size = 0;
+ context->bsr_stack = (opcode_t
**)mem_sys_allocate(size*sizeof(opcode_t *));
+ }
+ else if (context->bsr_stack_size >= context->bsr_allocated) {
+ /* Need a bigger stack. */
+ UINTVAL size = 2*context->bsr_allocated;
+ /* fprintf(stderr, "[realloc for %p (currently %p), from %d to %d]\n",
+ context, context->bsr_stack,
+ (int)context->bsr_allocated, (int)size); */
+ if (size > 1000000)
+ real_exception(interp, NULL, 1, "Too many 'bsr' calls\n");
+ context->bsr_stack = (opcode_t
**)mem_sys_realloc(context->bsr_stack, size*sizeof(opcode_t *));
+ context->bsr_allocated = size;
+ }
+
+ /* Push it. */
+ context->bsr_stack[context->bsr_stack_size++] = return_addr;
+}
+
+PARROT_API
+opcode_t *
+Parrot_bsr_stack_pop(PARROT_INTERP)
+{
+ parrot_context_t *context = CONTEXT(interp);
+
+ if (! context->bsr_stack || context->bsr_stack_size == 0)
+ real_exception(interp, NULL, 1, "'ret' without 'bsr'\n");
+ return context->bsr_stack[--context->bsr_stack_size];
+}
+
/*
=back
Index: src/ops/core.ops
===================================================================
--- src/ops/core.ops (revision 27087)
+++ src/ops/core.ops (working copy)
@@ -101,6 +101,8 @@
=cut
+/* ' */
+
inline op noop() :base_core {
goto NEXT();
}
@@ -217,8 +219,7 @@
=cut
inline op bsr(label INT) :base_core,check_event {
- stack_push(interp, &interp->dynamic_env,
- expr NEXT(), STACK_ENTRY_DESTINATION, STACK_CLEANUP_NULL);
+ Parrot_bsr_stack_push(interp, expr NEXT());
goto OFFSET($1);
}
@@ -229,7 +230,8 @@
=cut
inline op ret() {
- goto POP();
+ opcode_t * loc = Parrot_bsr_stack_pop(interp);
+ goto ADDRESS(loc);
}
@@ -244,8 +246,7 @@
inline op jsr(label INT) :base_core,check_event {
opcode_t * loc;
- stack_push(interp, &interp->dynamic_env,
- expr NEXT(), STACK_ENTRY_DESTINATION, STACK_CLEANUP_NULL);
+ Parrot_bsr_stack_push(interp, expr NEXT());
loc = INTVAL2PTR(opcode_t *, $1);
goto ADDRESS(loc);
}
End of diffs.
