Test failure Loader\\classloader\\TypeInitialization\\CctorsWithSideEffects\\CctorForWrite\\CctorForWrite.cmd #84007
Assignees
Labels
arch-arm64
arch-x64
arch-x86
area-crossgen2-coreclr
blocking-outerloop
Blocking the 'runtime-coreclr outerloop' and 'runtime-libraries-coreclr outerloop' runs
os-linux
Linux OS (any supported distro)
os-mac-os-x
macOS aka OSX
os-windows
Milestone
Comments
v-wenyuxu
added
arch-arm64
arch-x86
os-linux
|
@markples Any update on this? |
trylek
added a commit
to trylek/runtime
that referenced
this issue
Apr 3, 2023
|
I have seen that the IR coming out of the importer is incorrect but haven't debugged the importer itself to see how the code path is different for R2R to cause that. |
|
For reference, this test was created to show a fixed JIT bug for the code where the type initializer for ClassWithCctor was being run before Call was called. In the R2R case, the stsfld code in the JIT checks the static field and does not find CORINFO_FLG_FIELD_INITCLASS set. (Aside: This made me think maybe it was run at startup/early, but I have a callstack with the static field helper call on the stack and the type initialization exception being thrown.) |
|
|
|
If I'm right to interpret the name of the test so that it refers to the static constructor (.cctor), the last time I looked Crossgen2 didn't precompile any of these, I actually pointed that out several times in the past that we should as these are often involved in startup performance. |
ghost
added
the
in-pr
|
Thanks. I was specifically looking for not just precompiling but also executing (whether compiled or interpreted) since that would impact ordering. (and to be clear, that also doesn't seem to be the case, as it shouldn't since this one has side effects) The problem seems to be the JIT importer using CORINFO_FLG_FIELD_INITCLASS to mean "has a static constructor / type initializer" rather than "requires a separate call to initialize the type". The draft PR attached above addresses this but may not be the right overall fix. |
|
Original fix was in #80485 |
trylek
added a commit
that referenced
this issue
Apr 4, 2023
|
Failed again in: runtime-coreclr crossgen2 20230403.1 Failed test: Error message:
More failures
|
markples
added a commit
that referenced
this issue
Apr 21, 2023
Expand the fix in #80485 to cover helpers including common ones for R2R as well as some generic helpers for JIT. This would cause regressions (100ks code size in diffs) if done alone, so it also includes checks on whether a helper may trigger a cctor and whether it is beforefieldinit. The precise ordering of the fix is only required for a non-beforefieldinit ctor. This recovers the (small) losses from #80485 and mitigates this fix. As before, the primary diff is moving type initialization into (for correctness) or out of (the mitigation optimization) a JIT tree. These leads to secondary effects as values may need to be preserved around the type initialization code. The end result of the fix+mitigations is very little code size change, but viewing the diffs suggests possible future code improvements. However, the impact of any of these should be established first. - A type initializer will never invoke itself - When type initialization is removed/CSEed late, it is too late for forward subst/etc., so the loss of the intact tree can still impact code generation - A custom calling convention for the static helpers (tuned to the common case of no type initialization needing to occur) could help with the impact of calling it in the middle of a calculation - Code can use beforefieldinit in more place Fixes #84007
ghost
removed
the
in-pr
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Run: runtime-coreclr outerloop 20230327.2
Failed test:
Error message:
The text was updated successfully, but these errors were encountered: