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

[Chen Li via llvm-commits]

chenli added a comment.

In http://reviews.llvm.org/D15405#307177, @JosephTremoulet wrote:

> We want the new personality to generally be treated like Unknown, correct?  You should check for places in the code that look for EHPersonality::Unknown and make sure they agree for Token_LP.  From a quick look, I think you'd want to update `isNoOpWithoutInvoke` in EHPersonalities.h and `isCatchAll` in InstructionCombining.cpp.


Well, I don't think I want the new personality to be treated exactly like Unknown. I've looked at isNoOpWithoutInvoke when creating the patch, and I think in that case it should be treated as other known personalities and be safely removed (Unknown might have side effects that prevent the removal but we know this new personality doesn't have any, at least for now). I think I've missed 'isCatchAll', and I will update there.


================
Comment at: include/llvm/Analysis/EHPersonalities.h:29
@@ -29,1 +28,3 @@
+  CoreCLR,
+  Token_LP
 };
----------------
JosephTremoulet wrote:
> Since this one isn't actually some runtime's personality function, I think a comment explaining it would be helpful.  Something along the lines of "similar to Unknown, except that LandingPads have token type", maybe?
Will do.

================
Comment at: lib/Analysis/EHPersonalities.cpp:32
@@ -31,2 +31,3 @@
     .Case("ProcessCLRException",   EHPersonality::CoreCLR)
+    .Case("ProcessTokenLandingPad",EHPersonality::Token_LP)
     .Default(EHPersonality::Unknown);
----------------
JosephTremoulet wrote:
> nit: the name "ProcessTokenLandingPad" seems funny to me.  I presume you were modeling it after the line above?  That's the name of the method in the CLR runtime, and is so named because it processes an exception, but "process" sounds funny to me in your case since the landingpad isn't being processed.  Maybe something like "Token_LP_Personality"?
Thanks for the correction. I will modify to a proper name here.


http://reviews.llvm.org/D15405