Skip to content
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

Crossgen2 support for static virtual method resolution (take 2) #87438

Merged
merged 21 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0e68a2f
WIP: Crossgen2 support for static virtual method resolution
trylek Jun 11, 2021
8c69fcf
Rebase against latest main; address parts of Michal's PR feedback
trylek Jun 12, 2023
8513e6b
Address additional Michal's PR feedback
trylek Jun 12, 2023
475c190
Revert changes to MetadataVirtualMethodAlgorithm per Michal's PR feed…
trylek Jun 14, 2023
9fee598
Fix module token resolution in the presence of SVM devirtualization
trylek Jun 15, 2023
8940fd1
Fix a few bugs found in TypeHierarchy tests
trylek Jun 23, 2023
af42831
Fix two Crossgen2 codegen bugs found via SVM testing
trylek Jun 29, 2023
a6785a4
Fix bugs, address PR feedback
trylek Jul 10, 2023
bc60009
Remove instrumentation; fix instantiation stub flag in getCallInfo
trylek Jul 12, 2023
c75a1b0
Revert change to Argiterator that turned out to be incorrect
trylek Jul 16, 2023
c1d0603
Subtle fixes for SVM handling in ceeInfoGetCallInfo
trylek Jul 16, 2023
0916bfc
Fix delegate ctor and module token construction for SVMs
trylek Jul 17, 2023
168908f
Remove instrumentation and more fixes for special SVM cases
trylek Jul 18, 2023
ad44935
Fix assertion failure seen in readytorun/tests/generic tests
trylek Jul 18, 2023
5bad02e
Rebase against main; remove unused resource
trylek Jul 19, 2023
5ddc43c
Address Michal's PR feedback towards MethodImpl body lookup
trylek Jul 20, 2023
f5f530c
Fix typo and merge error
trylek Jul 20, 2023
56754ea
Add versioning resiliency test as suggested by DavidWr
trylek Jul 20, 2023
8d642c6
Remove superfluous blank lines
trylek Jul 20, 2023
26dd5cf
Fix composite issues by introducing a new token kind
trylek Jul 21, 2023
1374c32
Address David Wrighton's PR feedback
trylek Jul 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix two Crossgen2 codegen bugs found via SVM testing
1. Delegate cctor helper JIT interface method was missing support
for type constraint.

2. Methods wrapped in instantiation / unboxing stubs shouldn't
claim they have a generic dictionary slot in their GC refmap.

Thanks

Tomas
  • Loading branch information
trylek committed Aug 8, 2023
commit af42831e2ae3bfb302b7dbebe25bdf1bf760698a
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
}
else
{
bool isUnboxingStub = false;
if (methodNode is DelayLoadHelperImport methodImport)
bool isStub = false;
if (methodNode is Import methodImport && methodImport.Signature is MethodFixupSignature methodSignature)
{
isUnboxingStub = ((MethodFixupSignature)methodImport.ImportSignature.Target).IsUnboxingStub;
isStub = methodSignature.IsUnboxingStub || methodSignature.IsInstantiatingStub;
}
builder.GetCallRefMap(methodNode.Method, isUnboxingStub);
builder.GetCallRefMap(methodNode.Method, isStub);
}
if (methodIndex >= nextMethodIndex)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public class Import : EmbeddedObjectNode, ISymbolDefinitionNode, ISortableSymbol

internal readonly MethodDesc CallingMethod;

public Signature Signature => ImportSignature.Target;

public Import(ImportSectionNode tableNode, Signature importSignature, MethodDesc callingMethod = null)
{
Table = tableNode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,23 @@ private void getReadyToRunDelegateCtorHelper(ref CORINFO_RESOLVED_TOKEN pTargetM
TypeDesc delegateTypeDesc = HandleToObject(delegateType);
MethodDesc targetMethodDesc = HandleToObject(pTargetMethod.hMethod);
Debug.Assert(!targetMethodDesc.IsUnboxingThunk());
MethodWithToken targetMethod = new MethodWithToken(targetMethodDesc, HandleToModuleToken(ref pTargetMethod), constrainedType: null, unboxing: false, context: entityFromContext(pTargetMethod.tokenContext));

var typeOrMethodContext = (pTargetMethod.tokenContext == contextFromMethodBeingCompiled()) ?
MethodBeingCompiled : HandleToObject((void*)pTargetMethod.tokenContext);

TypeDesc constrainedType = null;
if (targetConstraint != 0)
{
MethodIL methodIL = _compilation.GetMethodIL(MethodBeingCompiled);
constrainedType = (TypeDesc)ResolveTokenInScope(methodIL, typeOrMethodContext, targetConstraint);
}

MethodWithToken targetMethod = new MethodWithToken(
targetMethodDesc,
HandleToModuleToken(ref pTargetMethod),
constrainedType: constrainedType,
unboxing: false,
context: typeOrMethodContext);

pLookup.lookupKind.needsRuntimeLookup = false;
pLookup.constLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.DelegateCtor(delegateTypeDesc, targetMethod));
Expand Down Expand Up @@ -2179,7 +2195,9 @@ private void ceeInfoGetCallInfo(
// class and not requesting the instantiating stub makes the runtime transform the
// owning type to its canonical equivalent that would need different codegen
// (supplying the instantiation argument).
if (!resolvedConstraint && !originalMethod.IsSharedByGenericInstantiations)
if (!resolvedConstraint &&
!originalMethod.IsSharedByGenericInstantiations &&
targetMethod.IsSharedByGenericInstantiations)
{
useInstantiatingStub = true;
}
Expand Down