Skip to content

Commit ad9281d

Browse files
authored
Refactor fgInitArgInfo to track non standard arg kinds (#58324)
This changes the JIT to keep track of the non standard arg kinds that are added to the arg info table. We currently have several places (`fgResetArgInfo`, morph for tailcall-via-helpers) that make "blind" assumptions on which arg is which depending on certain flags set in the call. This change makes `fgResetArgInfo` more general and allows us to add asserts to the tailcall logic to verify that we are removing the args we expect.
1 parent e7765d9 commit ad9281d

File tree

3 files changed

+161
-78
lines changed

3 files changed

+161
-78
lines changed

src/coreclr/jit/compiler.h

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,6 +1461,26 @@ struct FuncInfoDsc
14611461
// that isn't shared between the main function body and funclets.
14621462
};
14631463

1464+
enum class NonStandardArgKind : unsigned
1465+
{
1466+
None,
1467+
PInvokeFrame,
1468+
PInvokeTarget,
1469+
PInvokeCookie,
1470+
WrapperDelegateCell,
1471+
ShiftLow,
1472+
ShiftHigh,
1473+
FixedRetBuffer,
1474+
VirtualStubCell,
1475+
R2RIndirectionCell,
1476+
1477+
// If changing this enum also change getNonStandardArgKindName and isNonStandardArgAddedLate below
1478+
};
1479+
1480+
#ifdef DEBUG
1481+
const char* getNonStandardArgKindName(NonStandardArgKind kind);
1482+
#endif
1483+
14641484
struct fgArgTabEntry
14651485
{
14661486
GenTreeCall::Use* use; // Points to the argument's GenTreeCall::Use in gtCallArgs or gtCallThisArg.
@@ -1521,17 +1541,18 @@ struct fgArgTabEntry
15211541
// struct is passed as a scalar type, this is that type.
15221542
// Note that if a struct is passed by reference, this will still be the struct type.
15231543

1524-
bool needTmp : 1; // True when we force this argument's evaluation into a temp LclVar
1525-
bool needPlace : 1; // True when we must replace this argument with a placeholder node
1526-
bool isTmp : 1; // True when we setup a temp LclVar for this argument due to size issues with the struct
1527-
bool processed : 1; // True when we have decided the evaluation order for this argument in the gtCallLateArgs
1528-
bool isBackFilled : 1; // True when the argument fills a register slot skipped due to alignment requirements of
1529-
// previous arguments.
1530-
bool isNonStandard : 1; // True if it is an arg that is passed in a reg other than a standard arg reg, or is forced
1531-
// to be on the stack despite its arg list position.
1532-
bool isStruct : 1; // True if this is a struct arg
1533-
bool _isVararg : 1; // True if the argument is in a vararg context.
1534-
bool passedByRef : 1; // True iff the argument is passed by reference.
1544+
bool needTmp : 1; // True when we force this argument's evaluation into a temp LclVar
1545+
bool needPlace : 1; // True when we must replace this argument with a placeholder node
1546+
bool isTmp : 1; // True when we setup a temp LclVar for this argument due to size issues with the struct
1547+
bool processed : 1; // True when we have decided the evaluation order for this argument in the gtCallLateArgs
1548+
bool isBackFilled : 1; // True when the argument fills a register slot skipped due to alignment requirements of
1549+
// previous arguments.
1550+
NonStandardArgKind nonStandardArgKind : 4; // The non-standard arg kind. Non-standard args are args that are forced
1551+
// to be in certain registers or on the stack, regardless of where they
1552+
// appear in the arg list.
1553+
bool isStruct : 1; // True if this is a struct arg
1554+
bool _isVararg : 1; // True if the argument is in a vararg context.
1555+
bool passedByRef : 1; // True iff the argument is passed by reference.
15351556
#ifdef FEATURE_ARG_SPLIT
15361557
bool _isSplit : 1; // True when this argument is split between the registers and OutArg area
15371558
#endif // FEATURE_ARG_SPLIT
@@ -1557,6 +1578,34 @@ struct fgArgTabEntry
15571578
#endif
15581579
}
15591580

1581+
bool isNonStandard() const
1582+
{
1583+
return nonStandardArgKind != NonStandardArgKind::None;
1584+
}
1585+
1586+
// Returns true if the IR node for this non-standarg arg is added by fgInitArgInfo.
1587+
// In this case, it must be removed by GenTreeCall::ResetArgInfo.
1588+
bool isNonStandardArgAddedLate() const
1589+
{
1590+
switch (nonStandardArgKind)
1591+
{
1592+
case NonStandardArgKind::None:
1593+
case NonStandardArgKind::PInvokeFrame:
1594+
case NonStandardArgKind::ShiftLow:
1595+
case NonStandardArgKind::ShiftHigh:
1596+
case NonStandardArgKind::FixedRetBuffer:
1597+
return false;
1598+
case NonStandardArgKind::WrapperDelegateCell:
1599+
case NonStandardArgKind::VirtualStubCell:
1600+
case NonStandardArgKind::PInvokeCookie:
1601+
case NonStandardArgKind::PInvokeTarget:
1602+
case NonStandardArgKind::R2RIndirectionCell:
1603+
return true;
1604+
default:
1605+
unreached();
1606+
}
1607+
}
1608+
15601609
bool isLateArg() const
15611610
{
15621611
bool isLate = (_lateArgInx != UINT_MAX);

src/coreclr/jit/gentree.cpp

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,18 +1226,31 @@ void GenTreeCall::ResetArgInfo()
12261226
// only sets up fgArgInfo, it also adds non-standard args to the IR, and we need
12271227
// to remove that extra IR so it doesn't get added again.
12281228
//
1229-
// NOTE: this doesn't handle all possible cases. There might be cases where we
1230-
// should be removing non-standard arg IR but currently aren't.
1231-
CLANG_FORMAT_COMMENT_ANCHOR;
1229+
unsigned argNum = 0;
1230+
if (gtCallThisArg != nullptr)
1231+
{
1232+
argNum++;
1233+
}
12321234

1233-
#if !defined(TARGET_X86)
1234-
if (IsVirtualStub())
1235+
Use** link = &gtCallArgs;
1236+
while ((*link) != nullptr)
12351237
{
1236-
JITDUMP("Removing VSD non-standard arg [%06u] to prepare for re-morphing call [%06u]\n",
1237-
Compiler::dspTreeID(gtCallArgs->GetNode()), gtTreeID);
1238-
gtCallArgs = gtCallArgs->GetNext();
1238+
const fgArgTabEntry* entry = fgArgInfo->GetArgEntry(argNum);
1239+
if (entry->isNonStandard() && entry->isNonStandardArgAddedLate())
1240+
{
1241+
JITDUMP("Removing non-standarg arg %s [%06u] to prepare for re-morphing call [%06u]\n",
1242+
getNonStandardArgKindName(entry->nonStandardArgKind), Compiler::dspTreeID((*link)->GetNode()),
1243+
gtTreeID);
1244+
1245+
*link = (*link)->GetNext();
1246+
}
1247+
else
1248+
{
1249+
link = &(*link)->NextRef();
1250+
}
1251+
1252+
argNum++;
12391253
}
1240-
#endif // !defined(TARGET_X86)
12411254

12421255
fgArgInfo = nullptr;
12431256
}

0 commit comments

Comments
 (0)