Optimize __cxa_find_matching_catch to avoid stackAlloc. NFC #14440
Conversation
We can simply pass the adjusted pointer location as the inal argument to `__cxa_can_catch`. This function will set this location on success.
| var thrownType = info.get_type(); | ||
| var catchInfo = new CatchInfo(); | ||
| catchInfo.set_base_ptr(thrown); | ||
| catchInfo.set_adjusted_ptr(thrown); |
There was a problem hiding this comment.
How are catchInfo's base ptr and adjusted ptr different?
There was a problem hiding this comment.
I'm not sure exactly but this is features of libcxxabi. I think it relates to virtual inheritance.
| if (thrown !== adjusted) { | ||
| catchInfo.set_adjusted_ptr(adjusted); | ||
| } | ||
| if ({{{ exportedAsmFunc('___cxa_can_catch') }}}(caughtType, thrownType, catchInfo.get_adjusted_ptr_addr())) { |
There was a problem hiding this comment.
Previously, the adjusted ptr is set only when we can catch it, but now it is always set. Is this fine?
There was a problem hiding this comment.
that is still true.. __cxa_can_catch only writes to its third argument if it returns non-zero:
| var thrownType = info.get_type(); | ||
| var catchInfo = new CatchInfo(); | ||
| catchInfo.set_base_ptr(thrown); | ||
| catchInfo.set_adjusted_ptr(thrown); |
There was a problem hiding this comment.
I don't follow how this is NFC. This extra call will write a change to memory, and if we return on line 401 then it is noticeable, isn't it?
There was a problem hiding this comment.
Oh sorry, yes, this is setting the adjusted pointer in all cases, I forgot I ended up adding this line.
Basically __cxa_can_catch expects the existing value here to be the base_ptr and then it updates it accordingly.
The get_exception_ptr method will always prefer the adjusted pointer and fall back to the base ptr if its zero. I think in practice this now means that the fallback is not needed, but I also don't think there is any observable effect. The adjusted pointer will be equal the base pointer in the case that no adjustment was made.
There was a problem hiding this comment.
In modifying this code I'm fairly sure we have enough test coverage of this adjustment in our test suite, so if this was not NFC I think we would have seen a test failure.
There was a problem hiding this comment.
I see. Seems likely to be correct, yes.
We can simply pass the adjusted pointer location as the final argument to
__cxa_can_catch. This function will set this location on success.