At least for CoreCLR, a catchpad which immediately executes an
unreachable instruction indicates that the exception can never have a
matching type, and so such catchpads can be removed, and so can their
catchswitches if the catchswitch becomes empty.
Details
Diff Detail
Event Timeline
Can we get a test with a cleanupret which unwinds to a catchswitch which has unreachable catchpads with and without it being unwinds to caller?
lib/IR/Instructions.cpp | ||
---|---|---|
938 | Please end this with a period. | |
942 | Ditto. | |
lib/Transforms/Utils/SimplifyCFG.cpp | ||
3523–3560 | Please use auto *II here. | |
3528 | Please use auto *CSI here. | |
3545 | I'd write this as CSI->getNumHandlers() == 0. | |
3546 | Couldn't some other predecessor cause Changed to be true? I think we could rephrase this as an assertion that CSI->getNumHandlers() != 0 before the loop. |
lib/Transforms/Utils/SimplifyCFG.cpp | ||
---|---|---|
3528 | Sure. I went ahead and changed the rest of the function to use auto in these situations, to keep it consistent. | |
3546 | I guess the intention of the assertion wasn't clear -- you seem to be interpreting it as "assert that we only eliminate a catchswitch if the code just above is what made it empty", but that's not what I was worried about (empty catchswitches fail the verifier anyway). This was just "We're about to change the IR, but we don't need Changed = true; here because we know Changed is already true if we've reached this spot". I'm happy to add an assert message, or put in both asserts, or just replace the assertion with Changed = true; for simplicity; do you have a preference? |
- Expand test to cover all pred opcodes
- Fix handling for cleanupret preds (need to rewrite to unwind-to-caller, not replace with unreachable)
lib/Transforms/Utils/SimplifyCFG.cpp | ||
---|---|---|
3546 | Yes, I wasn't sure what you are asserting here. I'll leave this up to your judgement. |
replace assertion with code that handles the hypothetical failing case, for readability
Please end this with a period.