Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISC-V] Use common CodeGen::genHomeRegisterParams #101288

Merged
merged 16 commits into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/coreclr/jit/abi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,16 @@ bool ABIPassingInformation::HasExactlyOneStackSegment() const
//
bool ABIPassingInformation::IsSplitAcrossRegistersAndStack() const
{
bool anyReg = false;
bool anyStack = false;
for (unsigned i = 0; i < NumSegments; i++)
if (NumSegments < 2)
return false;

bool isFirstInReg = Segments[0].IsPassedInRegister();
for (unsigned i = 1; i < NumSegments; i++)
{
anyReg |= Segments[i].IsPassedInRegister();
anyStack |= Segments[i].IsPassedOnStack();
if (isFirstInReg != Segments[i].IsPassedInRegister())
return true;
}
return anyReg && anyStack;
return false;
}

//-----------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/abi.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ struct ABIPassingInformation
// - On loongarch64/riscv64, structs can be passed in two registers or
// can be split out over register and stack, giving
// multiple register segments and a struct segment.
unsigned NumSegments = 0;
ABIPassingSegment* Segments = nullptr;
unsigned NumSegments;
ABIPassingSegment* Segments;

ABIPassingInformation(unsigned numSegments = 0, ABIPassingSegment* segments = nullptr)
: NumSegments(numSegments)
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ class CodeGen final : public CodeGenInterface

void genHomeSwiftStructParameters(bool handleStack);

void genHomeStackPartOfSplitParameter(unsigned lclNum);

void genCheckUseBlockInit();
#if defined(UNIX_AMD64_ABI) && defined(FEATURE_SIMD)
void genClearStackVec3ArgUpperBits();
Expand Down
40 changes: 37 additions & 3 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2802,7 +2802,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
*/

#if !defined(TARGET_RISCV64)
struct RegNode;

struct RegNodeEdge
Expand Down Expand Up @@ -3069,6 +3068,7 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph)

unsigned baseOffset = varDsc->lvIsStructField ? varDsc->lvFldOffset : 0;
unsigned size = varDsc->lvExactSize();
bool isSpilled = false;

unsigned paramLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum;
LclVarDsc* paramVarDsc = compiler->lvaGetDesc(paramLclNum);
Expand Down Expand Up @@ -3102,6 +3102,7 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph)

GetEmitter()->emitIns_S_R(ins_Store(storeType), emitActualTypeSize(storeType), seg.GetRegister(), lclNum,
seg.Offset - baseOffset);
isSpilled = true;
}

if (!varDsc->lvIsInReg())
Expand Down Expand Up @@ -3138,6 +3139,11 @@ void CodeGen::genSpillOrAddRegisterParam(unsigned lclNum, RegGraph* graph)
graph->AddEdge(sourceReg, destReg, edgeType, seg.Offset - baseOffset);
}
}

if (isSpilled)
{
genHomeStackPartOfSplitParameter(lclNum);
}
Copy link
Member

@jakobbotsch jakobbotsch Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this handling outside genHomeRegisterParams, before it is called? Prolog generation becomes much more constrained after genHomeRegisterParams has run, and this handling doesn't have anything to do with moving registers to their initial locations. I would also switch to use initReg instead of REG_SCRATCH for the same reason (using REG_SCRATCH wouldn't work on other platforms at the point this runs, for example. I guess it works because RISCV64 never allocates REG_SCRATCH for anything, but on other platforms that's not a valid register to clobber after genHomeRegisterParams.).

You can factor the code in the stack case from genHomeSwiftStructParameters to avoid duplicating that logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking was split parameter homing can happen can only if there are some argument registers to home (otherwise the parameter wouldn't be split), but ok, it can also be outside.

}

// -----------------------------------------------------------------------------
Expand All @@ -3157,6 +3163,8 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed)
}
#endif

// RISC-V specific logic for homing the stack part of a split struct

tomeksowi marked this conversation as resolved.
Show resolved Hide resolved
regMaskTP paramRegs = intRegState.rsCalleeRegArgMaskLiveIn | floatRegState.rsCalleeRegArgMaskLiveIn;
if (compiler->opts.OptimizationDisabled())
{
Expand All @@ -3181,6 +3189,8 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed)
lclNum, seg.Offset);
}
}

genHomeStackPartOfSplitParameter(lclNum);
}

return;
Expand Down Expand Up @@ -3346,8 +3356,6 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed)
}
}

#endif

// -----------------------------------------------------------------------------
// genGetParameterHomingTempRegisterCandidates: Get the registers that are
// usable during register homing.
Expand Down Expand Up @@ -4226,6 +4234,32 @@ void CodeGen::genHomeSwiftStructParameters(bool handleStack)
}
#endif

/*-----------------------------------------------------------------------------
*
* Home the tail (stack) portion of a split parameter next to where the head (register) portion is homed.
*
* No-op on platforms where argument registers are already homed to form a contiguous space with incoming stack.
*/
void CodeGen::genHomeStackPartOfSplitParameter(unsigned lclNum)
{
#ifdef TARGET_RISCV64
const LclVarDsc* var = compiler->lvaGetDesc(lclNum);
if (!var->lvIsSplit)
return;

assert(varTypeIsStruct(var));
const ABIPassingInformation& abiInfo = compiler->lvaGetParameterABIInfo(lclNum);
assert(abiInfo.NumSegments == 2);
assert(abiInfo.Segments[0].GetRegister() == REG_ARG_LAST);
const ABIPassingSegment& seg = abiInfo.Segments[1];

int loadOffset =
-(isFramePointerUsed() ? genCallerSPtoFPdelta() : genCallerSPtoInitialSPdelta()) + (int)seg.GetStackOffset();
genInstrWithConstant(ins_Load(TYP_LONG), EA_8BYTE, REG_SCRATCH, genFramePointerReg(), loadOffset, REG_SCRATCH);
GetEmitter()->emitIns_S_R(ins_Store(TYP_LONG), EA_8BYTE, REG_SCRATCH, lclNum, seg.Offset);
#endif
}

/*-----------------------------------------------------------------------------
*
* Save the generic context argument.
Expand Down
Loading