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

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
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:
> > 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?).
> 'catch block of a WebAssemblyException' is confusing too because `WebAssemblyException` itself is only for a catch part. Should we go with just `WebAssemblyException`?
Yeah that sounds fine. In the other CL we should make it clear what WebAssemblyException actually refers to.


Repository:
  rL LLVM

https://reviews.llvm.org/D48262