-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RyuJIT] TailcallStress needs to check more conditions #8017
Comments
A tail call will implicitly discard the stack, so I am not sure how this can be a viable candidate. PS: Also this is just a tail call, not a tail call optimization. This will in fact run slower than a normal direct call in the current JIT. TCO is an ambiguous term that should not be used. |
@leppie yes, the JIT bug is that in this particular stress mode, the JIT does not do enough legality checking, therefore makes an illegal tail call. |
(Internal note: this came from VSO#278840) |
If/when fixed, enable the test added with dotnet/coreclr#11410 |
Tail call stress does not mix well with ZapDisable due to the issues described in #11408. Fixes #11648.
* Disable tail call stress in GH_10780 if ZapDisable is enabled. Tail call stress does not mix well with ZapDisable due to the issues described in #11408. Fixes #11648. * Add a missing semicolon.
Update fgMorphImplicitByRefArgs to rewrite references to struct-promoted implicit-by-reference parameters as references to the new struct temps created in fgRetypeImplicitByRefArgs; fgMorphStructField isn't updating these because it's only interested in field references, and runs upstream of fgRetypeImplicitByRefArgs where the full struct temp is created, anyway. Invert the sense of lvPromoted for implicit byref args in the interim between fgRetypeImplicitByRefArgs and fgMarkDemotedImplicitByRefArgs, since now fgMarkDemotedImplicitByRefArgs needs to update both and would look horribly backwards the other way around. Fixes #11408.
Tail call stress does not mix well with ZapDisable due to the issues described in #11408.
…if ZapDisable is enabled. Tail call stress does not mix well with ZapDisable due to the issues described in #11408.
Fixes #13796 Note that test GitHub_11408 is still disabled, now against issue #11408.
Fixes #13796, #12549 Note that test GitHub_11408 is still disabled, now against issue #11408.
Fixes #13796, #12549 Note that test GitHub_11408 is still disabled, now against issue #11408.
@jakobbotsch @jashook fyi |
IMO it does not make sense to spend time on trying to make tailcall stress work -- instead ABI stress should be expanded to cover the missing cases (generating tests that are correct by construction). |
Improve validation of tail calls that are not tail-prefixed in the IL but are marked as such because of TailCallStress. We now do the same correctness validation in morph for such tail calls as we do for implicit tail calls. That blocks tail calls when we have address-taken locals, struct promoted params, and pinned vars. Fixes dotnet#39398. Fixes dotnet#39309. Fixes dotnet#38892. Fixes dotnet#38889. Fixes dotnet#38887. Fixes dotnet#37117. Fixes dotnet#8017.
Improve validation of tail calls that are not tail-prefixed in the IL but are marked as such because of TailCallStress. We now do the same correctness validation in morph for such tail calls as we do for implicit tail calls. That blocks tail calls when we have address-taken locals, struct promoted params, and pinned vars. Fixes #39398. Fixes #39309. Fixes #38892. Fixes #38889. Fixes #38887. Fixes #37117. Fixes #8017.
Improve validation of tail calls that are not tail-prefixed in the IL but are marked as such because of TailCallStress. We now do the same correctness validation in morph for such tail calls as we do for implicit tail calls. That blocks tail calls when we have address-taken locals, struct promoted params, and pinned vars. Fixes dotnet/runtime#39398. Fixes dotnet/runtime#39309. Fixes dotnet/runtime#38892. Fixes dotnet/runtime#38889. Fixes dotnet/runtime#38887. Fixes dotnet/runtime#37117. Fixes dotnet/runtime#8017. Commit migrated from dotnet/runtime@7742b57
When
COMPlus_TailcallStress=1
is set, the JIT converts as many call/ret pairs as possible into tail call, which it treats astail.
prefixed "explicit" tail calls. It does not in this case run the legality checks that are run when checking to determine if implicit tailcall optimization can be done, specifically the code underFEATURE_TAILCALL_OPT
in fgMorphCall() that rejects a potential tail call if any variable in the function has had its address taken, if there are pinned locals, or if there are struct promoted struct arguments.This causes code such as this to fail:
category:correctness
theme:tail-call
skill-level:intermediate
cost:small
The text was updated successfully, but these errors were encountered: