[PATCH] D42176: [WebAssembly] Optimise relocation iteration to remove n^2 loop. NFC.

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added a comment.

In https://reviews.llvm.org/D42176#1018179, @ruiu wrote:

> Hmm, I think I'm confused. IIUC, a wasm object file has single .data and .code section, and there is one relocation section for each section.
>
> A "section" in wasm is actually not an atomic unit of inclusion/exclusion. It can be split into multiple "chunks", and "chunks"  is a unit of garbage collection.


Yes - I think that's right.

> But why does that two-level abstraction needed? Why can't everything just a section?

I don't follow what your alternative suggestion is ("why can't everything just a section"). Possibly one of these:

1. WasmObjectFile should implement llvm::object::ObjectFile to return an llvm::object::SectionRef for each chunk. Then there would be a one-to-one mapping between the LLVM "sections" and the LLD "chunks". You don't have any opinion on how Wasm files are encoded on disk - you're simply asking for the code I've written to essentially be pushed down into WasmObjectFile, so that LLD doesn't have to handle lists of relocations.
2. Or, maybe you are concerned about the Wasm file encoding on disk - you want to change that so that relocations are stored per chunk/function. You don't care about what WasmObjectFile's sections iterate over, as long as each input chunk (function etc) has a list of relocations associated with it by WasmObjectFile, so that LLD doesn't have to track that. I think this is what Sam was suggesting on GitHub.
3. Or maybe you're worried about both? The file format and the section abstraction are both wrong to you.

I don't have a strong opinion - I mean functionally they all end up doing the same thing! It's just a matter of shuffling around/renaming some code, to make future maintenance less confusing.

I'll go with whatever other people want - this PR is just a quick win that gets rid of an unoptimised loop, while avoiding changing the file format, and without redefining what "section" currently means in the context of Wasm. With those constraints, I don't think there's //that// much that could be done differently...


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42176