rnk added a comment.

The approach makes sense to me, but can you include test cases to illustrate 
the IR and code that gets generated?



================
Comment at: clang/lib/CodeGen/CGException.cpp:1833
+/// Find all local variable captures in the statement.
+struct ReturnStmtFinder : ConstStmtVisitor<ReturnStmtFinder> {
+  bool ContainsRetStmt = false;
----------------
We have the option to generalize the naming here to include other kinds of 
jumps that exit the scope of the finally. I think it's worth implementing 
`__leave` eventually, which I think users have asked for at some point. I could 
go either way, your choice.


================
Comment at: clang/lib/CodeGen/CGException.cpp:2205
+void CodeGenFunction::EnterSEHTryStmt(const SEHTryStmt &S,
+                                      bool &ContainsRetStmt) {
   CodeGenFunction HelperCGF(CGM, /*suppressNewContext=*/true);
----------------
If we were to generalize now, we'd want to name this something like 
`HasLocalUnwind`.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/WinException.cpp:604-614
   // Iterate over all the invoke try ranges. Unlike MSVC, LLVM currently only
   // models exceptions from invokes. LLVM also allows arbitrary reordering of
   // the code, so our tables end up looking a bit different. Rather than
   // trying to match MSVC's tables exactly, we emit a denormalized table.  For
   // each range of invokes in the same state, we emit table entries for all
   // the actions that would be taken in that state. This means our tables are
   // slightly bigger, which is OK.
----------------
Perhaps it's time to revisit this.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/WinException.cpp:631-634
+      // Mark up the destination of _local_unwind so it doesn't unwind
+      // too far.
+      //
+      // FIXME: Can this overlap with the EH_LABEL for an invoke?
----------------
I don't understand what this is doing, but I should look at the tests.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2942
+      if (!isa<CatchSwitchInst>(EHPadBB->getTerminator())) {
+        report_fatal_error("localunwind doesn't point to catchswitch");
+      }
----------------
Since this isn't verifier checked yet, is it possible to emit a diagnostic via 
the LLVMContext and select this instruction to nothing, or would that leave 
behind disconnected unreachable MBBs that break too many invariants later?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124642/new/

https://reviews.llvm.org/D124642

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to