[PATCH] D34487: Restrict the definition of loop preheader to avoid special blocks

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added inline comments.


================
Comment at: lib/IR/BasicBlock.cpp:364-371
+  // It isn't safe to hoist instructions across Windows exception handling
+  // boundary blocks and it doesn't make sense to hoist instructions into
+  // blocks that can't have successors.
+  if (isa<CatchReturnInst>(Term) || isa<CleanupReturnInst>(Term) ||
+      isa<CatchSwitchInst>(Term) || isa<InvokeInst>(Term) ||
+      isa<ResumeInst>(Term) || isa<ReturnInst>(Term) ||
+      isa<UnreachableInst>(Term))
----------------
majnemer wrote:
> It should be impossible to get here if Term has no successors, no? Couldn't we `assert(Term->getNumSuccessors > 0)`?
> 
> Also, could we simplify this by just returning return `!Term->IsExceptional()` ?
I wanted to write this as a generic function that could be used from anywhere.  It is difficult to imagine why anyone would ever call it for a block without successors, but it seems cheap and easy to give a sensible answer in that case.  I don't have strong feelings about it either way.  The assertion would serve as a useful flag to callers that they are doing something nonsensical.

You're right that I could use IsExceptional() here.  I guess my implementation reflects my thought process while writing the function a little more directly than it ought to.


Repository:
  rL LLVM

https://reviews.llvm.org/D34487