From 2086e59ab16bf5c370421c967dd5b4596e170456 Mon Sep 17 00:00:00 2001 From: Stephen Dolan Date: Wed, 22 Aug 2012 19:05:53 +0100 Subject: [PATCH] bugfix for backtracking over RET insns, and a truly evil testcase --- c/bytecode.c | 2 +- c/compile.c | 6 +++--- c/execute.c | 29 ++++++++++++++++------------- c/forkable_stack.h | 6 +++++- c/frame_layout.h | 44 +++++++++++++++++++++++++++++--------------- c/main.c | 1 - c/opcode.c | 19 ++++++++++--------- c/opcode.h | 14 +++++++++----- c/opcode_list.h | 4 ++-- c/testdata | 11 ++++++++--- 10 files changed, 83 insertions(+), 53 deletions(-) diff --git a/c/bytecode.c b/c/bytecode.c index 08d7fff3..c85be804 100644 --- a/c/bytecode.c +++ b/c/bytecode.c @@ -35,7 +35,7 @@ void dump_operation(struct bytecode* bc, uint16_t* codeptr) { printf("%04d ", pc); const struct opcode_description* op = opcode_describe(bc->code[pc++]); printf("%s", op->name); - if (op->flags & OP_HAS_IMMEDIATE || op->flags & OP_HAS_DOUBLE_IMMEDIATE) { + if (op->length > 1) { uint16_t imm = bc->code[pc++]; if (op->flags & OP_HAS_VARIABLE_LENGTH_ARGLIST) { for (int i=0; iflags & OP_HAS_IMMEDIATE)); + assert(opcode_describe(op)->length == 1); return inst_block(inst_new(op)); } @@ -364,8 +364,8 @@ static void compile(struct bytecode* bc, block b) { } } assert(found); - } else if (opflags & OP_HAS_IMMEDIATE) { - code[pos++] = curr->imm.intval; + } else if (op->length > 1) { + assert(0 && "codegen not implemented for this operation"); } } bc->constants = constant_pool; diff --git a/c/execute.c b/c/execute.c index 5245682b..a8dfc5e7 100644 --- a/c/execute.c +++ b/c/execute.c @@ -104,6 +104,7 @@ static struct closure make_closure(struct forkable_stack* stk, frame_ptr fr, uin assert(subfn_idx < frame_self(fr)->bc->nsubfunctions); return closure_new(stk, frame_self(fr)->bc->subfunctions[subfn_idx]); } else { + // FIXME: set pc return *frame_closure_arg(fr, idx); } } @@ -116,13 +117,14 @@ json_t* jq_next() { json_t* cfunc_input[MAX_CFUNCTION_ARGS] = {0}; json_t* cfunc_output[MAX_CFUNCTION_ARGS] = {0}; - assert(!forkable_stack_empty(&frame_stk)); - uint16_t* pc = *frame_current_pc(&frame_stk); + uint16_t* pc = *frame_current_retaddr(&frame_stk); + frame_pop(&frame_stk); + + assert(!forkable_stack_empty(&frame_stk)); int backtracking = 0; while (1) { - *frame_current_pc(&frame_stk) = pc; dump_operation(frame_current_bytecode(&frame_stk), pc); uint16_t opcode = *pc++; @@ -299,7 +301,7 @@ json_t* jq_next() { return 0; } stack_restore(); - pc = *frame_current_pc(&frame_stk); + pc = *frame_current_retaddr(&frame_stk); frame_pop(&frame_stk); backtracking = 1; break; @@ -320,7 +322,7 @@ json_t* jq_next() { case YIELD: { json_t* value = stack_pop().value; - *frame_current_pc(&frame_stk) = pc; + frame_push_backtrack(&frame_stk, pc); return value; } @@ -350,9 +352,9 @@ json_t* jq_next() { case CALL_1_1: { uint16_t nclosures = *pc++; - *frame_current_pc(&frame_stk) = pc + nclosures * 2; frame_ptr new_frame = frame_push(&frame_stk, - make_closure(&frame_stk, frame_current(&frame_stk), pc)); + make_closure(&frame_stk, frame_current(&frame_stk), pc), + pc + nclosures * 2); pc += 2; frame_ptr old_frame = forkable_stack_peek_next(&frame_stk, new_frame); for (int i=0; icode; break; } case RET: { + pc = *frame_current_retaddr(&frame_stk); frame_pop(&frame_stk); - pc = *frame_current_pc(&frame_stk); break; } } @@ -375,12 +377,13 @@ json_t* jq_next() { void jq_init(struct bytecode* bc, json_t* input) { - forkable_stack_init(&data_stk, sizeof(stackval) * 100); // FIXME: lower this number, see if it breaks - forkable_stack_init(&frame_stk, 1024); // FIXME: lower this number, see if it breaks - forkable_stack_init(&fork_stk, 1024); // FIXME: lower this number, see if it breaks + forkable_stack_init(&data_stk, sizeof(stackval) * 1000); // FIXME: lower this number, see if it breaks + forkable_stack_init(&frame_stk, 10240); // FIXME: lower this number, see if it breaks + forkable_stack_init(&fork_stk, 10240); // FIXME: lower this number, see if it breaks stack_push(stackval_root(input)); - frame_push(&frame_stk, closure_new_toplevel(bc)); + frame_push(&frame_stk, closure_new_toplevel(bc), 0); + frame_push_backtrack(&frame_stk, bc->code); } void run_program(struct bytecode* bc) { diff --git a/c/forkable_stack.h b/c/forkable_stack.h index 3de9f3e0..971b4ec0 100644 --- a/c/forkable_stack.h +++ b/c/forkable_stack.h @@ -76,7 +76,11 @@ static void* forkable_stack_peek(struct forkable_stack* s) { static void* forkable_stack_peek_next(struct forkable_stack* s, void* top) { forkable_stack_check(s); struct forkable_stack_header* elem = top; - return (void*)(s->stk + elem->next); + if (elem->next < s->length) { + return (void*)(s->stk + elem->next); + } else { + return 0; + } } static void forkable_stack_pop(struct forkable_stack* s) { diff --git a/c/frame_layout.h b/c/frame_layout.h index 373c2b86..b2e590eb 100644 --- a/c/frame_layout.h +++ b/c/frame_layout.h @@ -5,12 +5,18 @@ struct closure { struct bytecode* bc; - uint16_t* pc; stack_idx env; }; +struct continuation { + struct bytecode* bc; + stack_idx env; + uint16_t* retaddr; +}; + typedef union frame_elem { FORKABLE_STACK_HEADER; + struct continuation cont; struct closure closure; json_t* jsonval; } *frame_ptr; @@ -27,8 +33,8 @@ static int frame_size(struct bytecode* bc) { return sizeof(union frame_elem) * (bc->nclosures + bc->nlocals + 2); } -static struct closure* frame_self(frame_ptr fr) { - return &fr[1].closure; +static struct continuation* frame_self(frame_ptr fr) { + return &fr[1].cont; } static struct closure* frame_closure_arg(frame_ptr fr, int closure) { @@ -46,16 +52,20 @@ static json_t** frame_local_var(frame_ptr fr, int var) { static frame_ptr frame_current(struct forkable_stack* stk) { frame_ptr fp = forkable_stack_peek(stk); - struct bytecode* bc = frame_self(fp)->bc; - assert(frame_self(fp)->pc >= bc->code && frame_self(fp)->pc < bc->code + bc->codelen); + if (forkable_stack_peek_next(stk, fp)) { + struct bytecode* bc = frame_self(forkable_stack_peek_next(stk, fp))->bc; + assert(frame_self(fp)->retaddr >= bc->code && frame_self(fp)->retaddr < bc->code + bc->codelen); + } else { + assert(frame_self(fp)->retaddr == 0); + } return fp; } static struct bytecode* frame_current_bytecode(struct forkable_stack* stk) { return frame_self(frame_current(stk))->bc; } -static uint16_t** frame_current_pc(struct forkable_stack* stk) { - return &frame_self(frame_current(stk))->pc; +static uint16_t** frame_current_retaddr(struct forkable_stack* stk) { + return &frame_self(frame_current(stk))->retaddr; } static frame_ptr frame_get_parent(struct forkable_stack* stk, frame_ptr fr) { @@ -70,26 +80,30 @@ static frame_ptr frame_get_level(struct forkable_stack* stk, frame_ptr fr, int l } static struct closure closure_new_toplevel(struct bytecode* bc) { - struct closure cl = {bc, bc->code, -1}; + struct closure cl = {bc, -1}; return cl; } + static struct closure closure_new(struct forkable_stack* stk, struct bytecode* bc) { - struct closure cl = {bc, bc->code, + struct closure cl = {bc, forkable_stack_to_idx(stk, frame_current(stk))}; return cl; } -static frame_ptr frame_push(struct forkable_stack* stk, struct closure cl) { +static frame_ptr frame_push(struct forkable_stack* stk, struct closure cl, uint16_t* retaddr) { frame_ptr fp = forkable_stack_push(stk, frame_size(cl.bc)); - *frame_self(fp) = cl; + struct continuation* cc = frame_self(fp); + cc->bc = cl.bc; + cc->env = cl.env; + cc->retaddr = retaddr; return fp; } -static frame_ptr frame_push_backtrack(struct forkable_stack* stk, uint16_t* pc) { - struct closure curr = *frame_self(frame_current(stk)); +static frame_ptr frame_push_backtrack(struct forkable_stack* stk, uint16_t* retaddr) { + struct continuation cc = *frame_self(frame_current(stk)); frame_ptr fp = forkable_stack_push(stk, sizeof(union frame_elem) * 2); - curr.pc = pc; - *frame_self(fp) = curr; + cc.retaddr = retaddr; + *frame_self(fp) = cc; return fp; } diff --git a/c/main.c b/c/main.c index f8ea2149..66984842 100644 --- a/c/main.c +++ b/c/main.c @@ -54,7 +54,6 @@ void run_tests() { json_dumpf(actual, stdout, JSON_ENCODE_ANY); printf("\n"); pass = 0; - break; } } if (pass) { diff --git a/c/opcode.c b/c/opcode.c index 87179a51..8e107236 100644 --- a/c/opcode.c +++ b/c/opcode.c @@ -1,13 +1,14 @@ #include "opcode.h" -#define NONE 0 -#define CONSTANT (OP_HAS_IMMEDIATE | OP_HAS_CONSTANT) -#define VARIABLE (OP_HAS_DOUBLE_IMMEDIATE | OP_HAS_VARIABLE | OP_HAS_BINDING) -#define BRANCH (OP_HAS_IMMEDIATE | OP_HAS_BRANCH) -#define CFUNC (OP_HAS_IMMEDIATE | OP_HAS_SYMBOL | OP_HAS_CFUNC) -#define UFUNC (OP_HAS_IMMEDIATE | OP_HAS_UFUNC | OP_HAS_VARIABLE_LENGTH_ARGLIST) -#define CLOSURE_DEFINE (OP_HAS_IMMEDIATE | OP_HAS_BLOCK | OP_IS_CALL_PSEUDO | OP_HAS_BINDING) -#define CLOSURE_REF (OP_HAS_IMMEDIATE | OP_IS_CALL_PSEUDO | OP_HAS_BINDING) +// flags, length +#define NONE 0, 1 +#define CONSTANT OP_HAS_CONSTANT, 2 +#define VARIABLE (OP_HAS_VARIABLE | OP_HAS_BINDING), 3 +#define BRANCH OP_HAS_BRANCH, 2 +#define CFUNC (OP_HAS_SYMBOL | OP_HAS_CFUNC), 2 +#define UFUNC (OP_HAS_UFUNC | OP_HAS_VARIABLE_LENGTH_ARGLIST), 2 +#define CLOSURE_DEFINE (OP_HAS_BLOCK | OP_IS_CALL_PSEUDO | OP_HAS_BINDING), 2 +#define CLOSURE_ACCESS (OP_IS_CALL_PSEUDO | OP_HAS_BINDING), 2 #define OP(name, imm, in, out) \ {name, #name, imm, in, out}, @@ -17,7 +18,7 @@ static const struct opcode_description opcode_descriptions[] = { }; static const struct opcode_description invalid_opcode_description = { - -1, "#INVALID", 0, 0, 0 + -1, "#INVALID", 0, 0, 0, 0 }; diff --git a/c/opcode.h b/c/opcode.h index 87d24349..73948cd5 100644 --- a/c/opcode.h +++ b/c/opcode.h @@ -1,5 +1,7 @@ #ifndef OPCODE_H #define OPCODE_H +#include + typedef enum { #define OP(name, imm, in, out) name, #include "opcode_list.h" @@ -14,7 +16,6 @@ enum { }; enum { - OP_HAS_IMMEDIATE = 1, OP_HAS_CONSTANT = 2, OP_HAS_VARIABLE = 4, OP_HAS_BRANCH = 8, @@ -25,20 +26,23 @@ enum { OP_HAS_VARIABLE_LENGTH_ARGLIST = 256, OP_HAS_BLOCK = 512, OP_HAS_BINDING = 1024, - OP_HAS_DOUBLE_IMMEDIATE = 2048, }; struct opcode_description { opcode op; const char* name; + int flags; + + // length in 16-bit units + int length; + int stack_in, stack_out; }; const struct opcode_description* opcode_describe(opcode op); static inline int opcode_length(opcode op) { - return 1 + - (opcode_describe(op)->flags & OP_HAS_IMMEDIATE ? 1 : 0) + - (opcode_describe(op)->flags & OP_HAS_DOUBLE_IMMEDIATE ? 2 : 0); + return opcode_describe(op)->length; } + #endif diff --git a/c/opcode_list.h b/c/opcode_list.h index c54e1fda..497ae88a 100644 --- a/c/opcode_list.h +++ b/c/opcode_list.h @@ -20,6 +20,6 @@ OP(CALL_BUILTIN_3_1, CFUNC, 3, 1) OP(CALL_1_1, UFUNC, 1, 1) OP(RET, NONE, 1, 1) -OP(CLOSURE_PARAM, CLOSURE_REF, 0, 0) -OP(CLOSURE_REF, CLOSURE_REF, 0, 0) +OP(CLOSURE_PARAM, CLOSURE_ACCESS, 0, 0) +OP(CLOSURE_REF, CLOSURE_ACCESS, 0, 0) OP(CLOSURE_CREATE, CLOSURE_DEFINE, 0, 0) diff --git a/c/testdata b/c/testdata index cefc4b04..e22f09ce 100644 --- a/c/testdata +++ b/c/testdata @@ -138,6 +138,11 @@ def f: . + 1; def g: def g: . + 100; $$f | $$g | $$f; ($$f | $$g), $$g 106.0 105.0 -[[100,200][] as $x | def f: . + $x; $$f | $$f | $$f] -1 -[301.0, 601.0] +def f: (1000,2000); $$f +123412345 +1000 +2000 + +[[20,10][1,0] as $x | def f: (100,200) as $y | def g: [$x + $y, .]; . + $x | $$g; $$f[0] | [$$f][0][1] | $$f] +"woo, testing!" +[[110.0, 130.0], [210.0, 130.0], [110.0, 230.0], [210.0, 230.0], [120.0, 160.0], [220.0, 160.0], [120.0, 260.0], [220.0, 260.0]]