Skip to content

Commit

Permalink
Revert "Handle getting the address of an RVA static correctly in a co…
Browse files Browse the repository at this point in the history 
…mposite image (#55301)" (#55713)

This reverts commit e3d319b.
It breaks the crossgen2 outerloop runs
  • Loading branch information
@davidwrighton
davidwrighton authored Jul 15, 2021
1 parent 30257e6 commit 87fcf6e
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 61 deletions.
10 changes: 4 additions & 6 deletions src/coreclr/jit/emitxarch.cpp
View file
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
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
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
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
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

0 comments on commit 87fcf6e

Please sign in to comment.