-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Test failure: FindServicePoint_ReturnedServicePointMatchesExpectedValues #56689
Comments
Tagging subscribers to this area: @dotnet/ncl Issue DetailsTest: System.Net.Tests.ServicePointManagerTest.FindServicePoint_ReturnedServicePointMatchesExpectedValues Failures 6/1-8/1 (incl. PRs):
Failure:
|
@dotnet/jit-contrib this looks like PGO run failure only + missing method ... is it a known problem? |
@jakobbotsch PTAL. |
@jakobbotsch I'll probably be looking at this some too, since this is the last major failure symptom in the libraries-pgo runs, so let's share notes/progress. |
Assuming this is codegen, so updated area label. |
I've been trying to repro this by running the arm64 System.Console.Tests with checked jit/runtime/spc and with the right COMPlus settings on my RPI4; no luck so far. May try switching to a different host machine. |
Also note this test DLL seems unable to be run via corerun, you need to swap parts into the test host and run that way, as some tests try to re-exec |
Ok, I can repro if I bog the machine down with |
I've isolated this down to something PGO does to Suspect it involves inlining so will try and pin that down next. |
Inline analysis indicates it's likely this chain of inlines (plus maybe some others)
|
I don't have a general-purpose inline XML reducer (yet). The method normally does 136 inlines, and if I cut off inlining at 104, the tests pass, and at 105 they fail. Inline 105 is So I may work on reducing the inline set further; I suspect there's a version that does a much smaller number of inlines that perhaps will have more tractable diffs. Other options are to try and debug the failure, try disabling other parts of the jit, find a simpler test case that also fails, or rely on some psychic debugging skills. I've tried the latter two and so far they haven't panned out. |
I managed to narrow it down by first noting that the failure required assertion prop and then by limiting the number of allowed assertions. This lead to a one-instruction diff in the generated code:
and the upshot it looks like there's another invalid peephole:
wondering why arm64's |
This seems to fix things, as best I can tell: diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp
index 771ce88ec84..11263bd361b 100644
--- a/src/coreclr/jit/emitarm64.cpp
+++ b/src/coreclr/jit/emitarm64.cpp
@@ -15581,17 +15581,19 @@ bool emitter::IsRedundantMov(instruction ins, emitAttr size, regNumber dst, regN
regNumber prevSrc = emitLastIns->idReg2();
insFormat lastInsfmt = emitLastIns->idInsFmt();
- if ((prevDst == dst) && (prevSrc == src))
+
+
+ // Sometimes emitLastIns can be a mov with single register e.g. "mov reg, #imm". So ensure to
+ // optimize formats that does vector-to-vector or scalar-to-scalar register movs.
+ bool isValidLastInsFormats = ((lastInsfmt == IF_DV_3C) || (lastInsfmt == IF_DR_2G) || (lastInsfmt == IF_DR_2E));
+
+ if (isValidLastInsFormats && (prevDst == dst) && (prevSrc == src))
{
assert(emitLastIns->idOpSize() == size);
JITDUMP("\n -- suppressing mov because previous instruction already moved from src to dst register.\n");
return true;
}
- // Sometimes emitLastIns can be a mov with single register e.g. "mov reg, #imm". So ensure to
- // optimize formats that does vector-to-vector or scalar-to-scalar register movs.
- bool isValidLastInsFormats = ((lastInsfmt == IF_DV_3C) || (lastInsfmt == IF_DR_2G) || (lastInsfmt == IF_DR_2E));
-
if ((prevDst == src) && (prevSrc == dst) && isValidLastInsFormats)
{
// For mov with EA_8BYTE, ensure src/dst are both scalar or both vector. Issue was seemingly there from the start, eg #38179. |
Otherwise we may mistake an immediate for a register and do an incorrect optimization. Fixes dotnet#56689.
) Otherwise we may mistake an immediate for a register and do an incorrect optimization. Fixes #56689.
Test: System.Net.Tests.ServicePointManagerTest.FindServicePoint_ReturnedServicePointMatchesExpectedValues
Failures 6/1-8/1 (incl. PRs):
Failure:
The text was updated successfully, but these errors were encountered: