Delete the zero-offset field sequence map (#71455)

* Delete the unused GTF_ICON_FIELD_OFF

* Do not try to find "IND(struct ARR_ADDR)" handle

Leftover from GT_INDEX.

* Delete the zero-offset field sequence map

For statics at a zero offset, preserve the "CNS_INT 0" node instead.

* Make all field sequences singletons

* Print struct field offsets in the sequence

* Improve address mode marking to handle COMMAs

This avoids regressing cases where we now have new CSE
opportunities for statics with static init helper calls.

* Delete NotAField

Replace with using "nullptr" directly.

Note that "nullptr" can mean both an absense of a field
sequence and an "implicit" sequence of struct fields.

* Refactor the field sequence store

Now that we don't need the map to be a set, we can apply some optimizations.

* FieldSeqNode -> FieldSeq

* Turn asserts into "noway_assert"s

We don't expect empty sequences: they are either coming from
the "IsFieldAddr" code path, which disallows these, or from
the boxed static code path, where we should always have them.

* Fix formatting

* Take down SPMI

* Resurrect SPMI to work around a CG2 bug

src/coreclr/jit/assertionprop.cpp

@@ -1112,11 +1112,6 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse
  {
  printf(".%02u", curAssertion->op1.lcl.ssaNum);
  }
- if (curAssertion->op2.zeroOffsetFieldSeq != nullptr)
- {
- printf(" Zero");
- gtDispFieldSeq(curAssertion->op2.zeroOffsetFieldSeq);
- }
  break;
 
  case O2K_CONST_INT:
@@ -1674,14 +1669,10 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
  goto DONE_ASSERTION; // Don't make an assertion
  }
 
- FieldSeqNode* zeroOffsetFieldSeq = nullptr;
- GetZeroOffsetFieldMap()->Lookup(op2, &zeroOffsetFieldSeq);
-
- assertion.op2.kind = O2K_LCLVAR_COPY;
- assertion.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair);
- assertion.op2.lcl.lclNum = lclNum2;
- assertion.op2.lcl.ssaNum = op2->AsLclVarCommon()->GetSsaNum();
- assertion.op2.zeroOffsetFieldSeq = zeroOffsetFieldSeq;
+ assertion.op2.kind = O2K_LCLVAR_COPY;
+ assertion.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair);
+ assertion.op2.lcl.lclNum = lclNum2;
+ assertion.op2.lcl.ssaNum = op2->AsLclVarCommon()->GetSsaNum();
 
  // Ok everything has been set and the assertion looks good
  assertion.assertionKind = assertionKind;
@@ -3527,23 +3518,17 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion,
  }
 
  // Extract the matching lclNum and ssaNum, as well as the field sequence.
- unsigned copyLclNum;
- unsigned copySsaNum;
- FieldSeqNode* zeroOffsetFieldSeq;
+ unsigned copyLclNum;
+ unsigned copySsaNum;
  if (op1.lcl.lclNum == lclNum)
  {
- copyLclNum = op2.lcl.lclNum;
- copySsaNum = op2.lcl.ssaNum;
- zeroOffsetFieldSeq = op2.zeroOffsetFieldSeq;
+ copyLclNum = op2.lcl.lclNum;
+ copySsaNum = op2.lcl.ssaNum;
  }
  else
  {
- copyLclNum = op1.lcl.lclNum;
- copySsaNum = op1.lcl.ssaNum;
- zeroOffsetFieldSeq = nullptr; // Only the RHS of an assignment can have a FldSeq.
- assert(optLocalAssertionProp); // Were we to perform replacements in global propagation, that makes copy
- // assertions for control flow ("if (a == b) { ... }"), where both operands
- // could have a FldSeq, we'd need to save it for "op1" too.
+ copyLclNum = op1.lcl.lclNum;
+ copySsaNum = op1.lcl.ssaNum;
  }
 
  if (!optLocalAssertionProp)
