Skip to content

Commit

Permalink
Fewer AddressExposedReason::TOO_CONSERVATIVE (#59429)
Browse files Browse the repository at this point in the history
Delete some too_conservative addrExposed reasons.
  • Loading branch information
sandreenko authored Sep 28, 2021
1 parent 580d17f commit 181da3b
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 35 deletions.
6 changes: 0 additions & 6 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7469,14 +7469,8 @@ void CodeGen::genFnProlog()

if (compiler->info.compPublishStubParam)
{
#if CPU_LOAD_STORE_ARCH
GetEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, REG_SECRET_STUB_PARAM,
compiler->lvaStubArgumentVar, 0);
#else
// mov [lvaStubArgumentVar], EAX
GetEmitter()->emitIns_AR_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, REG_SECRET_STUB_PARAM, genFramePointerReg(),
compiler->lvaTable[compiler->lvaStubArgumentVar].GetStackOffset());
#endif
assert(intRegState.rsCalleeRegArgMaskLiveIn & RBM_SECRET_STUB_PARAM);

// It's no longer live; clear it out so it can be used after this in the prolog
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4476,6 +4476,11 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
assert(lvaStubArgumentVar == BAD_VAR_NUM);
lvaStubArgumentVar = lvaGrabTempWithImplicitUse(false DEBUGARG("stub argument"));
lvaTable[lvaStubArgumentVar].lvType = TYP_I_IMPL;
// TODO-CQ: there is no need to mark it as doNotEnreg. There are no stores for this local
// before codegen so liveness and LSRA mark it as "liveIn" and always allocate a stack slot for it.
// However, it would be better to process it like other argument locals and keep it in
// a reg for the whole method without spilling to the stack when possible.
lvaSetVarDoNotEnregister(lvaStubArgumentVar DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr));
}
};
DoPhase(this, PHASE_PRE_IMPORT, preImportPhase);
Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1758,11 +1758,6 @@ inline unsigned Compiler::lvaGrabTempWithImplicitUse(bool shortLifetime DEBUGARG

LclVarDsc* varDsc = &lvaTable[lclNum];

// This will prevent it from being optimized away
// TODO-CQ: We shouldn't have to go as far as to declare these
// address-exposed -- DoNotEnregister should suffice?
lvaSetVarAddrExposed(lclNum DEBUGARG(AddressExposedReason::TOO_CONSERVATIVE));

// Note the implicit use
varDsc->lvImplicitlyReferenced = 1;

Expand Down
7 changes: 5 additions & 2 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1404,8 +1404,11 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
if (!getNeedsGSSecurityCookie() && InlineeCompiler->getNeedsGSSecurityCookie())
{
setNeedsGSSecurityCookie();
const unsigned dummy = lvaGrabTempWithImplicitUse(false DEBUGARG("GSCookie dummy for inlinee"));
lvaTable[dummy].lvType = TYP_INT;
const unsigned dummy = lvaGrabTempWithImplicitUse(false DEBUGARG("GSCookie dummy for inlinee"));
LclVarDsc* gsCookieDummy = lvaGetDesc(dummy);
gsCookieDummy->lvType = TYP_INT;
gsCookieDummy->lvIsTemp = true; // It is not alive at all, set the flag to prevent zero-init.
lvaSetVarDoNotEnregister(dummy DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr));
}

// If there is non-NULL return, replace the GT_CALL with its return value expression,
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2757,6 +2757,9 @@ void Compiler::fgAddInternal()

lvaInlinedPInvokeFrameVar = lvaGrabTempWithImplicitUse(false DEBUGARG("Pinvoke FrameVar"));

// Lowering::InsertPInvokeMethodProlog will create a call with this local addr as an argument.
lvaSetVarAddrExposed(lvaInlinedPInvokeFrameVar DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS));

