[PATCH] D54660: [WebAssembly] Add null streamer support

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h:97
+
+  virtual void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override {}
+  virtual void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override {}
----------------
eush wrote:
> What is `virtual` here for? It seems to be redundant since these functions are already declared virtual in the base class (hence the `override`).
Thanks, I blindly copy-pasted this from the parent class and forgot to delete them.


================
Comment at: test/CodeGen/WebAssembly/null-streamer.ll:12
+  call void @llvm.wasm.throw(i32 0, i8* %p) ; This is to generate '.eventtype'
+  call void @g()
+  ret i32 0
----------------
eush wrote:
> Should there be a similar comment about `.functype` to explain `g`?
Actually we are planning changes (D54652, which will soon be landed) so that not only declared functions but also defined function will generate `.functype` directive. But anyway I think as you suggested having a second RUN line would be better than comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D54660