@@ -3575,19 +3560,6 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion,
  tree->SetLclNum(copyLclNum);
  tree->SetSsaNum(copySsaNum);
 
- // The sequence we are propagating (if any) represents the inner fields.
- if (zeroOffsetFieldSeq != nullptr)
- {
- FieldSeqNode* outerZeroOffsetFieldSeq = nullptr;
- if (GetZeroOffsetFieldMap()->Lookup(tree, &outerZeroOffsetFieldSeq))
- {
- zeroOffsetFieldSeq = GetFieldSeqStore()->Append(zeroOffsetFieldSeq, outerZeroOffsetFieldSeq);
- GetZeroOffsetFieldMap()->Remove(tree);
- }
-
- fgAddFieldSeqForZeroOffset(tree, zeroOffsetFieldSeq);
- }
-
 #ifdef DEBUG
  if (verbose)
  {

src/coreclr/jit/compiler.cpp

@@ -1933,11 +1933,10 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
  m_nodeTestData = nullptr;
  m_loopHoistCSEClass = FIRST_LOOP_HOIST_CSE_CLASS;
 #endif
- m_switchDescMap = nullptr;
- m_blockToEHPreds = nullptr;
- m_fieldSeqStore = nullptr;
- m_zeroOffsetFieldMap = nullptr;
- m_refAnyClass = nullptr;
+ m_switchDescMap = nullptr;
+ m_blockToEHPreds = nullptr;
+ m_fieldSeqStore = nullptr;
+ m_refAnyClass = nullptr;
  for (MemoryKind memoryKind : allMemoryKinds())
  {
  m_memorySsaMap[memoryKind] = nullptr;
@@ -9626,11 +9625,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
  chars += printf("[GTF_ICON_STATIC_BOX_PTR]");
  break;
 
- case GTF_ICON_FIELD_OFF:
-
- chars += printf("[ICON_FIELD_OFF]");
- break;
-
  default:
  assert(!"a forgotten handle flag");
  break;

src/coreclr/jit/compiler.h

@@ -2266,7 +2266,7 @@ class Compiler
  GenTree* op2 = nullptr);
 
  GenTreeIntCon* gtNewIconNode(ssize_t value, var_types type = TYP_INT);
- GenTreeIntCon* gtNewIconNode(unsigned fieldOffset, FieldSeqNode* fieldSeq);
+ GenTreeIntCon* gtNewIconNode(unsigned fieldOffset, FieldSeq* fieldSeq);
  GenTreeIntCon* gtNewNull();
  GenTreeIntCon* gtNewTrue();
  GenTreeIntCon* gtNewFalse();
@@ -2277,7 +2277,7 @@ class Compiler
 
  GenTree* gtNewIndOfIconHandleNode(var_types indType, size_t value, GenTreeFlags iconFlags, bool isInvariant);
 
- GenTreeIntCon* gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields = nullptr);
+ GenTreeIntCon* gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeq* fields = nullptr);
 
  GenTreeFlags gtTokenToIconFlags(unsigned token);
 
@@ -2838,7 +2838,6 @@ class Compiler
  unsigned gtDispMultiRegCount(GenTree* tree);
 #endif
  void gtDispRegVal(GenTree* tree);
- void gtDispZeroFieldSeq(GenTree* tree);
  void gtDispVN(GenTree* tree);
  void gtDispCommonEndLine(GenTree* tree);
 
@@ -2876,8 +2875,7 @@ class Compiler
  void gtGetArgMsg(GenTreeCall* call, CallArg* arg, char* bufp, unsigned bufLength);
  void gtGetLateArgMsg(GenTreeCall* call, CallArg* arg, char* bufp, unsigned bufLength);
  void gtDispArgList(GenTreeCall* call, GenTree* lastCallOperand, IndentStack* indentStack);
- void gtDispAnyFieldSeq(FieldSeqNode* fieldSeq);
- void gtDispFieldSeq(FieldSeqNode* pfsn);
+ void gtDispFieldSeq(FieldSeq* fieldSeq, ssize_t offset);
 
  void gtDispRange(LIR::ReadOnlyRange const& range);
 
