Skip to content

Commit

Permalink
Remove GTF_ADDR_ONSTACK and IsVarAddr.
Browse files Browse the repository at this point in the history 
IsVarAddr was checking GTF_ADDR_ONSTACK  to determine if
the GT_ADDR node is an address of a local. This change removes both
GTF_ADDR_ONSTACK and  IsVarAddr and uses IsLocalAdrExpr instead.
IsLocalAddrExpr uses opcodes to determine if GT_ADDR node is
a local address.

GTF_ADDR_ONSTACK flag is ancient, added before 2002 so I couldn't find
the checkin that introduced it.

I changed the assert to a check and an assignment since simplifications
inside fgMorphArgs between
https://github.com/dotnet/coreclr/blob/dotnet/coreclr@1a1e4c4d5a8030cb8d82a2e5b06c2ab357b92534/src/jit/morph.cpp#L3709
(which causes https://github.com/dotnet/coreclr/blob/dotnet/coreclr@1a1e4c4d5a8030cb8d82a2e5b06c2ab357b92534/src/jit/morph.cpp#L3057)
and
https://github.com/dotnet/coreclr/blob/dotnet/coreclr@1a1e4c4d5a8030cb8d82a2e5b06c2ab357b92534/src/jit/morph.cpp#L3790
may result in more GT_ADDR nodes recognized by IsLocalAdrExpr.

x86 and x64 pmi frameworks had no code diffs and some gcinfo reductions
(15 methods with gcinfo diffs in x86).

Fixes dotnet/coreclr#22190.


Commit migrated from dotnet/coreclr@4070994
  • Loading branch information
@erozenfeld
erozenfeld committed Jan 30, 2019
1 parent d4aaf64 commit b17d6e6
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 61 deletions.
6 changes: 4 additions & 2 deletions src/coreclr/src/jit/codegenarmarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1594,9 +1594,11 @@ void CodeGen::genCodeForLclAddr(GenTree* tree)
regNumber targetReg = tree->gtRegNum;

// Address of a local var.
noway_assert(targetType == TYP_BYREF);
noway_assert((targetType == TYP_BYREF) || (targetType == TYP_I_IMPL));

inst_RV_TT(INS_lea, targetReg, tree, 0, EA_BYREF);
emitAttr size = emitTypeSize(targetType);

inst_RV_TT(INS_lea, targetReg, tree, 0, size);
genProduceReg(tree);
}

Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/src/jit/codegenxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4379,9 +4379,11 @@ void CodeGen::genCodeForLclAddr(GenTree* tree)
regNumber targetReg = tree->gtRegNum;

// Address of a local var.
noway_assert(targetType == TYP_BYREF);
noway_assert((targetType == TYP_BYREF) || (targetType == TYP_I_IMPL));

inst_RV_TT(INS_lea, targetReg, tree, 0, EA_BYREF);
emitAttr size = emitTypeSize(targetType);

inst_RV_TT(INS_lea, targetReg, tree, 0, size);
genProduceReg(tree);
}

Expand Down
8 changes: 0 additions & 8 deletions src/coreclr/src/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9283,14 +9283,6 @@ int cTreeFlagsIR(Compiler* comp, GenTree* tree)
}
break;

case GT_ADDR:

if (tree->gtFlags & GTF_ADDR_ONSTACK)
{
chars += printf("[ADDR_ONSTACK]");
}
break;

case GT_MUL:
#if !defined(_TARGET_64BIT_)
case GT_MUL_LONG:
Expand Down
25 changes: 0 additions & 25 deletions src/coreclr/src/jit/compiler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -991,14 +991,6 @@ inline GenTree* Compiler::gtNewOperNode(genTreeOps oper, var_types type, GenTree

GenTree* node = new (this, oper) GenTreeOp(oper, type, op1, nullptr);

//
// the GT_ADDR of a Local Variable implies GTF_ADDR_ONSTACK
//
if ((oper == GT_ADDR) && (op1->OperGet() == GT_LCL_VAR))
{
node->gtFlags |= GTF_ADDR_ONSTACK;
}

return node;
}

Expand Down Expand Up @@ -1479,23 +1471,6 @@ inline void GenTree::ChangeOperUnchecked(genTreeOps oper)
gtFlags &= mask;
}

/*****************************************************************************
* Returns true if the node is &var (created by ldarga and ldloca)
*/

inline bool GenTree::IsVarAddr() const
{
if (gtOper == GT_ADDR)
{
if (gtFlags & GTF_ADDR_ONSTACK)
{
assert((gtType == TYP_BYREF) || (gtType == TYP_I_IMPL));
return true;
}
}
return false;
}

