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][x64] WiP: Passing empty struct fields #101796

Closed
wants to merge 70 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
c2c2bba
First, add a bunch of failing tests
tomeksowi May 2, 2024
03d4b23
Reduce empty megabyte field to 32k as msvc caps size of arguments at 64k
tomeksowi May 6, 2024
4403c4b
Don't stop calculating flags if struct size > 16 bytes
tomeksowi May 6, 2024
bf81116
Classify empty structs on x64 like padding
tomeksowi May 8, 2024
60ad834
Quell C-linkage warnings just in case
tomeksowi May 8, 2024
9064784
Add Pack=1 to bypass a known problem with field alignment requirement…
tomeksowi May 9, 2024
002c4b2
Merge branch 'main' into empty-struct-passing
tomeksowi May 10, 2024
b02c913
Check `size > 16` only when passing parameters according to integer c…
tomeksowi May 10, 2024
0fde22d
Don't assume struct size > 16 means return by implicit ref to a retur…
tomeksowi May 13, 2024
f89a68d
Don't assume if struct size > 16, IsArgPassedByRef should return true…
tomeksowi May 13, 2024
f5e3930
Adjust ArgIterator.GetNextOffset() for crossgen2 to be the same as th…
tomeksowi May 14, 2024
0a5ce05
Adjust crossgen2 version of ComputeReturnFlags (ComputeReturnValueTre…
tomeksowi May 14, 2024
6a3118b
Adjust IsArgPassedByRef for crossgen2 with native version for VM. Als…
tomeksowi May 14, 2024
0a1a7ff
Don't assume struct (size > 16) means pass by reference in Compiler::…
tomeksowi May 14, 2024
9aa2d10
Add a test for a single-float struct padded with empty struct field
tomeksowi May 14, 2024
9d9d0e2
Relax checks so the new test cases JIT without false positive assertions
tomeksowi May 16, 2024
983d423
New structure to store information about passing structs according to…
tomeksowi May 16, 2024
8366470
Implement GetRiscV64PassFPStructInfo for now with checks against exis…
tomeksowi May 17, 2024
c3e8578
Rename to match similar functions on other architectures
tomeksowi May 20, 2024
c2dd449
Use bitfields to keep the most important flags for decision making pa…
tomeksowi May 20, 2024
0d81d68
Fix JIT for structs with single float padded with empty structs
tomeksowi May 20, 2024
ec8d404
Replace bitfields with manual flags to avoid potential portability pr…
tomeksowi May 21, 2024
98e1861
Update C# version of GetRiscV64PassFpStructInRegistersInfo
tomeksowi May 21, 2024
623a2a6
Replace getRISCV64PassStructInRegisterFlags with getRiscV64PassFpStru…
tomeksowi May 21, 2024
f550c22
Fix offset assignment in C# GetRiscV64PassFpStructInRegistersInfo
tomeksowi May 22, 2024
338e244
Use enregistered struct field offsets in JIT new ABI classifiers
tomeksowi May 22, 2024
894fccd
Merge branch 'main' into empty-struct-passing
tomeksowi May 22, 2024
4cdd228
Fix build: formatting and using static ordering
tomeksowi May 22, 2024
99ec678
Include GC info in FpStructInRegistersInfo like on System V. While it…
tomeksowi May 23, 2024
b5e1cdc
Nicer size shift calculation routine
tomeksowi May 23, 2024
95e787d
Fix build
tomeksowi May 24, 2024
48e7944
Fix Empty8Float test
tomeksowi May 28, 2024
7102f31
Add EmptyFloatEmpty5(U)Byte tests
tomeksowi May 28, 2024
e5d67cd
Increase field visibility to match other tests
tomeksowi May 29, 2024
e25552e
Fix EmptyFloatEmpty5(U)Byte and LongEmptyDouble tests by using correc…
tomeksowi May 31, 2024
28a88b3
Fix LongEmptyGDoubleByImplicitRef and the rest of tests in StructABI …
tomeksowi Jun 4, 2024
123eaaf
Make tests harder
tomeksowi Jun 4, 2024
1a85e0b
Improve logging for GetRiscV64PassFpStructInRegistersInfo, similar to…
tomeksowi Jun 5, 2024
6163536
Format fix
tomeksowi Jun 5, 2024
6dc499d
Fix test EchoArrayOfEmptiesFloatDouble by returning FpStruct{ UseIntC…
tomeksowi Jun 5, 2024
afd95cd
Fix false positive marking a struct split between register and stack
tomeksowi Jun 6, 2024
de83bc7
Add more tests for cases where structs eligible for hardware floating…
tomeksowi Jun 6, 2024
978e851
Fix test EchoEmptyFloatEmpty5ByteSplitRiscV
tomeksowi Jun 6, 2024
dab9167
Remove LclVarDsc::lvIs4Field{1,2} because they were write-only
tomeksowi Jun 6, 2024
2d24bee
Fix ArgIteratorTemplate::GetNextOffset() for struct{Arr[], float}
tomeksowi Jun 6, 2024
9eb6b00
Merge branch 'main' into getnextoffset-for-struct-ptr-float
tomeksowi Jun 7, 2024
d3e974c
Merge branch 'getnextoffset-for-struct-ptr-float' into empty-struct-p…
tomeksowi Jun 7, 2024
a0ccb2a
Merge branch 'main' into empty-struct-passing
tomeksowi Jun 7, 2024
f469ef5
Fix arm32 build by reverting to using cSlotsToEnregister in lclvars.
tomeksowi Jun 10, 2024
811f947
Add failing tests for hardware floating-point calling convention by r…
tomeksowi Jun 10, 2024
b6bef68
Merge branch 'main' into empty-struct-passing
tomeksowi Jun 10, 2024
ac2670b
Fix small struct passing to calls by reflection: CopyStructToRegister…
tomeksowi Jun 11, 2024
76533b6
Fix FP structs returned from calls by reflection
tomeksowi Jun 12, 2024
c7e8a11
Fix the rest of the reflection tests by properly determining whether …
tomeksowi Jun 12, 2024
16b549f
Update crossgen2 C# version of ArgIterator to changes on the native side
tomeksowi Jun 14, 2024
3389bcf
Fix copying structs returned by hardware floating-point calling conve…
tomeksowi Jun 14, 2024
d4f81be
Merge branch 'main' into empty-struct-passing
tomeksowi Jun 14, 2024
7551d35
Merge branch 'main' into empty-struct-passing
tomeksowi Jun 18, 2024
4d2e2cb
Add signedness to integer field FpStructPassInRegistersInfo
tomeksowi Jun 21, 2024
907ac95
Better flag names
tomeksowi Jun 21, 2024
892ec22
Improve explanation why RISC-V can't use genActualType in genParamSta…
tomeksowi Jun 21, 2024
e73450e
Take signedness into account in CopyStructToRegisters
tomeksowi Jun 21, 2024
717cd59
Remove IsSize(1st|2nd)8 because they weren't used much
tomeksowi Jun 21, 2024
64de193
Improve getter names in FpStructInRegistersInfo
tomeksowi Jun 21, 2024
d2d1841
Add comment to fix JIT to AssignClassifiedEightByteTypes
tomeksowi Jun 21, 2024
47f9d87
Add helpers for IntKind and flag names to FpStructInRegistersInfo
tomeksowi Jun 21, 2024
09c0b38
Merge branch 'main' into empty-struct-passing
tomeksowi Jun 21, 2024
8d6a102
Fix C# build: Enum values should be on separate lines
tomeksowi Jun 21, 2024
5ebd2af
Merge branch 'main' into empty-struct-passing
tomeksowi Jun 21, 2024
7036dc0
Make a logging wrapper Compiler::GetPassFpStructInRegistersInfo, simi…
tomeksowi Jun 21, 2024
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
113 changes: 112 additions & 1 deletion src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,117 @@ enum StructFloatFieldInfoFlags
STRUCT_HAS_8BYTES_FIELDS_MASK = (STRUCT_FIRST_FIELD_SIZE_IS8 | STRUCT_SECOND_FIELD_SIZE_IS8),
};

// Bitfields for FpStructInRegistersInfo::flags
namespace FpStruct
{
enum class IntKind
{
Signed,
Unsigned,
GcRef,
GcByRef,
};

enum Flags
{
// Positions of flags and bitfields
PosOnlyOne = 0,
PosBothFloat = 1,
PosFloatInt = 2,
PosIntFloat = 3,
PosSizeShift1st = 4, // 2 bits
PosSizeShift2nd = 6, // 2 bits
PosIntFieldKind = 8, // 2 bits

UseIntCallConv = 0, // struct is passed according to integer calling convention

// The flags and bitfields
OnlyOne = 1 << PosOnlyOne, // has only one field, which is floating-point
BothFloat = 1 << PosBothFloat, // has two fields, both are floating-point
FloatInt = 1 << PosFloatInt, // has two fields, 1st is floating and 2nd is integer
IntFloat = 1 << PosIntFloat, // has two fields, 2nd is floating and 1st is integer
SizeShift1stMask = 0b11 << PosSizeShift1st, // log2(size) of 1st field
SizeShift2ndMask = 0b11 << PosSizeShift2nd, // log2(size) of 2nd field
IntFieldKindMask = 0b11 << PosIntFieldKind, // the kind of the integer field (FpStruct::IntKind)
// Note: flags OnlyOne, BothFloat, FloatInt, and IntFloat are mutually exclusive
};
}

// On RISC-V and LoongArch a struct with up to two non-empty fields, at least one of them floating-point,
// can be passed in registers according to hardware FP calling convention. FpStructInRegistersInfo represents
// passing information for such parameters.
struct FpStructInRegistersInfo
{
FpStruct::Flags flags;
uint32_t offset1st;
uint32_t offset2nd;

unsigned SizeShift1st() const { return (flags >> FpStruct::PosSizeShift1st) & 0b11; }
unsigned SizeShift2nd() const { return (flags >> FpStruct::PosSizeShift2nd) & 0b11; }

unsigned Size1st() const { return 1u << SizeShift1st(); }
unsigned Size2nd() const { return 1u << SizeShift2nd(); }

FpStruct::IntKind IntFieldKind() const
{
return (FpStruct::IntKind)((flags >> FpStruct::PosIntFieldKind) & 0b11);
}

const char* IntFieldKindName() const
{
static const char* intKindNames[] = { "Signed", "Unsigned", "GcRef", "GcByRef" };
return (flags & (FpStruct::FloatInt | FpStruct::IntFloat))
? intKindNames[(int)IntFieldKind()]
: "None";
}

const char* FlagName() const
{
switch (flags & (FpStruct::OnlyOne | FpStruct::BothFloat | FpStruct::FloatInt | FpStruct::IntFloat))
{
case FpStruct::OnlyOne: return "OnlyOne";
case FpStruct::BothFloat: return "BothFloat";
case FpStruct::FloatInt: return "FloatInt";
case FpStruct::IntFloat: return "IntFloat";
default: return "?";
}
}

StructFloatFieldInfoFlags ToOldFlags() const
{
return StructFloatFieldInfoFlags(
((flags & FpStruct::OnlyOne) ? STRUCT_FLOAT_FIELD_ONLY_ONE : 0) |
((flags & FpStruct::BothFloat) ? STRUCT_FLOAT_FIELD_ONLY_TWO : 0) |
((flags & FpStruct::FloatInt) ? STRUCT_FLOAT_FIELD_FIRST : 0) |
((flags & FpStruct::IntFloat) ? STRUCT_FLOAT_FIELD_SECOND : 0) |
((SizeShift1st() == 3) ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) |
((SizeShift2nd() == 3) ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0));
}

static FpStructInRegistersInfo FromOldFlags(StructFloatFieldInfoFlags flags)
{
unsigned sizeShift1st = (flags & STRUCT_FIRST_FIELD_SIZE_IS8) ? 3 : 2;
unsigned sizeShift2nd = (flags & STRUCT_SECOND_FIELD_SIZE_IS8) ? 3 : 2;
bool hasTwo = !(flags & STRUCT_FLOAT_FIELD_ONLY_ONE);
return {
FpStruct::Flags(
((flags & STRUCT_FLOAT_FIELD_ONLY_ONE) ? FpStruct::OnlyOne : 0) |
((flags & STRUCT_FLOAT_FIELD_ONLY_TWO) ? FpStruct::BothFloat : 0) |
((flags & STRUCT_FLOAT_FIELD_FIRST) ? FpStruct::FloatInt : 0) |
((flags & STRUCT_FLOAT_FIELD_SECOND) ? FpStruct::IntFloat : 0) |
(sizeShift1st << FpStruct::PosSizeShift1st) |
(hasTwo ? (sizeShift2nd << FpStruct::PosSizeShift2nd) : 0)
// No GC ref info in old flags
),
// Lacking actual field offsets, assume fields are naturally aligned without empty fields or padding
0,
hasTwo ? (1u << (sizeShift1st > sizeShift2nd ? sizeShift1st : sizeShift2nd)) : 0,
};
}
};

static_assert(sizeof(FpStructInRegistersInfo) == 3 * sizeof(uint32_t), "");

#include "corinfoinstructionset.h"

// CorInfoHelpFunc defines the set of helpers (accessed via the ICorDynamicInfo::getHelperFtn())
Expand Down Expand Up @@ -3075,7 +3186,7 @@ class ICorStaticInfo
virtual void getSwiftLowering(CORINFO_CLASS_HANDLE structHnd, CORINFO_SWIFT_LOWERING* pLowering) = 0;

virtual uint32_t getLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) = 0;
virtual uint32_t getRISCV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) = 0;
virtual FpStructInRegistersInfo getRiscV64PassFpStructInRegistersInfo(CORINFO_CLASS_HANDLE cls) = 0;
};

