This patch adds support for outlining cleanup handlers and refactors the existing catch handler outlining code to share common code as much as possible.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/CodeGen/WinEHPrepare.cpp | ||
---|---|---|
285–287 ↗ | (On Diff #20617) | Now that I think about it, I'm not sure we actually need to handle that case. All optimizations have to assume that landingpads can be entered for reasons other than those that are listed. Consider this case where we might hoist a store: void g(); static int r; int f() { try { g(); } catch (int) { r = 42; } catch (float) { r = 42; } return r; } An optimization that hoisted the r = 42 store into the landingpad block would be invalid, because the landingpad can be entered for other reasons. In particular, if we did that hoist and then inlined f into some other function with cleanups, now the store would be unconditional. |
288–292 ↗ | (On Diff #20617) | I'm not sure we can handle catches inside cleanups this way. Are you planning to switch to a more up front analysis in subsequent changes? I'm thinking of this case: void g(); struct MyRAII { ~MyRAII(); }; void runs_before_dtor(); void f() { MyRAII x; try { g(); } catch (int) { runs_before_dtor(); } } We need to make sure runs_before_dtor() runs first, but I guess we haven't gotten to building eh.actions. |
702–703 ↗ | (On Diff #20617) | Any reason not to map llvm.eh.begincatch to unreachable in a cleanup? |
708–737 ↗ | (On Diff #20617) | Similarly, I'd map this to unreachable. It isn't tested yet anyway. |
lib/CodeGen/WinEHPrepare.cpp | ||
---|---|---|
285–287 ↗ | (On Diff #20617) | Well, I'd be happy to never see the hoisting case arise, but I don't think it's actually going to be very difficult to handle if we wanted to. It might be better to have code to handle it that never gets executed than to have code to flags it as a fatal error if it does happen. Either way, I can take this comment out for now and we can decide when we get to the block analysis review. |
288–292 ↗ | (On Diff #20617) | Yeah, this only handles a single cleanup per landing pad and doesn't handle the case where a block of cleanup code is shared between landing pads. This is basically here as a place holder to move things along. I can add a comment explaining that. To handle cases like the one you describe I need the block analysis stuff in place. The actually outlining works the same way except that it gets a specific starting block and has different termination conditions. I have that working in my sandbox, but it seemed like a bit much to put into this review. |
702–703 ↗ | (On Diff #20617) | That's a good idea. I'll change it to that. |
708–737 ↗ | (On Diff #20617) | Agreed. |
Added comments clarifying the state of cleanup support.
Switched begin/end catch replacement to unreachable for cleanup cloning.