[PATCH] D40725: Wasm entrypoint changes #3 (add --no-entry argument to LLD) APPLY AFTER D40559

[Sam Clegg via Phabricator via llvm-commits]

sbc100 accepted this revision.
sbc100 added a comment.
This revision is now accepted and ready to land.

LGTM - with a couple more comments.



================
Comment at: test/wasm/data-layout.ll:3
 ; RUN: llc -filetype=obj %s -o %t.o
-; RUN: lld -flavor wasm --emit-relocs --allow-undefined --entry '' -o %t.wasm %t.o %t.hello.o
+; RUN: lld -flavor wasm --emit-relocs --allow-undefined --no-entry -o %t.wasm %t.o %t.hello.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
----------------
Is the  --allow-undefined still needed? (I was thinking it might be there simply because this test doesn't have an entry point).


================
Comment at: wasm/Config.h:38
   uint32_t ZStackSize;
-  llvm::StringRef Entry;
+  llvm::Optional<llvm::StringRef> Entry;
   llvm::StringRef OutputFile;
----------------
I still think its simpler just to use empty string to mean no entry and continue to assume that empty strings cannot be symbols.   I'm don't feel super strongly about it though.  I guess maybe we should agree to spec in Linking.md that symbols cannot be empty strings?


================
Comment at: wasm/Writer.cpp:264
   bool ExportHidden = Config->Relocatable;
-  Symbol *EntrySym = Symtab->find(Config->Entry);
+  Symbol *EntrySym = Config->Entry ? Symtab->find(Config->Entry.getValue())
+                                   : nullptr;
----------------
I think you'll need to rebase this since the symbol is now stored in the Config itself.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D40725