Skip to content

Commit

Permalink
Refactor fgInitArgInfo to track non standard arg kinds (#58324)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jakobbotsch authored Aug 30, 2021
1 parent e7765d9 commit ad9281d
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 78 deletions.
71 changes: 60 additions & 11 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1461,6 +1461,26 @@ struct FuncInfoDsc
// that isn't shared between the main function body and funclets.
};

enum class NonStandardArgKind : unsigned
{
None,
PInvokeFrame,
PInvokeTarget,
PInvokeCookie,
WrapperDelegateCell,
ShiftLow,
ShiftHigh,
FixedRetBuffer,
VirtualStubCell,
R2RIndirectionCell,

// If changing this enum also change getNonStandardArgKindName and isNonStandardArgAddedLate below
};

#ifdef DEBUG
const char* getNonStandardArgKindName(NonStandardArgKind kind);
#endif

struct fgArgTabEntry
{
GenTreeCall::Use* use; // Points to the argument's GenTreeCall::Use in gtCallArgs or gtCallThisArg.
Expand Down Expand Up @@ -1521,17 +1541,18 @@ struct fgArgTabEntry
// struct is passed as a scalar type, this is that type.
// Note that if a struct is passed by reference, this will still be the struct type.

bool needTmp : 1; // True when we force this argument's evaluation into a temp LclVar
bool needPlace : 1; // True when we must replace this argument with a placeholder node
bool isTmp : 1; // True when we setup a temp LclVar for this argument due to size issues with the struct
bool processed : 1; // True when we have decided the evaluation order for this argument in the gtCallLateArgs
bool isBackFilled : 1; // True when the argument fills a register slot skipped due to alignment requirements of
// previous arguments.
bool isNonStandard : 1; // True if it is an arg that is passed in a reg other than a standard arg reg, or is forced
// to be on the stack despite its arg list position.
bool isStruct : 1; // True if this is a struct arg
bool _isVararg : 1; // True if the argument is in a vararg context.
bool passedByRef : 1; // True iff the argument is passed by reference.
bool needTmp : 1; // True when we force this argument's evaluation into a temp LclVar
bool needPlace : 1; // True when we must replace this argument with a placeholder node
bool isTmp : 1; // True when we setup a temp LclVar for this argument due to size issues with the struct
bool processed : 1; // True when we have decided the evaluation order for this argument in the gtCallLateArgs
bool isBackFilled : 1; // True when the argument fills a register slot skipped due to alignment requirements of
// previous arguments.
NonStandardArgKind nonStandardArgKind : 4; // The non-standard arg kind. Non-standard args are args that are forced
// to be in certain registers or on the stack, regardless of where they
// appear in the arg list.
bool isStruct : 1; // True if this is a struct arg
bool _isVararg : 1; // True if the argument is in a vararg context.
bool passedByRef : 1; // True iff the argument is passed by reference.
#ifdef FEATURE_ARG_SPLIT
bool _isSplit : 1; // True when this argument is split between the registers and OutArg area
#endif // FEATURE_ARG_SPLIT
Expand All @@ -1557,6 +1578,34 @@ struct fgArgTabEntry
#endif
}

bool isNonStandard() const
{
return nonStandardArgKind != NonStandardArgKind::None;
}

// Returns true if the IR node for this non-standarg arg is added by fgInitArgInfo.
// In this case, it must be removed by GenTreeCall::ResetArgInfo.
bool isNonStandardArgAddedLate() const
{
switch (nonStandardArgKind)
{
case NonStandardArgKind::None:
case NonStandardArgKind::PInvokeFrame:
case NonStandardArgKind::ShiftLow:
case NonStandardArgKind::ShiftHigh:
case NonStandardArgKind::FixedRetBuffer:
return false;
case NonStandardArgKind::WrapperDelegateCell:
case NonStandardArgKind::VirtualStubCell:
case NonStandardArgKind::PInvokeCookie:
case NonStandardArgKind::PInvokeTarget:
case NonStandardArgKind::R2RIndirectionCell:
return true;
default:
unreached();
}
}

bool isLateArg() const
{
bool isLate = (_lateArgInx != UINT_MAX);
Expand Down
31 changes: 22 additions & 9 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1226,18 +1226,31 @@ void GenTreeCall::ResetArgInfo()
// only sets up fgArgInfo, it also adds non-standard args to the IR, and we need
// to remove that extra IR so it doesn't get added again.
//
// NOTE: this doesn't handle all possible cases. There might be cases where we
// should be removing non-standard arg IR but currently aren't.
CLANG_FORMAT_COMMENT_ANCHOR;
unsigned argNum = 0;
if (gtCallThisArg != nullptr)
{
argNum++;
}

#if !defined(TARGET_X86)
if (IsVirtualStub())
Use** link = &gtCallArgs;
while ((*link) != nullptr)
{
JITDUMP("Removing VSD non-standard arg [%06u] to prepare for re-morphing call [%06u]\n",
Compiler::dspTreeID(gtCallArgs->GetNode()), gtTreeID);
gtCallArgs = gtCallArgs->GetNext();
const fgArgTabEntry* entry = fgArgInfo->GetArgEntry(argNum);
if (entry->isNonStandard() && entry->isNonStandardArgAddedLate())
{
JITDUMP("Removing non-standarg arg %s [%06u] to prepare for re-morphing call [%06u]\n",
getNonStandardArgKindName(entry->nonStandardArgKind), Compiler::dspTreeID((*link)->GetNode()),
gtTreeID);

*link = (*link)->GetNext();
}
else
{
link = &(*link)->NextRef();
}

argNum++;
}
#endif // !defined(TARGET_X86)

fgArgInfo = nullptr;
}
Expand Down
Loading

0 comments on commit ad9281d

Please sign in to comment.