[PATCH] D48262: [WebAssembly] Add more utility functions

Derek Schuff via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 18 17:17:03 PDT 2018


dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

LGTM with the tweak to the comment.



================
Comment at: lib/Target/WebAssembly/WebAssemblyUtilities.h:58
+/// Return the "bottom" block of an entity, which can be either a loop or an
+/// exception, or something else. This differs from
+/// MachineLoop::getBottomBlock in that it works even if the entity is
----------------
aheejin wrote:
> dschuff wrote:
> > by 'exception' do you mean try block, catch block, both? what can 'something else' be?
> I  meant `WebAssemblyException` class defined in D44134. Actually this name itself is little confusing, because what it really means is the 'catch' of a wasm exception. I thought about renaming it to `WebAssemblyCatch`, `WebAssemblyExceptionCatch`, `WebAssemblyEHScope`, etc, but none of them really sounded better than this to me. (`WebAssemblyEHScope` is actually more confusing, because the definition of it is not really same with that of EHScope in general sense.)
> 
> Here 'something else' meaning, it can possibly be used for another data structure other than loop or exception if we get to have one. Maybe it sounds confusing and is better to be deleted..?
Maybe just say "either a loop or the catch block of a WebAssemblyException". I'd leave out the "something else" until we have a concrete use, or maybe a more concrete descriptor (e.g. something that will become encompassed by a wasm control-flow construct?).


Repository:
  rL LLVM

https://reviews.llvm.org/D48262





More information about the llvm-commits mailing list