This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Update WasmEHPrepare for the new spec
ClosedPublic

Authored by aheejin on Jan 4 2021, 3:39 PM.

Details

Summary

Clang generates wasm.get.exception and wasm.get.ehselector
intrinsics, which respectively return a caught exception value (a
pointer to some C++ exception struct) and a selector (an integer value
that tells which C++ catch clause the current exception matches, or
does not match any).

WasmEHPrepare is a pass that does some IR-level preparation before
instruction selection. Previously one of things we did in this pass was
to convert wasm.get.exception intrinsic calls to
wasm.extract.exception intrinsics. Their semantics were the same
except wasm.extract.exception did not have a token argument. We
maintained these two separate intrinsics with the same semantics because
instruction selection couldn't handle token arguments. This
wasm.extract.exception intrinsic was later converted to
extract_exception instruction in instruction selection, which was a
pseudo instruction to implement br_on_exn. Because br_on_exn pushed
an extracted value onto the value stack after the end instruction of a
block, but LLVM does not have a way of modeling that kind of behavior,
so this pseudo instruction was used to pull an extracted value out of
thin air, like this:

block $l0
  ...
  br_on_exn $cpp_exception $l0
  ...
end
extract_exception ;; pushes values onto the stack

In the new spec, we don't need this pseudo instruction anymore because
catch itself returns a value and we don't have br_on_exn anymore. In
the spec catch returns multiple values (like br_on_exn), but here we
assume it only returns a single i32, which is sufficient to support C++.

So this renames wasm.get.exception intrinsic to wasm.catch. Because
this CL does not yet contain instruction selection for wasm.catch
intrinsic, all RUN lines in exception.ll, eh-lsda.ll, and
cfg-stackify-eh.ll, and a single RUN line in wasm-eh.cpp (which is an
end-to-end test from C++ source to assembly) fail. So this CL
temporarily disables those RUN lines, and for those test files without
any valid remaining RUN lines, adds a dummy RUN line to make them
pass. These tests will be reenabled in later CLs.

Diff Detail

Event Timeline

aheejin created this revision.Jan 4 2021, 3:39 PM
aheejin requested review of this revision.Jan 4 2021, 3:39 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 4 2021, 3:39 PM

In the description I think "but LLVM does not have a way of that kind of behavior" is missing the word "modeling" => "but LLVM does not have a way of modeling that kind of behavior"

llvm/lib/CodeGen/WasmEHPrepare.cpp
374–375

What is the token argument used for? Could clang generate llvm.wasm.catch directly?

llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
166

Why do we need to keep this instruction definition? Is it removed in a later diff in this stack?

llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
611

What is this addition for?

dschuff added inline comments.Jan 6 2021, 3:57 PM
llvm/lib/CodeGen/WasmEHPrepare.cpp
374–375

Token arguments (https://llvm.org/docs/ExceptionHandling.html#wineh) are used to preserve the original scoping structure in mid-level optimization passes. I haven't looked at this intrinsic recently but I'd guess that maybe the token keeps it from getting moved out of its containing scope?

tlively added inline comments.Jan 7 2021, 10:15 AM
llvm/lib/CodeGen/WasmEHPrepare.cpp
374–375

Ah, that makes sense, thanks!

aheejin edited the summary of this revision. (Show Details)Jan 8 2021, 7:12 AM
aheejin marked an inline comment as done.Jan 8 2021, 7:23 AM

In the description I think "but LLVM does not have a way of that kind of behavior" is missing the word "modeling" => "but LLVM does not have a way of modeling that kind of behavior"

Thanks. Fixed.

llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
166

It is removed in D94041. I tried to keep removal of exnref and related instructions (br_on_exn, and also this, because this pseudoinstruction is used to support br_on_exn) in a separate CL because it was all mechanical stuff but touched a broad range of files, so mixing it with CLs that actually change stuff could look confusing.

llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
611

I think I added this in relation to the terminate pad handling (D94050) but while slicing up the whole thing into several CLs this was somehow put into this CL. But come to think of it I think this is not really necessary even in D94050 because we don't run llc on this file anyway. Will delete this.

aheejin updated this revision to Diff 315392.Jan 8 2021, 7:23 AM
aheejin marked an inline comment as done.

Delete declare void @_ZSt9terminatev()

aheejin marked an inline comment as done.Jan 8 2021, 7:24 AM
tlively accepted this revision.Jan 8 2021, 2:06 PM
This revision is now accepted and ready to land.Jan 8 2021, 2:06 PM
This revision was landed with ongoing or failed builds.Jan 8 2021, 11:57 PM
This revision was automatically updated to reflect the committed changes.