JIT: Refactor call argument representation (#67238)

This refactors the JIT's representation of call arguments. It replaces
`GenTreeCall::Use` and `fgArgTabEntry` with a single class `CallArg`.
`CallArg` always contains space for ABI information and contains two
intrusive linked list nodes: one for all args (similar to current
`gtCallArgs`) where all standard arguments are always in argument order,
and one for late args (similar to current `gtCallLateArgs`) that may be
reordered. The late args list may also not contain all arguments.

`fgArgInfo` is also replaced by a new class `CallArgs` that is stored
inline in `GenTreeCall`. It encapsulates all handling of arguments
(insertion/removal). The change also begins treating the 'this' argument
as a normal argument with `CallArgs` providing convenient access to it
when necessary.

The main benefit of this change is to avoid keeping track of the side
table `fgArgInfo` and having to scan through this side table repeatedly
when we need to query argument information. In addition it gives more
convenient ways to access well known arguments like 'this', the ret
buffer, VSD cell, etc. Finally, it also serves as a nice clean-up in the JIT.

Author: jakobbotsch

docs/design/coreclr/jit/jit-call-morphing.md

@@ -43,18 +43,21 @@ call site and is created by the Importer when it sees a call in the IL.  It is a
 used for internal calls that the JIT needs such as helper calls.  Every `GT_CALL` node
 should have a `GTF_CALL` flag set on it.  Nodes that may be implemented using a function
 call also should have the `GTF_CALL` flag set on them. The arguments for a single call
-site are held by three fields in the `GenTreeCall`: `gtCallObjp`, `gtCallArgs`, and
-`gtCallLateArgs`.  The first one, `gtCallObjp`, contains the instance pointer ("this"
-pointer) when you are calling a method that takes an instance pointer, otherwise it is
-null.  The `gtCallArgs` contains all of the normal arguments in a null terminated `GT_LIST`
-format.  When the `GenTreeCall` is first created the `gtCallLateArgs` is null and is
-set up later when we call `fgMorphArgs()` during the global Morph of all nodes. To
-accurately record and track all of the information about call site arguments we create
-a `fgArgInfo` that records information and decisions that we make about how each argument
-for this call site is handled.  It has a dynamically sized array member `argTable` that
-contains details about each argument. This per-argument information is contained in the
-`fgArgTabEntry` struct.
-
+site are encapsulated in the `CallArgs` class. Every call has an instance of this class
+in `GenTreeCall::gtArgs`. `CallArgs` contains two linked list of arguments: the "normal"
+linked list, which can be enumerated via `CallArgs::Args`, and the "late" linked list,
+enumerated via `CallArgs::LateArgs`.
+
+The normal linked list is a linked list of `CallArg` structures, in normal argument
+order. When the `GenTreeCall` is first created the late args list is empty and is
+set up later when we call `fgMorphArgs()` during the global Morph of all nodes. The short
+explanation of why we need two lists is that we may need to force the correct evaluation
+order of arguments and also architecture-specific ways of passing some arguments. See
+below and the documentation of `fgMorphArgs` and `AddFinalArgsAndDetermineABIInfo` for
+more information about late args.
+
+In addition to containing IR nodes, each `CallArg` entry also contains information about
+how it was evaluated and ABI information describing how to pass it.
 
 `FEATURE_FIXED_OUT_ARGS`
 -----------------
@@ -77,9 +80,9 @@ calls for x86 but do not allow them for the other architectures.
 Rules for when Arguments must be evaluated into temp LclVars
 -----------------
 
-During the first Morph phase known as global Morph we call `fgArgInfo::ArgsComplete()`
-after we have completed building the `argTable` for `fgArgInfo` struct. This method
-applies the following rules:
+During the first Morph phase known as global Morph we call `CallArgs::ArgsComplete()`
+after we have completed determining ABI information for each arg. This method applies
+the following rules:
 
 1. When an argument is marked as containing an assignment using `GTF_ASG`, then we
 force all previous non-constant arguments to be evaluated into temps.  This is very
@@ -93,7 +96,7 @@ we force that argument and any previous argument that is marked with any of the
     area is marked as needing a placeholder temp using `needPlace`.
 3. We force any arguments that use `localloc` to be evaluated into temps.
 4. We mark any address taken locals with the `GTF_GLOB_REF` flag. For two special
-cases we call `EvalToTmp()` and set up the temp in `fgMorphArgs`.  `EvalToTmp`
+cases we call `SetNeedsTemp()` and set up the temp in `fgMorphArgs`. `SetNeedsTemp`
 records the tmpNum used and sets `isTmp` so that we handle it like the other temps.
 The special cases are for `GT_MKREFANY` and for a `TYP_STRUCT` argument passed by
 value when we can't optimize away the extra copy.
@@ -104,7 +107,7 @@ Rules use to determine the order of argument evaluation
 
 After calling `ArgsComplete()` the `SortArgs()` method is called to determine the
 optimal way to evaluate the arguments.  This sorting controls the order that we place
-the nodes in the `gtCallLateArgs` list.
+the nodes in the late argument list.
 
 1. We iterate over the arguments and move any constant arguments to be evaluated
 last and remove them from further consideration by marking them as processed.
@@ -123,35 +126,35 @@ Evaluating Args into new LclVar temps and the creation of the LateArgs
 After calling `SortArgs()`, the `EvalArgsToTemps()` method is called to create
 the temp assignments and to populate the LateArgs list.
 
-Arguments that are marked with `needTmp == true`.
+For arguments that are marked as needing a temp:
 -----------------
 
 1. We create an assignment using `gtNewTempAssign`. This assignment replaces
-the original argument in the `gtCallArgs` list.  After we create the assignment
-the argument is marked as `isTmp`.  The new assignment is marked with the
+the original argument in the early argument list.  After we create the assignment
+the argument is marked with `m_isTmp = true`.  The new assignment is marked with the
 `GTF_LATE_ARG` flag.
-2. Arguments that are already marked with `isTmp` are treated similarly as
+2. Arguments that are already marked with `m_isTmp` are treated similarly as
 above except we don't create an assignment for them.
-3. A `TYP_STRUCT` argument passed by value will have `isTmp` set to true
+3. A `TYP_STRUCT` argument passed by value will have `m_isTmp` set to true
 and will use a `GT_COPYBLK` or a `GT_COPYOBJ` to perform the assignment of the temp.
 4. The assignment node or the CopyBlock node is referred to as `arg1 SETUP` in the JitDump.
 
 
-Argument that are marked with `needTmp == false`.
+For arguments that are marked as not needing a temp:
 -----------------
 
 1. If this is an argument that is passed in a register, then the existing
-node is moved to the `gtCallLateArgs` list and a new `GT_ARGPLACE` (placeholder)
-node replaces it in the `gtArgList` list.
-2. Additionally, if `needPlace` is true (only for `FEATURE_FIXED_OUT_ARGS`)
-then the existing node is moved to the `gtCallLateArgs` list and a new
-`GT_ARGPLACE` (placeholder) node replaces it in the `gtArgList` list.
-3. Otherwise the argument is left in the `gtCallArgs` and it will be
-evaluated into the outgoing arg area or pushed on the stack.
+node is moved to the late argument list and a new `GT_ARGPLACE` (placeholder)
+node replaces it in the early argument list.
+2. Additionally, if `m_needPlace` is true (only for `FEATURE_FIXED_OUT_ARGS`)
+then the existing node is moved to the late argument list and a new
+`GT_ARGPLACE` (placeholder) node replaces it in the `early argument list.
+3. Otherwise the argument is left in the early argument and it will be
+evaluated directly into the outgoing arg area or pushed on the stack.
 
 After the Call node is fully morphed the LateArgs list will contain the arguments
-passed in registers as well as additional ones for `needPlace` marked
+passed in registers as well as additional ones for `m_needPlace` marked
 arguments whenever we have a nested call for a stack based argument.
-When `needTmp` is true the LateArg will be a LclVar that was created
-to evaluate the arg (single-def/single-use).  When `needTmp` is false
+When `m_needTmp` is true the LateArg will be a LclVar that was created
+to evaluate the arg (single-def/single-use).  When `m_needTmp` is false
 the LateArg can be an arbitrary expression tree.

docs/design/coreclr/jit/struct-abi.md

@@ -50,7 +50,8 @@ when it comes to the handling of structs (aka value types).
 
 High-Level Proposed Design
 --------------------------
-This is a preliminary design, and is likely to change as the implementation proceeds:
+Note that much of the below work has already been carried out and further refactoring has replaced the side `fgArgInfo` table with `CallArgs`.
+The plan here is intended to provide some historical context and may not completely reflect JIT sources.
 
 First, the `fgArgInfo` is extended to contain all the information needed to determine
 how an argument is passed. Ideally, most of the `#ifdef`s relating to ABI differences

src/coreclr/inc/corinfo.h

@@ -1115,7 +1115,7 @@ struct CORINFO_SIG_INFO
     CorInfoCallConv     getCallConv()       { return CorInfoCallConv((callConv & CORINFO_CALLCONV_MASK)); }
     bool                hasThis()           { return ((callConv & CORINFO_CALLCONV_HASTHIS) != 0); }
     bool                hasExplicitThis()   { return ((callConv & CORINFO_CALLCONV_EXPLICITTHIS) != 0); }
-    unsigned            totalILArgs()       { return (numArgs + hasThis()); }
+    unsigned            totalILArgs()       { return (numArgs + (hasThis() ? 1 : 0)); }
     bool                isVarArg()          { return ((getCallConv() == CORINFO_CALLCONV_VARARG) || (getCallConv() == CORINFO_CALLCONV_NATIVEVARARG)); }
     bool                hasTypeArg()        { return ((callConv & CORINFO_CALLCONV_PARAMTYPE) != 0); }
 };

