This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Fix for PIC external symbol ISEL
ClosedPublic

Authored by aardappel on Apr 5 2021, 4:06 PM.

Details

Summary

wasm64 was missing DAG ISEL patterns for external symbol based global.get, but simply adding these analogous to the existing 32-bit versions doesn't work.
This is because we are conflating the 32-bit global index with the pointer represented by the external symbol, which for wasm32 happened to work.
The simplest fix is to pretend we have a 64-bit global index. This sounds incorrect, but is immaterial since once this index is stored as a MachineOperand it becomes 64-bit anyway (and has been all along). As such, the EmitInstrWithCustomInserter based implementation I experimented with become a no-op and no further changes in the C++ code are required.

Diff Detail

Event Timeline

aardappel created this revision.Apr 5 2021, 4:06 PM
aardappel requested review of this revision.Apr 5 2021, 4:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2021, 4:06 PM
sbc100 accepted this revision.Apr 5 2021, 4:13 PM
This revision is now accepted and ready to land.Apr 5 2021, 4:13 PM
tlively accepted this revision.Apr 6 2021, 1:40 PM

Very cool, I did not know any of these tricks were possible.

llvm/test/CodeGen/WebAssembly/load-store-pic.ll
4

Nice! I didn't know you could define match strings from the command line like this.

aardappel added inline comments.Apr 8 2021, 12:05 PM
llvm/test/CodeGen/WebAssembly/load-store-pic.ll
4

I didn't know either, but it was exactly what I needed, so I went to look in the docs for them :) The good thing about the LLVM eco-system is that whatever you're trying to do, someone has needed it before..

This revision was automatically updated to reflect the committed changes.