[PATCH] D45904: [CodeExtractor] Allow extracting blocks with exception handling

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 25 09:49:42 PDT 2018


rnk added inline comments.


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:207-209
+      if (BB->isEHPad()) {
+        llvm_unreachable("The first block cannot be an unwind block");
+        return {};
----------------
Do not attempt to recover from broken invariants. There are three reasonable options:
1. If this condition is possible but undesirable under normal operation, log with LLVM_DEBUG, dbgs(), and return {}.
2. Assert that this condition is impossible.
3. Use report_fatal_error to assert this condition is possible, but don't compile it out from release builds.


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:217-220
+      if (!Result.count(PBB)) {
+        llvm_unreachable("No blocks in this region may have entries from "
+                         "outside the region except for the first block!");
+        return {};
----------------
Ditto, this should probably be an assert, with the whole loop inside an NDEBUG block like it was before.


================
Comment at: test/Transforms/CodeExtractor/inline_eh.ll:1
+; RUN: opt < %s -skip-partial-inlining-cost-analysis -partial-inliner -S  | FileCheck %s
+; RUN: opt < %s -skip-partial-inlining-cost-analysis -passes=partial-inliner -S  | FileCheck %s
----------------
Please add a similar test that uses _CxxFrameHandler3 to get coverage of all the code you added to handle cleanupret, catchret, etc.


Repository:
  rL LLVM

https://reviews.llvm.org/D45904





More information about the llvm-commits mailing list