[PATCH] jit: fix conversion of invoke* instructions
It is incorrect to convert invocations into expressions which are pushed onto mimic stack because pushed expressions can be evaluated any time later after they are pushed. This can result in breaking the execution sequence, where methods are not invoked in the order they appear in bytecode. Invocations are now handled as statements and their result is pushed onto mimic stack. Signed-off-by: Tomek Grabiec --- Makefile |5 +- arch/x86/insn-selector.brg | 236 ++ include/jit/expression.h | 48 +- include/jit/statement.h | 21 jit/expression.c | 67 jit/invoke-bc.c | 142 ++ jit/ostack-bc.c | 14 +--- jit/statement.c | 14 +++ jit/tree-printer.c | 123 ++ regression/jvm/InvokeTest.j | 33 ++ regression/run-suite.sh |1 + test/jit/bc-test-utils.c | 15 ++- test/jit/bc-test-utils.h |3 +- test/jit/invoke-bc-test.c| 214 ++ test/jit/tree-printer-test.c | 49 - 15 files changed, 374 insertions(+), 611 deletions(-) create mode 100644 regression/jvm/InvokeTest.j diff --git a/Makefile b/Makefile index be74fb6..8d57627 100644 --- a/Makefile +++ b/Makefile @@ -307,10 +307,11 @@ REGRESSION_TEST_SUITE_CLASSES = \ JASMIN_REGRESSION_TEST_SUITE_CLASSES = \ regression/jvm/DupTest.j \ + regression/jvm/InvokeResultTest.j \ + regression/jvm/InvokeTest.j \ regression/jvm/PopTest.j \ regression/jvm/SubroutineTest.j \ - regression/jvm/WideTest.j \ - regression/jvm/InvokeResultTest.j + regression/jvm/WideTest.j java-regression: FORCE $(E) " JAVAC " $(REGRESSION_TEST_SUITE_CLASSES) diff --git a/arch/x86/insn-selector.brg b/arch/x86/insn-selector.brg index e587c96..43f6960 100644 --- a/arch/x86/insn-selector.brg +++ b/arch/x86/insn-selector.brg @@ -58,7 +58,7 @@ struct _MBState; static void select_insn(struct basic_block *bb, struct tree_node *tree, struct insn *instruction); static void select_exception_test(struct basic_block *bb, struct tree_node *tree); -void finvoke_return_value(struct _MBState *state, struct basic_block *s, struct tree_node *tree, enum vm_type ret_vm_type); +void save_invoke_result(struct basic_block *s, struct tree_node *tree, struct vm_method *method, struct statement *stmt); static unsigned char size_to_scale(int size) { @@ -697,130 +697,6 @@ reg: OP_XOR(reg, reg) 1 binop_reg_reg_high(state, s, tree, INSN_XOR_REG_REG); } -reg: EXPR_INVOKE(arg) 1 -{ - struct var_info *eax, *edx = NULL; - struct compilation_unit *cu; - struct vm_method *method; - struct expression *expr; - - expr= to_expr(tree); - method = expr->target_method; - cu = method->compilation_unit; - - eax = get_fixed_var(s->b_parent, MACH_REG_xAX); - state->reg1 = get_var(s->b_parent, J_INT); - - if (get_method_return_type(method->type) == J_LONG) { - edx = get_fixed_var(s->b_parent, MACH_REG_xDX); - state->reg2 = get_var(s->b_parent, J_INT); - } - - invoke(s, tree, cu, method); - - select_insn(s, tree, reg_reg_insn(INSN_MOV_REG_REG, eax, state->reg1)); - if (edx != NULL) - select_insn(s, tree, reg_reg_insn(INSN_MOV_REG_REG, edx, state->reg2)); -} - -freg: EXPR_FINVOKEINTERFACE(arg) 1 -{ - enum vm_type ret_vm_type; - struct expression *expr; - struct vm_method *method; - - expr= to_expr(tree); - method = expr->target_method; - - ret_vm_type = method_return_type(method); - state->reg1 = get_var(s->b_parent, ret_vm_type); - - invokeinterface(state, s, tree); - finvoke_return_value(state, s, tree, ret_vm_type); -} - -reg: EXPR_INVOKEINTERFACE(arg) 1 -{ - struct var_info *eax, *edx = NULL; - struct expression *expr; - struct vm_method *method; - - expr= to_expr(tree); - method = expr->target_method; - - eax = get_fixed_var(s->b_parent, MACH_REG_xAX); - state->reg1 = get_var(s->b_parent, J_INT); - - if (get_method_return_type(method->type) == J_LONG) { - edx = get_fixed_var(s->b_parent, MACH_REG_xDX); - state->reg2 = get_var(s->b_parent, J_INT); - } - - invokeinterface(state, s, tree); - - select_insn(s, tree, reg_reg_insn(INSN_MOV_REG_REG, eax, state->reg1)); - if (edx != NULL) - select_insn(s, tree, reg_reg_insn(INSN_MOV_REG_REG, edx, state->reg2)); -} - -freg: EXPR_FINVOKE(arg) 1 -{ - struct compilation_unit *cu; - enum vm_type ret_vm_type; - struct vm_method *method; - struct expression *expr; - - expr= to_expr(tree); - method = expr->target_method; - cu = method->compilation_unit; - -
[penberg/jato] dd86fa: regression: GC torture test
Branch: refs/heads/master Home: http://github.com/penberg/jato Commit: dd86fa95f7c3eb3ace8492e96ab77c4f4606ec07 http://github.com/penberg/jato/commit/dd86fa95f7c3eb3ace8492e96ab77c4f4606ec07 Author: Pekka Enberg Date: 2009-09-04 (Fri, 04 Sep 2009) Changed paths: M Makefile A regression/jvm/GcTortureTest.java M regression/run-suite.sh Log Message: --- regression: GC torture test This patch adds a simple torture test for GC. Signed-off-by: Pekka Enberg Commit: 0c960d430b8755bb169090c090947eacffef32e3 http://github.com/penberg/jato/commit/0c960d430b8755bb169090c090947eacffef32e3 Author: Tomek Grabiec Date: 2009-09-04 (Fri, 04 Sep 2009) Changed paths: M include/jit/basic-block.h M jit/basic-block.c M jit/exception-bc.c M jit/invoke-bc.c Log Message: --- jit: make clear_mimic_stack() work on stack instead of basic block Signed-off-by: Tomek Grabiec Signed-off-by: Pekka Enberg Commit: 93e5df794bbbc82dbee8e5bde2e0329007325cf8 http://github.com/penberg/jato/commit/93e5df794bbbc82dbee8e5bde2e0329007325cf8 Author: Tomek Grabiec Date: 2009-09-04 (Fri, 04 Sep 2009) Changed paths: M jit/basic-block.c M jit/switch-bc.c M test/jit/basic-block-test.c Log Message: --- jit: split tableswitch/lookupswitch bsaic block at the end. This way, the order of basic blocks matches the values of .start and .end It also makes created basic blocks have empty bytecode range (start == end). Signed-off-by: Tomek Grabiec Signed-off-by: Pekka Enberg Commit: ff538e8675c0acbac77f18d1e3ea23a1bd2bb957 http://github.com/penberg/jato/commit/ff538e8675c0acbac77f18d1e3ea23a1bd2bb957 Author: Tomek Grabiec Date: 2009-09-04 (Fri, 04 Sep 2009) Changed paths: M jit/switch-bc.c Log Message: --- jit: set .has_branch flag for created basic blocks for tableswitch/lookupswitch. We must set that flag so that mimic stack resolution will put moves before branch instructions. Signed-off-by: Tomek Grabiec Signed-off-by: Pekka Enberg Commit: cc5babc99ebae9e2e825a2d8c7499cf28e700f25 http://github.com/penberg/jato/commit/cc5babc99ebae9e2e825a2d8c7499cf28e700f25 Author: Tomek Grabiec Date: 2009-09-04 (Fri, 04 Sep 2009) Changed paths: M include/jit/basic-block.h M include/lib/stack.h M jit/basic-block.c M jit/bytecode-to-ir.c M lib/stack.c Log Message: --- jit: fix mimic stack content propagation The mimic stack propagation procedure was pushing elements onto successors' stacks on every CFG edge. This caused that the successor's stack was larger then it should be. Suppose we have basic block X, which has N predecessors. For each X's predecessor, Q elements were pushed onto X's mimic stack, so finally it had N*Q elements instead of Q. The X's mimic stack was then also propagated, along with extra elements. This led to generation of many unnecessary register moves. In some CFG configurations this may lead to degenerative mimic stack resolution, where STMT_STORE is inserted with src temporary that is assigned only on some paths. This leads to incorrect liveness analysis of the temporary register, because it is seen to be used before it is defined. The register appears to be always live before it's use position, which is wrong, and could cause problems with GC. Signed-off-by: Tomek Grabiec Signed-off-by: Pekka Enberg -- Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july ___ Jatovm-devel mailing list Jatovm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jatovm-devel
[PATCH 4/4] jit: fix mimic stack content propagation
The mimic stack propagation procedure was pushing elements onto successors' stacks on every CFG edge. This caused that the successor's stack was larger then it should be. Suppose we have basic block X, which has N predecessors. For each X's predecessor, Q elements were pushed onto X's mimic stack, so finally it had N*Q elements instead of Q. The X's mimic stack was then also propagated, along with extra elements. This led to generation of many unnecessary register moves. In some CFG configurations this may lead to degenerative mimic stack resolution, where STMT_STORE is inserted with src temporary that is assigned only on some paths. This leads to incorrect liveness analysis of the temporary register, because it is seen to be used before it is defined. The register appears to be always live before it's use position, which is wrong, and could cause problems with GC. Signed-off-by: Tomek Grabiec --- include/jit/basic-block.h |9 include/lib/stack.h | 11 +- jit/basic-block.c |1 + jit/bytecode-to-ir.c | 50 lib/stack.c | 17 +++ 5 files changed, 73 insertions(+), 15 deletions(-) diff --git a/include/jit/basic-block.h b/include/jit/basic-block.h index b3e3cc1..e175eba 100644 --- a/include/jit/basic-block.h +++ b/include/jit/basic-block.h @@ -46,6 +46,10 @@ struct basic_block { Adl-Tabatabai et al (1998) for more in-depth explanation. */ struct stack *mimic_stack; + /* Holds the size of mimic stack at basic block entry. If + mimic stack has not yet been resolved it's set to -1. */ + long entry_mimic_stack_size; + /* Is this basic block an exception handler? */ bool is_eh; @@ -74,6 +78,11 @@ static inline struct basic_block *bb_entry(struct list_head *head) return list_entry(head, struct basic_block, bb_list_node); } +static inline bool bb_entry_mimic_stack_set(struct basic_block *bb) +{ + return bb->entry_mimic_stack_size != -1; +} + struct basic_block *alloc_basic_block(struct compilation_unit *, unsigned long, unsigned long); struct basic_block *get_basic_block(struct compilation_unit *, unsigned long, unsigned long); void free_basic_block(struct basic_block *); diff --git a/include/lib/stack.h b/include/lib/stack.h index e4e79d9..2a5f6bc 100644 --- a/include/lib/stack.h +++ b/include/lib/stack.h @@ -1,9 +1,11 @@ #ifndef LIB_STACK_H #define LIB_STACK_H +#include +#include +#include #include #include -#include struct stack { unsigned long nr_elements; @@ -37,4 +39,11 @@ static inline bool stack_is_empty(struct stack *stack) return !stack->nr_elements; } +static inline unsigned long stack_size(struct stack *stack) +{ + return stack->nr_elements; +} + +void stack_copy(struct stack *src, struct stack *dst); + #endif diff --git a/jit/basic-block.c b/jit/basic-block.c index d478315..ff8c11c 100644 --- a/jit/basic-block.c +++ b/jit/basic-block.c @@ -41,6 +41,7 @@ struct basic_block *alloc_basic_block(struct compilation_unit *b_parent, unsigne bb->b_parent = b_parent; bb->start = start; bb->end = end; + bb->entry_mimic_stack_size = -1; return bb; } diff --git a/jit/bytecode-to-ir.c b/jit/bytecode-to-ir.c index 2923890..b9d08a4 100644 --- a/jit/bytecode-to-ir.c +++ b/jit/bytecode-to-ir.c @@ -181,8 +181,14 @@ static int do_convert_bb_to_ir(struct basic_block *bb) buffer.buffer = cu->method->code_attribute.code; buffer.pos = bb->start; - if (bb->is_eh) + if (bb->is_eh) { + assert(!bb_entry_mimic_stack_set(bb)); stack_push(bb->mimic_stack, exception_ref_expr()); + bb->entry_mimic_stack_size = 1; + } + + if (!bb_entry_mimic_stack_set(bb)) + bb->entry_mimic_stack_size = 0; while (buffer.pos < bb->end) { ctx.offset = ctx.buffer->pos; /* this is fragile */ @@ -194,6 +200,32 @@ static int do_convert_bb_to_ir(struct basic_block *bb) return err; } +static int reload_mimic_stack(struct basic_block *bb, struct stack *reload) +{ + unsigned int i; + + for (i = 0; i < reload->nr_elements; i++) + bb_add_mimic_stack_expr(bb, reload->elements[i]); + + if (bb_entry_mimic_stack_set(bb)) { + if (stack_size(reload) == (unsigned long) bb->entry_mimic_stack_size) + return 0; + + return warn("stack size differs on different paths"), -EINVAL; + } + + for (i = 0; i < reload->nr_elements; i++) { + struct expression *elem; + + elem = reload->elements[reload->nr_elements - i - 1]; + expr_get(elem); + stack_push(bb->mimic_stack, elem); + } + + bb->entry_mimic_stack_size = stack_size(bb->mimic_stack); + return 0; +} + static int convert_bb_to_ir(struct bas
[PATCH 1/4] jit: make clear_mimic_stack() work on stack instead of basic block
Signed-off-by: Tomek Grabiec --- include/jit/basic-block.h |2 +- jit/basic-block.c |6 +++--- jit/exception-bc.c|2 +- jit/invoke-bc.c |4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/jit/basic-block.h b/include/jit/basic-block.h index d5452a4..b3e3cc1 100644 --- a/include/jit/basic-block.h +++ b/include/jit/basic-block.h @@ -88,7 +88,7 @@ unsigned char *bb_native_ptr(struct basic_block *bb); void resolution_block_init(struct resolution_block *block); bool branch_needs_resolution_block(struct basic_block *from, int idx); int bb_lookup_successor_index(struct basic_block *from, struct basic_block *to); -void clear_mimic_stack(struct basic_block *bb); +void clear_mimic_stack(struct stack *); #define for_each_basic_block(bb, bb_list) list_for_each_entry(bb, bb_list, bb_list_node) #define for_each_basic_block_reverse(bb, bb_list) list_for_each_entry_reverse(bb, bb_list, bb_list_node) diff --git a/jit/basic-block.c b/jit/basic-block.c index e19ee7e..401377c 100644 --- a/jit/basic-block.c +++ b/jit/basic-block.c @@ -235,12 +235,12 @@ bool branch_needs_resolution_block(struct basic_block *from, int idx) return !list_is_empty(&from->resolution_blocks[idx].insns); } -void clear_mimic_stack(struct basic_block *bb) +void clear_mimic_stack(struct stack *stack) { struct expression *expr; - while (!stack_is_empty(bb->mimic_stack)) { - expr = stack_pop(bb->mimic_stack); + while (!stack_is_empty(stack)) { + expr = stack_pop(stack); expr_put(expr); } } diff --git a/jit/exception-bc.c b/jit/exception-bc.c index 94de121..c76772f 100644 --- a/jit/exception-bc.c +++ b/jit/exception-bc.c @@ -61,7 +61,7 @@ int convert_athrow(struct parse_context *ctx) * reference is not transferred to exception handlers in * BC2IR layer. */ - clear_mimic_stack(ctx->bb); + clear_mimic_stack(ctx->bb->mimic_stack); convert_statement(ctx, stmt); diff --git a/jit/invoke-bc.c b/jit/invoke-bc.c index cac01a9..b0a993a 100644 --- a/jit/invoke-bc.c +++ b/jit/invoke-bc.c @@ -35,7 +35,7 @@ int convert_xreturn(struct parse_context *ctx) expr = stack_pop(ctx->bb->mimic_stack); return_stmt->return_value = &expr->node; convert_statement(ctx, return_stmt); - clear_mimic_stack(ctx->bb); + clear_mimic_stack(ctx->bb->mimic_stack); return 0; } @@ -48,7 +48,7 @@ int convert_return(struct parse_context *ctx) return_stmt->return_value = NULL; convert_statement(ctx, return_stmt); - clear_mimic_stack(ctx->bb); + clear_mimic_stack(ctx->bb->mimic_stack); return 0; } -- 1.6.3.3 -- Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july ___ Jatovm-devel mailing list Jatovm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jatovm-devel
[PATCH 2/4] jit: split tableswitch/lookupswitch bsaic block at the end.
This way, the order of basic blocks matches the values of .start and .end It also makes created basic blocks have empty bytecode range (start == end). Signed-off-by: Tomek Grabiec --- jit/basic-block.c |2 +- jit/switch-bc.c |6 +++--- test/jit/basic-block-test.c |2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/jit/basic-block.c b/jit/basic-block.c index 401377c..d478315 100644 --- a/jit/basic-block.c +++ b/jit/basic-block.c @@ -104,7 +104,7 @@ struct basic_block *bb_split(struct basic_block *orig_bb, unsigned long offset) { struct basic_block *new_bb; - if (offset < orig_bb->start || offset >= orig_bb->end) + if (offset < orig_bb->start || offset > orig_bb->end) return NULL; new_bb = alloc_basic_block(orig_bb->b_parent, offset, orig_bb->end); diff --git a/jit/switch-bc.c b/jit/switch-bc.c index 32cb7fc..b1ece1a 100644 --- a/jit/switch-bc.c +++ b/jit/switch-bc.c @@ -93,8 +93,8 @@ int convert_tableswitch(struct parse_context *ctx) master_bb = ctx->bb; - b1 = bb_split(master_bb, ctx->offset); - b2 = bb_split(b1, ctx->offset); + b1 = bb_split(master_bb, master_bb->end); + b2 = bb_split(b1, master_bb->end); assert(b1 && b2); @@ -179,7 +179,7 @@ int convert_lookupswitch(struct parse_context *ctx) master_bb = ctx->bb; - b1 = bb_split(master_bb, ctx->offset); + b1 = bb_split(master_bb, master_bb->end); assert(b1); diff --git a/test/jit/basic-block-test.c b/test/jit/basic-block-test.c index 7195487..a22d3bf 100644 --- a/test/jit/basic-block-test.c +++ b/test/jit/basic-block-test.c @@ -24,7 +24,7 @@ void test_split_with_out_of_range_offset(void) bb = get_basic_block(cu, 1, 2); assert_ptr_equals(NULL, bb_split(bb, 0)); - assert_ptr_equals(NULL, bb_split(bb, 2)); + assert_ptr_equals(NULL, bb_split(bb, 3)); free_compilation_unit(cu); } -- 1.6.3.3 -- Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july ___ Jatovm-devel mailing list Jatovm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jatovm-devel
[PATCH 3/4] jit: set .has_branch flag for created basic blocks for tableswitch/lookupswitch.
We must set that flag so that mimic stack resolution will put moves before branch instructions. Signed-off-by: Tomek Grabiec --- jit/switch-bc.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/jit/switch-bc.c b/jit/switch-bc.c index b1ece1a..fb3fef2 100644 --- a/jit/switch-bc.c +++ b/jit/switch-bc.c @@ -98,8 +98,9 @@ int convert_tableswitch(struct parse_context *ctx) assert(b1 && b2); - b1->is_converted = true; - b2->is_converted = true; + master_bb->has_branch = true; + b1->has_branch = true; + b2->has_branch = true; bb_add_successor(master_bb, default_bb ); bb_add_successor(master_bb, b1); @@ -183,7 +184,7 @@ int convert_lookupswitch(struct parse_context *ctx) assert(b1); - b1->is_converted = true; + b1->has_branch = true; bb_add_successor(master_bb, default_bb ); bb_add_successor(master_bb, b1); -- 1.6.3.3 -- Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july ___ Jatovm-devel mailing list Jatovm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jatovm-devel