src/coreclr/jit/assertionprop.cpp

@@ -2548,9 +2548,8 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree)
         (call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ISINSTANCEOFCLASS)) ||
         (call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ISINSTANCEOFANY)))
     {
-        fgArgInfo* const argInfo         = call->fgArgInfo;
-        GenTree*         objectNode      = argInfo->GetArgNode(1);
-        GenTree*         methodTableNode = argInfo->GetArgNode(0);
+        GenTree* objectNode      = call->gtArgs.GetArgByIndex(1)->GetNode();
+        GenTree* methodTableNode = call->gtArgs.GetArgByIndex(0)->GetNode();
 
         assert(objectNode->TypeGet() == TYP_REF);
         assert(methodTableNode->TypeGet() == TYP_I_IMPL);
@@ -2697,7 +2696,7 @@ void Compiler::optAssertionGen(GenTree* tree)
             if (call->NeedsNullCheck() || (call->IsVirtual() && !call->IsTailCall()))
             {
                 //  Retrieve the 'this' arg.
-                GenTree* thisArg = gtGetThisArg(call);
+                GenTree* thisArg = call->gtArgs.GetThisArg()->GetNode();
                 assert(thisArg != nullptr);
                 assertionInfo = optCreateAssertion(thisArg, nullptr, OAK_NOT_EQUAL);
             }
@@ -4485,7 +4484,7 @@ GenTree* Compiler::optNonNullAssertionProp_Call(ASSERT_VALARG_TP assertions, Gen
     {
         return nullptr;
     }
-    GenTree* op1 = gtGetThisArg(call);
+    GenTree* op1 = call->gtArgs.GetThisArg()->GetNode();
     noway_assert(op1 != nullptr);
     if (op1->gtOper != GT_LCL_VAR)
     {
@@ -4544,13 +4543,13 @@ GenTree* Compiler::optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCal
             call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_CHKCASTANY) ||
             call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_CHKCASTCLASS_SPECIAL))
         {
-            GenTree* arg1 = gtArgEntryByArgNum(call, 1)->GetNode();
+            GenTree* arg1 = call->gtArgs.GetArgByIndex(1)->GetNode();
             if (arg1->gtOper != GT_LCL_VAR)
             {
                 return nullptr;
             }
 
-            GenTree* arg2 = gtArgEntryByArgNum(call, 0)->GetNode();
+            GenTree* arg2 = call->gtArgs.GetArgByIndex(0)->GetNode();
 
             unsigned index = optAssertionIsSubtype(arg1, arg2, assertions);
             if (index != NO_ASSERTION_INDEX)

src/coreclr/jit/codegen.h

@@ -1365,7 +1365,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 #endif
     bool genEmitOptimizedGCWriteBarrier(GCInfo::WriteBarrierForm writeBarrierForm, GenTree* addr, GenTree* data);
     GenTree* getCallTarget(const GenTreeCall* call, CORINFO_METHOD_HANDLE* methHnd);
-    regNumber getCallIndirectionCellReg(const GenTreeCall* call);
+    regNumber getCallIndirectionCellReg(GenTreeCall* call);
     void genCall(GenTreeCall* call);
     void genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackArgBytes));
     void genJmpMethod(GenTree* jmp);

