GC runtime export APIs implement #2154

kylo5aby · 2023-04-26T08:34:51Z

No description provided.

wenyongh

Use clang-format-12 -i --style=file <path/to/source/file> to format the source files.

wenyongh · 2023-04-27T00:23:09Z

core/iwasm/common/gc/gc_common.c

@@ -4,6 +4,8 @@
 */

 #include "../wasm_runtime_common.h"
+#include "gc_object.h"
+#include <stdlib.h>


Don't include stdblib.h, it should have been included in wasm_runtime_common.h

Don't include stdblib.h, it should have been included in wasm_runtime_common.h

get it, thanks

wenyongh · 2023-04-27T00:25:46Z

core/iwasm/common/gc/gc_common.c

@@ -53,6 +78,33 @@ wasm_func_type_get_param_count(WASMFuncType *const func_type)
    return 0;
 }

+wasm_ref_type_t *
+wasm_ref_type_normalize(wasm_ref_type_t *ref_type) {


put { to the next line

wenyongh · 2023-04-27T00:27:20Z

core/iwasm/common/gc/gc_common.c

@@ -53,6 +78,33 @@ wasm_func_type_get_param_count(WASMFuncType *const func_type)
    return 0;
 }

+wasm_ref_type_t *
+wasm_ref_type_normalize(wasm_ref_type_t *ref_type) {
+    if (ref_type->value_type != REF_TYPE_HT_NULLABLE) {


Had better set ref_type->nullable = false also.

wenyongh · 2023-04-27T00:35:11Z

core/iwasm/common/gc/gc_common.c

+        return ref_type;
+    }
+    int32_t heap_type = ref_type->heap_type;
+    switch (heap_type) {


Can convert it directly:

if (heap_type >= HEAP_TYPE_NONE && heap_type <= HEAP_TYPE_FUNC) { ref_type->value_type = (uint8)(REF_TYPE_NULLREF + heap_type - HEAP_TYPE_NONE); ref_type->nullable = false; ref_type->heap_type = 0; } else { ref_type->nullable = true; }

wenyongh · 2023-04-27T00:36:50Z

core/iwasm/common/gc/gc_common.c

+    XX(HEAP_TYPE_NONE, REF_TYPE_NULLREF);
+#undef XX
+    }
+    ref_type->heap_type = 0;


Remove this line

wenyongh · 2023-04-27T00:51:59Z

core/iwasm/common/gc/gc_common.c

@@ -153,15 +262,41 @@ wasm_ref_type_is_subtype_of(const wasm_ref_type_t *ref_type1,
 WASMStructObjectRef
 wasm_struct_obj_new_with_typeidx(WASMExecEnv *exec_env, uint32 type_idx)
 {
-    /* TODO */
-    return NULL;
+    wasm_module_inst_t module_inst = wasm_runtime_get_module_inst(exec_env);


Use WASMModuleInstanceCommon * in internal code

wenyongh · 2023-04-27T00:57:13Z

core/iwasm/common/gc/gc_common.c

+    WASMRttTypeRef rtt_type = (WASMRttTypeRef)malloc(sizeof(WASMRttTypeRef));
+
+    rtt_type->type_flag = WASM_TYPE_STRUCT;
+    rtt_type->inherit_depth = type->inherit_depth;
+    rtt_type->defined_type = (WASMType*)type;
+    rtt_type->root_type = type->root_type;


Use wasm_rtt_type_new to create/get the rtt_type, refer to
https://github.com/bytecodealliance/wasm-micro-runtime/blob/dev/gc_refactor/core/iwasm/interpreter/wasm_interp_classic.c#L1998-L2002

Note that we should check the return value

Use wasm_rtt_type_new to create/get the rtt_type, refer to https://github.com/bytecodealliance/wasm-micro-runtime/blob/dev/gc_refactor/core/iwasm/interpreter/wasm_interp_classic.c#L1998-L2002

Note that we should check the return value

get it, thanks

wenyongh · 2023-04-27T00:57:24Z

core/iwasm/common/gc/gc_common.c

+    rtt_type->root_type = type->root_type;
+
+    struct_obj = wasm_struct_obj_new(exec_env, rtt_type);
+    free(rtt_type);


Remove the line

wenyongh · 2023-04-27T01:00:23Z

core/iwasm/common/gc/gc_common.c

+#endif
+
+    for ( ; type_idx < type_count; type_idx++) {
+        if (wasm_defined_type_equal(types[type_idx], defined_type, module)) {


Use if (types[type_idx] == defined_type)
Or add a common field type_idx for WASMTYPE, WASMFuncType, WASMStructType, WASMArrayType, init it in the loader, and here we get it directly.

wenyongh · 2023-04-27T01:01:28Z

core/iwasm/include/gc_export.h

@@ -7,6 +7,7 @@
 #define _GC_EXPORT_H

 #include "wasm_export.h"
+#include <stdbool.h>


Seems no need to change the return type of wasm_ref_type_set_type_idx, and no need to include stdbool.h?

xujuntwt95329 · 2023-04-27T06:03:31Z

core/iwasm/common/gc/gc_common.c

@@ -14,15 +15,39 @@
 uint32
 wasm_get_defined_type_count(WASMModuleCommon *const module)
 {
-    /* TODO */
-    return 0;
+    WASMModule *wasm_module;


Suggested change

WASMModule *wasm_module;

WASMModule is only used when #if WASM_ENABLE_INTERP != 0, move this after line 21

xujuntwt95329 · 2023-04-27T06:03:42Z

core/iwasm/common/gc/gc_common.c

+
+#if WASM_ENABLE_INTERP != 0
+    if (module->module_type == Wasm_Module_Bytecode) {
+        wasm_module = (WASMModule *)module;


Suggested change

wasm_module = (WASMModule *)module;

WASMModule *wasm_module = (WASMModule *)module;

core/iwasm/common/gc/gc_common.c

xujuntwt95329 · 2023-04-27T06:08:03Z

core/iwasm/common/gc/gc_common.c

+    if (ref_type->value_type != REF_TYPE_HT_NULLABLE) {
+        return ref_type;
+    }
+    int32_t heap_type = ref_type->heap_type;


use int32 in our internal file, and put the variable declaration to the front of the scope

core/iwasm/common/gc/gc_common.c

xujuntwt95329 · 2023-04-27T06:18:47Z

core/iwasm/common/gc/gc_common.c

+    // TODO
+#endif
+
+    for (; type_idx < type_count; type_idx++) {


Suggested change

for (; type_idx < type_count; type_idx++) {

for (type_idx = 0; type_idx < type_count; type_idx++) {

core/iwasm/common/gc/gc_common.c

xujuntwt95329 · 2023-04-27T10:27:58Z

core/iwasm/include/gc_export.h

@zhenweijin Can you update the comments for the APIs you implemented?
You can refer to wasm_export.h

@zhenweijin Can you update the comments for the APIs you implemented? You can refer to wasm_export.h

Do you mean gc_export.h? some APIs are not declared in gc_common.c, but there have been implemented in gc_object.c, for example wasm_obj_is_anyref_obj/wasm_obj_is_i31_obj

here are the APIs in gc_export.h I have implemented:

wasm_get_defined_type_count(const wasm_module_t module);

wasm_get_defined_type(const wasm_module_t module, uint32_t index);

wasm_defined_type_is_func_type(const wasm_defined_type_t def_type);

wasm_defined_type_is_struct_type(const wasm_defined_type_t def_type);

wasm_defined_type_is_array_type(const wasm_defined_type_t def_type);

wasm_func_type_get_param_count(const wasm_func_type_t func_type);

wasm_ref_type_normalize(wasm_ref_type_t *ref_type);

wasm_func_type_get_param_type(const wasm_func_type_t func_type,
uint32_t param_idx);

wasm_func_type_get_result_count(const wasm_func_type_t func_type);

wasm_func_type_get_result_type

wasm_struct_type_get_field_count(const wasm_struct_type_t struct_type);

wasm_struct_type_get_field_type

wasm_array_type_get_elem_type

wasm_defined_type_equal

wasm_defined_type_is_subtype_of

wasm_ref_type_set_type_idx

wasm_ref_type_set_heap_type

wasm_ref_type_equal

wasm_ref_type_is_subtype_of

wasm_struct_obj_new_with_typeidx(wasm_exec_env_t exec_env, uint32_t type_idx);

wasm_struct_obj_new_with_type

wasm_struct_obj_set_field (gc_object.c)

wasm_struct_obj_get_field (gc_object.c)

wasm_array_obj_new_with_typeidx(wasm_exec_env_t exec_env, uint32_t type_idx);

wasm_array_obj_new_with_type

wasm_array_obj_set_elem

wasm_array_obj_get_elem

wasm_array_obj_copy

wasm_array_obj_length(const wasm_array_obj_t array_obj);

wasm_array_obj_first_elem_addr(const wasm_array_obj_t array_obj);

wasm_array_obj_elem_addr(const wasm_array_obj_t array_obj, uint32_t elem_idx);

wasm_func_obj_new_with_typeidx(wasm_exec_env_t exec_env, uint32_t type_idx);

wasm_func_obj_new_with_type(wasm_exec_env_t exec_env, wasm_func_type_t type);

wasm_func_obj_get_func_idx_bound(const wasm_func_obj_t func_obj);

wasm_func_obj_get_func_type(const wasm_func_obj_t func_obj);

wasm_externref_obj_new(wasm_exec_env_t exec_env, const void *host_obj);

wasm_externref_obj_get_value(const wasm_externref_obj_t externref_obj);

wasm_anyref_obj_new(wasm_exec_env_t exec_env, const void *host_obj); (gc_object.c)

wasm_anyref_obj_get_value(const wasm_anyref_obj_t anyref_obj); (gc_object.c)

wasm_externref_obj_to_internal_obj(const wasm_externref_obj_t externref_obj);

wasm_internal_obj_to_externref_obj

wasm_i31_obj_new(uint32_t i31_value); (gc_object.c)

wasm_i31_obj_get_value(wasm_i31_obj_t i31_obj, bool sign_extend); (gc_object.c)

wasm_runtime_pin_object(wasm_exec_env_t exec_env, wasm_obj_t obj);

wasm_runtime_unpin_object(wasm_exec_env_t exec_env, wasm_obj_t obj);

wasm_obj_is_struct_obj(const wasm_obj_t obj); (gc_object.c)

wasm_obj_is_array_obj(const wasm_obj_t obj);

wasm_obj_is_func_obj(const wasm_obj_t obj);

wasm_obj_is_i31_obj(const wasm_obj_t obj); (gc_object.c)

wasm_obj_is_externref_obj(const wasm_obj_t obj);

wasm_obj_is_anyref_obj(const wasm_obj_t obj); (gc_object.c)

wasm_obj_is_internal_obj(const wasm_obj_t obj);

wasm_obj_is_eq_obj(const wasm_obj_t obj);

wasm_obj_is_instance_of_defined_type

wasm_obj_is_instance_of_type_idx

wasm_obj_is_instance_of_ref_type

wasm_runtime_push_local_object_ref

wasm_runtime_pop_local_object_ref(wasm_exec_env_t exec_env);

wasm_runtime_pop_local_object_refs(wasm_exec_env_t exec_env, uint32_t n);

core/iwasm/common/gc/gc_common.c

wenyongh · 2023-05-03T03:50:10Z

core/iwasm/common/gc/gc_common.c

@@ -4,6 +4,11 @@
 */

 #include "../wasm_runtime_common.h"
+#include "bh_assert.h"


Don't include header files under core/shared/utils, include "bh_platform.h" instead.

wenyongh · 2023-05-03T03:50:37Z

core/iwasm/common/gc/gc_common.c

+#include "gc_export.h"
+#include "gc_object.h"
+#include "wasm.h"
+#include <stdbool.h>


No need to include stdbool.h

wenyongh · 2023-05-03T03:51:24Z

core/iwasm/common/gc/gc_common.c

+    }
+#endif
+#if WASM_ENABLE_AOT != 0
+    // TODO


Use /* .. */ to add comments in C file

wenyongh · 2023-05-03T03:52:03Z

core/iwasm/common/gc/gc_common.c

+    }
+#endif
+#if WASM_ENABLE_AOT != 0
+    // TODO


Same as above

wenyongh · 2023-05-03T03:59:09Z

core/iwasm/common/gc/gc_common.c

+    wasm_value_type_t value_type = ref_type->value_type;
+    int32 heap_type = ref_type->heap_type;
+
+    if (value_type < VALUE_TYPE_NULLREF || value_type > VALUE_TYPE_I32) {


Had better

if ((value_type >= VALUE_TYPE_V128 && value_type <= VALUE_TYPE_I32) || (value_type >= VALUE_TYPE_NULLREF && value_type <= VALUE_TYPE_FUNCREF))

Since there are no value types between VALUE_TYPE_FUNCREF (0x70) and VALUE_TYPE_V128 (0x7B).

Had better

if ((value_type >= VALUE_TYPE_V128 && value_type <= VALUE_TYPE_I32) || (value_type >= VALUE_TYPE_NULLREF && value_type <= VALUE_TYPE_FUNCREF))

Since there are no value types between VALUE_TYPE_FUNCREF (0x70) and VALUE_TYPE_V128 (0x7B).

Get it, thanks

core/iwasm/common/gc/gc_common.c

wenyongh · 2023-05-03T04:19:08Z

core/iwasm/common/gc/gc_common.c

+    // TODO
+#endif
+
+    bh_assert(rtt_type);


Don't bh_assert here, rtt_type may be NULL, just check the return value.

Don't bh_assert here, rtt_type may be NULL, just check the return value.

get it.

wenyongh · 2023-05-03T04:20:27Z

core/iwasm/common/gc/gc_common.c

+        WASMModule *module = ((WASMModuleInstance *)module_inst)->module;
+        uint32 type_count = module->type_count;
+
+        for (uint32 i = 0; i < type_count; i++) {


Declare uint32 i; at the beginning of function.

wenyongh · 2023-05-03T04:25:03Z

core/iwasm/common/gc/gc_common.c

+        for (uint32 i = 0; i < type_count; i++) {
+            if (module->types[i]->type_flag != WASM_TYPE_STRUCT) {
+                continue;
+            }
+            if (module->types[i] == (WASMType *)type) {
+                rtt_type = wasm_rtt_type_new(
+                    (WASMType *)type, i, module->rtt_types, module->type_count,
+                    &module->rtt_type_lock);
+                break;
+            }
+        }


How about get i firstly:

for (i = 0; i < i < type_count; i++) { if (module->types[i] == (WASMType *)type) { break; } } bh_assert(i < type_count); bh_assert(type->type_flag == WASM_TYPE_STRUCT);

And at the end of function:

rtt_type = wasm_rtt_type_new( (WASMType *)type, i, module->rtt_types, module->type_count, &module->rtt_type_lock); if (!rtt_type) return NULL; struct_obj = wasm_struct_obj_new(exec_env, rtt_type); return struct_obj;

wenyongh · 2023-05-03T04:25:58Z

core/iwasm/common/gc/gc_common.c

+    // TODO
+#endif
+
+    bh_assert(rtt_type);


Should check the return value but not bh_assert it.

wenyongh · 2023-05-04T00:59:16Z

core/iwasm/common/gc/gc_common.c

+    bh_memcpy_s(&ref_type2_norm, (uint32)sizeof(wasm_ref_type_t), ref_type2,
+                (uint32)sizeof(wasm_ref_type_t));
+    if (!wasm_ref_type_normalize(&ref_type1_norm)) {
+        bh_assert(0);


Don't bh_assert(0), return false instead

wenyongh · 2023-05-04T00:59:27Z

core/iwasm/common/gc/gc_common.c

+        bh_assert(0);
+    }
+    if (!wasm_ref_type_normalize(&ref_type2_norm)) {
+        bh_assert(0);


Don't bh_assert(0), return false instead

wenyongh · 2023-05-04T01:01:32Z

core/iwasm/common/gc/gc_common.c

+    bh_memcpy_s(&ref_type2_norm, (uint32)sizeof(wasm_ref_type_t), ref_type2,
+                (uint32)sizeof(wasm_ref_type_t));
+    if (!wasm_ref_type_normalize(&ref_type1_norm)) {
+        bh_assert(0);


Don't bh_assert(0), return false instead

wenyongh · 2023-05-04T01:01:37Z

core/iwasm/common/gc/gc_common.c

+        bh_assert(0);
+    }
+    if (!wasm_ref_type_normalize(&ref_type2_norm)) {
+        bh_assert(0);


Don't bh_assert(0), return false instead

core/iwasm/common/gc/gc_common.c

wenyongh

LGTM

xujuntwt95329

LGTM

kylo5aby added 2 commits April 26, 2023 08:28

add struct & type def related GC export APIs

f7acb3e

add typedef for wasm_heap_type_enum

10add0e

wenyongh reviewed Apr 27, 2023

View reviewed changes

kylo5aby added 2 commits April 27, 2023 05:59

update struct & type def related GC export APIs

6aa8fc1

clang-format

f15bf00

xujuntwt95329 reviewed Apr 27, 2023

View reviewed changes

update code format

47f3f67

xujuntwt95329 reviewed Apr 27, 2023

View reviewed changes

kylo5aby added 3 commits April 28, 2023 09:38

update code format

0c4fc15

add wasm_ref_type_is_subtype & wasm_ref_type_equal

7717ae2

add micro

38ff8fd

kylo5aby commented Apr 28, 2023

View reviewed changes

core/iwasm/common/gc/gc_common.c Show resolved Hide resolved

kylo5aby added 2 commits April 28, 2023 16:07

update wasm_ref_type_t normalize

ec8c255

update wasm_ref_type_normalize

6b9f813

wenyongh reviewed May 3, 2023

View reviewed changes

wenyongh reviewed May 4, 2023

View reviewed changes

wenyongh mentioned this pull request May 4, 2023

Implement gc common API of funcref, array, etc #2159

Closed

update header & API implement related

0612c62

wenyongh reviewed May 5, 2023

View reviewed changes

core/iwasm/common/gc/gc_common.c Outdated Show resolved Hide resolved

core/iwasm/common/gc/gc_common.c Outdated Show resolved Hide resolved

core/iwasm/common/gc/gc_common.c Outdated Show resolved Hide resolved

wenyongh reviewed May 5, 2023

View reviewed changes

core/iwasm/common/gc/gc_common.c Show resolved Hide resolved

core/iwasm/common/gc/gc_common.c Show resolved Hide resolved

core/iwasm/common/gc/gc_common.c Outdated Show resolved Hide resolved

core/iwasm/common/gc/gc_common.c Show resolved Hide resolved

update some GC APIs

ce3e75e

wenyongh reviewed May 5, 2023

View reviewed changes

core/iwasm/common/gc/gc_common.c Show resolved Hide resolved

add (void)ret to avoid warning

6c8c2e4

wenyongh approved these changes May 5, 2023

View reviewed changes

xujuntwt95329 approved these changes May 5, 2023

View reviewed changes

wenyongh merged commit 2e5ba11 into bytecodealliance:dev/gc_refactor May 5, 2023
493 checks passed

	wasm_module = (WASMModule *)module;
	WASMModule wasm_module = (WASMModule )module;

	for (; type_idx < type_count; type_idx++) {
	for (type_idx = 0; type_idx < type_count; type_idx++) {

GC runtime export APIs implement #2154

GC runtime export APIs implement #2154

Conversation

kylo5aby commented Apr 26, 2023

wenyongh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xujuntwt95329 Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylo5aby Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenyongh left a comment

Choose a reason for hiding this comment

xujuntwt95329 left a comment

Choose a reason for hiding this comment

xujuntwt95329 Apr 27, 2023 •

edited

Loading

kylo5aby Apr 27, 2023 •

edited

Loading