Skip to content

Commit

Permalink
AOT/JIT native stack bound check improvement (#2244)
Browse files Browse the repository at this point in the history
Move the native stack overflow check from the caller to the callee because the
former doesn't work for call_indirect and imported functions.

Make the stack usage estimation more accurate. Instead of making a guess from
the number of wasm locals in the function, use the LLVM's idea of the stack size
of each MachineFunction. The former is inaccurate because a) it doesn't reflect
optimization passes, and b) wasm locals are not the only reason to use stack.

To use the post-compilation stack usage information without requiring 2-pass
compilation or machine-code imm rewriting, introduce a global array to store
stack consumption of each functions:
For JIT, use a custom IRCompiler with an extra pass to fill the array.
For AOT, use `clang -fstack-usage` equivalent because we support external llc.

Re-implement function call stack usage estimation to reflect the real calling
conventions better. (aot_estimate_stack_usage_for_function_call)

Re-implement stack estimation logic (--enable-memory-profiling) based on the new
machinery.

Discussions: #2105.
  • Loading branch information
yamt authored Jun 21, 2023
1 parent 8797c75 commit cd7941c
Show file tree
Hide file tree
Showing 12 changed files with 1,350 additions and 254 deletions.
9 changes: 9 additions & 0 deletions core/iwasm/compilation/aot.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ extern "C" {
#define AOT_FUNC_PREFIX "aot_func#"
#endif

#ifndef AOT_FUNC_INTERNAL_PREFIX
#define AOT_FUNC_INTERNAL_PREFIX "aot_func_internal#"
#endif

#ifndef AOT_STACK_SIZES_NAME
#define AOT_STACK_SIZES_NAME "aot_stack_sizes"
#endif
extern const char *aot_stack_sizes_name;

typedef InitializerExpression AOTInitExpr;
typedef WASMType AOTFuncType;
typedef WASMExport AOTExport;
Expand Down
97 changes: 95 additions & 2 deletions core/iwasm/compilation/aot_compiler.c
Original file line number Diff line number Diff line change
Expand Up @@ -2761,6 +2761,16 @@ aot_compile_wasm(AOTCompContext *comp_ctx)
aot_handle_llvm_errmsg("failed to addIRModule", err);
return false;
}

if (comp_ctx->stack_sizes != NULL) {
LLVMOrcJITTargetAddress addr;
if ((err = LLVMOrcLLLazyJITLookup(comp_ctx->orc_jit, &addr,
aot_stack_sizes_name))) {
aot_handle_llvm_errmsg("failed to look up stack_sizes", err);
return false;
}
comp_ctx->jit_stack_sizes = (uint32 *)addr;
}
}

return true;
Expand Down Expand Up @@ -2815,6 +2825,55 @@ aot_emit_llvm_file(AOTCompContext *comp_ctx, const char *file_name)
return true;
}

static bool
aot_move_file(const char *dest, const char *src)
{
FILE *dfp = fopen(dest, "w");
FILE *sfp = fopen(src, "r");
size_t rsz;
char buf[128];
bool success = false;

if (dfp == NULL || sfp == NULL) {
LOG_DEBUG("open error %s %s", dest, src);
goto fail;
}
do {
rsz = fread(buf, 1, sizeof(buf), sfp);
if (rsz > 0) {
size_t wsz = fwrite(buf, 1, rsz, dfp);
if (wsz < rsz) {
LOG_DEBUG("write error");
goto fail;
}
}
if (rsz < sizeof(buf)) {
if (ferror(sfp)) {
LOG_DEBUG("read error");
goto fail;
}
}
} while (rsz > 0);
success = true;
fail:
if (dfp != NULL) {
if (fclose(dfp)) {
LOG_DEBUG("close error");
success = false;
}
if (!success) {
(void)unlink(dest);
}
}
if (sfp != NULL) {
(void)fclose(sfp);
}
if (success) {
(void)unlink(src);
}
return success;
}

bool
aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
{
Expand All @@ -2830,7 +2889,25 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
int ret;

if (comp_ctx->external_llc_compiler) {
const char *stack_usage_flag = "";
char bc_file_name[64];
char su_file_name[65]; /* See the comment below */

if (comp_ctx->stack_usage_file != NULL) {
/*
* Note: we know the caller uses 64 byte buffer for
* file_name. It will get 1 byte longer because we
* replace ".o" with ".su".
*/
size_t len = strlen(file_name);
bh_assert(len + 1 <= sizeof(su_file_name));
bh_assert(len > 3);
bh_assert(file_name[len - 2] == '.');
bh_assert(file_name[len - 1] == 'o');
snprintf(su_file_name, sizeof(su_file_name), "%.*s.su",
(int)(len - 2), file_name);
stack_usage_flag = " -fstack-usage";
}

if (!aot_generate_tempfile_name("wamrc-bc", "bc", bc_file_name,
sizeof(bc_file_name))) {
Expand All @@ -2842,8 +2919,8 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
return false;
}

snprintf(cmd, sizeof(cmd), "%s %s -o %s %s",
comp_ctx->external_llc_compiler,
snprintf(cmd, sizeof(cmd), "%s%s %s -o %s %s",
comp_ctx->external_llc_compiler, stack_usage_flag,
comp_ctx->llc_compiler_flags ? comp_ctx->llc_compiler_flags
: "-O3 -c",
file_name, bc_file_name);
Expand All @@ -2858,6 +2935,22 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
"with external LLC compiler.");
return false;
}
if (comp_ctx->stack_usage_file != NULL) {
/*
* move the temporary .su file to the specified location.
*
* Note: the former is automatimally inferred from the output
* filename (file_name here) by clang.
*
* Note: the latter might be user-specified.
* (wamrc --stack-usage=<file>)
*/
if (!aot_move_file(comp_ctx->stack_usage_file, su_file_name)) {
aot_set_last_error("failed to move su file.");
(void)unlink(su_file_name);
return false;
}
}
}
else if (comp_ctx->external_asm_compiler) {
char asm_file_name[64];
Expand Down
Loading

0 comments on commit cd7941c

Please sign in to comment.