[PATCH] Fix inlining to not produce duplicate landingpad clauses

[Mark Seaborn]

On 3 December 2013 15:25, Chandler Carruth <chandlerc at google.com> wrote:

> Mind using phabricator? Makes the review much faster.
>

OK.  Posted as http://llvm-reviews.chandlerc.com/D2342 (which has showed up
as a separate thread,
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131202/197796.html
).



> I'd rather someone more familiar with exception handling in LLVM review
> this. I worry about it mostly because there were explicit tests for the
> duplication that you've had to remove.
>

Those tests look like they were copy-and-pasted from the output of Clang
and 'opt' without being checked closely -- hence use of mangled names such
as "_ZTIi" (rather than more readable names that you might write if writing
the LLVM assembly by hand, as I did for the test case in this patch).

Having the same "catch <foo>" clause twice in the same landingpad is
definitely unnecessary.  When the personality function is checking whether
to pass control to a landingpad, it checks each clause against the
exception in order.  If the first "catch <foo>" clause matches the
exception, the second "catch <foo>" clause won't be looked at, and if the
first doesn't match the second won't match either.  So the second "catch
<foo>" will have no effect.



> While in the abstract your description makes perfect sense and I have no
> idea why those were there, I also don't know the EH side of LLVM very well
> at all. Bill is really the perfect reviewer here I suspect. =]
>

OK.  I CC'd Bill originally but he hasn't responded.

Cheers,
Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131205/32758fa4/attachment.html>