/*****************************************************************************
*
* Returns true if the node is of the "ovf" variety, for example, add.ovf.i1.
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/src/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9456,15 +9456,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, __in __in_z _
}
goto DASH;

case GT_ADDR:
if (tree->gtFlags & GTF_ADDR_ONSTACK)
{
printf("L");
--msgLength;
break;
} // L means LclVar
goto DASH;

case GT_LCL_FLD:
case GT_LCL_VAR:
case GT_LCL_VAR_ADDR:
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/src/jit/gentree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -826,8 +826,6 @@ struct GenTree
#define GTF_CLS_VAR_ASG_LHS 0x04000000 // GT_CLS_VAR -- this GT_CLS_VAR node is (the effective val) of the LHS
// of an assignment; don't evaluate it independently.

#define GTF_ADDR_ONSTACK 0x80000000 // GT_ADDR -- this expression is guaranteed to be on the stack

#define GTF_ADDRMODE_NO_CSE 0x80000000 // GT_ADD/GT_MUL/GT_LSH -- Do not CSE this node only, forms complex
// addressing mode

Expand Down Expand Up @@ -2031,7 +2029,6 @@ struct GenTree
}
inline bool IsHelperCall();

bool IsVarAddr() const;
bool gtOverflow() const;
bool gtOverflowEx() const;
bool gtSetFlags() const;
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/src/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2928,12 +2928,12 @@ CORINFO_CLASS_HANDLE Compiler::impGetObjectClass()
/* static */
void Compiler::impBashVarAddrsToI(GenTree* tree1, GenTree* tree2)
{
if (tree1->IsVarAddr())
if (tree1->IsLocalAddrExpr() != nullptr)
{
tree1->gtType = TYP_I_IMPL;
}

if (tree2 && tree2->IsVarAddr())
if (tree2 && (tree2->IsLocalAddrExpr() != nullptr))
{
tree2->gtType = TYP_I_IMPL;
}
Expand Down Expand Up @@ -11442,7 +11442,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
// We had better assign it a value of the correct type
assertImp(
genActualType(lclTyp) == genActualType(op1->gtType) ||
genActualType(lclTyp) == TYP_I_IMPL && op1->IsVarAddr() ||
genActualType(lclTyp) == TYP_I_IMPL && (op1->IsLocalAddrExpr() != nullptr) ||
(genActualType(lclTyp) == TYP_I_IMPL && (op1->gtType == TYP_BYREF || op1->gtType == TYP_REF)) ||
(genActualType(op1->gtType) == TYP_I_IMPL && lclTyp == TYP_BYREF) ||
(varTypeIsFloating(lclTyp) && varTypeIsFloating(op1->TypeGet())) ||
Expand All @@ -11451,7 +11451,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
/* If op1 is "&var" then its type is the transient "*" and it can
be used either as TYP_BYREF or TYP_I_IMPL */

if (op1->IsVarAddr())
if (op1->IsLocalAddrExpr() != nullptr)
{
assertImp(genActualType(lclTyp) == TYP_I_IMPL || lclTyp == TYP_BYREF);

Expand Down Expand Up @@ -12247,7 +12247,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
op3 = impPopStack().val;

assertImp(op3->gtType == TYP_REF);
if (op2->IsVarAddr())
if (op2->IsLocalAddrExpr() != nullptr)
{
op2->gtType = TYP_I_IMPL;
}
Expand Down Expand Up @@ -19148,7 +19148,7 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo)
assert(sigType == TYP_I_IMPL);

/* If possible change the BYREF to an int */
if (thisArg->IsVarAddr())
if (thisArg->IsLocalAddrExpr() != nullptr)
{
thisArg->gtType = TYP_I_IMPL;
lclVarInfo[0].lclVerTypeInfo = typeInfo(varType2tiType(TYP_I_IMPL));
Expand Down Expand Up @@ -19230,7 +19230,7 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo)
assert(varTypeIsIntOrI(sigType));

/* If possible bash the BYREF to an int */
if (inlArgNode->IsVarAddr())
if (inlArgNode->IsLocalAddrExpr() != nullptr)
{
inlArgNode->gtType = TYP_I_IMPL;
lclVarInfo[i].lclVerTypeInfo = typeInfo(varType2tiType(TYP_I_IMPL));
Expand Down
12 changes: 7 additions & 5 deletions src/coreclr/src/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1619,7 +1619,7 @@ void fgArgInfo::ArgsComplete()
case 7:
// If we have a stack based LclVar we can perform a wider read of 4 or 8 bytes
//
if (argObj->gtObj.gtOp1->IsVarAddr() == false) // Is the source not a LclVar?
if (argObj->gtObj.gtOp1->IsLocalAddrExpr() == nullptr) // Is the source not a LclVar?
{
// If we don't have a LclVar we need to read exactly 3,5,6 or 7 bytes
// For now we use a a GT_CPBLK to copy the exact size into a GT_LCL_VAR temp.
Expand Down Expand Up @@ -3052,7 +3052,7 @@ void Compiler::fgInitArgInfo(GenTreeCall* call)
// Change the node to TYP_I_IMPL so we don't report GC info
// NOTE: We deferred this from the importer because of the inliner.

if (argx->IsVarAddr())
if (argx->IsLocalAddrExpr() != nullptr)
{
argx->gtType = TYP_I_IMPL;
}
Expand Down Expand Up @@ -3786,8 +3786,10 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
assert(size != 0);
argSlots += argEntry->getSlotCount();

// lclVar address should have been retyped to TYP_I_IMPL.
assert(!argx->IsVarAddr() || (argx->gtType == TYP_I_IMPL));
if (argx->IsLocalAddrExpr() != nullptr)
{
argx->gtType = TYP_I_IMPL;
}

// Get information about this argument.
var_types hfaType = argEntry->hfaType;
Expand Down Expand Up @@ -4077,7 +4079,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
// There are a few special cases where we can omit using a CopyBlk
// where we normally would need to use one.

if (argObj->gtObj.gtOp1->IsVarAddr()) // Is the source a LclVar?
if (argObj->gtObj.gtOp1->IsLocalAddrExpr() != nullptr) // Is the source a LclVar?
{
copyBlkClass = NO_CLASS_HANDLE;
}
Expand Down

0 comments on commit b17d6e6

Please sign in to comment.