bugfixes and performance improvements

diff --git a/include/vm_codebuilder.h b/include/vm_codebuilder.h
index 5261fadc021ba6ad62fe65955ad49e1e1d199d1c..6ecadad972ac5d70ef0dc37f50291fc8cf86ab9d 100644 (file)
--- a/include/vm_codebuilder.h
+++ b/include/vm_codebuilder.h
@@ -51,19 +51,21 @@ struct branch_patch_t {
     i32 label_idx;
     i32 static_arr;
     i32 static_idx;
+    bool targets_else;
 };
 
 ovm_code_builder_t ovm_code_builder_new(ovm_program_t *program, i32 param_count, i32 local_count);
 label_target_t     ovm_code_builder_wasm_target_idx(ovm_code_builder_t *builder, i32 idx);
 i32                ovm_code_builder_push_label_target(ovm_code_builder_t *builder, label_kind_t kind);
 void               ovm_code_builder_pop_label_target(ovm_code_builder_t *builder);
+void               ovm_code_builder_patch_else(ovm_code_builder_t *builder, label_target_t if_target);
 void               ovm_code_builder_free(ovm_code_builder_t *builder);
 void               ovm_code_builder_add_nop(ovm_code_builder_t *builder);
 void               ovm_code_builder_add_binop(ovm_code_builder_t *builder, u32 instr);
 void               ovm_code_builder_add_unop(ovm_code_builder_t *builder, u32 instr);
 void               ovm_code_builder_add_imm(ovm_code_builder_t *builder, u32 ovm_type, void *imm);
 void               ovm_code_builder_add_branch(ovm_code_builder_t *builder, i32 label_idx);
-void               ovm_code_builder_add_cond_branch(ovm_code_builder_t *builder, i32 label_idx, bool branch_if_true);
+void               ovm_code_builder_add_cond_branch(ovm_code_builder_t *builder, i32 label_idx, bool branch_if_true, bool targets_else);
 void               ovm_code_builder_add_branch_table(ovm_code_builder_t *builder, i32 count, i32 *label_indicies, i32 default_label_idx);
 void               ovm_code_builder_add_return(ovm_code_builder_t *builder);
 void               ovm_code_builder_add_call(ovm_code_builder_t *builder, i32 func_idx, i32 param_count, bool has_return_value);
@@ -71,6 +73,7 @@ void               ovm_code_builder_add_indirect_call(ovm_code_builder_t *builde
 void               ovm_code_builder_drop_value(ovm_code_builder_t *builder);
 void               ovm_code_builder_add_local_get(ovm_code_builder_t *builder, i32 local_idx);
 void               ovm_code_builder_add_local_set(ovm_code_builder_t *builder, i32 local_idx);
+void               ovm_code_builder_add_local_tee(ovm_code_builder_t *builder, i32 local_idx);
 void               ovm_code_builder_add_register_get(ovm_code_builder_t *builder, i32 local_idx);
 void               ovm_code_builder_add_register_set(ovm_code_builder_t *builder, i32 local_idx);
 void               ovm_code_builder_add_load(ovm_code_builder_t *builder, u32 ovm_type, i32 offset);

diff --git a/src/vm/code_builder.c b/src/vm/code_builder.c
index fa252fa9ecfecd8579216178bad5943eee25aeaa..0ed8ba6febdb26358e2144880c510bbeabfe9e90 100644 (file)
--- a/src/vm/code_builder.c
+++ b/src/vm/code_builder.c
@@ -101,6 +101,24 @@ void ovm_code_builder_pop_label_target(ovm_code_builder_t *builder) {
     }
 }
 
