Enable checking of GTF_EXCEPT and GTF_ASG flags. (#13668)

* Enable checking of GTF_EXCEPT and GTF_ASG flags. fgDebugCheckFlags is modified to check that GTF_EXCEPT and GTF_ASG are set precisely when needed. It's also modified to handle several special operators correctly. fgAddrCouldBeNull is updated to check for handles, implicit byref locals, and stack byrefs. OperMayThrow is modified to handle several operators correctly. GTF_IND_NONFAULTING is reused on operations for which OperIsIndir() is true and on GT_ARR_LENGTH. Various places in morph are updated to set side effect flags correctly. gtUpdateSideEffects is re-written so that it's precise for GTF_ASG and GTF_EXCEPT and conservatively correct for the other side effects. It's now called from more places to keep the flags up-to-date after transformations. NoThrow in HelperCallProperties is updated and GTF_EXCEPT flag is set on helper calls according to that property. optRemoveRangeCheck is cleaned up and simplified.

Author: erozenfeld

src/jit/assertionprop.cpp

@@ -3421,7 +3421,7 @@ GenTreePtr Compiler::optAssertionProp_Comma(ASSERT_VALARG_TP assertions, const G
 if ((tree->gtGetOp1()->OperGet() == GT_ARR_BOUNDS_CHECK) &&
  ((tree->gtGetOp1()->gtFlags & GTF_ARR_BOUND_INBND) != 0))
  {
-optRemoveRangeCheck(tree, stmt, true, GTF_ASG, true/* force remove */);
+optRemoveRangeCheck(tree, stmt);
 returnoptAssertionProp_Update(tree, tree, stmt);
  }
 returnnullptr;
@@ -3475,6 +3475,7 @@ GenTreePtr Compiler::optAssertionProp_Ind(ASSERT_VALARG_TP assertions, const Gen
  }
 #endif
  tree->gtFlags &= ~GTF_EXCEPT;
+ tree->gtFlags |= GTF_IND_NONFAULTING;
 
 // Set this flag to prevent reordering
  tree->gtFlags |= GTF_ORDER_SIDEEFF;

src/jit/compiler.cpp

@@ -9765,7 +9765,7 @@ int cTreeFlagsIR(Compiler* comp, GenTree* tree)
 #endif
  if (tree->gtFlags & GTF_IND_NONFAULTING)
  {
- if ((op == GT_IND) || (op == GT_STOREIND))
+ if (tree->OperIsIndirOrArrLength())
  {
  chars += printf("[IND_NONFAULTING]");
  }

src/jit/compiler.h

@@ -2056,10 +2056,7 @@ class Compiler
  GenTreeArgList* args,
  IL_OFFSETX ilOffset = BAD_IL_OFFSET);
 
- GenTreeCall* gtNewHelperCallNode(unsigned helper,
- var_types type,
-unsigned flags = 0,
- GenTreeArgList* args = nullptr);
+ GenTreeCall* gtNewHelperCallNode(unsigned helper, var_types type, GenTreeArgList* args = nullptr);
 
  GenTreePtr gtNewLclvNode(unsigned lnum, var_types type, IL_OFFSETX ILoffs = BAD_IL_OFFSET);
 
@@ -2086,6 +2083,10 @@ class Compiler
 
  GenTreePtr gtNewIndexRef(var_types typ, GenTreePtr arrayOp, GenTreePtr indexOp);
 
+ GenTreeArrLen* gtNewArrLen(var_types typ, GenTree* arrayOp, int lenOffset);
+
+ GenTree* gtNewIndir(var_types typ, GenTree* addr);
+
  GenTreeArgList* gtNewArgList(GenTreePtr op);
  GenTreeArgList* gtNewArgList(GenTreePtr op1, GenTreePtr op2);
  GenTreeArgList* gtNewArgList(GenTreePtr op1, GenTreePtr op2, GenTreePtr op3);
@@ -2139,7 +2140,13 @@ class Compiler
 
  GenTreePtr gtReplaceTree(GenTreePtr stmt, GenTreePtr tree, GenTreePtr replacementTree);
 
-voidgtUpdateSideEffects(GenTreePtr tree, unsigned oldGtFlags, unsigned newGtFlags);
+voidgtUpdateSideEffects(GenTree* stmt, GenTree* tree);
+
+voidgtUpdateTreeAncestorsSideEffects(GenTree* tree);
+
+voidgtUpdateStmtSideEffects(GenTree* stmt);
+
+voidgtResetNodeSideEffects(GenTree* tree);
 
 // Returns "true" iff the complexity (not formally defined, but first interpretation
 // is #of nodes in subtree) of "tree" is greater than "limit".
@@ -4748,7 +4755,11 @@ class Compiler
 
 voidfgFixupStructReturn(GenTreePtr call);
  GenTreePtr fgMorphLocalVar(GenTreePtr tree, bool forceRemorph);
+
+public:
 boolfgAddrCouldBeNull(GenTreePtr addr);
+
+private:
  GenTreePtr fgMorphField(GenTreePtr tree, MorphAddrContext* mac);
 boolfgCanFastTailCall(GenTreeCall* call);
 voidfgMorphTailCall(GenTreeCall* call);
@@ -4913,6 +4924,9 @@ class Compiler
 voidfgMarkAddressExposedLocals();
 boolfgNodesMayInterfere(GenTree* store, GenTree* load);
 
+static fgWalkPreFn fgUpdateSideEffectsPre;
+static fgWalkPostFn fgUpdateSideEffectsPost;
+
 // Returns true if the type of tree is of size at least "width", or if "tree" is not a
 // local variable.
 boolfgFitsInOrNotLoc(GenTreePtr tree, unsigned width);
@@ -4956,8 +4970,7 @@ class Compiler
  LclVarDsc* optIsTrackedLocal(GenTreePtr tree);
 
 public:
-voidoptRemoveRangeCheck(
- GenTreePtr tree, GenTreePtr stmt, bool updateCSEcounts, unsigned sideEffFlags = 0, bool forceRemove = false);
+voidoptRemoveRangeCheck(GenTreePtr tree, GenTreePtr stmt);
 booloptIsRangeCheckRemovable(GenTreePtr tree);
 
 protected:

src/jit/compiler.hpp

@@ -1121,8 +1121,21 @@ inline GenTreePtr Compiler::gtNewIconEmbFldHndNode(CORINFO_FIELD_HANDLE fldHnd,
 
 /*****************************************************************************/
 
-inline GenTreeCall* Compiler::gtNewHelperCallNode(unsigned helper, var_types type, unsigned flags, GenTreeArgList* args)
+//------------------------------------------------------------------------------
+// gtNewHelperCallNode : Helper to create a call helper node.
+//
+//
+// Arguments:
+// helper - Call helper
+// type - Type of the node
+// args - Call args
+//
+// Return Value:
+// New CT_HELPER node
+
+inline GenTreeCall* Compiler::gtNewHelperCallNode(unsigned helper, var_types type, GenTreeArgList* args)
 {
+ unsigned flags = s_helperCallProperties.NoThrow((CorInfoHelpFunc)helper) ? 0 : GTF_EXCEPT;
  GenTreeCall* result = gtNewCallNode(CT_HELPER, eeFindHelper(helper), type, args);
  result->gtFlags |= flags;
 
@@ -1228,6 +1241,43 @@ inline GenTreePtr Compiler::gtNewIndexRef(var_types typ, GenTreePtr arrayOp, Gen
  return gtIndx;
 }
 
+//------------------------------------------------------------------------------
+// gtNewArrLen : Helper to create an array length node.
+//
+//
+// Arguments:
+// typ - Type of the node
+// arrayOp - Array node
+// lenOffset - Offset of the length field
+//
+// Return Value:
+// New GT_ARR_LENGTH node
+
+inline GenTreeArrLen* Compiler::gtNewArrLen(var_types typ, GenTree* arrayOp, int lenOffset)
+{
+ GenTreeArrLen* arrLen = new (this, GT_ARR_LENGTH) GenTreeArrLen(typ, arrayOp, lenOffset);
+ static_assert_no_msg(GTF_ARRLEN_NONFAULTING == GTF_IND_NONFAULTING);
+ arrLen->SetIndirExceptionFlags(this);
+ return arrLen;
+}
+
+//------------------------------------------------------------------------------
+// gtNewIndir : Helper to create an indirection node.
+//
+// Arguments:
+// typ - Type of the node
+// addr - Address of the indirection
+//
+// Return Value:
+// New GT_IND node
+
+inline GenTree* Compiler::gtNewIndir(var_types typ, GenTree* addr)
+{
+ GenTree* indir = gtNewOperNode(GT_IND, typ, addr);
+ indir->SetIndirExceptionFlags(this);
+ return indir;
+}
+
 /*****************************************************************************
  *
  * Create (and check for) a "nothing" node, i.e. a node that doesn't produce
@@ -1512,8 +1562,13 @@ inline void GenTree::ChangeOper(genTreeOps oper, ValueNumberUpdate vnUpdate)
 {
  assert(!OperIsConst(oper)); // use ChangeOperLeaf() instead
 
+ unsigned mask = GTF_COMMON_MASK;
+ if (this->OperIsIndirOrArrLength() && OperIsIndirOrArrLength(oper))
+ {
+ mask |= GTF_IND_NONFAULTING;
+ }
  SetOper(oper, vnUpdate);
- gtFlags &= GTF_COMMON_MASK;
+ gtFlags &= mask;
 
  // Do "oper"-specific initializations...
  switch (oper)
@@ -1529,8 +1584,13 @@ inline void GenTree::ChangeOper(genTreeOps oper, ValueNumberUpdate vnUpdate)
 
 inline void GenTree::ChangeOperUnchecked(genTreeOps oper)
 {
+ unsigned mask = GTF_COMMON_MASK;
+ if (this->OperIsIndirOrArrLength() && OperIsIndirOrArrLength(oper))
+ {
+ mask |= GTF_IND_NONFAULTING;
+ }
  SetOperRaw(oper); // Trust the caller and don't use SetOper()
- gtFlags &= GTF_COMMON_MASK;
+ gtFlags &= mask;
 }
 
 /*****************************************************************************

src/jit/copyprop.cpp

@@ -110,13 +110,17 @@ int Compiler::optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDs
 return score + ((preferOp2) ? 1 : -1);
 }
 
-/**************************************************************************************
- *
- * Perform copy propagation on a given tree as we walk the graph and if it is a local
- * variable, then look up all currently live definitions and check if any of those
- * definitions share the same value number. If so, then we can make the replacement.
- *
-*/
+//------------------------------------------------------------------------------
+// optCopyProp : Perform copy propagation on a given tree as we walk the graph and if it is a local
+// variable, then look up all currently live definitions and check if any of those
+// definitions share the same value number. If so, then we can make the replacement.
+//
+// Arguments:
+// block - Block the tree belongs to
+// stmt - Statement the tree belongs to
+// tree - The tree to perform copy propagation on
+// curSsaName - The map from lclNum to its recently live definitions as a stack
+
 voidCompiler::optCopyProp(BasicBlock* block, GenTreePtr stmt, GenTreePtr tree, LclNumToGenTreePtrStack* curSsaName)
 {
 // TODO-Review: EH successor/predecessor iteration seems broken.
@@ -259,6 +263,7 @@ void Compiler::optCopyProp(BasicBlock* block, GenTreePtr stmt, GenTreePtr tree,
  lvaTable[newLclNum].incRefCnts(block->getBBWeight(this), this);
  tree->gtLclVarCommon.SetLclNum(newLclNum);
  tree->AsLclVarCommon()->SetSsaNum(newSsaNum);
+gtUpdateSideEffects(stmt, tree);
 #ifdef DEBUG
 if (verbose)
  {
@@ -280,13 +285,15 @@ bool Compiler::optIsSsaLocal(GenTreePtr tree)
 return tree->IsLocal() && !fgExcludeFromSsa(tree->AsLclVarCommon()->GetLclNum());
 }
 
-/**************************************************************************************
- *
- * Perform copy propagation using currently live definitions on the current block's
- * variables. Also as new definitions are encountered update the "curSsaName" which
- * tracks the currently live definitions.
- *
-*/
+//------------------------------------------------------------------------------
+// optBlockCopyProp : Perform copy propagation using currently live definitions on the current block's
+// variables. Also as new definitions are encountered update the "curSsaName" which
+// tracks the currently live definitions.
+//
+// Arguments:
+// block - Block the tree belongs to
+// curSsaName - The map from lclNum to its recently live definitions as a stack
+
 voidCompiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName)
 {
 JITDUMP("Copy Assertion for BB%02u\n", block->bbNum);
@@ -302,6 +309,7 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
 for (GenTreePtr tree = stmt->gtStmt.gtStmtList; tree; tree = tree->gtNext)
  {
  compUpdateLife</*ForCodeGen*/false>(tree);
+
 optCopyProp(block, stmt, tree, curSsaName);
 
 // TODO-Review: Merge this loop with the following loop to correctly update the

src/jit/decomposelongs.cpp

@@ -1429,7 +1429,7 @@ GenTree* DecomposeLongs::DecomposeShift(LIR::Use& use)
 
  GenTreeArgList* argList = m_compiler->gtNewArgList(loOp1, hiOp1, shiftByOp);
 
- GenTree* call = m_compiler->gtNewHelperCallNode(helper, TYP_LONG, 0, argList);
+ GenTree* call = m_compiler->gtNewHelperCallNode(helper, TYP_LONG, argList);
  call->gtFlags |= shift->gtFlags & GTF_ALL_EFFECT;
 
  if (shift->IsUnusedValue())

src/jit/earlyprop.cpp

@@ -196,11 +196,12 @@ void Compiler::optEarlyProp()
  {
  if (optEarlyPropRewriteTree(tree))
  {
+ gtUpdateSideEffects(stmt, tree);
  isRewritten = true;
  }
  }
 
- // Morph the stmt and update the evaluation order if the stmt has been rewritten.
+ // Update the evaluation order and the statement info if the stmt has been rewritten.
  if (isRewritten)
  {
  gtSetStmtInfo(stmt);
@@ -611,6 +612,7 @@ void Compiler::optFoldNullCheck(GenTreePtr tree)
 
  // Set this flag to prevent reordering
  nullCheckTree->gtFlags |= GTF_ORDER_SIDEEFF;
+ nullCheckTree->gtFlags |= GTF_IND_NONFAULTING;
 
  defRHS->gtFlags &= ~(GTF_EXCEPT | GTF_DONT_CSE);
  defRHS->gtFlags |=