[PATCH] D90948: [WebAssembly] call_indirect issues table number relocs

[Thomas Lively via Phabricator via llvm-commits]

tlively added a comment.

> In D90948#2527474 <https://reviews.llvm.org/D90948#2527474>, @tlively wrote:
>
>> I agree that we just need a single `call_indirect` that can take either a constant zero table operand or a symbolic table operand. Both types of operands need to be able to coexist in the same link or even in the same object file to make MVP object files linkable with reference-types object files.
>
> Whoa -- is linking MVP object files with reference-types object files a requirement?  This was totally not on my radar.  Note that with feature flags, we can prohibit linking between MVP and reftypes object files.

Yes, I expect we will receive bug reports from confused users if we don't allow linking MVP and reftypes object files together. Consider a library author who has no use for reference types and wants their library to run on as many engines as possible, including those that may not support reference types. That library author might distribute just a single build of their library that does not have the reference-types feature enabled. An application author who does use reference types should still be able to link against that library without requiring a separate build. Although you're right that we could just make that an error, that would inconvenience the library author who now has to distribute two builds of their library, even though the builds do not differ in the features they use.

>>> I think this goes against what was already done in D91637 <https://reviews.llvm.org/D91637> and D92840 <https://reviews.llvm.org/D92840>.  If individual object files can signal their use of the indirect function table via the presence or absence of the `__indirect_function_table` import, the linker can then only include it in the final linking stage when needed -- a minor win.  I can see the point in treating `__indirect_function_table` as a good default, and which in the absence of reference-types will be table number 0, but reserving table 0 only complicates things when reference types *are* enabled -- because then you have two kinds of tables.
>>
>> Yes, I agree that it's better not to emit the table at all if it's not needed. I'm not totally clear on whether those patches to emit the table only when necessary are ABI-breaking changes, though. Do I understand correctly that they are not ABI-breaking because wasm-ld previously ignored the presence of the table import and that those patches signal whether the table is used by the presence of the import in the object file and not via new symbol or relocation types? If so, I agree that table 0 (i.e. '__indirect_function_table') should be live only if necessary.
>
> This is correct, yes.  Old object files always have an `__indirect_function_table` import, and the old linker always produces `__indirect_function_table`.  Newer object files may omit `__indirect_function_table` if it is not needed, and newer linkers may omit `__indirect_function_table` in the output if no imput file requires it.  It is always safe (but can be suboptimal) to emit an indirect function table, so old linkers can work with new object files.  New linkers can also work with old object files, because the `__indirect_function_table` import is always present, so the table will not be omitted.  So no ABI break AFAIU.

Thanks for confirming!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90948