WASMAssembler: Initial support #1657
Conversation
|
I would like to request if someone could review (at a high level) the current state of |
| Vec<uint8_t> m_type_section; | ||
| Vec<uint8_t> m_import_section; | ||
| Vec<uint8_t> m_func_section; | ||
| Vec<uint8_t> m_memory_section; | ||
| Vec<uint8_t> m_global_section; | ||
| Vec<uint8_t> m_export_section; | ||
| Vec<uint8_t> m_code_section; | ||
| Vec<uint8_t> m_data_section; | ||
|
|
||
| // no_of_types indicates total (imported + defined) no of functions | ||
| uint32_t no_of_types; | ||
| uint32_t no_of_functions; | ||
| uint32_t no_of_memories; | ||
| uint32_t no_of_globals; | ||
| uint32_t no_of_exports; | ||
| uint32_t no_of_imports; | ||
| uint32_t no_of_data_segments; | ||
| uint32_t avail_mem_loc; | ||
| uint32_t digits_mem_loc; |
There was a problem hiding this comment.
I would wish the above variables to be private and not accessible by outer entities.
| wasm::emit_b8(m_type_section, m_al, 0x60); | ||
| m_wa.emit_b8(m_wa.m_type_section, m_al, 0x60); | ||
|
|
||
| /********************* Parameter Types List *********************/ | ||
| s->referenced_vars.reserve(m_al, x.n_args); | ||
| uint32_t len_idx_type_section_param_types_list = | ||
| wasm::emit_len_placeholder(m_type_section, m_al); | ||
| m_wa.emit_len_placeholder(m_wa.m_type_section, m_al); | ||
| for (size_t i = 0; i < x.n_args; i++) { | ||
| ASR::Variable_t *arg = ASRUtils::EXPR2VAR(x.m_args[i]); | ||
| LCOMPILERS_ASSERT(ASRUtils::is_arg_dummy(arg->m_intent)); | ||
| m_var_idx_map[get_hash((ASR::asr_t *)arg)] = s->no_of_variables; | ||
| emit_var_type(m_type_section, arg, s->no_of_variables, false); | ||
| emit_var_type(m_wa.m_type_section, arg, s->no_of_variables, false); | ||
| if (isRefVar(arg)) { | ||
| s->referenced_vars.push_back(m_al, arg); | ||
| } | ||
| } | ||
| wasm::fixup_len(m_type_section, m_al, | ||
| m_wa.fixup_len(m_wa.m_type_section, m_al, | ||
| len_idx_type_section_param_types_list); |
There was a problem hiding this comment.
The variables noted in #1657 (comment) are needed at several places in the ASRToWASMVisitor Class. Thus, I am unable to make them private.
It seems that without those variables being private, we may/might not have truly decoupled WASMAssembler from ASRToWASMVisitor Class.
There was a problem hiding this comment.
This is fixed in #1675.
|
It looks like the assembler itself only writes to one section in the WASM binary, correct? In that case, why not make the WASMAssembler just to return the binary instructions, and have another class that handles the WASM format? That way at least the WASMAssembler will be very simple. Overall this direction is good I think. |
We have several sections in the wasm binary. After completion of wasm generation, we combine these section into one single array and return it. Currently, as per the main branch,
One class that handles instructions (like |
|
It seems to me the assembler class should only construct the "code" section. It seems we only use the assembler to construct bytes in the global and type sections, but we should rather use the WasmFormat class for that I think, it would be more readable and robust. Please correct me if I missed something. |
I implemented a |
|
Closing in favour of #1675. |
towards lfortran/lfortran#589
It seems our existing
wasmassembly functions and theASRToWASMVisitorClass are tightly coupled together. In this PR, I am hoping to decouple them and have a separateWASMAssemblerClass responsible forwasmgeneration.