+void ovm_code_builder_patch_else(ovm_code_builder_t *builder, label_target_t if_target) {
+    assert(if_target.kind == label_kind_if);
+
+    fori (i, 0, bh_arr_length(builder->branch_patches)) {
+        branch_patch_t patch = builder->branch_patches[i];
+        if (patch.label_idx != if_target.idx) continue;
+        if (!patch.targets_else) continue;
+
+        int br_delta = bh_arr_length(builder->program->code) - patch.branch_instr - 1;
+        assert(patch.kind == branch_patch_instr_a);
+
+        builder->program->code[patch.branch_instr].a = br_delta;
+
+        bh_arr_fastdelete(builder->branch_patches, i);
+        return;
+    }
+}
+
 void ovm_code_builder_add_nop(ovm_code_builder_t *builder) {
     ovm_instr_t nop = {0};
     nop.full_instr = OVMI_NOP;
@@ -163,13 +181,14 @@ void ovm_code_builder_add_branch(ovm_code_builder_t *builder, i32 label_idx) {
     patch.kind = branch_patch_instr_a;
     patch.branch_instr = bh_arr_length(builder->program->code);
     patch.label_idx = label_idx;
+    patch.targets_else = false;
 
     bh_arr_push(builder->branch_patches, patch);
 
     ovm_program_add_instructions(builder->program, 1, &branch_instr);
 }
 
-void ovm_code_builder_add_cond_branch(ovm_code_builder_t *builder, i32 label_idx, bool branch_if_true) {
+void ovm_code_builder_add_cond_branch(ovm_code_builder_t *builder, i32 label_idx, bool branch_if_true, bool targets_else) {
     ovm_instr_t branch_instr = {0};
     if (branch_if_true) {
         branch_instr.full_instr = OVMI_BR_NZ;
@@ -184,6 +203,7 @@ void ovm_code_builder_add_cond_branch(ovm_code_builder_t *builder, i32 label_idx
     patch.kind = branch_patch_instr_a;
     patch.branch_instr = bh_arr_length(builder->program->code);
     patch.label_idx = label_idx;
+    patch.targets_else = targets_else;
 
     bh_arr_push(builder->branch_patches, patch);
 
@@ -232,6 +252,7 @@ void ovm_code_builder_add_branch_table(ovm_code_builder_t *builder, i32 count, i
         patch.label_idx = label_indicies[i];
         patch.static_arr = table_idx;
         patch.static_idx = i;
+        patch.targets_else = false;
         bh_arr_push(builder->branch_patches, patch);
     }
 
@@ -239,6 +260,7 @@ void ovm_code_builder_add_branch_table(ovm_code_builder_t *builder, i32 count, i
     default_patch.kind = branch_patch_instr_a;
     default_patch.branch_instr = bh_arr_length(builder->program->code) + 2;
     default_patch.label_idx = default_label_idx;
+    default_patch.targets_else = false;
     bh_arr_push(builder->branch_patches, default_patch);
 
     ovm_program_add_instructions(builder->program, 5, instrs);
@@ -351,6 +373,21 @@ void ovm_code_builder_add_local_set(ovm_code_builder_t *builder, i32 local_idx)
     ovm_program_add_instructions(builder->program, 1, &instr);
 }
 
+void ovm_code_builder_add_local_tee(ovm_code_builder_t *builder, i32 local_idx) {
+    ovm_instr_t instr = {0};
+    instr.full_instr = OVMI_MOV;
+    instr.r = local_idx; // This makes the assumption that the params will be in
+                         // the lower "address space" of the value numbers. This
+                         // will be true for web assembly, because that's how it
+                         // it was spec'd; but in the future for other things,
+                         // this will be incorrect.
+    instr.a = POP_VALUE(builder);
+
+    ovm_program_add_instructions(builder->program, 1, &instr);
+
+    PUSH_VALUE(builder, instr.a);
+}
+
 void ovm_code_builder_add_register_get(ovm_code_builder_t *builder, i32 reg_idx) {
     ovm_instr_t instr = {0};
     instr.full_instr = OVMI_REG_GET;

diff --git a/src/wasm/instance.c b/src/wasm/instance.c
index 3300a7f475b829377296af2ee6cab93a061d2a9f..916c29b1bd3e86a10c713d9125123fb8ee67bff8 100644 (file)
--- a/src/wasm/instance.c
+++ b/src/wasm/instance.c
@@ -17,6 +17,7 @@ struct ovm_wasm_binding {
     int param_count;
     int result_count;
     wasm_func_t *func;
+    wasm_val_vec_t param_buffer;
 };
 
 #define WASM_TO_OVM(w, o) { \
@@ -102,12 +103,8 @@ static wasm_trap_t *wasm_to_ovm_func_call_binding(void *vbinding, const wasm_val
 static void ovm_to_wasm_func_call_binding(void *env, ovm_value_t* params, ovm_value_t *res) {
     ovm_wasm_binding *binding = (ovm_wasm_binding *) env;
 
-    wasm_val_vec_t wasm_params;
-    wasm_params.data = alloca(sizeof(wasm_val_t) * binding->param_count);
-    wasm_params.size = binding->param_count;
-
     fori (i, 0, binding->param_count) {
-        OVM_TO_WASM(params[i], wasm_params.data[i]);
+        OVM_TO_WASM(params[i], binding->param_buffer.data[i]);
     }
 
     wasm_val_t return_value;
@@ -115,7 +112,7 @@ static void ovm_to_wasm_func_call_binding(void *env, ovm_value_t* params, ovm_va
     wasm_results.data = &return_value;
     wasm_results.size = binding->result_count;
 
-    wasm_trap_t *trap = wasm_func_call(binding->func, &wasm_params, &wasm_results);
+    wasm_trap_t *trap = wasm_func_call(binding->func, &binding->param_buffer, &wasm_results);
     assert(!trap);
 
     if (binding->result_count > 0) {
@@ -162,6 +159,8 @@ static void prepare_instance(wasm_instance_t *instance, const wasm_extern_vec_t
                 binding->param_count  = functype->params.size;
                 binding->result_count = functype->results.size;
                 binding->func         = func;
+                binding->param_buffer.data = bh_alloc(ovm_store->arena_allocator, sizeof(wasm_val_t) * binding->param_count);
+                binding->param_buffer.size = binding->param_count; 
 
                 ovm_state_register_external_func(ovm_state, importtype->external_func_idx, ovm_to_wasm_func_call_binding, binding);
                 break;

diff --git a/src/wasm/module_parsing.h b/src/wasm/module_parsing.h
index 7ee0fce3801cbd57ea2154443a26b8687b6dc3f2..61d810151c987eea6ec1d65105dd8c11c0be7cfc 100644 (file)
--- a/src/wasm/module_parsing.h
+++ b/src/wasm/module_parsing.h
@@ -497,23 +497,14 @@ static void parse_instruction(build_context *ctx) {
             //
             // This uses the pattern of "branch if zero" to skip a section of
             // code if the condition was not true.
-            ovm_code_builder_add_cond_branch(&ctx->builder, if_target, false);
+            ovm_code_builder_add_cond_branch(&ctx->builder, if_target, false, true);
             break;
         }
 
         case 0x05: {
-            //
-            // HACK HACK HACK
-            // I'm not entirely happy with the way that this happens, but let me explain.
-            // When trying to make an "else", you need to have a unconditional branch that
-            // skips the alternate code path. This branch instruction both needs to know
-            // the label index of the else statment, and be part of the existing block.
-            // Therefore, this "peeks" at what the next label index will be, so it can
-            // know what label to track for where to jump. It feels like there should be
-            // a better way to do this.
-            ovm_code_builder_add_branch(&ctx->builder, ctx->builder.next_label_idx);
-            ovm_code_builder_pop_label_target(&ctx->builder);
-            ovm_code_builder_push_label_target(&ctx->builder, label_kind_block);
+            label_target_t if_target = ovm_code_builder_wasm_target_idx(&ctx->builder, 0);
+            ovm_code_builder_add_branch(&ctx->builder, if_target.idx);
+            ovm_code_builder_patch_else(&ctx->builder, if_target);
             break;
         }
 
@@ -534,7 +525,7 @@ static void parse_instruction(build_context *ctx) {
             int label_idx = uleb128_to_uint(ctx->binary.data, &ctx->offset);
 
             label_target_t target = ovm_code_builder_wasm_target_idx(&ctx->builder, label_idx);
-            ovm_code_builder_add_cond_branch(&ctx->builder, target.idx, true);
+            ovm_code_builder_add_cond_branch(&ctx->builder, target.idx, true, false);
             break;
         }
 
@@ -605,8 +596,7 @@ static void parse_instruction(build_context *ctx) {
 
         case 0x22: {
             int local_idx = uleb128_to_uint(ctx->binary.data, &ctx->offset);
-            ovm_code_builder_add_local_set(&ctx->builder, local_idx);
-            ovm_code_builder_add_local_get(&ctx->builder, local_idx);
+            ovm_code_builder_add_local_tee(&ctx->builder, local_idx);
             break;
         }