Improve jit tailcall decision reporting #26149
Conversation
On platforms with unsupported helper, report the reason why we did not do a fast tailcall instead of the fact that no copy args thunk is available.
jakobbotsch
changed the title
|
@jakobbotsch please confirm this change has no diffs when possible. |
Yep, 0 diffs over {unix x64, windows x64, windows x86} x {crossgen, pmi}. |
|
Also cc @dotnet/jit-contrib. |
|
@jakobbotsch can you fix the conflicts? We can merge after that is done. |
…ecision-reporting
* Update dependencies from https://github.com/dotnet/core-setup build 20190904.1 - Microsoft.NETCore.App - 5.0.0-alpha1.19454.1
…ecision-reporting
|
@BruceForstall @jashook PTAL again, the parts in |
|
/azp run coreclr-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
It might be useful to declare an enum as for |
|
I think the main difference between inlining and tailcall decisions is that the inlining observations are also used to guide the decisions while for tailcalls the string is purely for logging. I don't know if an enum would add much due to that. However it could certainly be useful in the future if/when we decide to do more profitability checks for fast tailcalls. |
|
@jakobbotsch - I wasn't so much interested in how the JIT might use the decisions, but how the JIT might expose them in event logging. I'm not sure at this point how useful that might be, and probably something that can be considered in future. |
|
If I'm not wrong I think even the inlining decisions are reported as strings in event logging, not as enum values. The enum values are only used internally I believe. |
|
Thanks; I didn't realize they were even being reported in event logging. It seems to me that we might want to be more concise in event logs than using strings, but in any event I don't think it's something to hold this up over (and I didn't intend to; it was just a passing thought) |
|
We have used strings for inline reporting forever, and they have some very nice advantages -- minor changes to the string text, or adding or removing new types is simple. If we used enums we'd need to make changes in the runtime each time we did this, and it would create versioning issues for diagnostic tools, as the meaning of enums might now be jit or runtime version-dependent. My only real complaint on the inlining side is that failure reasons generated by the runtime don't get propagated back to the jit, so we just get a generic "VM said no" failure instead of something more specific. |
|
The failing test also seems to be failing in other outerloop runs. This should be good to go. |
|
Correct it is #26013 |
On platforms with unsupported helper, report the reason why we did not
do a fast tailcall instead of just the general reason that we couldn't.
On Windows we might also consider both reporting a fail decision and a success reason when we end up doing a slow tailcall, so that we record the reason a fast tailcall wasn't done. There is also a "reason" field in the success decision that we could use instead. Thoughts?
cc @jashook