src/coreclr/jit/codegenarmarch.cpp

@@ -763,9 +763,9 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
     unsigned argOffsetOut = treeNode->getArgOffset();
 
 #ifdef DEBUG
-    fgArgTabEntry* curArgTabEntry = compiler->gtArgEntryByNode(treeNode->gtCall, treeNode);
-    assert(curArgTabEntry != nullptr);
-    DEBUG_ARG_SLOTS_ASSERT(argOffsetOut == (curArgTabEntry->slotNum * TARGET_POINTER_SIZE));
+    CallArg* callArg = treeNode->gtCall->gtArgs.FindByNode(treeNode);
+    assert(callArg != nullptr);
+    DEBUG_ARG_SLOTS_ASSERT(argOffsetOut == (callArg->AbiInfo.SlotNum * TARGET_POINTER_SIZE));
 #endif // DEBUG
 
     // Whether to setup stk arg in incoming or out-going arg area?
@@ -3125,23 +3125,21 @@ void CodeGen::genCodeForInitBlkHelper(GenTreeBlk* initBlkNode)
 void CodeGen::genCall(GenTreeCall* call)
 {
     // Consume all the arg regs
-    for (GenTreeCall::Use& use : call->LateArgs())
+    for (CallArg& arg : call->gtArgs.LateArgs())
     {
-        GenTree* argNode = use.GetNode();
-
-        fgArgTabEntry* curArgTabEntry = compiler->gtArgEntryByNode(call, argNode);
-        assert(curArgTabEntry);
+        CallArgABIInformation& abiInfo = arg.AbiInfo;
+        GenTree*               argNode = arg.GetLateNode();
 
         // GT_RELOAD/GT_COPY use the child node
         argNode = argNode->gtSkipReloadOrCopy();
 
-        if (curArgTabEntry->GetRegNum() == REG_STK)
+        if (abiInfo.GetRegNum() == REG_STK)
             continue;
 
         // Deal with multi register passed struct args.
         if (argNode->OperGet() == GT_FIELD_LIST)
         {
-            regNumber argReg = curArgTabEntry->GetRegNum();
+            regNumber argReg = abiInfo.GetRegNum();
             for (GenTreeFieldList::Use& use : argNode->AsFieldList()->Uses())
             {
                 GenTree* putArgRegNode = use.GetNode();
@@ -3162,22 +3160,22 @@ void CodeGen::genCall(GenTreeCall* call)
 #endif // TARGET_ARM
             }
         }
-        else if (curArgTabEntry->IsSplit())
+        else if (abiInfo.IsSplit())
         {
             assert(compFeatureArgSplit());
-            assert(curArgTabEntry->numRegs >= 1);
+            assert(abiInfo.NumRegs >= 1);
             genConsumeArgSplitStruct(argNode->AsPutArgSplit());
-            for (unsigned idx = 0; idx < curArgTabEntry->numRegs; idx++)
+            for (unsigned idx = 0; idx < abiInfo.NumRegs; idx++)
             {
-                regNumber argReg   = (regNumber)((unsigned)curArgTabEntry->GetRegNum() + idx);
+                regNumber argReg   = (regNumber)((unsigned)abiInfo.GetRegNum() + idx);
                 regNumber allocReg = argNode->AsPutArgSplit()->GetRegNumByIdx(idx);
                 inst_Mov_Extend(argNode->TypeGet(), /* srcInReg */ true, argReg, allocReg, /* canSkip */ true,
                                 emitActualTypeSize(TYP_I_IMPL));
             }
         }
         else
         {
-            regNumber argReg = curArgTabEntry->GetRegNum();
+            regNumber argReg = abiInfo.GetRegNum();
             genConsumeReg(argNode);
             inst_Mov_Extend(argNode->TypeGet(), /* srcInReg */ true, argReg, argNode->GetRegNum(), /* canSkip */ true,
                             emitActualTypeSize(TYP_I_IMPL));
@@ -3409,12 +3407,11 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
             trashedByEpilog |= genRegMask(REG_GSCOOKIE_TMP_1);
         }
 
-        for (unsigned i = 0; i < call->fgArgInfo->ArgCount(); i++)
+        for (CallArg& arg : call->gtArgs.Args())
         {
-            fgArgTabEntry* entry = call->fgArgInfo->GetArgEntry(i);
-            for (unsigned j = 0; j < entry->numRegs; j++)
+            for (unsigned j = 0; j < arg.AbiInfo.NumRegs; j++)
             {
-                regNumber reg = entry->GetRegNum(j);
+                regNumber reg = arg.AbiInfo.GetRegNum(j);
                 if ((trashedByEpilog & genRegMask(reg)) != 0)
                 {
                     JITDUMP("Tail call node:\n");

src/coreclr/jit/codegencommon.cpp

@@ -6488,38 +6488,29 @@ GenTree* CodeGen::getCallTarget(const GenTreeCall* call, CORINFO_METHOD_HANDLE*
 // Notes:
 //   We currently use indirection cells for VSD on all platforms and for R2R calls on ARM architectures.
 //
-regNumber CodeGen::getCallIndirectionCellReg(const GenTreeCall* call)
+regNumber CodeGen::getCallIndirectionCellReg(GenTreeCall* call)
 {
     regNumber result = REG_NA;
     switch (call->GetIndirectionCellArgKind())
     {
-        case NonStandardArgKind::None:
+        case WellKnownArg::None:
             break;
-        case NonStandardArgKind::R2RIndirectionCell:
+        case WellKnownArg::R2RIndirectionCell:
             result = REG_R2R_INDIRECT_PARAM;
             break;
-        case NonStandardArgKind::VirtualStubCell:
+        case WellKnownArg::VirtualStubCell:
             result = compiler->virtualStubParamInfo->GetReg();
             break;
         default:
             unreached();
     }
 
 #ifdef DEBUG
-    regNumber       foundReg = REG_NA;
-    unsigned        argCount = call->fgArgInfo->ArgCount();
-    fgArgTabEntry** argTable = call->fgArgInfo->ArgTable();
-    for (unsigned i = 0; i < argCount; i++)
+    if (call->GetIndirectionCellArgKind() != WellKnownArg::None)
     {
-        NonStandardArgKind kind = argTable[i]->nonStandardArgKind;
-        if ((kind == NonStandardArgKind::R2RIndirectionCell) || (kind == NonStandardArgKind::VirtualStubCell))
-        {
-            foundReg = argTable[i]->GetRegNum();
-            break;
-        }
+        CallArg* indirCellArg = call->gtArgs.FindWellKnownArg(call->GetIndirectionCellArgKind());
+        assert((indirCellArg != nullptr) && (indirCellArg->AbiInfo.GetRegNum() == result));
     }
-
-    assert(foundReg == result);
 #endif
 
     return result;