[PATCH] D41038: [WebAssembly] Preserve ordering of global symbols

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

In https://reviews.llvm.org/D41038#950727, @ncw wrote:

> Looks good. Is the ordering mainly for the stability of the test output, or is it expected to have a tiny improvement to code locality at runtime?
>
> There might be a couple of other places where we could do the same change. For example `calculateImports` has exactly the same iteration pattern, where the imports are ordered according to the first-importing file, rather than ordered by the file that actually uses the import. Harder to see how to fix that one though. I wouldn't have thought it's worth trying.


I think an aesthetic things.  It makes sense for function and global definitions to appear in declaration order.

For import, I think it makes sense for them to appear in the order of the first object the contains the import.  I can't imagine a object that imports something but doesn't use it.. at least clang shouldn't produce such a thing.



================
Comment at: wasm/Writer.cpp:617-618
     for (Symbol *Sym : File->getSymbols()) {
-      if (Sym->hasOutputIndex() || !Sym->isDefined())
-        continue;
-
-      if (Sym->isFunction()) {
-        if (Sym->getFile() && isa<ObjFile>(Sym->getFile())) {
+      // Assign indexes for symbols defined with this file.
+      if (Sym->isDefined() && File == Sym->getFile()) {
+        if (Sym->isFunction()) {
----------------
ruiu wrote:
> Could you invert the boolean expression so that we can still do early continue (to keep the indentation shallower)?
I personally found it more readable this way around.  But sure, will change it back


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D41038