This is an archive of the discontinued LLVM Phabricator instance.

[SimplifyCFG] simplifyUnreachable(): erase instructions iff they are guaranteed to transfer execution to unreachable
ClosedPublic

Authored by lebedev.ri on Jul 2 2021, 1:39 PM.

Details

Summary

This replaces the current ad-hoc implementation,
by syncing the code from InstCombine's implementation in InstCombinerImpl::visitUnreachableInst(),
with one exception that here in SimplifyCFG we are allowed to remove EH instructions.

Effectively, this now allows SimplifyCFG to remove calls (iff they won't throw and will return),
arithmetic/logic operations, etc.

Diff Detail

Event Timeline

lebedev.ri created this revision.Jul 2 2021, 1:39 PM
lebedev.ri requested review of this revision.Jul 2 2021, 1:39 PM
lebedev.ri added inline comments.Jul 2 2021, 1:41 PM
llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll
86–87

What happens here is that call void @llvm.stackrestore() ends up being located
right before unreachable, and we now happily erase it.
I think that makes sense, because resume would cause us to "return" from the function,
freeing all the stack anyways.

lebedev.ri edited the summary of this revision. (Show Details)Jul 2 2021, 1:47 PM
efriedma added inline comments.Jul 2 2021, 2:26 PM
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
4685

Invoke or catchswitch.

nikic added inline comments.Jul 2 2021, 2:54 PM
llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll
86–87

This test is probably supposed to have a cleanup on the landingpad.

Speaking of, can someone point me to the relevant wording in LangRef/ExceptionHandling on why resuming from a non-cleanup landingpad is UB? At least that's how I understand the DwarfEHPrepare code.

efriedma added inline comments.Jul 2 2021, 3:24 PM
llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll
86–87

I'm not sure it's really justified anywhere.

In practice, I think it works out for clang because we only use "resume" for cleanups; to rethrow an exception, we use __cxa_rethrow().

lebedev.ri marked an inline comment as done.

Tune comment.

lebedev.ri marked 2 inline comments as done.

Fix test instead of showing how it changes.

nikic accepted this revision.Jul 3 2021, 12:35 AM

LGTM

This revision is now accepted and ready to land.Jul 3 2021, 12:35 AM

Thank you for the review.