[PATCH] D44313: [WebAssembly] Implement GC for imports

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

Nice!

I don't really like the yaml test case, but hopefully it will be shortlived.



================
Comment at: wasm/Symbols.cpp:85
+  }
+  if (InputChunk *C = getChunk()) {
+    C->Live = true;
----------------
I think getChunk can now be removed and inlines into `isLive` and `getLive`, it think it will be more readable that wasy.


================
Comment at: wasm/Symbols.cpp:89
+  }
+  Live = true;
 }
----------------
Maybe assert `isUndefined` here?


================
Comment at: wasm/Symbols.h:78-79
 
+  // Marks the symbol as Live, so that it will be included in the final image.
+  void markLive();
+
----------------
ruiu wrote:
> I don't think we should add a notion of "liveness" to symbols. It is easy to be confused, but "liveness" is for chunks, data, globals, etc, and symbols themselves are not subject of GC. I understand that since each wasm global have only one symbol associated with it, that distinction is ambiguous, but I still want to maintain that distinction to make things easy to understand.
But for undefined symbols we need to know which ones to write to the relocatable output.  For undefined symbols there is no chunk.

We already have in in ELF/Symbols.h: `unsigned Used : 1;`

If you prefer we can give it the same name here for consistency.

I'm not sure there is any difference between `Used` and `Live` though, so it might be worth renaming the one in ELF?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44313