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

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 1 10:58:48 PST 2018


sbc100 added a comment.

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

> Maybe you are right. I didn't have time to take a look and understand the wasm file format (that's why I didn't send any comments to https://github.com/WebAssembly/tool-conventions yet), but by reading wasm/InputChunks.h and wasm/InputFiles.cpp again, there's no two-level containers in the file format. What I found a bit confusing is the name of "InputSegment" -- that sounds like it is some base class, but it is actually a leaf class for data segment.
>
> To be clear, I'm not suggesting to make wasm look simliar to ELF. I'm of course not complaining that wasm is different from ELF. All file formats should have equal rights in lld. :) When I say something seems weird, that's just that. I don't think wasm needs to be similar to ELF because the two file formats are naturally different for a good reason. That's why you guys have invented a new file format in the first place.
>
> If "section" is used for other stuff in wasm, we shouldn't use the same terminology for something else to avoid confusion. I think this is more like a problem of lack of documentation or comments in wasm lld. Maybe we should expand the file comment of InputChunk.h to describe what Chunk is and how a file consists with chunks. I'll try to do that.
>
> I also think that we should rename InputSegment InputData. A function is called InputFunction, so it is I believe a better name.


The term "data segment" in the wasm world has a specific meaning.  It refers to the things in the data section.  The data section is made up of segments.  I know this is directly counter to the meaning in the ELF world but it make some sense.  Each wasm data segment is a contiguous indivisible run of of bytes.    InputSegment already has a comment specifying why its called what it is, and why it might be counter intuitive to someone coming from ELF.

I think the name InputSegment makes sense when this is understood, but I don't really might what we call it.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42176





More information about the llvm-commits mailing list