Skip to content

Commit

Permalink
Address code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@loganek
loganek committed Sep 9, 2024
1 parent e5ae586 commit 4dae5a7
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 37 deletions.
17 changes: 10 additions & 7 deletions core/iwasm/compilation/aot_compiler.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,7 @@ aot_gen_commit_ip(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
return aot_standard_frame_gen_commit_ip(comp_ctx, func_ctx,
ip_value, is_64bit);
case AOT_STACK_FRAME_TYPE_TINY:
return aot_tiny_frame_gen_commit_ip(comp_ctx, func_ctx, ip_value,
is_64bit);
return aot_tiny_frame_gen_commit_ip(comp_ctx, func_ctx, ip_value);
default:
aot_set_last_error(
"unsupported mode when generating commit_ip code");
Expand Down Expand Up @@ -989,6 +988,7 @@ static bool
aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index)
{
AOTFuncContext *func_ctx = comp_ctx->func_ctxes[func_index];
LLVMValueRef func_index_ref;
uint8 *frame_ip = func_ctx->aot_func->code, opcode, *p_f32, *p_f64;
uint8 *frame_ip_end = frame_ip + func_ctx->aot_func->code_size;
uint8 *param_types = NULL;
Expand Down Expand Up @@ -1017,11 +1017,14 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index)
func_ctx->block_stack.block_list_head->llvm_entry_block);