@@ -4806,14 +4804,10 @@ class Compiler
 
  void fgValueNumberArrayElemStore(GenTree* storeNode, VNFuncApp* addrFunc, unsigned storeSize, ValueNum value);
 
- void fgValueNumberFieldLoad(GenTree* loadTree, GenTree* baseAddr, FieldSeqNode* fieldSeq, ssize_t offset);
+ void fgValueNumberFieldLoad(GenTree* loadTree, GenTree* baseAddr, FieldSeq* fieldSeq, ssize_t offset);
 
- void fgValueNumberFieldStore(GenTree* storeNode,
- GenTree* baseAddr,
- FieldSeqNode* fieldSeq,
- ssize_t offset,
- unsigned storeSize,
- ValueNum value);
+ void fgValueNumberFieldStore(
+ GenTree* storeNode, GenTree* baseAddr, FieldSeq* fieldSeq, ssize_t offset, unsigned storeSize, ValueNum value);
 
  // Compute the value number for a byref-exposed load of the given type via the given pointerVN.
  ValueNum fgValueNumberByrefExposedLoad(var_types type, ValueNum pointerVN);
@@ -4928,7 +4922,6 @@ class Compiler
 
 #ifdef DEBUG
  void fgDebugCheckExceptionSets();
- void fgDebugCheckValueNumberedTree(GenTree* tree);
 #endif
 
  // These are the current value number for the memory implicit variables while
