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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/wasm/Writer.h:32
+
+  size_t finalize();
+};
----------------
if I'm not mistaken this method is not really "finalize", 
the main thing what it does - it properly encodes Sections + computes the total size.

What would you say to the following reorganization of this code:

Writer.h:

   classWriter {
        ....
        Error write(); 

    private:   
        uint64_t totalSize() const { ... }
    };

Writer.cpp:
    uint64_t calculateSerializedSectionSize(const Section &S) {
         uint64_t Size = S.Content.size();
         if (S.SectionType == WASM_SEC_CUSTOM)
              Size += (getULEB128Size(S.Name.size()) + S.Name.size());
         return Size;
     }

     void writeSection(const Section &S, uint8_t* Out) {
          ....
     }

     uint64_t Writer::totalSize() const {
         ....
     }

     Error Writer::write() {
         ....
     }

Side note: it looks like in this case we eliminate SectionData - decrease the number of copies/ memory consumption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70930