This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Add an option to make get_local/set_local explicit.
ClosedPublic

Authored by sunfish on Oct 20 2016, 12:05 PM.

Details

Summary

This patch adds a pass, controlled by an option and off by default for now, for making implicit get_local/set_local explicit. This simplifies emitting wasm with MC.

Diff Detail

Repository
rL LLVM

Event Timeline

sunfish updated this revision to Diff 75330.Oct 20 2016, 12:05 PM
sunfish retitled this revision from to [WebAssembly] Add an option to make get_local/set_local explicit..
sunfish updated this object.
sunfish set the repository for this revision to rL LLVM.
sunfish added subscribers: dschuff, jgravelle-google.
dschuff added a subscriber: sbc100.Oct 24 2016, 8:46 AM
dschuff added inline comments.Oct 24 2016, 9:58 AM
lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
61 ↗(On Diff #75330)

use LLVM's lovely functionStyleCapitalization here and below. And might as well clang-format this file if it hasn't been, too.

293 ↗(On Diff #75330)

So, after this pass runs, we expect every VReg to be stackified, right? Should we run a pass over the MIs (maybe behind !NDEBUG) to check this?

lib/Target/WebAssembly/WebAssemblyInstrInfo.td
155 ↗(On Diff #75330)

might as well add the binary opcodes for the local operations too.

162 ↗(On Diff #75330)

why is this modeled as mayStore? just because it's no longer a register-like operation? Or we want LLVM to think of locals as something like memory? We should probably have some more explanatory text (maybe in this file) about how we model the locals.

lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
44 ↗(On Diff #75330)

general question: should we be somehow prefixing our wasm-specific options, e.g. wasm-enable-explicit-locals?

sunfish updated this revision to Diff 75624.Oct 24 2016, 12:31 PM
sunfish added a reviewer: dschuff.

Address review comments.

sunfish updated this revision to Diff 75629.Oct 24 2016, 12:47 PM
sunfish marked an inline comment as done.

Minor fixes.

sunfish marked 4 inline comments as done.Oct 24 2016, 12:48 PM
dschuff accepted this revision.Oct 24 2016, 12:49 PM
dschuff edited edge metadata.
dschuff added inline comments.
lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
299 ↗(On Diff #75624)

registe->register

lib/Target/WebAssembly/WebAssemblyInstrInfo.td
143 ↗(On Diff #75624)

get_locals->get_local

This revision is now accepted and ready to land.Oct 24 2016, 12:49 PM
This revision was automatically updated to reflect the committed changes.