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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
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
----------------
fitzgen wrote:
> sbc100 wrote:
> > Why `.ll` and not `.s` here? 
> I used `.ll` because it was the first test I wrote and it was derived from the C snippet. I'm happy to change it to `.s` if you prefer.
Yes please, I've been trying to covert all the existing .ll tests .s for a while so would rather not any any new .ll if possible.


================
Comment at: lld/test/wasm/duplicate-global-imports.s:30
+_start:
+  .functype _start () -> ()
+  end_function
----------------
fitzgen wrote:
> sbc100 wrote:
> > 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
> Regarding this and removing `--no-gc-sections`: is it generally preferred to create uses of things rather than to pass `--no-gc-sections` in tests?
> 
> Happy to update this either way, just curious.
My general philosophy is to try to minimize the number of flags passed in a given test.   This tends to make to more obvious what is being tested (less noise) and makes makes it easier to find tests for a given open by simply grepping for it.    There could also be an argument that we want to do most of our testing on the default path since that is the most well trodden one... maybe?

Of course its a balance and we don't want to be jumping through too many hoops to avoid extra flags. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105519