[PATCH] D15405: [EHPersonality] Add a new personality enum to represent langindPad of token type

[Joseph Tremoulet via llvm-commits]

JosephTremoulet added a comment.

> I don't know if we should tie the token-valued landingpad to a specific runtime


I don't think of it as tying them together.  I think of it as your runtime/personality being the first, and currently only, runtime that uses token-valued landingpads.  Another personality that also wants token-valued landingpads could add its entry to the enum and update the `usesTokenLandingPads` method to include itself.

> see a token returning personality as "wrapping" another existing personality


Introducing a hierarchy here when we aren't for the other varying properties of personalities seems awkward to me.

> add a personality function to our downstream tree and then add an upstream personality function for testing purposes which really is "unknown but with token return"


I don't see a reason why that wouldn't work, but it seems like it could be a hassle to maintain your out-of-tree personality.
_

I think several of these comments are getting at the underlying pre-existing issue of "isn't having an enum of recognized personalities with string matching to identify them a scaling issue?".  I wouldn't mind having a long-term direction in mind for addressing that.  I haven't seen any feedback on the scheme I suggested on the dev alias:

> since we have llvm::Functions for the personality functions, maybe we could make them more self-describing via attributes?  We'd have to come up with the set of relevant properties that characterize personality functions (the predicates in Analysis/EHPersonalities would be a start, but we'd also have to look at other places testing/switching on the enum), and for each define an attribute that a front-end could either attach to the personality Function or not to indicate whether that property applies, and we could get rid of the string matching and hard-coded enum


I think that in reality we have few enough personalities that it's fine if we don't address the scaling issue right now, but then if we're not addressing it I'd prefer we stick to the current pattern which is to have a hard-coded list of recognized personalities, plus "Unknown".

If you don't like adding your runtime's personality to the enum, perhaps we should go back to Chen's original suggestion of just checking the type of the landingpad in SelectionDAG.  The reason we started going in this direction instead was "It seems a little backwards to check the landingpad's type as the first check, since the personality dictates the landingpad's type".  But maybe it makes sense to allow that much variation within Unknown, since it is after all a bucket for "all the other personalities" -- if we wanted to be pedantic, SelectionDAG could allow either token or struct for Unknown, but require struct for all the known personalities that use landingpads.


http://reviews.llvm.org/D15405