[PATCH] D42233: [WebAssembly] Better support for WASM Object format

[Patrick Cheng via Phabricator via llvm-commits]

patcheng added a comment.

re: wasm2yaml

Is it only used for unit tests?
Before I process, just want to make sure I am causing issues. Basically, I am thinking changing the output from one block for entire Data section

  - Type:            DATA
    Segments:        
      - SectionOffset:   6
        MemoryIndex:     0
        Offset:          
          Opcode:          I32_CONS
          Value:           0
        Content:         '00000000'
  ...    

to a Data Segment.

  - Type:            DATA
    SectionOffset:   6
    MemoryIndex:     0
    Offset:          
      Opcode:          I32_CONS
      Value:           0
    Content:         '00000000'
  ...    



================
Comment at: lib/Object/WasmObjectFile.cpp:1058
-bool WasmObjectFile::isSectionBitcode(DataRefImpl Sec) const { return false; }
-
 relocation_iterator WasmObjectFile::section_rel_begin(DataRefImpl Ref) const {
----------------
sbc100 wrote:
> This seems reasonable, and could even be a separate change that could land right away if your prefer.
Looks like it's used by clang when -fembed-bitcode is specified:

https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/BackendUtil.cpp#L1246

Probably makes more sense to separate it.


================
Comment at: lib/Object/WasmObjectFile.cpp:1149
   Ref.d.a = 0;
+  Ref.d.b = 0;
   return section_iterator(SectionRef(Ref, this));
----------------
sbc100 wrote:
> Should do this for section_end() too? and symbol_begin()/end() too?
> 
> Is there a particular motivation for this? I someone reading this value somewhere?
See changes to moveSectionNext() above. 

Now when iterating through sections(), we are using Ref.d.a and Ref.d.b.

Don't think it's needed in other methods.

Actually, looking into a bit more, looks like DataRefImpl has a constructor that memset() everything. Maybe it's not strictly necessary (?)


Repository:
  rL LLVM

https://reviews.llvm.org/D42233