[PATCH] D70930: [llvm-objcopy][WebAssembly] Initial support for wasm in llvm-objcopy

Thu Dec 12 10:05:34 PST 2019

dschuff marked an inline comment as done.
dschuff added inline comments.

================
Comment at: llvm/tools/llvm-objcopy/wasm/Writer.cpp:35
+      SectionSize += getULEB128Size(S.Name.size()) + S.Name.size();
+    encodeULEB128(SectionSize, OS, 5);
+    if (HasName) {
----------------
jhenderson wrote:
> dschuff wrote:
> > jhenderson wrote:
> > > What is  the `5` here? Is there a constant for it somewhere if applicable?
> > No constant (it's just the max encoded LEB size that can encode 32 bits) but added a comment.
> Okay, thanks. Why 32-bit though? Also, do we need to handle data that is greater in size than max 32-bits in some way?
The spec (e.g. for the [[ https://webassembly.github.io/spec/core/binary/modules.html | module format ]]) specifies the size of each kind of field, and also specifies how the values are [[ https://webassembly.github.io/spec/core/binary/values.html | encoded ]]. In the MVP/1.0 version of wasm everything has 32-bit types (with a few exceptions, e.g. i64/f64 constants; but definitely everything in the module format). A future extension will allow 64-bit pointers and memory sizes and will likely also extend some of the module-level encodings as a result. That would likely be treated by LLVM as a separate architecture and possibly a separate object format variant akin to ELFCLASS64. 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70930/new/

https://reviews.llvm.org/D70930