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.

Sign up for GitHub

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

Jump to bottom

Revert "Handle getting the address of an RVA static correctly in a composite image (#55301)" #55713

Merged
merged 1 commit into from
Jul 15, 2021
Merged

Revert "Handle getting the address of an RVA static correctly in a composite image (#55301)" #55713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 4 additions & 6 deletions src/coreclr/jit/emitxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12226,12 +12226,10 @@ BYTE* emitter::emitOutputRR(BYTE* dst, instrDesc* id)
regMaskTP regMask;
regMask = genRegMask(reg1) | genRegMask(reg2);

// Assert disabled as requested in PR 55301. A use of the Unsafe api
// which produces correct code, but isn't handled correctly here.
// r1/r2 could have been a GCREF as GCREF + int=BYREF
// or BYREF+/-int=BYREF
// assert(((regMask & emitThisGCrefRegs) && (ins == INS_add)) ||
// ((regMask & emitThisByrefRegs) && (ins == INS_add || ins == INS_sub)));
// r1/r2 could have been a GCREF as GCREF + int=BYREF
// or BYREF+/-int=BYREF
assert(((regMask & emitThisGCrefRegs) && (ins == INS_add)) ||
((regMask & emitThisByrefRegs) && (ins == INS_add || ins == INS_sub)));
#endif
// Mark r1 as holding a byref
emitGCregLiveUpd(GCT_BYREF, reg1, dst);
Expand Down
27 changes: 3 additions & 24 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7704,24 +7704,14 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
void** pFldAddr = nullptr;
void* fldAddr = info.compCompHnd->getFieldAddress(pResolvedToken->hField, (void**)&pFldAddr);

// We should always be able to access this static's address directly unless this is a field RVA
// used within a composite image during R2R composite image building.
//
#ifdef FEATURE_READYTORUN_COMPILER
assert((pFldAddr == nullptr) ||
(pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_RVA_ADDRESS && opts.IsReadyToRun()));
#else
// We should always be able to access this static's address directly
//
assert(pFldAddr == nullptr);
#endif

FieldSeqNode* fldSeq = GetFieldSeqStore()->CreateSingleton(pResolvedToken->hField);

/* Create the data member node */
op1 = gtNewIconHandleNode(pFldAddr == nullptr ? (size_t)fldAddr : (size_t)pFldAddr,
#ifdef FEATURE_READYTORUN_COMPILER
pFldAddr != nullptr ? GTF_ICON_CONST_PTR :
#endif
GTF_ICON_STATIC_HDL,
op1 = gtNewIconHandleNode(pFldAddr == nullptr ? (size_t)fldAddr : (size_t)pFldAddr, GTF_ICON_STATIC_HDL,
fldSeq);
#ifdef DEBUG
op1->AsIntCon()->gtTargetHandle = op1->AsIntCon()->gtIconVal;
Expand All @@ -7731,17 +7721,6 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
{
op1->gtFlags |= GTF_ICON_INITCLASS;
}

#ifdef FEATURE_READYTORUN_COMPILER
if (pFldAddr != nullptr)
{
// Indirection used to get to initial actual field RVA when building a composite image
// where the actual field does not move from the original file
assert(!varTypeIsGC(lclTyp));
op1 = gtNewOperNode(GT_IND, TYP_I_IMPL, op1);
op1->gtFlags |= GTF_IND_INVARIANT | GTF_IND_NONFAULTING;
}
#endif
}
else // We need the value of a static field
{
Expand Down
15 changes: 0 additions & 15 deletions ...ools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,6 @@ private void CreateNodeCaches()
new ReadyToRunInstructionSetSupportSignature(key));
});

_precodeFieldAddressCache = new NodeCache<FieldDesc, ISymbolNode>(key =>
{
return new PrecodeHelperImport(
_codegenNodeFactory,
new FieldFixupSignature(ReadyToRunFixupKind.FieldAddress, key, _codegenNodeFactory)
);
});

_fieldAddressCache = new NodeCache<FieldDesc, ISymbolNode>(key =>
{
return new DelayLoadHelperImport(
Expand Down Expand Up @@ -406,13 +398,6 @@ public ISymbolNode FieldAddress(FieldDesc fieldDesc)
return _fieldAddressCache.GetOrAdd(fieldDesc);
}

private NodeCache<FieldDesc, ISymbolNode> _precodeFieldAddressCache;

public ISymbolNode PrecodeFieldAddress(FieldDesc fieldDesc)
{
return _precodeFieldAddressCache.GetOrAdd(fieldDesc);
}

private NodeCache<FieldDesc, ISymbolNode> _fieldOffsetCache;

public ISymbolNode FieldOffset(FieldDesc fieldDesc)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ protected override void ComputeDependencyNodeDependencies(List<DependencyNodeCor
}
}

public ISymbolNode GetFieldRvaData(FieldDesc field) => NodeFactory.CompilationModuleGroup.IsCompositeBuildMode ? SymbolNodeFactory.PrecodeFieldAddress(field) : NodeFactory.CopiedFieldRva(field);
public ISymbolNode GetFieldRvaData(FieldDesc field) => NodeFactory.CopiedFieldRva(field);

public override void Dispose()
{
Expand Down
15 changes: 0 additions & 15 deletions src/coreclr/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14034,21 +14034,6 @@ BOOL LoadDynamicInfoEntry(Module *currentModule,
break;
#endif // PROFILING_SUPPORTED

case ENCODE_FIELD_ADDRESS:
{
FieldDesc *pField = ZapSig::DecodeField(currentModule, pInfoModule, pBlob);

pField->GetEnclosingMethodTable()->CheckRestore();

// We can only take address of RVA field here as we don't handle scenarios where the static variable may move
// Also, this cannot be used with a ZapImage as it relies on a separate signatures block as an RVA static
// address may be unaligned which would interfere with the tagged pointer approach.
_ASSERTE(currentModule->IsReadyToRun());
_ASSERTE(pField->IsRVA());
result = (size_t)pField->GetStaticAddressHandle(NULL);
}
break;

case ENCODE_STATIC_FIELD_ADDRESS:
{
FieldDesc *pField = ZapSig::DecodeField(currentModule, pInfoModule, pBlob);
Expand Down