[PATCH] D43264: [WebAssembly] Add explicit symbol table

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: wasm/InputChunks.h:13
+//
+// They are writen directly to the mmap'd output file afterwhich relcoations
+// are applied.  Because each Chunk is independed they can be written in
----------------
sbc100 wrote:
> ruiu wrote:
> > after relocations are applied
> But relocations are applies after the memcpy, no?
I meant there was a typo in this line.


================
Comment at: wasm/InputGlobal.h:42
+
+  WasmGlobal Global;
+
----------------
sbc100 wrote:
> ruiu wrote:
> > How large is WasmGlobal? I wonder which is better, keeping it through a reference or copying it here.
> Around 16 bytes or so.   And there is currently only one of them :)  There may be more soon, but currently we are only talking about __stack_pointer.   If we find users are generating a lot of these we could reconsider this.    In that case we could need to handle the __stack_pointer differently since we currently really on the mutability of this field to set the SP value.
> 
> For now I think this is fine.
Agreed.


================
Comment at: wasm/InputGlobal.h:45
+protected:
+  llvm::Optional<uint32_t> OutputIndex;
+};
----------------
sbc100 wrote:
> ruiu wrote:
> > It is better to keep this kind of things trivially destructible.
> Why?  I think its better to match the pattern used in InputChunks.h (initially anyway).  
> 
> (Also, Optional is only non-trivial on gcc due to a compiler bug)
Because I thought you would allocate a lot of instances of this class for a large program, just like we do to Symbol.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D43264