LclVarDsc* varDsc = &lvaTable[lvaInlinedPInvokeFrameVar];
varDsc->lvType = TYP_BLK;
// Make room for the inlined frame.
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12206,7 +12206,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
/* The ARGLIST cookie is a hidden 'last' parameter, we have already
adjusted the arg count cos this is like fetching the last param */
assertImp(0 < numArgs);
assert(lvaTable[lvaVarargsHandleArg].IsAddressExposed());
lclNum = lvaVarargsHandleArg;
op1 = gtNewLclvNode(lclNum, TYP_I_IMPL DEBUGARG(opcodeOffs + sz + 1));
op1 = gtNewOperNode(GT_ADDR, TYP_BYREF, op1);
Expand Down
27 changes: 17 additions & 10 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,11 @@ void Compiler::lvaInitTypeRef()
{
// Ensure that there will be at least one stack variable since
// we require that the GSCookie does not have a 0 stack offset.
unsigned dummy = lvaGrabTempWithImplicitUse(false DEBUGARG("GSCookie dummy"));
lvaTable[dummy].lvType = TYP_INT;
unsigned dummy = lvaGrabTempWithImplicitUse(false DEBUGARG("GSCookie dummy"));
LclVarDsc* gsCookieDummy = lvaGetDesc(dummy);
gsCookieDummy->lvType = TYP_INT;
gsCookieDummy->lvIsTemp = true; // It is not alive at all, set the flag to prevent zero-init.
lvaSetVarDoNotEnregister(dummy DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr));
}

// Allocate the lvaOutgoingArgSpaceVar now because we can run into problems in the
Expand Down Expand Up @@ -1228,14 +1231,17 @@ void Compiler::lvaInitVarArgsHandle(InitVarDscInfo* varDscInfo)
LclVarDsc* varDsc = varDscInfo->varDsc;
varDsc->lvType = TYP_I_IMPL;
varDsc->lvIsParam = 1;
// Make sure this lives in the stack -- address may be reported to the VM.
// TODO-CQ: This should probably be:
// lvaSetVarDoNotEnregister(varDscInfo->varNum DEBUGARG(VMNeedsStackAddr));
// But that causes problems, so, for expedience, I switched back to this heavyweight
// hammer. But I think it should be possible to switch; it may just work now
// that other problems are fixed.
lvaSetVarAddrExposed(varDscInfo->varNum DEBUGARG(AddressExposedReason::TOO_CONSERVATIVE));
#if defined(TARGET_X86)
// Codegen will need it for x86 scope info.
varDsc->lvImplicitlyReferenced = 1;
#endif // TARGET_X86

lvaSetVarDoNotEnregister(lvaVarargsHandleArg DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr));

assert(mostRecentlyActivePhase == PHASE_PRE_IMPORT);

// TODO-Cleanup: this is preImportation phase, why do we try to work with regs here?
// Should it be just deleted?
if (varDscInfo->canEnreg(TYP_I_IMPL))
{
/* Another register argument */
Expand Down Expand Up @@ -4323,6 +4329,7 @@ void Compiler::lvaMarkLocalVars()
lvaPSPSym = lvaGrabTempWithImplicitUse(false DEBUGARG("PSPSym"));
LclVarDsc* lclPSPSym = &lvaTable[lvaPSPSym];
lclPSPSym->lvType = TYP_I_IMPL;
lvaSetVarDoNotEnregister(lvaPSPSym DEBUGARG(DoNotEnregisterReason::VMNeedsStackAddr));
}
#endif // FEATURE_EH_FUNCLETS

Expand Down Expand Up @@ -7360,7 +7367,7 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r
}

// The register or stack location field is 11 characters wide.
if (varDsc->lvRefCnt() == 0)
if ((varDsc->lvRefCnt() == 0) && !varDsc->lvImplicitlyReferenced)
{
printf("zero-ref ");
}
Expand Down
20 changes: 14 additions & 6 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4066,8 +4066,11 @@ void Lowering::InsertPInvokeMethodProlog()
const CORINFO_EE_INFO* pInfo = comp->eeGetEEInfo();
const CORINFO_EE_INFO::InlinedCallFrameInfo& callFrameInfo = pInfo->inlinedCallFrameInfo;

// First arg: &compiler->lvaInlinedPInvokeFrameVar + callFrameInfo.offsetOfFrameVptr

