Add comments

src/coreclr/jit/flowgraph.cpp

@@ -62,29 +62,27 @@ static bool blockNeedsGCPoll(BasicBlock* block)
 
 PhaseStatus Compiler::fgInsertClsInitChecks()
 {
-
- if (!strcmp(info.compMethodName, "Test"))
- {
- fgDispBasicBlocks(true);
- }
-
  if (!opts.OptimizationEnabled())
  {
  return PhaseStatus::MODIFIED_NOTHING;
  }
 
  bool modified = false;
  BasicBlock* block;
+
+ BasicBlock* prevBb = nullptr;
  for (block = fgFirstBB; block; block = block->bbNext)
  {
- if (!block->isRunRarely() && block->bbFlags)
+ if (!block->isRunRarely())
  {
  for (Statement* stmt : block->Statements())
  {
  for (GenTree* tree = stmt->GetTreeList(); tree != nullptr; tree = tree->gtNext)
  {
- if (!tree->IsCall())
+ // we only need GT_CALL nodes with helper funcs
+ if (!tree->IsCall() || (tree->gtFlags & GTF_CALL_HOISTABLE))
  {
+ // TODO: remove that GTF_CALL_HOISTABLE check
  continue;
  }
 
@@ -95,20 +93,46 @@ PhaseStatus Compiler::fgInsertClsInitChecks()
  }
 
  CorInfoHelpFunc helpFunc = eeGetHelperNum(call->gtCallMethHnd);
- if (helpFunc != CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE)
+ if ((helpFunc != CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE) ||
+ (call->fgArgInfo->ArgCount() != 2))
  {
  continue;
  }
 
- GenTree* moduleIdArg = call->LateArgs().begin()->GetNode();
- GenTree* clsIdArg = call->LateArgs().begin()->GetNext()->GetNode();
+ GenTree* moduleIdArg = call->fgArgInfo->GetArgNode(0);
+ GenTree* clsIdArg = call->fgArgInfo->GetArgNode(1);
 
  if (!moduleIdArg->IsCnsIntOrI() || !clsIdArg->IsCnsIntOrI())
  {
  // Looks like moduleId or/and clsId were passed as indirect loads
  continue;
  }
 
+ if (clsIdArg->AsIntCon()->IconValue() < 0)
+ {
+ // Unknown clsId
+ continue;
+ }
+
+ if (prevBb == nullptr)
+ {
+ // We're going to emit a BB in front of fgFirstBB 
+ fgEnsureFirstBBisScratch();
+ prevBb = fgFirstBB;
+ if (prevBb == block)
+ {
+ continue;
+ }
+ }
+
+ // So, we found a helper call inside the "block" - let's extract it to a
+ // separate block "callInitBb" and guard it with a fast "isInitedBb" bb.
+ // The final layout should look like this:
+
+ // BB0 "prevBb":
+ // ...
+ //
+ // BB1 "isInitedBb": (preds: BB0 + %current preds of BB3%)
  //
  // * JTRUE void
  // \--* NE int
@@ -118,62 +142,119 @@ PhaseStatus Compiler::fgInsertClsInitChecks()
  // | \--* CNS_INT int isInitMask
  // \--* CNS_INT int 0
  //
+ //
+ // BB2 "callInitBb": (preds: BB1)
+ //
  // * CALL help long HELPER.CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
  // +--* CNS_INT long moduleIdArg
  // \--* CNS_INT int clsIdArg
  //
-
+ // BB3 "block" (preds: BB1, BB2)
+ // ...
+ //
+
+
+ // Let's start from emitting that BB2 "callInitBb"
  BasicBlock* callInitBb = fgNewBBbefore(BBJ_NONE, block, true);
- fgAddRefPred(block, callInitBb);
+ // it's executed only once so can be marked as cold
  callInitBb->bbSetRunRarely();
- callInitBb->bbFlags |= block->bbFlags & (BBF_SPLIT_GAINED | BBF_IMPORTED | BBF_HAS_CALL);
-
+ callInitBb->bbFlags |= (BBF_INTERNAL | BBF_HAS_CALL | BBF_HAS_LABEL);
  GenTree* clonedHelperCall = gtCloneExprCallHelper(call);
  clonedHelperCall->gtFlags |= call->gtFlags;
- fgInsertStmtAtEnd(callInitBb, fgNewStmtFromTree(clonedHelperCall));
 
+ Statement* callStmt = fgNewStmtFromTree(clonedHelperCall);
+ if (fgStmtListThreaded)
+ {
+ gtSetStmtInfo(callStmt);
+ fgSetStmtSeq(callStmt);
+ }
+ fgInsertStmtAtEnd(callInitBb, callStmt);
+ gtUpdateStmtSideEffects(callStmt);
+
+ // BB1 "isInitedBb"
  BasicBlock* isInitedBb = fgNewBBbefore(BBJ_COND, callInitBb, true);
- fgAddRefPred(callInitBb, isInitedBb);
- fgAddRefPred(block, isInitedBb);
  isInitedBb->inheritWeight(block);
- isInitedBb->bbFlags |= block->bbFlags & (BBF_SPLIT_GAINED | BBF_IMPORTED | BBF_HAS_CALL);
+ isInitedBb->bbFlags |= (BBF_INTERNAL | BBF_HAS_LABEL | BBF_HAS_JMP);
 
  // TODO: ask VM for these constants:
- const int dataBlobOffset = 48;
- const int isInitMask = 1;
+ const int dataBlobOffset = 48; // DomainLocalModule::GetOffsetOfDataBlob()
+ const int isInitMask = 1; // ClassInitFlags::INITIALIZED_FLAG;
 
  size_t address = moduleIdArg->AsIntCon()->IconValue() + dataBlobOffset + clsIdArg->AsIntCon()->IconValue();
  GenTree* indir = gtNewIndir(TYP_UBYTE, gtNewIconNode(address, TYP_I_IMPL));
  indir->gtFlags = (GTF_IND_NONFAULTING | GTF_IND_INVARIANT);
 
  GenTree* isInitedMask = gtNewOperNode(GT_AND, TYP_INT, indir, gtNewIconNode(isInitMask));
- GenTree* isInitedCmp = gtNewOperNode(GT_NE, TYP_INT, isInitedMask, gtNewIconNode(0));
+ GenTree* isInitedCmp = gtNewOperNode(GT_GT, TYP_INT, isInitedMask, gtNewIconNode(0));
+ isInitedCmp->gtFlags |= (GTF_UNSIGNED | GTF_RELOP_JMP_USED);
 
- fgInsertStmtAtEnd(isInitedBb, fgNewStmtFromTree(gtNewOperNode(GT_JTRUE, TYP_VOID, isInitedCmp)));
+ Statement* isInitedStmt = fgNewStmtFromTree(gtNewOperNode(GT_JTRUE, TYP_VOID, isInitedCmp));
+ if (fgStmtListThreaded)
+ {
+ gtSetStmtInfo(isInitedStmt);
+ fgSetStmtSeq(isInitedStmt);
+ }
+
+ fgInsertStmtAtEnd(isInitedBb, isInitedStmt);
  isInitedBb->bbJumpDest = block;
- gtReplaceTree(stmt, call, gtNewNothingNode());
+ block->bbFlags |= BBF_JMP_TARGET;
+
+ // Now we can remove the call from the current block
+ // We're going to replace the call with just "moduleId" node (it's what it was supposed to return)
+ gtReplaceTree(stmt, call, gtNewIconNode(moduleIdArg->AsIntCon()->IconValue(), call->TypeGet()));
+
+ // Now we need to fix all the preds:
+
+ // isInitedBb is a pred of callInitBb
+ fgAddRefPred(callInitBb, isInitedBb);
+ for (flowList* pred = block->bbPreds; pred != nullptr; pred = pred->flNext)
+ {
+ // Redirect all the preds from the current block to isInitedBb
+
+ // TODO: should I check EH region here?
+ // TODO: should I update some loop info if I'm inside a loop?
+
+ BasicBlock* predBlock = pred->getBlock();
+ if (predBlock->bbJumpDest == block)
+ {
+ predBlock->bbJumpDest = isInitedBb;
+ isInitedBb->bbFlags |= BBF_JMP_TARGET;
+ }
+ fgRemoveRefPred(block, predBlock);
+ fgAddRefPred(isInitedBb, predBlock);
+ }
+ // Both callInitBb and isInitedBb are preds of block now
+ fgAddRefPred(block, callInitBb);
+ fgAddRefPred(block, isInitedBb);
 
+ // Make sure all three basic blocks are in the same EH region:
+ BasicBlock::sameEHRegion(callInitBb, block);
+ BasicBlock::sameEHRegion(isInitedBb, block);
 
  modified = true;
  }
  if (modified)
  {
- // clear GTF_CALL and GTF_EXC flags
+ // clear GTF_CALL and GTF_EXC flags (we've just removed a call)
  gtUpdateStmtSideEffects(stmt);
  }
  }
  }
- }
-
- if (!strcmp(info.compMethodName, "Test"))
- {
- fgDispBasicBlocks(true);
+ prevBb = block;
  }
 
  if (modified)
  {
+#ifdef DEBUG
+ if (verbose)
+ {
+ printf("\nAfter fgInsertClsInitChecks:");
+ fgDispBasicBlocks(true);
+ }
+#endif // DEBUG
  fgReorderBlocks();
- fgUpdateChangedFlowGraph(false);
+ constexpr bool computeDoms = false;
+ fgUpdateChangedFlowGraph(computeDoms);
  return PhaseStatus::MODIFIED_EVERYTHING;
  }
  return PhaseStatus::MODIFIED_NOTHING;