@@ -7076,11 +7069,7 @@ class Compiler
  GenTreeFlags iconFlags; // gtFlags
  };
  union {
- struct
- {
- SsaVar lcl;
- FieldSeqNode* zeroOffsetFieldSeq;
- };
+ SsaVar lcl;
  IntVal u1;
  __int64 lconVal;
  double dconVal;
@@ -7183,8 +7172,7 @@ class Compiler
 
  case O2K_LCLVAR_COPY:
  return (op2.lcl.lclNum == that->op2.lcl.lclNum) &&
- (!vnBased || op2.lcl.ssaNum == that->op2.lcl.ssaNum) &&
- (op2.zeroOffsetFieldSeq == that->op2.zeroOffsetFieldSeq);
+ (!vnBased || (op2.lcl.ssaNum == that->op2.lcl.ssaNum));
 
  case O2K_SUBRANGE:
  return op2.u2.Equals(that->op2.u2);
@@ -10486,54 +10474,20 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  void JitTestCheckVN(); // Value numbering tests.
 #endif // DEBUG
 
- // The "FieldSeqStore", for canonicalizing field sequences. See the definition of FieldSeqStore for
- // operations.
  FieldSeqStore* m_fieldSeqStore;
 
  FieldSeqStore* GetFieldSeqStore()
  {
  Compiler* compRoot = impInlineRoot();
  if (compRoot->m_fieldSeqStore == nullptr)
  {
- // Create a CompAllocator that labels sub-structure with CMK_FieldSeqStore, and use that for allocation.
- CompAllocator ialloc(getAllocator(CMK_FieldSeqStore));
- compRoot->m_fieldSeqStore = new (ialloc) FieldSeqStore(ialloc);
+ CompAllocator alloc = getAllocator(CMK_FieldSeqStore);
+ compRoot->m_fieldSeqStore = new (alloc) FieldSeqStore(alloc);
  }
  return compRoot->m_fieldSeqStore;
  }
 
- typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, FieldSeqNode*> NodeToFieldSeqMap;
-
- // Some nodes of "TYP_BYREF" or "TYP_I_IMPL" actually represent the address of a field within a struct, but since
- // the offset of the field is zero, there's no "GT_ADD" node. We normally attach a field sequence to the constant
- // that is added, but what do we do when that constant is zero, and is thus not present? We use this mechanism to
- // attach the field sequence directly to the address node.
- NodeToFieldSeqMap* m_zeroOffsetFieldMap;
-
- NodeToFieldSeqMap* GetZeroOffsetFieldMap()
- {
- // Don't need to worry about inlining here
- if (m_zeroOffsetFieldMap == nullptr)
- {
- // Create a CompAllocator that labels sub-structure with CMK_ZeroOffsetFieldMap, and use that for
- // allocation.
- CompAllocator ialloc(getAllocator(CMK_ZeroOffsetFieldMap));
- m_zeroOffsetFieldMap = new (ialloc) NodeToFieldSeqMap(ialloc);
- }
- return m_zeroOffsetFieldMap;
- }
-
- // Requires that "op1" is a node of type "TYP_BYREF" or "TYP_I_IMPL". We are dereferencing this with the fields in
- // "fieldSeq", whose offsets are required all to be zero. Ensures that any field sequence annotation currently on
- // "op1" or its components is augmented by appending "fieldSeq". In practice, if "op1" is a GT_LCL_FLD, it has
- // a field sequence as a member; otherwise, it may be the addition of an a byref and a constant, where the const
- // has a field sequence -- in this case "fieldSeq" is appended to that of the constant; otherwise, we
- // record the field sequence using the ZeroOffsetFieldMap described above.
- //
- // One exception above is that "op1" is a node of type "TYP_REF" where "op1" is a GT_LCL_VAR.
- // This happens when System.Object vtable pointer is a regular field at offset 0 in System.Private.CoreLib in
- // NativeAOT. Such case is handled same as the default case.
- void fgAddFieldSeqForZeroOffset(GenTree* op1, FieldSeqNode* fieldSeq);
+ typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, FieldSeq*> NodeToFieldSeqMap;
 
  NodeToUnsignedMap* m_memorySsaMap[MemoryKindCount];
 

src/coreclr/jit/compiler.hpp

@@ -902,15 +902,9 @@ inline GenTree* Compiler::gtNewLargeOperNode(genTreeOps oper, var_types type, Ge
  * that may need to be fixed up).
  */
 
-inline GenTreeIntCon* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields)
+inline GenTreeIntCon* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeq* fields)
 {
- assert((flags & (GTF_ICON_HDL_MASK | GTF_ICON_FIELD_OFF)) != 0);
-
- // Interpret "fields == NULL" as "not a field."
- if (fields == nullptr)
- {
- fields = FieldSeqStore::NotAField();
- }
+ assert((flags & GTF_ICON_HDL_MASK) != 0);
 
  GenTreeIntCon* node;
 #if defined(LATE_DISASM)
@@ -1429,7 +1423,7 @@ inline void GenTree::SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate)
  switch (oper)
  {
  case GT_CNS_INT:
- AsIntCon()->gtFieldSeq = FieldSeqStore::NotAField();
+ AsIntCon()->gtFieldSeq = nullptr;
  INDEBUG(AsIntCon()->gtTargetHandle = 0);
  break;
 #if defined(TARGET_ARM)
@@ -1611,7 +1605,7 @@ void GenTree::BashToConst(T value, var_types type /* = TYP_UNDEF */)
  }
 
  AsIntCon()->SetIconValue(static_cast<ssize_t>(value));
- AsIntCon()->gtFieldSeq = FieldSeqStore::NotAField();
+ AsIntCon()->gtFieldSeq = nullptr;
  break;
 
 #if !defined(TARGET_64BIT)

src/coreclr/jit/compmemkind.h

@@ -34,7 +34,6 @@ CompMemKindMacro(FixedBitVect)
 CompMemKindMacro(Generic)
 CompMemKindMacro(LocalAddressVisitor)
 CompMemKindMacro(FieldSeqStore)
-CompMemKindMacro(ZeroOffsetFieldMap)
 CompMemKindMacro(MemorySsaMap)
 CompMemKindMacro(MemoryPhiArg)
 CompMemKindMacro(CSE)