Implement BR_TABLE #1064
Implement BR_TABLE #1064
Conversation
| /* Reload block parameters */ | ||
| if (!load_block_params(cc, block)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Why moving these lines here? Here we load block params again for else branch as similar to if branch, moving here causes loading again for if branch and missing for else branch?
| cur_basic_block = cc->cur_basic_block; | ||
| gen_commit_values(jit_frame, jit_frame->lp, jit_frame->sp); | ||
| clear_values(jit_frame); | ||
| SET_BB_END_BCIP(cur_basic_block, *p_frame_ip - 1); |
There was a problem hiding this comment.
Current block hasn't ended yet, it is br_if but not br, the following opcodes also belongs to current basic block.
| if (depth_idx < br_count) { | ||
| br_depth = br_depths[depth_idx]; | ||
| else { | ||
| opnd->match_pairs[i].value = br_depths[i]; |
There was a problem hiding this comment.
Should be opnd->match_pairs[i].value = i.
br_depths[i] is used in L925 handle_op_br(cc, br_depths[i], p_frame_ip)
| } | ||
|
|
||
| /* continue processing opcodes after BR_TABLE */ | ||
| load_block_params(cc, cc->block_stack.block_list_end); |
There was a problem hiding this comment.
No need to load_block_params again, the following codes of current block are useless after br_table.
Instead, should change to return handle_next_reachable_block(cc, p_frame_ip);
core/iwasm/fast-jit/jit_frontend.c
Outdated
| read_leb_uint32(frame_ip, frame_ip_end, br_depths[i]); | ||
|
|
||
| if (!jit_compile_op_br_table(cc, br_depths, br_count, | ||
| if (!jit_compile_op_br_table(cc, br_depths, br_count + 1, |
There was a problem hiding this comment.
The original code is ported from core/iwasm/compilation/aot_compiler.c:
https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/core/iwasm/compilation/aot_compiler.c#L282-L283
Could we not change it and make it similar to aot_compiler.c? We can increase br_count in jit_emit_control.c
There was a problem hiding this comment.
Sure we can keep following AOT code.
Just a little wired if the length parameter (br_count) is actually longer than the size of the array(br_depths).
| jit_frame = cc->jit_frame; | ||
| cur_basic_block = cc->cur_basic_block; | ||
| gen_commit_values(jit_frame, jit_frame->lp, jit_frame->sp); | ||
| clear_values(jit_frame); |
There was a problem hiding this comment.
Should not remove this line, current basic block's register may be used by other basic block, for example:
(module
(func $test (export "test") (param $l0 i32) (result i32)
i32.const 1
(block $l (param i32) (result i32)
i32.const 2
i32.add
local.get $l0
local.get $l0
br_if $l
drop
local.get $l0
i32.add
)
)
)The IRs in L3 generated are:
L3:
; PREDS()
; BEGIN_BCIP=0x002f END_BCIP=0x002f
ADD I21, I0, 0x94L
STI32 i8, I21, 0x0 => unknown/unloaded i8 used
JMP L4
; SUCCS(L4)The running results:
iwasm -f test test.wasm 1
# return 3, the right result should be 1| POP_I32(value); | ||
|
|
||
| /* append LOOKUPSWITCH to current basic block */ | ||
| gen_commit_values(cc->jit_frame, cc->jit_frame->lp, cc->jit_frame->sp); |
There was a problem hiding this comment.
Should also clear_values() after gen_commit_values.
No description provided.