[PATCH] D105519: [WebAssembly] Deduplicate imports of the same module name, field name, and type

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

Nice!   I think I'm sold on the idea that this a good think.  I'm curious what @dschuff and @sunfish think?   Is wasm-ld the right place to be doing this?



================
Comment at: lld/test/wasm/duplicate-function-imports.ll:1
+; RUN: llc -filetype=obj %s -o %t.o
+; RUN: wasm-ld -o %t.wasm %t.o
----------------
Why `.ll` and not `.s` here? 


================
Comment at: lld/test/wasm/duplicate-global-imports.s:2
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t.o
+# RUN: wasm-ld --no-check-features --no-gc-sections --allow-undefined -o %t1.wasm %t.o
+# RUN: obj2yaml %t1.wasm | FileCheck %s
----------------
I think we can remove `--no-gc-sections` (if you use each on in a global.get) and `--allow-undefined`.


================
Comment at: lld/test/wasm/duplicate-global-imports.s:30
+_start:
+  .functype _start () -> ()
+  end_function
----------------
Maybe we should do a `global.get` on each of these globals so we can see that the de-duplication is working in the relocations.  

Sadly obj2yaml doesn't do disassembly so the test will be a little opaque, at least until https://reviews.llvm.org/D105539 lands


================
Comment at: lld/test/wasm/duplicate-table-imports.s:2
+# RUN: llvm-mc -mattr=+reference-types -triple=wasm32-unknown-unknown -filetype=obj -o %t.o %s
+# RUN: wasm-ld --no-entry --allow-undefined --export=read_externref --export=read_funcref_1 --export=read_funcref_2 -o %t1.wasm %t.o
+# RUN: obj2yaml %t1.wasm | FileCheck %s
----------------
Can you remove `--allow-undefined` ?   And maybe you can `--export-all` rather then exporting each one ?


================
Comment at: lld/test/wasm/duplicate-table-imports.s:40
+  i32.const 0
+  table.get t3
+  end_function
----------------
Lets do a `.get` on each of the tables?


================
Comment at: lld/wasm/SyntheticSections.h:198
+  llvm::DenseMap<ImportKey<WasmSignature>, uint32_t> alreadyImportedFunctions;
+  llvm::DenseMap<ImportKey<WasmTableType>, uint32_t> alreadyImportedTables;
 };
----------------
Maybe just `importedGlobals`, `importedFunctions` and `importedTables`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105519