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

Use a simpler representation for array addresses after morph #64581

Merged
merged 14 commits into from
Apr 1, 2022
11 changes: 0 additions & 11 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3225,19 +3225,8 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion,
}
else
{
bool isArrIndex = ((tree->gtFlags & GTF_VAR_ARR_INDEX) != 0);

assert(varTypeIsIntegralOrI(tree));

newTree->BashToConst(curAssertion->op2.u1.iconVal, genActualType(tree));

// If we're doing an array index address, assume any constant propagated contributes to the index.
if (isArrIndex)
{
newTree->AsIntCon()->gtFieldSeq =
GetFieldSeqStore()->CreateSingleton(FieldSeqStore::ConstantIndexPseudoField);
}
newTree->gtFlags &= ~GTF_VAR_ARR_INDEX;
}
break;

Expand Down
13 changes: 7 additions & 6 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1916,7 +1916,6 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
m_blockToEHPreds = nullptr;
m_fieldSeqStore = nullptr;
m_zeroOffsetFieldMap = nullptr;
m_arrayInfoMap = nullptr;
m_refAnyClass = nullptr;
for (MemoryKind memoryKind : allMemoryKinds())
{
Expand Down Expand Up @@ -9161,10 +9160,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
{
chars += printf("[VAR_DEATH]");
}
if (tree->gtFlags & GTF_VAR_ARR_INDEX)
{
chars += printf("[VAR_ARR_INDEX]");
}
#if defined(DEBUG)
if (tree->gtDebugFlags & GTF_DEBUG_VAR_CSE_REF)
{
Expand Down Expand Up @@ -9310,8 +9305,14 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
}
break;

case GT_CNS_INT:
case GT_ARR_ADDR:
if (tree->gtFlags & GTF_ARR_ADDR_NONNULL)
{
chars += printf("[ARR_ADDR_NONNULL]");
}
break;

case GT_CNS_INT:
{
GenTreeFlags handleKind = (tree->gtFlags & GTF_ICON_HDL_MASK);

Expand Down
74 changes: 5 additions & 69 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1377,27 +1377,6 @@ class LinearScanInterface

LinearScanInterface* getLinearScanAllocator(Compiler* comp);

// Information about arrays: their element type and size, and the offset of the first element.
// We label GT_IND's that are array indices with GTF_IND_ARR_INDEX, and, for such nodes,
// associate an array info via the map retrieved by GetArrayInfoMap(). This information is used,
// for example, in value numbering of array index expressions.
struct ArrayInfo
{
var_types m_elemType;
CORINFO_CLASS_HANDLE m_elemStructType;
unsigned m_elemSize;
unsigned m_elemOffset;

ArrayInfo() : m_elemType(TYP_UNDEF), m_elemStructType(nullptr), m_elemSize(0), m_elemOffset(0)
{
}

ArrayInfo(var_types elemType, unsigned elemSize, unsigned elemOffset, CORINFO_CLASS_HANDLE elemStructType)
: m_elemType(elemType), m_elemStructType(elemStructType), m_elemSize(elemSize), m_elemOffset(elemOffset)
{
}
};

// This enumeration names the phases into which we divide compilation. The phases should completely
// partition a compilation.
enum Phases
Expand Down Expand Up @@ -5604,6 +5583,8 @@ class Compiler
// Does value-numbering for an intrinsic tree.
void fgValueNumberIntrinsic(GenTree* tree);

void fgValueNumberArrIndexAddr(GenTreeArrAddr* arrAddr);

#ifdef FEATURE_SIMD
// Does value-numbering for a GT_SIMD tree
void fgValueNumberSimd(GenTreeSIMD* tree);
Expand Down Expand Up @@ -11219,52 +11200,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
// CoreRT. Such case is handled same as the default case.
void fgAddFieldSeqForZeroOffset(GenTree* op1, FieldSeqNode* fieldSeq);

typedef JitHashTable<const GenTree*, JitPtrKeyFuncs<GenTree>, ArrayInfo> NodeToArrayInfoMap;
NodeToArrayInfoMap* m_arrayInfoMap;

NodeToArrayInfoMap* GetArrayInfoMap()
{
Compiler* compRoot = impInlineRoot();
if (compRoot->m_arrayInfoMap == nullptr)
{
// Create a CompAllocator that labels sub-structure with CMK_ArrayInfoMap, and use that for allocation.
CompAllocator ialloc(getAllocator(CMK_ArrayInfoMap));
compRoot->m_arrayInfoMap = new (ialloc) NodeToArrayInfoMap(ialloc);
}
return compRoot->m_arrayInfoMap;
}

//-----------------------------------------------------------------------------------------------------------------
// Compiler::TryGetArrayInfo:
// Given an indirection node, checks to see whether or not that indirection represents an array access, and
// if so returns information about the array.
//
// Arguments:
// indir - The `GT_IND` node.
// arrayInfo (out) - Information about the accessed array if this function returns true. Undefined otherwise.
//
// Returns:
// True if the `GT_IND` node represents an array access; false otherwise.
bool TryGetArrayInfo(GenTreeIndir* indir, ArrayInfo* arrayInfo)
{
if ((indir->gtFlags & GTF_IND_ARR_INDEX) == 0)
{
return false;
}

if (indir->gtOp1->OperIs(GT_INDEX_ADDR))
{
GenTreeIndexAddr* const indexAddr = indir->gtOp1->AsIndexAddr();
*arrayInfo = ArrayInfo(indexAddr->gtElemType, indexAddr->gtElemSize, indexAddr->gtElemOffset,
indexAddr->gtStructElemClass);
return true;
}

bool found = GetArrayInfoMap()->Lookup(indir, arrayInfo);
assert(found);
return true;
}

NodeToUnsignedMap* m_memorySsaMap[MemoryKindCount];

// In some cases, we want to assign intermediate SSA #'s to memory states, and know what nodes create those memory
Expand All @@ -11282,8 +11217,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
Compiler* compRoot = impInlineRoot();
if (compRoot->m_memorySsaMap[memoryKind] == nullptr)
{
// Create a CompAllocator that labels sub-structure with CMK_ArrayInfoMap, and use that for allocation.
CompAllocator ialloc(getAllocator(CMK_ArrayInfoMap));
// Create a CompAllocator that labels sub-structure with CMK_MemorySsaMap, and use that for allocation.
CompAllocator ialloc(getAllocator(CMK_MemorySsaMap));
compRoot->m_memorySsaMap[memoryKind] = new (ialloc) NodeToUnsignedMap(ialloc);
}
return compRoot->m_memorySsaMap[memoryKind];
Expand Down Expand Up @@ -11551,6 +11486,7 @@ class GenTreeVisitor
case GT_RETURN:
case GT_RETFILT:
case GT_RUNTIMELOOKUP:
case GT_ARR_ADDR:
case GT_KEEPALIVE:
case GT_INC_SATURATE:
{
Expand Down
17 changes: 6 additions & 11 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -844,28 +844,22 @@ inline GenTree* Compiler::gtNewOperNode(genTreeOps oper, var_types type, GenTree

if (doSimplifications)
{
// We do some simplifications here.
// If this gets to be too many, try a switch...
// TODO-Cleanup: With the factoring out of array bounds checks, it should not be the
// case that we need to check for the array index case here, but without this check
// we get failures (see for example jit\Directed\Languages\Python\test_methods_d.exe)
// We do some simplifications here. If this gets to be too many, try a switch...
if (oper == GT_IND)
{
// IND(ADDR(IND(x)) == IND(x)
if (op1->gtOper == GT_ADDR)
if (op1->OperIs(GT_ADDR))
{
GenTreeUnOp* addr = op1->AsUnOp();
GenTree* indir = addr->gtGetOp1();
if (indir->OperIs(GT_IND) && ((indir->gtFlags & GTF_IND_ARR_INDEX) == 0))
GenTree* indir = op1->AsUnOp()->gtGetOp1();
if (indir->OperIs(GT_IND))
{
op1 = indir->AsIndir()->Addr();
}
}
}
else if (oper == GT_ADDR)
{
// if "x" is not an array index, ADDR(IND(x)) == x
if (op1->gtOper == GT_IND && (op1->gtFlags & GTF_IND_ARR_INDEX) == 0)
if (op1->OperIs(GT_IND))
{
return op1->AsOp()->gtOp1;
}
Expand Down Expand Up @@ -4231,6 +4225,7 @@ void GenTree::VisitOperands(TVisitor visitor)
case GT_ALLOCOBJ:
case GT_INIT_VAL:
case GT_RUNTIMELOOKUP:
case GT_ARR_ADDR:
case GT_JTRUE:
case GT_SWITCH:
case GT_NULLCHECK:
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compmemkind.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ CompMemKindMacro(Generic)
CompMemKindMacro(LocalAddressVisitor)
CompMemKindMacro(FieldSeqStore)
CompMemKindMacro(ZeroOffsetFieldMap)
CompMemKindMacro(ArrayInfoMap)
CompMemKindMacro(MemorySsaMap)
CompMemKindMacro(MemoryPhiArg)
CompMemKindMacro(CSE)
CompMemKindMacro(GC)
Expand Down
10 changes: 0 additions & 10 deletions src/coreclr/jit/earlyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,6 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck
actualValClone->gtType = tree->gtType;
}

// Propagating a constant into an array index expression requires calling
// LabelIndex to update the FieldSeq annotations. EarlyProp may replace
// array length expressions with constants, so check if this is an array
// length operator that is part of an array index expression.
bool isIndexExpr = (tree->OperGet() == GT_ARR_LENGTH && ((tree->gtFlags & GTF_ARRLEN_ARR_IDX) != 0));
if (isIndexExpr)
{
actualValClone->LabelIndex(this);
}

// actualValClone has small tree node size, it is safe to use CopyFrom here.
tree->ReplaceWith(actualValClone, this);

Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2951,8 +2951,7 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
expectedFlags |= GTF_CALL;
}

// We reuse GTF_REVERSE_OPS as GTF_VAR_ARR_INDEX for LCL_VAR nodes.
if (((tree->gtFlags & GTF_REVERSE_OPS) != 0) && !tree->OperIs(GT_LCL_VAR))
if ((tree->gtFlags & GTF_REVERSE_OPS) != 0)
{
assert(tree->OperSupportsReverseOpEvalOrder(this));
}
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,10 @@ bool Compiler::fgAddrCouldBeNull(GenTree* addr)
{
return false;
}
else if (addr->OperIs(GT_ARR_ADDR))
{
return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0;
}
else if (addr->gtOper == GT_LCL_VAR)
{
unsigned varNum = addr->AsLclVarCommon()->GetLclNum();
Expand Down
Loading