| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
Change #53567 introduced new functionality to the JIT-EE
resolveVirtualMethod() API. This fixes the SuperPMI implementation
in the case where the API returns `false`, where some data fields
are not initialized.
Fixes crossgen2 SuperPMI collections.
Fixes #54310
|
|
Address deficiencies in current devirtualization infrastructure
- Remove the responsibility of creating a CORINFO_RESOLVED_TOKEN structure from the JIT and make it a responsibility of the VM side of the jit interface.
- This enables the component (crossgen2) which has deeper understanding of the requirements here to correctly handle scenarios that would otherwise require expressing crossgen2 specific details across the jit interface.
- Add a new set of fixups (`READYTORUN_FIXUP_Check_VirtualFunctionOverride` and `READYTORUN_FIXUP_Verify_VirtualFunctionOverride`) these are used to validate that the behavior of the runtime and crossgen2 compiler is equivalent for a virtual resolution event
- `READYTORUN_FIXUP_Check_VirtualFunctionOverride` will ensure that the virtual resolution decision is the same at crossgen2 time and runtime, and if the decision differs, any generated code affected by the decision will not be used.
- `READYTORUN_FIXUP_Verify_VirtualFunctionOverride` will perform the same checks as `READYTORUN_FIXUP_Check_VirtualFunctionOverride`, but if it fails the check, the process will be terminated with a fail-fast. It is intended for use under the `--verify-type-and-field-layout` stress mode.
- Currently only the `READYTORUN_FIXUP_Verify_VirtualFunctionOverride` is actually generated, and it is only generated when using the `--verify-type-and-field-layout` switch to crossgen2. Future work will identify if there are scenarios where we need to generate the `READYTORUN_FIXUP_Check_VirtualFunctionOverride` flag. One area of possible concern is around covariant returns, another is around handling of type equivalence.
- In order to express the fixup signature for the VirtualFunctionOverride fixups, a new flag has been added to `ReadyToRunMethodSigFlags`. `READYTORUN_METHOD_SIG_UpdateContext` will allow the method signature to internally specify the assembly which is associated with the method token, instead of relying on the ambient context.
- R2RDump and the ReadyToRun format documentation have been updated with the details of the new fixups/flags.
- Update the rules for handling unboxing stubs
- See #51918 for details. This adds a new test, as well as proper handling for unboxing stubs to match the JIT behavior
- Also revert #52605, which avoided the problem by simply disabling devirtualization in the presence of structs
- Adjust the rules for when it is legal to devirtualize and maintain version resiliency
- The VersionsWithCode and VersionsWithType rules are unnecessarily restrictive.
- Instead Validate that the metadata is safely checkable, and rely on the canInline logic to ensure that no IL that can't be handled is inlined.
- This also involved adding a check that the chain of types from the implementation type to the declaration method table type is within the version bubble.
- And changing the `VersionsWithType` check on the implementation type, to a new `VersionsWithTypeReference` check which can be used to validate that the type can be referred to, in combination with using `VersionsWithType` on the type definition.
- By adjusting the way that the declMethod is referred to, it becomes possible to use the declMethod without checking the full method is `VersionsWithCode`, and it just needs a relationship to version matching code.
- In `resolveVirtualMethod` generate the `CORINFO_RESOLVED_TOKEN` structures for the jit
- In particular we are now able to resolve to methods where the decl method is the resolution result but is not within the version bubble itself. This can happen if we can prove that the decl method is the only method which can possibly implement a virtual.
- Add support for devirtualization reasons to crossgen2
- Port all devirtualization abort conditions to crossgen2 from runtime that were not already present
- Fix devirtualization from a canonical virtual method when the actual implementation is more exact
- Fix variant interface override scenario where there is an interface that requires implementation of the variant interface as well as the variant interface itself.
|
|
* Remove crossgen SPMI collection
* Remove reference of crossgen from superpmi.py
* some misc crossgen references
* put back deleted things
|
|
* Fix gcc armel build
Mostly signed/unsigned comparisons, etc.
* Fix gcc arm64 build
|
|
|
|
* Fix `SetShimDebugVariables` for counter and simple.
* fix dumps for replays
|
|
* Fix SPMI dump.
* more fixes
|
|
* Fix SuperPMI collect with getIntConfigValue
PR #52427 introduced a per-compilation call to getIntConfigValue
on "SuperPMIMethodContextNumber". This pseudo-config should not
be collected. It exposed a pre-existing multi-threading issue in
the superpmi collector shim. I got rid of the per-compilation
override of the jithost MethodContext, which is problematic, and
currently unneeded, but documented the considerations around collecting
per-compilation data.
Fixes #53440
* Update src/coreclr/ToolBox/superpmi/superpmi-shim-collector/jithost.cpp
Co-authored-by: Kunal Pathak <Kunal.Pathak@microsoft.com>
Co-authored-by: Kunal Pathak <Kunal.Pathak@microsoft.com>
|
|
Several changes to help better diagnose PGO and devirtualization issues:
* Report the source of the PGO data to the jit
* Report the reason for a devirtualization failure to the jit
* Add checking mode that compares result of devirtualization to class profile
* Add reporting mode to assess overall rates of devirtualization failure
when the jit has exact type information.
Also fix a loophole where in some case we'd still devirtualize if not
optimizing.
Note crossgen2 does not yet set devirtualization failure reasons.
|
|
|
|
1. Print the Unicode pattern properly
2. Return an error code
|
|
* Modify JIT to support double mapped memory
This change modifies JIT so that it can generate code into
double mapped memory. The code is written into RW mapped memory,
but the relative offsets are computed using the related RX locations.
The change doesn't modify the runtime to provide double mapped memory
yet, the JIT2EE interface allocMem returns the same addresses as RW and
RX. The runtime changes will be part of follow-up PRs. However, it was
already tested with the double mapping enabled locally.
It also changes signature of allocMem to pass everything in a single structure
|
|
|
|
I've been trying to correlate methods in SPMI collections with methods
seen in profile data (say perfview). But for generic classes or generic
methods this is not easy.
To make it more feasible, in the "extra query" mode, get the names of
all the generic parameters, and then add these to the class name in MCS's
dumpMap output.
Also dump out the jit flags, so it's easier to find a particular instance
if the method was jitted more than once.
|
|
* Improve SuperPMI missing data asserts
SuperPMI has many asserts for missing data that are specially handled
to return a "missing data" error code. We typically ignore these, except
to note that a new collection might be needed. In particular, we prefer a
"missing data" error to an unidentified (caught) crash, which might indicate a JIT crash.
A recent change exposed a case where we were missing an assert, leading
to a crash. I went through all the methodcontext.cpp "rep" functions and added
appropriate asserts to all cases where they were missing.
To do this, add new `AssertMapExists`, `AssertKeyExists`, and `AssertMapAndKeyExist`
macros to make this consistent, easy, and compact, and converted all existing map and key
asserts to use these new forms.
In addition,
1. Rewrite the code to be much more regular and consistent.
2. Add many missing `DEBUG_REC` and `DEBUG_REP` cases, and fix some that were wrong.
3. Create a `key` variable almost everywhere, to avoid repeated `CastHandle` calls.
4. Simplify/commonize the `ZeroMemory` calls.
5. Remove all BOOL; use bool as appropriate.
There are no asm diffs (as expected). I verified the new asserts properly fire by using
`superpmi.py replay -jitoption JitStress=2`.
(Re-revert change, with fix)
* Fix clang compilation failures, more
clang is less permissive about empty variatic macros `__VA_ARGS__`,
so add `AssertMapExistsNoMessage`, `AssertKeyExistsNoMessage`, and
`AssertMapAndKeyExistNoMessage` for cases where we haven't bothered
to write a failure message, to avoid compiler problems. It's also more
clear in the code.
Change all the functions to explicity compute and use a `value` variable
for the key=>value map, to reduce duplicate casting, and make the code
more clear.
* clang build fix
|
|
Add COMPlus_JitCollect64BitCounts which makes the JIT instrument using
64-bit counts instead of 32-bit counts. No support for consuming these
counts is added, only support for producing them.
I also changed the printing of relocs to include their values when
diffable disassembly is off.
|
|
This reverts commit 7512c89d0605f9ec99a580b4d6cc5ded39d2f907.
|
|
SuperPMI has many asserts for missing data that are specially handled
to return a "missing data" error code. We typically ignore these, except
to note that a new collection might be needed. In particular, we prefer a
"missing data" error to an unidentified (caught) crash, which might indicate a JIT crash.
A recent change exposed a case where we were missing an assert, leading
to a crash. I went through all the methodcontext.cpp "rep" functions and added
appropriate asserts to all cases where they were missing.
To do this, add new `AssertMapExists`, `AssertKeyExists`, and `AssertMapAndKeyExist`
macros to make this consistent, easy, and compact, and converted all existing map and key
asserts to use these new forms.
In addition,
1. Rewrite the code to be much more regular and consistent.
2. Add many missing `DEBUG_REC` and `DEBUG_REP` cases, and fix some that were wrong.
3. Create a `key` variable almost everywhere, to avoid repeated `CastHandle` calls.
4. Simplify/commonize the `ZeroMemory` calls.
5. Remove all BOOL; use bool as appropriate.
There are no asm diffs (as expected). I verified the new asserts properly fire by using
`superpmi.py replay -jitoption JitStress=2`.
|
|
The jit can see the SPMI method context number by querying the
jit host for `SuperPMIMethodContextNumber`. Update the way this
value is computed, so it gives the same answer when running SPMI
in parallel that it does when running serially.
|
|
Co-authored-by: David Wrighton <davidwr@microsoft.com>
|
|
Records of this type are created when class profile histograms in dynamic PGO
data are summarized by the static PGO tooling.
These records can appear in both prejit and jit schemas when the static PGO data is
passed back to the jit.
|
|
SPMI wasn't capturing enough data for PGO schemas that had class profiles,
leading to replay failures.
|
|
* Run SPMI collection on libraries-tests
* Only run for libraries-tests and change GUID
* fix error
* fix the collection name
* fix the timeoutInMinutes
* Download libraries test artifacts
* try to fix the libs+tests artifacts name
* Use artifacts name as is
* Fix the libraries test artifacts name
* Use librares_test.zip
* Modify the filter to unzip all *Tests.dll files
* Revert "Modify the filter to unzip all *Tests.dll files"
This reverts commit b62f7132ec084453495a92a3e336688585366908.
* Try to unzip libraries_zipped
* overwriteExistingFiles=true
* Fix the asset extension
- Also exclude files present in core_root
* fix artifacts name
* remove extra .
* Ignore permission error
* Copy common test files in CORE_ROOT
- And do not copy not *.Tests.dll" for PMI
* Perform collection for all libraries_tests assets
* Add UnicodeEncodeError EH
* Add PermissionError for copy2
* Copy all the test assests to core_root
* Make input_directory readonly
* make ch_mod recursive
* Add option of error_limit to superpmi.py
* Pass -failureLimit flag to parallel mode
* Revert "Only run for libraries-tests and change GUID"
This reverts commit 7f4cdda9cf4f49fc21a354f616a67554f65cd6ea.
|
|
- Move processing of likely class histogram into the JIT
- Fix cases where the devirtualization logic in crossgen2 behaved incorrectly in the presence of likely class histogram data
- Pre-process histogram at crossgen2 time to reduce the size of the pgo data
- This removes 3/4 of the data from System.Private.CoreLib
|
|
represents an intrinsic method (#51124)
* Updating the JIT/EE interface to expose `isJitIntrinsic`
* Update JITEEVersionIdentifier
* Update fgFindJumpTargets to use the much cheaper `eeIsJitIntrinsic`
* Ensure IsJitIntrinsic exists in the LightWeightMap and fix a copy/paste error
* Run ThunkGenerator
* Don't resolve tokens for CEE_CONSTRAINED when making inlining observations
|
|
|
|
* Limit superpmi replay failures
* Introduce errorCount2
* reword error message
* Introduce failureLimit commandline option
* Add error handling
|
|
system is untouched (#49906)
|
|
Adjust the error code determination for asm diffs to not report
diffs if all the compilation failures were due to missing data
in the MCH files.
Update the SuperPMI end status line to report the count of MC
compilation failures due to missing data (and adjust parallel
SuperPMI to handle it as well).
As a result, with `superpmi.py asmdiffs` you won't see the message
"Asm diffs" if all the failures were due to missing data.
|
|
When generating disasm for an SPMI replay, we can call
`eeGetMethodFullName` for callees for which we are missing data.
Fix `MethodContext::repGetArgNext` to raise an appropriate exception,
and not crash, when it encounters missing data.
|
|
SuperPMI logging was doing a lot of work even if no log output was
asked for. For example, there are LogDebug() calls in the recordRelocation
path that were doing all the printf work and memory allocation even though
we were not logging anything.
Add a fast-path return to avoid all this unnecessary work.
|
|
* COMPlus_JitDisablePGO will now block the jit from using profile data. Useful
for investigations and (someday) bug triage. Also disable GDV if we're disabling PGO
* `-base_jit_option` and `-diff_jit_option` can be used to pass options to
superpmy.py asmdiffs.
* Fix SPMI file name computation to better handle cases where the directory is
a relative path.
* Enhance `mcs -jitflags` output to describe how many method contexts have
PGO data, and which kinds of data they hold. Useful for validating that an
SPMI collection done via dynamic PGO has actually collected an interesting
set of jit requests.
|
|
|
|
* Fix SuperPMI on Linux for crossgen2
1. crossgen2 doesn't call DllMain (https://github.com/dotnet/runtime/issues/49525),
so add a call to initialize the PAL and other things in jitStartup() (the JIT does
it this way already). Note that crossgen2 doesn't call DllMain for shutdown, either,
so we won't properly shut down the logger. Normally, however, we don't use the logger
during collections.
1.1. I only changed the collector, not the other two shims (which aren't used).
2. We don't seem to be able to get the correct exception code for exceptions thrown
from crossgen2. So, as a workaround, instead of assuming we're going to succeed,
assume we're going to fail with a non-zero exception code, and then set it to zero
if no exception was thrown. This only applies to the `getCallInfo` API.
* Add crossgen2 collections for Linux platforms
* Clarify comment
|
|
This ensures that pgo and non-pgo machine contexts for the same method with
otherwise identical jit flags won't get merged, and likewise for pgo contexts
with different counter values.
|
|
* Do not pass --use-zapdisable to dotnet
But instead pass to individual CoreRun.exe that is launched during benchmark run. That way we do not slow down the launch of dotnet.exe process.
* Fail superpmi collection pipeline if there was no valid .mch files were found to merge
* Upload merged .mch files as zipped artifacts
TODO: Upload the superpmi.log from all the partition runs
* TEMP: New Jit guid
- Add new JIT guid so we don't overwrite the existing .mch files
- Disable all the runs except benchmarks
* add trailing path sep
* Upload superpmi.logs separately
* Use PublishPipelineArtifacts instead of upload artifacts
* properly pass zapdisable to CoreRun.exe
* Revert "TEMP: New Jit guid"
This reverts commit e89ef69625e4e57bb622fc35ff66d82f14b34285.
* Remove CoreClrTestBuildHost from benchmark run
* fix --envVars for benchmarks run
* add missing space
* temporary disable other runs
* Revert "temporary disable other runs"
This reverts commit 2926d1f582061eee6c7852046982ffb5107887b3.
|
|
|
|
|
|
|
|
* Add assembly name to SuperPMI collection
For each class, add JIT/EE calls to getAssemblyName so the SuperPMI
collections contain this information. This can be useful to try and track
down where a particular function in a large method context hive came
from.
Only do this when `COMPlus_EnableExtraSuperPmiQueries=1` is set.
* Formatting
|
|
Include `GetIndex` method calls in the assert message, to distinguish
the different assert cases in error output. E.g., the asserts
change from:
```
'index != -1' failed ...
```
(which was identical for several cases), to:
```
'GetClassAttribs->GetIndex(CastHandle(classHandle)) != -1' failed ...
```
This makes it easier to know where the problem is without debugging the failure.
|
|
Capture data in the filter; don't save a PEXCEPTION_POINTERS
that points to data that is is transient to the filter.
|
|
may be included without the PAL (#46055)
This was done via simple substitution of the standard sized integer types, and dealing with the fallout. It is tested by using the ICorJitInfo interface directly within the crossgen2 jitinterface thunk library.
Effort was made to use a minimum number of casts, as casts in such a large change are likely to be wrong somewhere.
- Exceptions are the use of casting to handle some calls to the existing internal metadata api and around use of char16_t.
In addition, a small amount of logic from the PAL headers were pulled into the proper header to allow compilation in the presence of some SAL annotation, and other small details.
|
|
|
|
Useful for determining what sorts of jit compilations are found in an SPMI
collection.
|
|
* SPMI hacks/fixes.
* review response.
|
|
MethodContext::repGetAddrOfCaptureThreadGlobal() if there is corresponding GetAddrOfCaptureThreadGlobal LWM (#48260)
|
|
Add a clause that maps result1 and result2 offsets to a common base and
then compares that result.
This fixes some noisy diffs from instrumented Tier0 compiles, where the
probe addresses can vary from run to run.
|
