[PATCH] D92321: [WebAssembly] Allow element sections for nonzero table numbers

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: llvm/lib/MC/WasmObjectWriter.cpp:936
+    W->OS << ElemKind;
+  }
+
----------------
wingo wrote:
> sbc100 wrote:
> > This is dead code no?  Since that flag cannot be set?
> > 
> > In fact all these changes to writeElemSection look unused (and therefore untested), perhaps we can split this out and make it part of a future PR?
> So the tableNumber changes above are definitely used and tested.  Definitely possible to have non-zero table numbers for the indirect function table.  I could remove this HAS_ELEM_KIND bit here but let me push back gently -- we would like to test that this output is spec-conforming and also that it "makes" sense" from a LLVM point of view (e.g. can round trip through `tables.s`).  But the former is more important than the latter.  By structuring this function to look more like the [binary spec]((https://webassembly.github.io/reference-types/core/binary/modules.html#binary-elemsec)) -- e.g. checking the `HAS_TABLE_NUMBER` flag instead of checking `TableNumber` -- I have greater confidence reading the code that it does what the spec says.
Sure, I don't really care if you check TableNumber of HAS_TABLE_NUMBER above.

But these six lines seems compltely pointless and in fact that are dead code right?    This function doesn't write arbitrary tables it *only* writes the special __indirect_function_table (at least today).   Can that ever have an ElemKind?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92321