Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

aot: avoid possible relocations around "stack_sizes" for indirect mode #2322

Merged
merged 4 commits into from Jun 29, 2023
Merged

aot: avoid possible relocations around "stack_sizes" for indirect mode #2322

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e0f44a9
aot: avoid possible relocations around "stack_sizes" for indirect mode
yamt Jun 27, 2023
f7ec28b
restore aot abi compatibility
yamt Jun 28, 2023
361d83d
style fixes
yamt Jun 29, 2023
c66e62d
use DefPointer for the new stack_sizes member
yamt Jun 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions core/iwasm/aot/aot_loader.c
View file
Open in desktop
Expand Up @@ -1843,6 +1843,13 @@ get_data_section_addr(AOTModule *module, const char *section_name,
return NULL;
}

const void *
aot_get_data_section_addr(AOTModule *module, const char *section_name,
uint32 *p_data_size)
{
return get_data_section_addr(module, section_name, p_data_size);
}

static void *
resolve_target_sym(const char *symbol, int32 *p_index)
{
Expand Down
5 changes: 5 additions & 0 deletions core/iwasm/aot/aot_runtime.c
View file
Open in desktop
Expand Up @@ -42,6 +42,8 @@ bh_static_assert(offsetof(AOTModuleInstance, cur_exception)
bh_static_assert(offsetof(AOTModuleInstance, global_table_data)
== 13 * sizeof(uint64) + 128 + 11 * sizeof(uint64));

bh_static_assert(offsetof(AOTModuleInstanceExtra, stack_sizes) == 0);

static void
set_error_buf(char *error_buf, uint32 error_buf_size, const char *string)
{
Expand Down Expand Up @@ -1210,6 +1212,9 @@ aot_instantiate(AOTModule *module, bool is_sub_inst, WASMExecEnv *exec_env_main,
#endif
module_inst->default_wasm_stack_size = stack_size;

((AOTModuleInstanceExtra *)module_inst->e)->stack_sizes =
aot_get_data_section_addr(module, AOT_STACK_SIZES_SECTION_NAME, NULL);

#if WASM_ENABLE_PERF_PROFILING != 0
total_size = (uint64)sizeof(AOTFuncPerfProfInfo)
* (module->import_func_count + module->func_count);
Expand Down
5 changes: 5 additions & 0 deletions core/iwasm/aot/aot_runtime.h
View file
Open in desktop
Expand Up @@ -88,6 +88,7 @@ typedef struct AOTFunctionInstance {
} AOTFunctionInstance;

typedef struct AOTModuleInstanceExtra {
const uint32 *stack_sizes;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had better use DefPointer(const uint32 *, stack_sizes)? Suppose the aot code may access more fields here in the future, we can ensure the offsets of these fields are fixed, and are the same in compilation time and execution time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

CApiFuncImport *c_api_func_imports;
} AOTModuleInstanceExtra;

Expand Down Expand Up @@ -633,6 +634,10 @@ aot_dump_perf_profiling(const AOTModuleInstance *module_inst);
const uint8 *
aot_get_custom_section(const AOTModule *module, const char *name, uint32 *len);

const void *
aot_get_data_section_addr(AOTModule *module, const char *section_name,
uint32 *p_data_size);

#if WASM_ENABLE_STATIC_PGO != 0
void
llvm_profile_instrument_target(uint64 target_value, void *data,
Expand Down
37 changes: 36 additions & 1 deletion core/iwasm/compilation/aot_llvm.c
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@
#include "aot_llvm_extra2.h"
#include "aot_compiler.h"
#include "aot_emit_exception.h"
#include "aot_emit_table.h"
#include "../aot/aot_runtime.h"
#include "../aot/aot_intrinsic.h"

Expand Down Expand Up @@ -230,6 +231,17 @@ aot_estimate_stack_usage_for_function_call(const AOTCompContext *comp_ctx,
return size;
}

static uint32
get_inst_extra_offset(AOTCompContext *comp_ctx)
{
const AOTCompData *comp_data = comp_ctx->comp_data;
uint32 table_count = comp_data->import_table_count + comp_data->table_count;
uint64 offset = get_tbl_inst_offset(comp_ctx, NULL, table_count);
bh_assert(offset <= UINT_MAX);
offset = align_uint(offset, 8);
return offset;
}

/*
* a "precheck" function performs a few things before calling wrapped_func.
*
Expand Down Expand Up @@ -327,9 +339,32 @@ aot_add_precheck_function(AOTCompContext *comp_ctx, LLVMModuleRef module,
/*
* load the value for this wrapped function from the stack_sizes array
*/
LLVMValueRef stack_sizes;
if (comp_ctx->is_indirect_mode) {
uint32 offset_u32 = get_inst_extra_offset(comp_ctx);
offset_u32 += offsetof(AOTModuleInstanceExtra, stack_sizes);
LLVMValueRef offset = I32_CONST(offset_u32);
if (!offset) {
goto fail;
}
LLVMValueRef stack_sizes_p =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had better declare the variables at the beginning of code piece { ... }. And had better add empty lines if needed. We usually keep that coding style to make the code easier to read.
For example, there is no empty line from L342 to L402, a little intensive:)

Copy link
Collaborator Author

@yamt yamt Jun 29, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i generally feel less lines are easier to read. but ok. i will follow your style.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

LLVMBuildInBoundsGEP2(b, INT8_TYPE, func_ctx->aot_inst, &offset, 1,
"aot_inst_stack_sizes_p");
if (!stack_sizes_p) {
goto fail;
}
stack_sizes =
LLVMBuildLoad2(b, INT32_PTR_TYPE, stack_sizes_p, "stack_sizes");
if (!stack_sizes) {
goto fail;
}
}
else {
stack_sizes = comp_ctx->stack_sizes;
}
LLVMValueRef func_index_const = I32_CONST(func_index);
LLVMValueRef sizes =
LLVMBuildBitCast(b, comp_ctx->stack_sizes, INT32_PTR_TYPE, "sizes");
LLVMBuildBitCast(b, stack_sizes, INT32_PTR_TYPE, "sizes");
if (!sizes) {
goto fail;
}
Expand Down