/*****************************************************************************
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ void getSwiftLowering(
uint32_t getLoongArch64PassStructInRegisterFlags(
CORINFO_CLASS_HANDLE structHnd) override;

uint32_t getRISCV64PassStructInRegisterFlags(
FpStructInRegistersInfo getRiscV64PassFpStructInRegistersInfo(
CORINFO_CLASS_HANDLE structHnd) override;

uint32_t getThreadTLSIndex(
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* e428e66d-5e0e-4320-ad8a-fa5a50f6da07 */
0xe428e66d,
0x5e0e,
0x4320,
{0xad, 0x8a, 0xfa, 0x5a, 0x50, 0xf6, 0xda, 0x07}
constexpr GUID JITEEVersionIdentifier = { /* 1907f831-d9c8-4b42-a549-0cdce990d1c6 */
0x1907f831,
0xd9c8,
0x4b42,
{0xa5, 0x49, 0x0c, 0xdc, 0xe9, 0x90, 0xd1, 0xc6}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/ICorJitInfo_names_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ DEF_CLR_API(getMethodHash)
DEF_CLR_API(getSystemVAmd64PassStructInRegisterDescriptor)
DEF_CLR_API(getSwiftLowering)
DEF_CLR_API(getLoongArch64PassStructInRegisterFlags)
DEF_CLR_API(getRISCV64PassStructInRegisterFlags)
DEF_CLR_API(getRiscV64PassFpStructInRegistersInfo)
DEF_CLR_API(getThreadTLSIndex)
DEF_CLR_API(getAddrOfCaptureThreadGlobal)
DEF_CLR_API(getHelperFtn)
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1238,12 +1238,12 @@ uint32_t WrapICorJitInfo::getLoongArch64PassStructInRegisterFlags(
return temp;
}

uint32_t WrapICorJitInfo::getRISCV64PassStructInRegisterFlags(
FpStructInRegistersInfo WrapICorJitInfo::getRiscV64PassFpStructInRegistersInfo(
CORINFO_CLASS_HANDLE structHnd)
{
API_ENTER(getRISCV64PassStructInRegisterFlags);
uint32_t temp = wrapHnd->getRISCV64PassStructInRegisterFlags(structHnd);
API_LEAVE(getRISCV64PassStructInRegisterFlags);
API_ENTER(getRiscV64PassFpStructInRegistersInfo);
FpStructInRegistersInfo temp = wrapHnd->getRiscV64PassFpStructInRegistersInfo(structHnd);
API_LEAVE(getRiscV64PassFpStructInRegistersInfo);
return temp;
}

Expand Down
12 changes: 5 additions & 7 deletions src/coreclr/jit/buildstring.cpp
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#define STRINGIFY(L) #L
#define MAKESTRING(M, L) M(L)
#define STRINGIZE(X) MAKESTRING(STRINGIFY, X)

#if defined(__clang__)
#define BUILD_COMPILER \
"Clang " STRINGIZE(__clang_major__) "." STRINGIZE(__clang_minor__) "." STRINGIZE(__clang_patchlevel__)
"Clang " STRINGIFY(__clang_major__) "." STRINGIFY(__clang_minor__) "." STRINGIFY(__clang_patchlevel__)
#elif defined(_MSC_VER)
#define BUILD_COMPILER "MSVC " STRINGIZE(_MSC_FULL_VER)
#define BUILD_COMPILER "MSVC " STRINGIFY(_MSC_FULL_VER)
#elif defined(__GNUC__)
#define BUILD_COMPILER "GCC " STRINGIZE(__GNUC__) "." STRINGIZE(__GNUC_MINOR__) "." STRINGIZE(__GNUC_PATCHLEVEL__)
#define BUILD_COMPILER "GCC " STRINGIFY(__GNUC__) "." STRINGIFY(__GNUC_MINOR__) "." STRINGIFY(__GNUC_PATCHLEVEL__)
#else
#define BUILD_COMPILER "Unknown"
#endif
Expand All @@ -26,6 +22,8 @@
#define TARGET_ARCH_STRING "arm64"
#elif defined(TARGET_LOONGARCH64)
#define TARGET_ARCH_STRING "loongarch64"
#elif defined(TARGET_RISCV64)
#define TARGET_ARCH_STRING "riscv64"
#else
#define TARGET_ARCH_STRING "Unknown"
#endif
Expand Down
31 changes: 22 additions & 9 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3170,6 +3170,13 @@ var_types CodeGen::genParamStackType(LclVarDsc* dsc, const ABIPassingSegment& se
// can always use the full register size here. This allows us to
// use stp more often.
return TYP_I_IMPL;
#elif defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
// On RISC-V/LoongArch structs passed according to floating-point calling convention are enregistered one
// field per register regardless of the layout of the fields in memory, so the small int load/store
// instructions must not be upsized to 4 bytes, otherwise for example:
// * struct { struct{} e1,e2,e3; byte b; float f; } -- 4-byte store for 'b' would trash 'f'
// * struct { float f; struct{} e1,e2,e3; byte b; } -- 4-byte store for 'b' would trash adjacent stack slot
return seg.GetRegisterType();
#else
return genActualType(seg.GetRegisterType());
#endif
Expand Down Expand Up @@ -7360,16 +7367,17 @@ void CodeGen::genStructReturn(GenTree* treeNode)
#if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
// On LoongArch64, for a struct like "{ int, double }", "retTypeDesc" will be "{ TYP_INT, TYP_DOUBLE }",
// i. e. not include the padding for the first field, and so the general loop below won't work.
var_types type = retTypeDesc.GetReturnRegType(0);
regNumber toReg = retTypeDesc.GetABIReturnReg(0, compiler->info.compCallConv);
GetEmitter()->emitIns_R_S(ins_Load(type), emitTypeSize(type), toReg, lclNode->GetLclNum(), 0);
var_types type = retTypeDesc.GetReturnRegType(0);
regNumber toReg = retTypeDesc.GetABIReturnReg(0, compiler->info.compCallConv);
unsigned offset = retTypeDesc.GetReturnFieldOffset(0);
GetEmitter()->emitIns_R_S(ins_Load(type), emitTypeSize(type), toReg, lclNode->GetLclNum(), offset);
if (regCount > 1)
{
assert(regCount == 2);
int offset = genTypeSize(type);
type = retTypeDesc.GetReturnRegType(1);
offset = (int)((unsigned int)offset < genTypeSize(type) ? genTypeSize(type) : offset);
toReg = retTypeDesc.GetABIReturnReg(1, compiler->info.compCallConv);
assert(offset + genTypeSize(type) <= retTypeDesc.GetReturnFieldOffset(1));
type = retTypeDesc.GetReturnRegType(1);
toReg = retTypeDesc.GetABIReturnReg(1, compiler->info.compCallConv);
offset = retTypeDesc.GetReturnFieldOffset(1);
GetEmitter()->emitIns_R_S(ins_Load(type), emitTypeSize(type), toReg, lclNode->GetLclNum(), offset);
}
#else // !TARGET_LOONGARCH64 && !TARGET_RISCV64
Expand Down Expand Up @@ -7644,6 +7652,11 @@ void CodeGen::genMultiRegStoreToLocal(GenTreeLclVar* lclNode)
assert(regCount == varDsc->lvFieldCnt);
}

#if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
// genMultiRegStoreToLocal is only used for calls on RISC-V and LoongArch
const ReturnTypeDesc* returnTypeDesc = actualOp1->AsCall()->GetReturnTypeDesc();
#endif

#ifdef SWIFT_SUPPORT
const uint32_t* offsets = nullptr;
if (actualOp1->IsCall() && (actualOp1->AsCall()->GetUnmanagedCallConv() == CorInfoCallConvExtension::Swift))
Expand Down Expand Up @@ -7694,8 +7707,8 @@ void CodeGen::genMultiRegStoreToLocal(GenTreeLclVar* lclNode)
else
{
#if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
// should consider the padding field within a struct.
offset = (offset % genTypeSize(srcType)) ? AlignUp(offset, genTypeSize(srcType)) : offset;
// Should consider the padding, empty struct fields, etc within a struct.
offset = returnTypeDesc->GetReturnFieldOffset(i);
#endif
#ifdef SWIFT_SUPPORT
if (offsets != nullptr)
Expand Down
Loading
Loading