This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Always start table index at 1, even for relocatable output
ClosedPublic

Authored by ncw on Jan 16 2018, 3:16 AM.

Details

Summary

Previously llvm was using 0 as the first table index for wasm object files
but now that has switched to 1 we can have the output of lld do the same
and simplify the code.


No need to commit simultaneously with the matching LLVM change (D42095), they are completely independent. Third chunk split out of D41955.

No functional changes, since the table is ignored of object files.

Diff Detail

Repository
rL LLVM

Event Timeline

ncw created this revision.Jan 16 2018, 3:16 AM
ncw added inline comments.Jan 16 2018, 3:21 AM
test/wasm/relocatable.ll
132 ↗(On Diff #129928)

Test expectations updated: as for D42095, relocatable Wasm files now match non-relocatable ones (avoiding random/unnecessary differences between them is good). The ELEM section has offset one, causing the indexes below to be incremented by one as well.

sbc100 added inline comments.Jan 16 2018, 1:42 PM
wasm/Writer.cpp
111 ↗(On Diff #129928)

Just use a global static "static const int kInitialTableOffset"?

ncw updated this revision to Diff 130107.Jan 17 2018, 1:36 AM
ncw marked an inline comment as done.

Updated:

  • Renamed constant
ncw updated this revision to Diff 130649.Jan 19 2018, 10:20 AM

Updated:

  • Simple rebase
ncw updated this revision to Diff 130721.Jan 19 2018, 4:38 PM

Updated:

  • Another rebase after #4c and #4d landed
ncw updated this revision to Diff 131078.Jan 23 2018, 8:07 AM

Updated:

  • Rebased to avoid conflict after previous change was landed with some reformating

Can you update the description and title, now that this change is so specific.

ncw retitled this revision from [WebAssembly] Symbol changes #3: Cosmetic table, LLD. NFC. to [WebAssembly] Symbol changes #3: Make LLD relocatable object files match Clang output. NFC..Jan 23 2018, 2:02 PM
ncw edited the summary of this revision. (Show Details)
ncw updated this revision to Diff 131244.Jan 24 2018, 6:03 AM

Updated:

  • After weak-alias.ll test was extended (to test relocatable output as well) this patch had to be updated...

Could it be merged soon to avoid further rebasing?

sbc100 added a comment.EditedJan 24 2018, 11:48 AM

I was imagining a more simple CL description. Something like:

[WebAssembly] Always start table index at 1, even for relocatable output

Previously llvm was using 0 as the first table index for wasm object files
but now that has switched to 1 we can have the output of lld to the same
and simply the code.

Happy to land make this change myself as I land it but i think its nice to have
the review match the commit.

sbc100 accepted this revision.Jan 24 2018, 11:50 AM
This revision is now accepted and ready to land.Jan 24 2018, 11:50 AM
ncw retitled this revision from [WebAssembly] Symbol changes #3: Make LLD relocatable object files match Clang output. NFC. to [WebAssembly] Always start table index at 1, even for relocatable output.Jan 24 2018, 1:11 PM
ncw edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.