[PATCH] D43940: [WebAssembly] Reorder reloc sections to come between symtab and name

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added inline comments.


================
Comment at: tools/yaml2obj/yaml2wasm.cpp:545
+  }
+
   return 0;
----------------
sbc100 wrote:
> ncw wrote:
> > sbc100 wrote:
> > > I thought we decided this didn't need to change?  i.e. we don't care about relative ordering of name vs reloc or linking sections.  At least this change relates to reloc vs linking sections, so I'm not sure why the name section moved too.   
> > I think it would be nice to have "name" come after "linking" at least. (Since we set the name for inputchunks based on the first symbol that references them or the name from the name section, with the name section "winning".)
> > 
> > I'd still personally prefer to nail down the order of the sections, so that compat between tools is guaranteed and no-one needs to try to handle different orderings for the sections; I haven't quite been convinced on the benefits being lenient in section ordering.
> > 
> > This change does exactly what the PR title says: moves reloc to come between linking/symtab and name sections. So in that sense, I've done a matching change in yaml2wasm and WasmObjectWriter.
> But we are not the only consumer or producer or wasm files, so we need to as flexible as the spec allows.
> 
> I you revert this file this change still does what you want for now, doesn't it?    If you don't mind doing that then this LGTM and we can discuss moving the name section separately.
I'd be OK with splitting this out - but D43946 changes LLD to output "symtab/relocs/name", and what this change is trying to achieve is to make llc and yaml2wasm match. So one way or another we would need to make the bits agree before we could apply D44024 to validate the section order.

> But we are not the only consumer or producer or wasm files, so we need to as flexible as the spec allows.

But we are the producer of the Linking.md spec so we can make it as unflexible as we like :) Currently the "core" spec has no flexibility at all (including the "name" section, which the spec says must come after all other core sections). The only flexibility currently is due to an omission in the Linking.md one that doesn't specify the ordering for the new sections it defines - which would ensure compat with other producers/consumers of wasm, if they agreed on an order.

Sorry to be stubborn - I'll give in and follow your lead on Monday!


Repository:
  rL LLVM

https://reviews.llvm.org/D43940