[PATCH] D44134: [WebAssembly] Add WebAssemblyException information analysis

[Derek Schuff via Phabricator via llvm-commits]

dschuff added a comment.

Looks pretty reasonable otherwise. I like the MIR unittest 👌



================
Comment at: lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp:14
+///
+/// WebAssembly instructions for exception handling are structured as follows:
+///   try
----------------
Ah, this is the explanation I was looking for before. I would put it (at least the part that describes the class) in the header where the class is defined. 


================
Comment at: lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp:18
+///   catch             ----|
+///     instructions*       | -> This part consists of WebAssemblyException
+///   end               ----|
----------------
I would reverse it and say  something like "A WebAssemblyException represents/contains/consists of these blocks/this region"


================
Comment at: lib/Target/WebAssembly/WebAssemblyExceptionInfo.h:27
+
+class WebAssemblyException {
+  MachineBasicBlock *EHPad = nullptr;
----------------
Per our discussion on the previous CL, a big comment here explaining what this class represents, and how it is used.


Repository:
  rL LLVM

https://reviews.llvm.org/D44134