[PATCH] D12433: [WinEH] Add cleanupendpad instruction
Joseph Tremoulet via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 28 08:43:33 PDT 2015
JosephTremoulet added inline comments.
================
Comment at: lib/CodeGen/WinEHPrepare.cpp:3308-3313
@@ -3294,3 +3307,8 @@
IsUnreachableCleanupret = CRI->getCleanupPad() != CleanupPad;
- if (IsUnreachableRet || IsUnreachableCatchret || IsUnreachableCleanupret) {
+ // The token consumed by a CleanupEndPadInst must match the funclet token.
+ bool IsUnreachableCleanupendpad = false;
+ if (auto *CEPI = dyn_cast<CleanupEndPadInst>(TI))
+ IsUnreachableCleanupendpad = CEPI->getCleanupPad() != CleanupPad;
+ if (IsUnreachableRet || IsUnreachableCatchret ||
+ IsUnreachableCleanupret || IsUnreachableCleanupendpad) {
new UnreachableInst(BB->getContext(), TI);
----------------
This part is likely wrong or incomplete; I have some questions about how to proceed that I felt were best asked in the context of a code review:
1. As written, this will leave an `unreachable` as the first instruction in a block that's the target of unwind edges, which doesn't verify. I can see that if we were to run `SimplifyCFGOpt::SimplifyUnreachable` on it, it would rewrite any invoke predecessors as calls. Does that mean we need to run SimplifyCFGOpt on the function?
2. Should `SimplifyCFGOpt::SimplifyUnreachable` be extended to rewrite catchendpad/cleanupendpad/terminatepad/cleanupret predecessors as 'unwind to caller'? Does it matter that doing so would make those predecessors 'mayThrow()' where they weren't before? From the comment "This would be a good place to note that the call does not throw though" in SimplifyUnreachable, I'm guessing the same happens for the invoke->call rewrite so it's ok (or at least won't make things worse), but not sure I'm intepreting that correctly.
3. I think we'll need to do something similar for unwind edges that target implausible catchendpads, but since the plausiblity check there is 'is this catchendpad in the parent funclet', I think that will need to happen after the treeifying duplication that makes "*the* parent funclet" well-defined. [I guess this one was a comment, not a question.]
http://reviews.llvm.org/D12433
More information about the llvm-commits
mailing list