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

A few small jit changes. #31816

Merged
merged 4 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/coreclr/src/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4557,7 +4557,8 @@ void CodeGen::genCodeForLclVar(GenTreeLclVar* tree)
// lcl_vars are not defs
assert((tree->gtFlags & GTF_VAR_DEF) == 0);

bool isRegCandidate = compiler->lvaTable[tree->GetLclNum()].lvIsRegCandidate();
LclVarDsc* varDsc = compiler->lvaGetDesc(tree);
bool isRegCandidate = varDsc->lvIsRegCandidate();

// If this is a register candidate that has been spilled, genConsumeReg() will
// reload it at the point of use. Otherwise, if it's not in a register, we load it here.
Expand Down
27 changes: 14 additions & 13 deletions src/coreclr/src/jit/eeinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,23 +102,24 @@ const char* Compiler::eeGetMethodFullName(CORINFO_METHOD_HANDLE hnd)
/* add length of methodName and opening bracket */
length += strlen(methodName) + 1;

/* figure out the signature */
PAL_TRY(FilterSuperPMIExceptionsParam_eeinterface*, pParam, &param)
{

param.pThis->eeGetMethodSig(param.hnd, &param.sig);
/* figure out the signature */

// allocate space to hold the class names for each of the parameters
pParam->pThis->eeGetMethodSig(pParam->hnd, &pParam->sig);

if (param.sig.numArgs > 0)
{
param.pArgNames = getAllocator(CMK_DebugOnly).allocate<const char*>(param.sig.numArgs);
}
else
{
param.pArgNames = nullptr;
}
// allocate space to hold the class names for each of the parameters

if (pParam->sig.numArgs > 0)
{
pParam->pArgNames = pParam->pThis->getAllocator(CMK_DebugOnly).allocate<const char*>(pParam->sig.numArgs);
}
else
{
pParam->pArgNames = nullptr;
}

PAL_TRY(FilterSuperPMIExceptionsParam_eeinterface*, pParam, &param)
{
unsigned i;
pParam->argLst = pParam->sig.args;

Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/src/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16869,6 +16869,9 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree)
case GT_OBJ:
structHnd = tree->AsObj()->GetLayout()->GetClassHandle();
break;
case GT_BLK:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use this path only in optIsCSEcandidate, but later optIsCSEcandidate doesn't know how to work with these opcodes, so it doesn't produce any diffs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, normally the layouts used by BLK do not have a class handle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually get such BLK nodes from OBJ nodes for structs that do not contain GC pointers.
I'll check if I see them in master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I've managed to confuse myself. Yes, BLKs can have class handles now, it was my plan to have handles only in OBJs but it didn't happen.

structHnd = tree->AsBlk()->GetLayout()->GetClassHandle();
break;
case GT_CALL:
structHnd = tree->AsCall()->gtRetClsHnd;
break;
Expand Down Expand Up @@ -16944,6 +16947,10 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree)
}
}
}
else if (addr->OperGet() == GT_LCL_VAR)
{
structHnd = gtGetStructHandleIfPresent(addr);
}
}
}
break;
Expand Down
14 changes: 8 additions & 6 deletions src/coreclr/src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1214,21 +1214,23 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr,
// If it is a multi-reg struct return, don't change the oper to GT_LCL_FLD.
// That is, the IR will be of the form lclVar = call for multi-reg return
//
GenTreeLclVar* lcl = destAddr->AsOp()->gtOp1->AsLclVar();
GenTreeLclVar* lcl = destAddr->AsOp()->gtOp1->AsLclVar();
unsigned lclNum = lcl->GetLclNum();
LclVarDsc* varDsc = lvaGetDesc(lclNum);
if (src->AsCall()->HasMultiRegRetVal())
{
// Mark the struct LclVar as used in a MultiReg return context
// which currently makes it non promotable.
// TODO-1stClassStructs: Eliminate this pessimization when we can more generally
// handle multireg returns.
lcl->gtFlags |= GTF_DONT_CSE;
lvaTable[lcl->AsLclVarCommon()->GetLclNum()].lvIsMultiRegRet = true;
varDsc->lvIsMultiRegRet = true;
}
else if (lcl->gtType != src->gtType)
{
// We change this to a GT_LCL_FLD (from a GT_ADDR of a GT_LCL_VAR)
lcl->ChangeOper(GT_LCL_FLD);
fgLclFldAssign(lcl->AsLclVarCommon()->GetLclNum());
fgLclFldAssign(lclNum);
lcl->gtType = src->gtType;
asgType = src->gtType;
}
Expand All @@ -1238,7 +1240,7 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr,
#if defined(TARGET_ARM)
// TODO-Cleanup: This should have been taken care of in the above HasMultiRegRetVal() case,
// but that method has not been updadted to include ARM.
impMarkLclDstNotPromotable(lcl->AsLclVarCommon()->GetLclNum(), src, structHnd);
impMarkLclDstNotPromotable(lclNum, src, structHnd);
lcl->gtFlags |= GTF_DONT_CSE;
#elif defined(UNIX_AMD64_ABI)
// Not allowed for FEATURE_CORCLR which is the only SKU available for System V OSs.
Expand All @@ -1250,7 +1252,7 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr,
// TODO-Cleanup: Why is this needed here? This seems that it will set this even for
// non-multireg returns.
lcl->gtFlags |= GTF_DONT_CSE;
lvaTable[lcl->AsLclVarCommon()->GetLclNum()].lvIsMultiRegRet = true;
varDsc->lvIsMultiRegRet = true;
#endif
}
else // we don't have a GT_ADDR of a GT_LCL_VAR
Expand Down Expand Up @@ -2558,7 +2560,7 @@ BasicBlock* Compiler::impPushCatchArgOnStack(BasicBlock* hndBlk, CORINFO_CLASS_H
#if defined(JIT32_GCENCODER)
const bool forceInsertNewBlock = isSingleBlockFilter || compStressCompile(STRESS_CATCH_ARG, 5);
#else
const bool forceInsertNewBlock = compStressCompile(STRESS_CATCH_ARG, 5);
const bool forceInsertNewBlock = compStressCompile(STRESS_CATCH_ARG, 5);
#endif // defined(JIT32_GCENCODER)

/* Spill GT_CATCH_ARG to a temp if there are jumps to the beginning of the handler */
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/instr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,7 @@ instruction CodeGenInterface::ins_Load(var_types srcType, bool aligned /*=false*
*/
instruction CodeGen::ins_Copy(var_types dstType)
{
assert(emitTypeActSz[dstType] != 0);
#if defined(TARGET_XARCH)
if (varTypeIsSIMD(dstType))
{
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/src/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ GenTree* Lowering::LowerNode(GenTree* node)
break;

case GT_RETURN:
LowerRet(node);
LowerRet(node->AsUnOp());
break;

case GT_RETURNTRAP:
Expand Down Expand Up @@ -3084,7 +3084,7 @@ void Lowering::LowerJmpMethod(GenTree* jmp)
}

// Lower GT_RETURN node to insert PInvoke method epilog if required.
void Lowering::LowerRet(GenTree* ret)
void Lowering::LowerRet(GenTreeUnOp* ret)
{
assert(ret->OperGet() == GT_RETURN);

Expand All @@ -3097,7 +3097,7 @@ void Lowering::LowerRet(GenTree* ret)
(varTypeUsesFloatReg(ret) != varTypeUsesFloatReg(ret->gtGetOp1())))
{
GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, ret->TypeGet(), ret->gtGetOp1(), nullptr);
ret->AsOp()->gtOp1 = bitcast;
ret->gtOp1 = bitcast;
BlockRange().InsertBefore(ret, bitcast);
ContainCheckBitCast(bitcast);
}
Expand All @@ -3107,7 +3107,7 @@ void Lowering::LowerRet(GenTree* ret)
{
InsertPInvokeMethodEpilog(comp->compCurBB DEBUGARG(ret));
}
ContainCheckRet(ret->AsOp());
ContainCheckRet(ret);
}

GenTree* Lowering::LowerDirectCall(GenTreeCall* call)
Expand Down Expand Up @@ -5764,7 +5764,7 @@ void Lowering::ContainCheckLclHeap(GenTreeOp* node)
// Arguments:
// node - pointer to the node
//
void Lowering::ContainCheckRet(GenTreeOp* ret)
void Lowering::ContainCheckRet(GenTreeUnOp* ret)
{
assert(ret->OperIs(GT_RETURN));

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/jit/lower.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class Lowering final : public Phase
void ContainCheckReturnTrap(GenTreeOp* node);
void ContainCheckArrOffset(GenTreeArrOffs* node);
void ContainCheckLclHeap(GenTreeOp* node);
void ContainCheckRet(GenTreeOp* node);
void ContainCheckRet(GenTreeUnOp* ret);
void ContainCheckJTrue(GenTreeOp* node);

void ContainCheckBitCast(GenTree* node);
Expand Down Expand Up @@ -133,7 +133,7 @@ class Lowering final : public Phase
GenTree* LowerJTrue(GenTreeOp* jtrue);
GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition);
void LowerJmpMethod(GenTree* jmp);
void LowerRet(GenTree* ret);
void LowerRet(GenTreeUnOp* ret);
GenTree* LowerDelegateInvoke(GenTreeCall* call);
GenTree* LowerIndirectNonvirtCall(GenTreeCall* call);
GenTree* LowerDirectCall(GenTreeCall* call);
Expand Down
12 changes: 4 additions & 8 deletions src/coreclr/src/jit/lsraarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
//
int LinearScan::BuildIndir(GenTreeIndir* indirTree)
{
int srcCount = 0;
// If this is the rhs of a block copy (i.e. non-enregisterable struct),
// it has no register requirements.
if (indirTree->TypeGet() == TYP_STRUCT)
{
return srcCount;
}
// struct typed indirs are expected only on rhs of a block copy,
// but in this case they must be contained.
assert(indirTree->TypeGet() != TYP_STRUCT);

GenTree* addr = indirTree->Addr();
GenTree* index = nullptr;
Expand Down Expand Up @@ -109,7 +105,7 @@ int LinearScan::BuildIndir(GenTreeIndir* indirTree)
}
#endif // FEATURE_SIMD

srcCount = BuildIndirUses(indirTree);
int srcCount = BuildIndirUses(indirTree);
buildInternalRegisterUses();

if (indirTree->gtOper != GT_STOREIND)
Expand Down
8 changes: 6 additions & 2 deletions src/coreclr/src/jit/lsrabuild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2473,14 +2473,18 @@ void setTgtPref(Interval* interval, RefPosition* tgtPrefUse)
RefPosition* LinearScan::BuildDef(GenTree* tree, regMaskTP dstCandidates, int multiRegIdx)
{
assert(!tree->isContained());
RegisterType type = getDefType(tree);

if (dstCandidates != RBM_NONE)
{
assert((tree->GetRegNum() == REG_NA) || (dstCandidates == genRegMask(tree->GetRegByIndex(multiRegIdx))));
}

if (tree->IsMultiRegNode())
RegisterType type = getDefType(tree);
if (!tree->IsMultiRegNode())
{
type = getDefType(tree);
}
else
{
type = tree->GetRegTypeByIndex(multiRegIdx);
}
Expand Down
9 changes: 3 additions & 6 deletions src/coreclr/src/jit/lsraxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2751,12 +2751,9 @@ int LinearScan::BuildCast(GenTreeCast* cast)
//
int LinearScan::BuildIndir(GenTreeIndir* indirTree)
{
// If this is the rhs of a block copy (i.e. non-enregisterable struct),
// it has no register requirements.
if (indirTree->TypeGet() == TYP_STRUCT)
{
return 0;
}
// struct typed indirs are expected only on rhs of a block copy,
// but in this case they must be contained.
assert(indirTree->TypeGet() != TYP_STRUCT);

#ifdef FEATURE_SIMD
RefPosition* internalFloatDef = nullptr;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8888,7 +8888,8 @@ GenTree* Compiler::fgMorphOneAsgBlockOp(GenTree* tree)

if (dest == destLclVarTree)
{
dest = gtNewIndir(asgType, gtNewOperNode(GT_ADDR, TYP_BYREF, dest));
GenTree* addr = gtNewOperNode(GT_ADDR, TYP_BYREF, dest);
dest = gtNewIndir(asgType, addr);
}
}
}
Expand Down