This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Fix PIC/GOT codegen for wasm64
ClosedPublic

Authored by aardappel on May 3 2021, 12:54 PM.

Details

Summary

table_base is know 64-bit, since in LLVM it represents a function pointer offset
table_base32 is a copy in wasm32 for use in elem init expr, since no truncation may be used there.
New reloc R_WASM_TABLE_INDEX_REL_SLEB64 added

Diff Detail

Event Timeline

aardappel created this revision.May 3 2021, 12:54 PM
aardappel requested review of this revision.May 3 2021, 12:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2021, 12:54 PM
sbc100 added inline comments.May 6 2021, 12:01 PM
lld/wasm/Driver.cpp
587–589

Since config->is64.getValueOr(false) appears so often in the code I wonder if we could make config->is64 just work (less typing and easier to read.. less likely to cache in a local variable). I know we had a good reason for making it an optional... but maybe we should re-visit?

(not blocking this change just mentioning it..)

680

How about:

if (config->is64.getValueOr(false))
  WasmSym::definedTableBase32 = symtab->addOptionalDataSymbol("__table_base32");
lld/wasm/InputElement.h
55

This seems a little out of place here... although I don't know if we have a better home for such utilities. If it was just used in one source file I would say move it out of the header.

lld/wasm/Symbols.h
566

Can we mention that this is workaround for lack or https://github.com/WebAssembly/extended-const and can potentially be removed if/wehn this lands?

lld/wasm/SyntheticSections.cpp
343

Maybe keep these spacer lines? Then seem harmless at worst.

llvm/test/MC/WebAssembly/reloc-pic64.s
5

"entries" .. which looks one of my spelling mistakes that you cargo culted :)

aardappel marked 4 inline comments as done.May 6 2021, 4:20 PM
aardappel added inline comments.
lld/wasm/Driver.cpp
587–589

We had a single boolean, until we decided we needed to know wether it was set or not, which is exactly in one place. We could also just add a second is64set boolean or whatever, that way the other 15 or so uses can be simplified. Up to you.

lld/wasm/InputElement.h
55

It's used in 3 source files.

aardappel updated this revision to Diff 343529.May 6 2021, 4:29 PM

Addressed code review.

sbc100 accepted this revision.May 6 2021, 4:45 PM
This revision is now accepted and ready to land.May 6 2021, 4:45 PM
dschuff accepted this revision.May 7 2021, 10:46 AM
dschuff added inline comments.
lld/test/wasm/data-layout.s
4

--check-prefixes isn't needed anymore

aardappel updated this revision to Diff 344089.May 10 2021, 9:30 AM
aardappel marked an inline comment as done.

fixed --check-prefixes

reworked test for .s format and rebasing

This revision was landed with ongoing or failed builds.May 20 2021, 10:01 AM
This revision was automatically updated to reflect the committed changes.