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

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp:79-90
+  for (auto &MI : *MBB) {
+    if (MI.getOpcode() == WebAssembly::CATCH_ALL) {
+      SeenCatchAll = true;
+      continue;
+    }
+    if (SeenCatchAll && MI.isCall()) {
+      const MachineOperand &CalleeOp = MI.getOperand(0);
----------------
majnemer wrote:
> aheejin wrote:
> > aheejin wrote:
> > > majnemer wrote:
> > > > aheejin wrote:
> > > > > majnemer wrote:
> > > > > > aheejin wrote:
> > > > > > > majnemer wrote:
> > > > > > > > Is it not possible for the MBB to branch to block which then calls std::terminate?
> > > > > > > Hmm, good point... I assumed a terminatepad would be always a single BB but it may not hold. Do you think I should DFS all descendants from here?
> > > > > > We handled problems like this one by using getFuncletMembership on the MachineFunction. This does a DFS to let you know which MBBs are part of which catch scopes. I think I'd find a way to reuse it for this.
> > > > > Hmm, no, I guess I shouldn't.. doing DFS from all catchpads or cleanuppads sounds like unnecessarily expensive. Should I search for a function call to `__clang_call_terminate` and search upward from there..?
> > > > IIRC, getFuncletMembership is O(# MBBs) which is no worse than the fundamental amount of work you are doing here. It shouldn't be a considerable amount of overhead.
> > > Oh, I added the last comment before seeing your new comments about `getFuncletMembership`, sorry.
> > > 
> > > You mean, I can replace the whole analysis (not only terminate pad thing) with `getFuncletMembership`? Or are you talking only about this terminatepad detection?
> > > 
> > > If you meant the former, I actually thought about not doing this DFS myself but just use the result from `getFuncletMembership`, but I couldn't find a way to create parent-child relationship between exception objects with `getFuncletMembership`. That function just returns a map of <BB, funclet number>, which does not say anything about whether a funclet is included in another funclet or is a top-level funclet. Can I reconstruct that information using `getFuncletMembership`?
> > > 
> > > And by the way I changed the function name to `getEHScopeMembership` in D47005 after you accepted it, per @dschuff's request. PTAL if it looks OK.
> > Oh, maybe I can just search for a call to `__clang_call_terminate()` and figure out which `WebAssemblyException` object it belongs to, and search for BBs that are contained in that exception object...
> Wouldn't finding the other BBs be equivalent to inverting the map given by getEHScopeMembership?
As I said above, can I reconstruct LoopInfo-like parent-child relationship with `getFuncletMembership`?


Repository:
  rL LLVM

https://reviews.llvm.org/D44134