if (comp_ctx->aux_stack_frame_type
&& comp_ctx->call_stack_features.frame_per_function
&& !aot_alloc_frame_per_function_frame_for_aot_func(
comp_ctx, func_ctx,
I32_CONST(func_index + comp_ctx->comp_data->import_func_count))) {
return false;
&& comp_ctx->call_stack_features.frame_per_function) {
INT_CONST(func_index_ref,
func_index + comp_ctx->comp_data->import_func_count, I32_TYPE,
true);
if (!aot_alloc_frame_per_function_frame_for_aot_func(comp_ctx, func_ctx,
func_index_ref)) {
return false;
}
}
if (comp_ctx->aux_stack_frame_type) {
if (!init_comp_frame(comp_ctx, func_ctx, func_index)) {
Expand Down
9 changes: 9 additions & 0 deletions core/iwasm/compilation/aot_compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,15 @@ set_local_gc_ref(AOTCompFrame *frame, int n, LLVMValueRef value, uint8 ref_type)
#define F64_CONST(v) LLVMConstReal(F64_TYPE, v)
#define I8_CONST(v) LLVMConstInt(INT8_TYPE, v, true)

#define INT_CONST(variable, value, type, is_signed) \
do { \
variable = LLVMConstInt(type, value, is_signed); \
if (!variable) { \
aot_set_last_error("llvm build const failed"); \
return false; \
} \
} while (0)

#define LLVM_CONST(name) (comp_ctx->llvm_consts.name)
#define I1_ZERO LLVM_CONST(i1_zero)
#define I1_ONE LLVM_CONST(i1_one)
Expand Down
49 changes: 35 additions & 14 deletions core/iwasm/compilation/aot_emit_control.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,24 @@ format_block_name(char *name, uint32 name_size, uint32 block_index,
snprintf(name, name_size, "%s", "func_end");
}

#define CREATE_BLOCK(new_llvm_block, name) \
do { \
if (!(new_llvm_block = LLVMAppendBasicBlockInContext( \
comp_ctx->context, func_ctx->func, name))) { \
aot_set_last_error("add LLVM basic block failed."); \
goto fail; \
} \
#define CREATE_BLOCK(new_llvm_block, name) \
do { \
if (!(new_llvm_block = LLVMAppendBasicBlockInContext( \
comp_ctx->context, func_ctx->func, name))) { \
aot_set_last_error("add LLVM basic block failed."); \
goto fail; \
} \
if (!strcmp(name, "func_end") && comp_ctx->aux_stack_frame_type \
&& comp_ctx->call_stack_features.frame_per_function) { \
LLVMBasicBlockRef cur_block = \
LLVMGetInsertBlock(comp_ctx->builder); \
SET_BUILDER_POS(new_llvm_block); \
if (!aot_free_frame_per_function_frame_for_aot_func(comp_ctx, \
func_ctx)) { \
goto fail; \
} \
SET_BUILDER_POS(cur_block); \
} \
} while (0)

#define CURR_BLOCK() LLVMGetInsertBlock(comp_ctx->builder)
Expand Down Expand Up @@ -94,6 +105,11 @@ format_block_name(char *name, uint32 name_size, uint32 block_index,
goto fail; \
} \
SET_BUILDER_POS(block->llvm_end_block); \
LLVMValueRef first_instr = \
get_first_non_phi(block->llvm_end_block); \
if (first_instr) { \
LLVMPositionBuilderBefore(comp_ctx->builder, first_instr); \
} \
for (_i = 0; _i < block->result_count; _i++) { \
if (!(block->result_phis[_i] = LLVMBuildPhi( \
comp_ctx->builder, \
Expand Down Expand Up @@ -159,6 +175,18 @@ get_target_block(AOTFuncContext *func_ctx, uint32 br_depth)
return block;
}

LLVMValueRef
get_first_non_phi(LLVMBasicBlockRef block)
{
LLVMValueRef instr = LLVMGetFirstInstruction(block);

while (instr && LLVMIsAPHINode(instr)) {
instr = LLVMGetNextInstruction(instr);
}

return instr;
}

static void
clear_frame_locals(AOTCompFrame *aot_frame)
{
Expand Down Expand Up @@ -815,13 +843,6 @@ aot_compile_op_end(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
if ((next_llvm_end_block = find_next_llvm_end_block(block)))
MOVE_BLOCK_BEFORE(block->llvm_end_block, next_llvm_end_block);
}
if (block->label_type == LABEL_TYPE_FUNCTION
&& comp_ctx->aux_stack_frame_type
&& comp_ctx->call_stack_features.frame_per_function
&& !aot_free_frame_per_function_frame_for_aot_func(comp_ctx,
func_ctx)) {
return false;
}

if (comp_ctx->aot_frame) {
if (block->label_type != LABEL_TYPE_FUNCTION && comp_ctx->enable_gc
Expand Down
26 changes: 15 additions & 11 deletions core/iwasm/compilation/aot_emit_function.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,7 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
LLVMValueRef *param_values = NULL, value_ret = NULL, func;
LLVMValueRef import_func_idx, res;
LLVMValueRef ext_ret, ext_ret_ptr, ext_ret_idx;
LLVMValueRef func_idx_ref;
int32 i, j = 0, param_count, result_count, ext_ret_count;
uint64 total_size;
uint8 wasm_ret_type;
Expand Down Expand Up @@ -1452,8 +1453,9 @@ aot_compile_op_call(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
if (comp_ctx->aux_stack_frame_type) {
if (func_idx < import_func_count
&& comp_ctx->call_stack_features.frame_per_function) {
INT_CONST(func_idx_ref, func_idx, I32_TYPE, true);
if (!aot_alloc_frame_per_function_frame_for_aot_func(
comp_ctx, func_ctx, I32_CONST(func_idx))) {
comp_ctx, func_ctx, func_idx_ref)) {
return false;
}
}
Expand Down Expand Up @@ -2539,6 +2541,12 @@ aot_compile_op_call_indirect(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
/* Translate call import block */
LLVMPositionBuilderAtEnd(comp_ctx->builder, block_call_import);

if (comp_ctx->aot_frame && comp_ctx->call_stack_features.frame_per_function
&& !aot_alloc_frame_per_function_frame_for_aot_func(comp_ctx, func_ctx,
func_idx)) {
return false;
}

if (comp_ctx->aux_stack_frame_type == AOT_STACK_FRAME_TYPE_STANDARD
&& !commit_params_to_frame_of_import_func(comp_ctx, func_ctx, func_type,
param_values + 1)) {
Expand All @@ -2565,12 +2573,6 @@ aot_compile_op_call_indirect(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
goto fail;
}

if (comp_ctx->aot_frame
&& comp_ctx->call_stack_features.frame_per_function) {
aot_alloc_frame_per_function_frame_for_aot_func(comp_ctx, func_ctx,
func_idx);
}

if (!call_aot_call_indirect_func(
comp_ctx, func_ctx, func_type, ftype_idx, tbl_idx_value, elem_idx,
param_types + 1, param_values + 1, func_param_count, param_cell_num,
Expand All @@ -2582,9 +2584,10 @@ aot_compile_op_call_indirect(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
&& !check_call_return(comp_ctx, func_ctx, res))
goto fail;

if (comp_ctx->aot_frame
&& comp_ctx->call_stack_features.frame_per_function) {
aot_free_frame_per_function_frame_for_aot_func(comp_ctx, func_ctx);
if (comp_ctx->aot_frame && comp_ctx->call_stack_features.frame_per_function
&& !aot_free_frame_per_function_frame_for_aot_func(comp_ctx,
func_ctx)) {
return false;
}

block_curr = LLVMGetInsertBlock(comp_ctx->builder);
Expand Down Expand Up @@ -3177,7 +3180,8 @@ aot_compile_op_call_ref(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
PUSH(result_phis[i], func_type->types[func_param_count + i]);
}

if (comp_ctx->aux_stack_frame_type == AOT_STACK_FRAME_TYPE_STANDARD) {
if (comp_ctx->aux_stack_frame_type
&& !comp_ctx->call_stack_features.frame_per_function) {
#if WASM_ENABLE_AOT_STACK_FRAME != 0
if (!free_frame_for_aot_func(comp_ctx, func_ctx))
goto fail;
Expand Down
10 changes: 6 additions & 4 deletions core/iwasm/compilation/aot_stack_frame_comp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ aot_alloc_tiny_frame_for_aot_func(AOTCompContext *comp_ctx,
ADD_STORE(func_index, wasm_stack_top);

/* increment the stack pointer */
offset = I32_CONST(sizeof(AOTTinyFrame));
INT_CONST(offset, sizeof(AOTTinyFrame), I32_TYPE, true);
ADD_IN_BOUNDS_GEP(wasm_stack_top, INT8_TYPE, wasm_stack_top, &offset, 1);
ADD_STORE(wasm_stack_top, wasm_stack_top_ptr);

Expand All @@ -90,7 +90,8 @@ aot_free_tiny_frame_for_aot_func(AOTCompContext *comp_ctx,

ADD_LOAD(wasm_stack_top, INT8_PTR_TYPE, wasm_stack_top_ptr);

offset = I32_CONST(-sizeof(AOTTinyFrame));
INT_CONST(offset, -sizeof(AOTTinyFrame),
comp_ctx->pointer_size == 8 ? I64_TYPE : I32_TYPE, true);
ADD_IN_BOUNDS_GEP(wasm_stack_top, INT8_TYPE, wasm_stack_top, &offset, 1);
ADD_STORE(wasm_stack_top, wasm_stack_top_ptr);

Expand All @@ -99,7 +100,7 @@ aot_free_tiny_frame_for_aot_func(AOTCompContext *comp_ctx,

bool
aot_tiny_frame_gen_commit_ip(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
LLVMValueRef ip_value, bool is_64bit)
LLVMValueRef ip_value)
{
LLVMValueRef wasm_stack_top_ptr = func_ctx->wasm_stack_top_ptr,
wasm_stack_top;
Expand All @@ -109,7 +110,8 @@ aot_tiny_frame_gen_commit_ip(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,

ADD_LOAD(wasm_stack_top, INT8_PTR_TYPE, wasm_stack_top_ptr);

offset = I32_CONST(-4);
INT_CONST(offset, -4, comp_ctx->pointer_size == 8 ? I64_TYPE : I32_TYPE,
true);
ADD_IN_BOUNDS_GEP(ip_addr, INT8_TYPE, wasm_stack_top, &offset, 1);

ADD_STORE(ip_value, ip_addr);
Expand Down
2 changes: 1 addition & 1 deletion core/iwasm/compilation/aot_stack_frame_comp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ aot_free_frame_per_function_frame_for_aot_func(AOTCompContext *comp_ctx,

bool
aot_tiny_frame_gen_commit_ip(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx,
LLVMValueRef ip_value, bool is_64bit);
LLVMValueRef ip_value);

#ifdef __cplusplus
}
Expand Down

0 comments on commit 4dae5a7

Please sign in to comment.