[PATCH] D126950: [lld][WebAssembly] Retain data segments referenced via __start/__stop

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: lld/test/wasm/gc-sections-metadata-startstop.ll:1
+; RUN: llc -filetype=obj -o %t.o %s
+; RUN: wasm-ld %t.o --gc-sections -o %t.wasm
----------------
We are trying to move away from `.ll` tests and stick to use `.s` tests as they tend to be easier to read/faster/fewer dependencies.

Would you mind converting this test to `.s`?


================
Comment at: lld/wasm/InputChunks.cpp:531
+    return ".tdata";
+  StringRef name = this->getName();
+  if (!config->mergeDataSegments)
----------------
I don't think you need the `this->` qualifiers here .


================
Comment at: lld/wasm/MarkLive.cpp:89
 
+void MarkLive::enqueueWholeChunk(InputChunk *chunk) {
+  LLVM_DEBUG(dbgs() << "markLive: " << chunk->getName() << "\n");
----------------
Just `enqueueChunk` seems descriptive enough, or maybe even just overload `enqueue`?


================
Comment at: lld/wasm/MarkLive.cpp:121
+      if (symtab->find(("__start_" + name).str()) ||
+          symtab->find(("__stop_" + name).str())) {
+        enqueueWholeChunk(segment);
----------------
So, does this mean that if I reference the symbol `__start_data`, then all the normal data segment in my whole program will be included (non will be GC'd)?

Is this how ELF works?  It seems like a shame to loose the ability to GC data segments just because i want to know where my data starts and/or stops.  I suppose we have other symbols such as `_edata` which can be used to get the start/end of the static data region without disabling GC?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126950/new/

https://reviews.llvm.org/D126950