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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: llvm/test/MC/WebAssembly/debug-info.ll:14
 ; CHECK-NEXT:    Type: IMPORT (0x2)
-; CHECK-NEXT:    Size: 81
 ; CHECK-NEXT:    Offset: 18
----------------
wingo wrote:
> sbc100 wrote:
> > Is this because we don't import that table anymore?
> > 
> > I wonder if this change, along with all the other test changes that are just about removing the table-by-default can land first.
> > 
> > i.e. can we make a change "don't emit table import in object files that don't have table relocation".    
> > 
> > Just in the interest of shrinking this PR?
> Yeah I think that's the reason, yes.
> 
> Regarding patch splitting -- The issue is that this patch does two things:
>  - makes call_indirect have a table operand, thus causing table relocs.  Also makes call_indirect take up more space because of sleb128 relocs.
>  - emits the indirect function table import if and only if there are relocs against it (either implicitly via `TABLE_INDEX` or explicity via `TABLE_NUMBER`)
> 
> If you just do the first part, you have to then hack around to avoid a double-import of the indirect function table (the one that comes from the table operand, and the hard-coded import).  If you just do the second, you miss references from call_indirect.
> 
> I think this change may make sense as a unit, rather than split.  WDYT?
I'm not clear why we can't to the second part first... why can't we emit the table if and only if there is a TABLE_INDEX relocation? 

Are you suggesting that there could be some call_indirect instruction generated by MC that doesn't have a TABLE_INDEX relocation attached to it?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90948