-
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
Fewer AddressExposedReason::TOO_CONSERVATIVE #59429
Conversation
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsA few improvements here and there, no actual regressions. The good benefit is that we can tail-call more methods and it should affect the performance better than just the size decrease.
Enreg stat: Locals enregistration statistics:
-total number of locals: 346731, number of enregistered: 309775, notEnreg: 36956, ratio: 0.89
+total number of locals: 346731, number of enregistered: 309776, notEnreg: 36955, ratio: 0.89
total number of struct locals: 10920, number of enregistered: 86, notEnreg: 10834, ratio: 0.01
-total number of primitive locals: 335811, number of enregistered: 309689, notEnreg: 26122, ratio: 0.92
+total number of primitive locals: 335811, number of enregistered: 309690, notEnreg: 26121, ratio: 0.92
-m_addrExposed 16176, ratio: 0.44
+m_addrExposed 14059, ratio: 0.38
m_notRegSizeStruct 403, ratio: 0.01
m_localField 3219, ratio: 0.09
+m_VMNeedsStackAddr 2116, ratio: 0.06
m_liveInOutHndlr 642, ratio: 0.02
m_blockOp 449, ratio: 0.01
m_structArg 26, ratio: 0.00
m_depField 952, ratio: 0.03
m_noRegVars 14819, ratio: 0.40
m_castTakesAddr 12, ratio: 0.00
m_swizzleArg 171, ratio: 0.00
m_blockOpRet 87, ratio: 0.00
Addr exposed details:
-m_parentExposed 5123, ratio: 0.32
+m_parentExposed 5123, ratio: 0.36
-m_tooConservative 3434, ratio: 0.21
+m_tooConservative 235, ratio: 0.02
-m_escapeAddress 7384, ratio: 0.46
+m_escapeAddress 8466, ratio: 0.60
m_wideIndir 7, ratio: 0.00
-m_copyFldByFld 228, ratio: 0.01
+m_copyFldByFld 228, ratio: 0.02 Diff example: Before, tail call is blocked because JIT thinks there is an address exposed var:
After, do tailcall.
|
@@ -4348,6 +4347,7 @@ void Compiler::lvaMarkLocalVars() | |||
lvaPSPSym = lvaGrabTempWithImplicitUse(false DEBUGARG("PSPSym")); | |||
LclVarDsc* lclPSPSym = &lvaTable[lvaPSPSym]; | |||
lclPSPSym->lvType = TYP_I_IMPL; | |||
lvaSetVarDoNotEnregister(lvaPSPSym DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now this is the main reason for VMNeedsStackAddr
but if I understand correctly we actually report this to VM so we can't do anything about it.
There is an x86 test failure related to varargs it seems, is it unrelated? |
looks related, there are no asm diffs for this method but maybe we tell VM something differently. I will check. |
The failure was fixed and I have improved the dump a little. |
Failures are #59541 |
A few improvements here and there, no actual regressions. The good benefit is that we can tail-call more methods and it should affect the performance better than just the size decrease.
Enreg stat:
Diff example:
Before, tail call is blocked because JIT thinks there is an address exposed var:
After, do tailcall.