// First arg: &compiler->lvaInlinedPInvokeFrameVar + callFrameInfo.offsetOfFrameVptr
#if defined(DEBUG)
const LclVarDsc* inlinedPInvokeDsc = comp->lvaGetDesc(comp->lvaInlinedPInvokeFrameVar);
assert(inlinedPInvokeDsc->IsAddressExposed());
#endif // DEBUG
GenTree* frameAddr = new (comp, GT_LCL_FLD_ADDR)
GenTreeLclFld(GT_LCL_FLD_ADDR, TYP_BYREF, comp->lvaInlinedPInvokeFrameVar, callFrameInfo.offsetOfFrameVptr);

Expand Down Expand Up @@ -4111,7 +4114,7 @@ void Lowering::InsertPInvokeMethodProlog()
GenTreeLclFld(GT_STORE_LCL_FLD, TYP_I_IMPL, comp->lvaInlinedPInvokeFrameVar, callFrameInfo.offsetOfCallSiteSP);
storeSP->gtOp1 = PhysReg(REG_SPBASE);
storeSP->gtFlags |= GTF_VAR_DEF;
comp->lvaSetVarDoNotEnregister(comp->lvaInlinedPInvokeFrameVar DEBUGARG(DoNotEnregisterReason::LocalField));
assert(inlinedPInvokeDsc->lvDoNotEnregister);

firstBlockRange.InsertBefore(insertionPoint, LIR::SeqTree(comp, storeSP));
DISPTREERANGE(firstBlockRange, storeSP);
Expand All @@ -4127,6 +4130,8 @@ void Lowering::InsertPInvokeMethodProlog()
GenTreeLclFld* storeFP =
new (comp, GT_STORE_LCL_FLD) GenTreeLclFld(GT_STORE_LCL_FLD, TYP_I_IMPL, comp->lvaInlinedPInvokeFrameVar,
callFrameInfo.offsetOfCalleeSavedFP);
assert(inlinedPInvokeDsc->lvDoNotEnregister);

storeFP->gtOp1 = PhysReg(REG_FPBASE);
storeFP->gtFlags |= GTF_VAR_DEF;

Expand Down Expand Up @@ -4408,9 +4413,12 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call)
noway_assert(comp->lvaInlinedPInvokeFrameVar != BAD_VAR_NUM);

// First argument is the address of the frame variable.
GenTree* frameAddr =
new (comp, GT_LCL_VAR) GenTreeLclVar(GT_LCL_VAR, TYP_BYREF, comp->lvaInlinedPInvokeFrameVar);
frameAddr->SetOperRaw(GT_LCL_VAR_ADDR);
GenTree* frameAddr = comp->gtNewLclVarAddrNode(comp->lvaInlinedPInvokeFrameVar, TYP_BYREF);

#if defined(DEBUG)
const LclVarDsc* inlinedPInvokeDsc = comp->lvaGetDesc(comp->lvaInlinedPInvokeFrameVar);
assert(inlinedPInvokeDsc->IsAddressExposed());
#endif // DEBUG

// Insert call to CORINFO_HELP_JIT_PINVOKE_END
GenTreeCall* helperCall =
Expand Down
7 changes: 2 additions & 5 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2098,7 +2098,8 @@ GenTree* Compiler::fgMakeTmpArgNode(fgArgTabEntry* curArgTabEntry)
{
var_types addrType = TYP_BYREF;
arg = gtNewOperNode(GT_ADDR, addrType, arg);
addrNode = arg;
lvaSetVarAddrExposed(tmpVarNum DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS));
addrNode = arg;

#if FEATURE_MULTIREG_ARGS
#ifdef TARGET_ARM64
Expand Down Expand Up @@ -2137,10 +2138,6 @@ GenTree* Compiler::fgMakeTmpArgNode(fgArgTabEntry* curArgTabEntry)
if (addrNode != nullptr)
{
assert(addrNode->gtOper == GT_ADDR);

// This will prevent this LclVar from being optimized away
lvaSetVarAddrExposed(tmpVarNum DEBUGARG(AddressExposedReason::TOO_CONSERVATIVE));

// the child of a GT_ADDR is required to have this flag set
addrNode->AsOp()->gtOp1->gtFlags |= GTF_DONT_CSE;
}
Expand Down

0 comments on commit 181da3b

Please sign in to comment.