[PATCH] D14344: [WinEH] Set ExceptionPointerRegister for CoreCLR

[Joseph Tremoulet via llvm-commits]

JosephTremoulet added a comment.

In http://reviews.llvm.org/D14344#281435, @rnk wrote:

> Maybe getExceptionPointerRegister() should be virtual and accept a personality function instead. I'm imagining a situation where CoreCLR personality functions are in a module with SEH personality functions, and this change as-is won't work.
>
> You can avoid updating all targets by leaving behind the default implementation that returns the ExceptionPointerRegister field for targets that don't have split EH personality disorder. :)


I agree that conditionalizing this on the personality sounds more correct than doing it on the environment as I have it.

It feels funny to leave around setExceptionPointerRegister if it's sometimes effectively ignored.  Grepping the in-tree targets, I just see 10 callers, 8 of which set to a constant value and 2 of which are parameterized on a property available on the TargetLowering, so updating those wouldn't seem to be much of a struggle (and likewise for ExceptionSelectorRegister).
Does that mean the Right Thing(TM) is to remove the backing field and update all the in-tree targets to encode the information in their override of the new virtual method instead, or is it better to just back off and leave the backing-field-based default implementation as you suggested, for the sake of out-of-tree target maintainers?


http://reviews.llvm.org/D14344