From 454596fb450a5e01d5823c79e49251f76858337b Mon Sep 17 00:00:00 2001 From: Dong-Heon Jung Date: Tue, 12 Mar 2024 21:50:12 +0900 Subject: [PATCH 01/18] [RISC-V] Fix struct info value in crossgen2 --- .../tools/Common/JitInterface/RISCV64PassStructInRegister.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs index d6c820b8f904c9..9f33bf6936f863 100644 --- a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs +++ b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs @@ -153,7 +153,7 @@ public static uint GetRISCV64PassStructInRegisterFlags(TypeDesc typeDesc) Debug.Assert(fieldIndex == 1); if ((floatFieldFlags2 & (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) { - floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_MERGE_FIRST_SECOND; + floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND; } if (field.FieldType.GetElementSize().AsInt == 8) { From 2d80de26caa331db9198c8ab7bf5433a571d26fb Mon Sep 17 00:00:00 2001 From: Dong-Heon Jung Date: Fri, 15 Mar 2024 20:38:07 +0900 Subject: [PATCH 02/18] [RISC-V] Fix assertion in crossgen Asserts in `./Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp/MarshalStructAsLayoutExp.sh` Error message is `Assertion failed 'roundUp(structSize, TARGET_POINTER_SIZE) == roundUp(loadExtent, TARGET_POINTER_SIZE)' in 'Managed:MarshalStructAsParam_AsExpByVal(int)' during 'Morph - Global' (IL size 2208; hash 0x9fd9734a; MinOpts)` Copied missed codes of GetRiscV64PassStructInRegiste in vm to crossgen2 --- .../Common/JitInterface/RISCV64PassStructInRegister.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs index 9f33bf6936f863..8edac07cdfa1f9 100644 --- a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs +++ b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs @@ -56,6 +56,13 @@ public static uint GetRISCV64PassStructInRegisterFlags(TypeDesc typeDesc) { continue; } + else if (fieldIndex == 1) + { + if (firstField.Offset.AsInt != 0 || field.Offset.AsInt == 0 || firstFieldSize > field.Offset.AsInt) + { + return (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; + } + } Debug.Assert(fieldIndex < numIntroducedFields); From 1462c932c5b5760e5564f28f3867a714de1bb950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Wed, 20 Mar 2024 13:26:53 +0100 Subject: [PATCH 03/18] Check size in GetRiscV64PassStructInRegisterFlags early, use named constant --- src/coreclr/vm/methodtable.cpp | 806 ++++++++++++++++----------------- 1 file changed, 399 insertions(+), 407 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 24f93f4c14899c..02bbc5e77a6e10 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -2803,45 +2803,45 @@ bool MethodTable::IsOnlyOneField(MethodTable * pMT) { TypeHandle th(pMT); + if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) + return false; + bool ret = false; if (!th.IsTypeDesc()) { MethodTable* pMethodTable = th.AsMethodTable(); - if (th.GetSize() <= 16 /*MAX_PASS_MULTIREG_BYTES*/) - { - DWORD numIntroducedFields = pMethodTable->GetNumIntroducedInstanceFields(); + DWORD numIntroducedFields = pMethodTable->GetNumIntroducedInstanceFields(); - if (numIntroducedFields == 1) - { - FieldDesc *pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); + if (numIntroducedFields == 1) + { + FieldDesc *pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); - CorElementType fieldType = pFieldStart[0].GetFieldType(); + CorElementType fieldType = pFieldStart[0].GetFieldType(); - // InlineArray types and fixed buffer types have implied repeated fields. - // Checking if a type is an InlineArray type is cheap, so we'll do that first. - bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); + // InlineArray types and fixed buffer types have implied repeated fields. + // Checking if a type is an InlineArray type is cheap, so we'll do that first. + bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); - if (hasImpliedRepeatedFields) + if (hasImpliedRepeatedFields) + { + numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); + if (numIntroducedFields != 1) { - numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); - if (numIntroducedFields != 1) - { - goto _End_arg; - } + goto _End_arg; } + } - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - ret = true; - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) + if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + { + ret = true; + } + else if (fieldType == ELEMENT_TYPE_VALUETYPE) + { + pMethodTable = pFieldStart->GetApproxFieldTypeHandleThrowing().GetMethodTable(); + if (pMethodTable->GetNumIntroducedInstanceFields() == 1) { - pMethodTable = pFieldStart->GetApproxFieldTypeHandleThrowing().GetMethodTable(); - if (pMethodTable->GetNumIntroducedInstanceFields() == 1) - { - ret = IsOnlyOneField(pMethodTable); - } + ret = IsOnlyOneField(pMethodTable); } } } @@ -2849,54 +2849,50 @@ bool MethodTable::IsOnlyOneField(MethodTable * pMT) else { MethodTable* pMethodTable = th.AsNativeValueType(); + DWORD numIntroducedFields = pMethodTable->GetNativeLayoutInfo()->GetNumFields(); + FieldDesc *pFieldStart = nullptr; - if (th.GetSize() <= 16 /*MAX_PASS_MULTIREG_BYTES*/) + if (numIntroducedFields == 1) { - DWORD numIntroducedFields = pMethodTable->GetNativeLayoutInfo()->GetNumFields(); - FieldDesc *pFieldStart = nullptr; - - if (numIntroducedFields == 1) - { - pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); + pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); - CorElementType fieldType = pFieldStart->GetFieldType(); + CorElementType fieldType = pFieldStart->GetFieldType(); - // InlineArray types and fixed buffer types have implied repeated fields. - // Checking if a type is an InlineArray type is cheap, so we'll do that first. - bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); + // InlineArray types and fixed buffer types have implied repeated fields. + // Checking if a type is an InlineArray type is cheap, so we'll do that first. + bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); - if (hasImpliedRepeatedFields) + if (hasImpliedRepeatedFields) + { + numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); + if (numIntroducedFields != 1) { - numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); - if (numIntroducedFields != 1) - { - goto _End_arg; - } + goto _End_arg; } + } - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + { + ret = true; + } + else if (fieldType == ELEMENT_TYPE_VALUETYPE) + { + const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); + NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); + if (nfc == NativeFieldCategory::NESTED) { - ret = true; + pMethodTable = pNativeFieldDescs->GetNestedNativeMethodTable(); + ret = IsOnlyOneField(pMethodTable); } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) + else if (nfc != NativeFieldCategory::ILLEGAL) { - const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); - if (nfc == NativeFieldCategory::NESTED) - { - pMethodTable = pNativeFieldDescs->GetNestedNativeMethodTable(); - ret = IsOnlyOneField(pMethodTable); - } - else if (nfc != NativeFieldCategory::ILLEGAL) - { - ret = true; - } + ret = true; } } - else - { - ret = false; - } + } + else + { + ret = false; } } _End_arg: @@ -3446,498 +3442,475 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) { TypeHandle th(cls); + if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) + return STRUCT_NO_FLOAT_FIELD; + int size = STRUCT_NO_FLOAT_FIELD; if (!th.IsTypeDesc()) { MethodTable* pMethodTable = th.AsMethodTable(); - if (th.GetSize() <= 16 /*MAX_PASS_MULTIREG_BYTES*/) - { - DWORD numIntroducedFields = pMethodTable->GetNumIntroducedInstanceFields(); - if (numIntroducedFields == 1) - { - FieldDesc *pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); + DWORD numIntroducedFields = pMethodTable->GetNumIntroducedInstanceFields(); + if (numIntroducedFields == 1) + { + FieldDesc *pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); - CorElementType fieldType = pFieldStart[0].GetFieldType(); + CorElementType fieldType = pFieldStart[0].GetFieldType(); - // InlineArray types and fixed buffer types have implied repeated fields. - // Checking if a type is an InlineArray type is cheap, so we'll do that first. - bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); + // InlineArray types and fixed buffer types have implied repeated fields. + // Checking if a type is an InlineArray type is cheap, so we'll do that first. + bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); - if (hasImpliedRepeatedFields) + if (hasImpliedRepeatedFields) + { + numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); + if (numIntroducedFields > 2) { - numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); - if (numIntroducedFields > 2) - { - goto _End_arg; - } + goto _End_arg; + } - if (fieldType == ELEMENT_TYPE_R4) + if (fieldType == ELEMENT_TYPE_R4) + { + if (numIntroducedFields == 1) { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FLOAT_FIELD_ONLY_TWO; - } - goto _End_arg; + size = STRUCT_FLOAT_FIELD_ONLY_ONE; } - else if (fieldType == ELEMENT_TYPE_R8) + else if (numIntroducedFields == 2) { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FIELD_TWO_DOUBLES; - } - goto _End_arg; + size = STRUCT_FLOAT_FIELD_ONLY_TWO; } + goto _End_arg; } - - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + else if (fieldType == ELEMENT_TYPE_R8) { - if (fieldType == ELEMENT_TYPE_R4) + if (numIntroducedFields == 1) { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; + size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; } - else if (fieldType == ELEMENT_TYPE_R8) + else if (numIntroducedFields == 2) { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; + size = STRUCT_FIELD_TWO_DOUBLES; } - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - pMethodTable = pFieldStart->GetApproxFieldTypeHandleThrowing().GetMethodTable(); - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); + goto _End_arg; } } - else if (numIntroducedFields == 2) + + if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) { - FieldDesc *pFieldSecond; - FieldDesc *pFieldFirst = pMethodTable->GetApproxFieldDescListRaw(); - if (pFieldFirst->GetOffset() == 0) + if (fieldType == ELEMENT_TYPE_R4) { - pFieldSecond = pFieldFirst + 1; + size = STRUCT_FLOAT_FIELD_ONLY_ONE; } - else + else if (fieldType == ELEMENT_TYPE_R8) { - pFieldSecond = pFieldFirst; - pFieldFirst = pFieldFirst + 1; + size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; } - assert(pFieldFirst->GetOffset() == 0); + } + else if (fieldType == ELEMENT_TYPE_VALUETYPE) + { + pMethodTable = pFieldStart->GetApproxFieldTypeHandleThrowing().GetMethodTable(); + size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); + } + } + else if (numIntroducedFields == 2) + { + FieldDesc *pFieldSecond; + FieldDesc *pFieldFirst = pMethodTable->GetApproxFieldDescListRaw(); + if (pFieldFirst->GetOffset() == 0) + { + pFieldSecond = pFieldFirst + 1; + } + else + { + pFieldSecond = pFieldFirst; + pFieldFirst = pFieldFirst + 1; + } + assert(pFieldFirst->GetOffset() == 0); - if (pFieldFirst->GetSize() > 8) + if (pFieldFirst->GetSize() > 8) + { + goto _End_arg; + } + + if (pFieldFirst->GetSize() > pFieldSecond->GetOffset()) + { + goto _End_arg; + } + + CorElementType fieldType = pFieldFirst[0].GetFieldType(); + if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + { + if (fieldType == ELEMENT_TYPE_R4) { - goto _End_arg; + size = STRUCT_FLOAT_FIELD_FIRST; } - - if (pFieldFirst->GetSize() > pFieldSecond->GetOffset()) + else if (fieldType == ELEMENT_TYPE_R8) { - goto _End_arg; + size = STRUCT_FIRST_FIELD_DOUBLE; + } + else if (pFieldFirst[0].GetSize() == 8) + { + size = STRUCT_FIRST_FIELD_SIZE_IS8; } - CorElementType fieldType = pFieldFirst[0].GetFieldType(); - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + } + else if (fieldType == ELEMENT_TYPE_VALUETYPE) + { + pMethodTable = pFieldFirst->GetApproxFieldTypeHandleThrowing().GetMethodTable(); + if (IsOnlyOneField(pMethodTable)) { - if (fieldType == ELEMENT_TYPE_R4) + size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); + if ((size & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) { - size = STRUCT_FLOAT_FIELD_FIRST; + size = pFieldFirst[0].GetSize() == 8 ? STRUCT_FIRST_FIELD_DOUBLE : STRUCT_FLOAT_FIELD_FIRST; } - else if (fieldType == ELEMENT_TYPE_R8) + else if (size == STRUCT_NO_FLOAT_FIELD) { - size = STRUCT_FIRST_FIELD_DOUBLE; + size = pFieldFirst[0].GetSize() == 8 ? STRUCT_FIRST_FIELD_SIZE_IS8: 0; } - else if (pFieldFirst[0].GetSize() == 8) + else { - size = STRUCT_FIRST_FIELD_SIZE_IS8; + size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; } + } + else + { + size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; + } + } + else if (pFieldFirst[0].GetSize() == 8) + { + size = STRUCT_FIRST_FIELD_SIZE_IS8; + } + fieldType = pFieldSecond[0].GetFieldType(); + if (pFieldSecond[0].GetSize() > 8) + { + size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; + } + else if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + { + if (fieldType == ELEMENT_TYPE_R4) + { + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) + else if (fieldType == ELEMENT_TYPE_R8) { - pMethodTable = pFieldFirst->GetApproxFieldTypeHandleThrowing().GetMethodTable(); - if (IsOnlyOneField(pMethodTable)) + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); + } + else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) + { + size = STRUCT_NO_FLOAT_FIELD; + } + else if (pFieldSecond[0].GetSize() == 8) + { + size |= STRUCT_SECOND_FIELD_SIZE_IS8; + } + } + else if (fieldType == ELEMENT_TYPE_VALUETYPE) + { + pMethodTable = pFieldSecond[0].GetApproxFieldTypeHandleThrowing().GetMethodTable(); + if (IsOnlyOneField(pMethodTable)) + { + int size2 = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); + if ((size2 & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) { - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); - if ((size & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) + if (pFieldSecond[0].GetSize() == 8) { - size = pFieldFirst[0].GetSize() == 8 ? STRUCT_FIRST_FIELD_DOUBLE : STRUCT_FLOAT_FIELD_FIRST; - } - else if (size == STRUCT_NO_FLOAT_FIELD) - { - size = pFieldFirst[0].GetSize() == 8 ? STRUCT_FIRST_FIELD_SIZE_IS8: 0; + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); } else { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); } } + else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) + { + size = STRUCT_NO_FLOAT_FIELD; + } + else if (size2 == STRUCT_NO_FLOAT_FIELD) + { + size |= pFieldSecond[0].GetSize() == 8 ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0; + } else { size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; } } - else if (pFieldFirst[0].GetSize() == 8) + else { - size = STRUCT_FIRST_FIELD_SIZE_IS8; + size = STRUCT_NO_FLOAT_FIELD; } + } + else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) + { + size = STRUCT_NO_FLOAT_FIELD; + } + else if (pFieldSecond[0].GetSize() == 8) + { + size |= STRUCT_SECOND_FIELD_SIZE_IS8; + } + } + } + else + { + MethodTable* pMethodTable = th.AsNativeValueType(); + DWORD numIntroducedFields = pMethodTable->GetNativeLayoutInfo()->GetNumFields(); + FieldDesc *pFieldStart = nullptr; - fieldType = pFieldSecond[0].GetFieldType(); - if (pFieldSecond[0].GetSize() > 8) + if (numIntroducedFields == 1) + { + pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); + + CorElementType fieldType = pFieldStart->GetFieldType(); + + // InlineArray types and fixed buffer types have implied repeated fields. + // Checking if a type is an InlineArray type is cheap, so we'll do that first. + bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); + + if (hasImpliedRepeatedFields) + { + numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); + if (numIntroducedFields > 2) { - size = STRUCT_NO_FLOAT_FIELD; goto _End_arg; } - else if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + + if (fieldType == ELEMENT_TYPE_R4) { - if (fieldType == ELEMENT_TYPE_R4) + if (numIntroducedFields == 1) { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); + size = STRUCT_FLOAT_FIELD_ONLY_ONE; } - else if (fieldType == ELEMENT_TYPE_R8) + else if (numIntroducedFields == 2) { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldSecond[0].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; + size = STRUCT_FLOAT_FIELD_ONLY_TWO; } + goto _End_arg; } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) + else if (fieldType == ELEMENT_TYPE_R8) { - pMethodTable = pFieldSecond[0].GetApproxFieldTypeHandleThrowing().GetMethodTable(); - if (IsOnlyOneField(pMethodTable)) + if (numIntroducedFields == 1) { - int size2 = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); - if ((size2 & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - if (pFieldSecond[0].GetSize() == 8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (size2 == STRUCT_NO_FLOAT_FIELD) - { - size |= pFieldSecond[0].GetSize() == 8 ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0; - } - else - { - size = STRUCT_NO_FLOAT_FIELD; - } + size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; } - else + else if (numIntroducedFields == 2) { - size = STRUCT_NO_FLOAT_FIELD; + size = STRUCT_FIELD_TWO_DOUBLES; } + goto _End_arg; } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) + } + + if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + { + if (fieldType == ELEMENT_TYPE_R4) { - size = STRUCT_NO_FLOAT_FIELD; + size = STRUCT_FLOAT_FIELD_ONLY_ONE; } - else if (pFieldSecond[0].GetSize() == 8) + else if (fieldType == ELEMENT_TYPE_R8) { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; + size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; } } - } - } - else - { - MethodTable* pMethodTable = th.AsNativeValueType(); - - if (th.GetSize() <= 16 /*MAX_PASS_MULTIREG_BYTES*/) - { - DWORD numIntroducedFields = pMethodTable->GetNativeLayoutInfo()->GetNumFields(); - FieldDesc *pFieldStart = nullptr; - - if (numIntroducedFields == 1) + else if (fieldType == ELEMENT_TYPE_VALUETYPE) { - pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); - - CorElementType fieldType = pFieldStart->GetFieldType(); - - // InlineArray types and fixed buffer types have implied repeated fields. - // Checking if a type is an InlineArray type is cheap, so we'll do that first. - bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); - - if (hasImpliedRepeatedFields) + const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); + NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); + if (nfc == NativeFieldCategory::NESTED) { - numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); - if (numIntroducedFields > 2) - { - goto _End_arg; - } - - if (fieldType == ELEMENT_TYPE_R4) - { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FLOAT_FIELD_ONLY_TWO; - } - goto _End_arg; - } - else if (fieldType == ELEMENT_TYPE_R8) - { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FIELD_TWO_DOUBLES; - } - goto _End_arg; - } + pMethodTable = pNativeFieldDescs->GetNestedNativeMethodTable(); + size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); + return size; } - - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + else if (nfc == NativeFieldCategory::FLOAT) { - if (fieldType == ELEMENT_TYPE_R4) + if (pFieldStart->GetSize() == 4) { size = STRUCT_FLOAT_FIELD_ONLY_ONE; } - else if (fieldType == ELEMENT_TYPE_R8) + else if (pFieldStart->GetSize() == 8) { size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; } } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); - if (nfc == NativeFieldCategory::NESTED) - { - pMethodTable = pNativeFieldDescs->GetNestedNativeMethodTable(); - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); - return size; - } - else if (nfc == NativeFieldCategory::FLOAT) - { - if (pFieldStart->GetSize() == 4) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (pFieldStart->GetSize() == 8) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - } - } } - else if (numIntroducedFields == 2) + } + else if (numIntroducedFields == 2) + { + pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); + + if (pFieldStart->GetSize() > 8) { - pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); + goto _End_arg; + } - if (pFieldStart->GetSize() > 8) + if (pFieldStart->GetOffset() || !pFieldStart[1].GetOffset() || (pFieldStart[0].GetSize() > pFieldStart[1].GetOffset())) + { + goto _End_arg; + } + + CorElementType fieldType = pFieldStart[0].GetFieldType(); + if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + { + if (fieldType == ELEMENT_TYPE_R4) { - goto _End_arg; + size = STRUCT_FLOAT_FIELD_FIRST; } - - if (pFieldStart->GetOffset() || !pFieldStart[1].GetOffset() || (pFieldStart[0].GetSize() > pFieldStart[1].GetOffset())) + else if (fieldType == ELEMENT_TYPE_R8) { - goto _End_arg; + size = STRUCT_FIRST_FIELD_DOUBLE; + } + else if (pFieldStart[0].GetSize() == 8) + { + size = STRUCT_FIRST_FIELD_SIZE_IS8; } - CorElementType fieldType = pFieldStart[0].GetFieldType(); + fieldType = pFieldStart[1].GetFieldType(); if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) { if (fieldType == ELEMENT_TYPE_R4) { - size = STRUCT_FLOAT_FIELD_FIRST; + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); } else if (fieldType == ELEMENT_TYPE_R8) { - size = STRUCT_FIRST_FIELD_DOUBLE; + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); } - else if (pFieldStart[0].GetSize() == 8) + else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) { - size = STRUCT_FIRST_FIELD_SIZE_IS8; + size = STRUCT_NO_FLOAT_FIELD; } - - fieldType = pFieldStart[1].GetFieldType(); - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + else if (pFieldStart[1].GetSize() == 8) { - if (fieldType == ELEMENT_TYPE_R4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - goto _End_arg; + size |= STRUCT_SECOND_FIELD_SIZE_IS8; } + goto _End_arg; } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); + } + else if (fieldType == ELEMENT_TYPE_VALUETYPE) + { + const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); + NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); - if (nfc == NativeFieldCategory::NESTED) + if (nfc == NativeFieldCategory::NESTED) + { + if (pNativeFieldDescs->GetNumElements() != 1) { - if (pNativeFieldDescs->GetNumElements() != 1) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } + size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; + } - MethodTable* pMethodTable2 = pNativeFieldDescs->GetNestedNativeMethodTable(); + MethodTable* pMethodTable2 = pNativeFieldDescs->GetNestedNativeMethodTable(); - if (!IsOnlyOneField(pMethodTable2)) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } + if (!IsOnlyOneField(pMethodTable2)) + { + size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; + } - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable2); - if ((size & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - if (pFieldStart->GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_DOUBLE; - } - else - { - size = STRUCT_FLOAT_FIELD_FIRST; - } - } - else if (pFieldStart->GetSize() == 8) + size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable2); + if ((size & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) + { + if (pFieldStart->GetSize() == 8) { - size = STRUCT_FIRST_FIELD_SIZE_IS8; + size = STRUCT_FIRST_FIELD_DOUBLE; } else { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; + size = STRUCT_FLOAT_FIELD_FIRST; } } - else if (nfc == NativeFieldCategory::FLOAT) + else if (pFieldStart->GetSize() == 8) { - if (pFieldStart[0].GetSize() == 4) - { - size = STRUCT_FLOAT_FIELD_FIRST; - } - else if (pFieldStart[0].GetSize() == 8) - { - _ASSERTE((pMethodTable->GetNativeSize() == 8) || (pMethodTable->GetNativeSize() == 16)); - size = STRUCT_FIRST_FIELD_DOUBLE; - } + size = STRUCT_FIRST_FIELD_SIZE_IS8; + } + else + { + size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; + } + } + else if (nfc == NativeFieldCategory::FLOAT) + { + if (pFieldStart[0].GetSize() == 4) + { + size = STRUCT_FLOAT_FIELD_FIRST; } else if (pFieldStart[0].GetSize() == 8) { - size = STRUCT_FIRST_FIELD_SIZE_IS8; + _ASSERTE((pMethodTable->GetNativeSize() == 8) || (pMethodTable->GetNativeSize() == 16)); + size = STRUCT_FIRST_FIELD_DOUBLE; } } else if (pFieldStart[0].GetSize() == 8) { size = STRUCT_FIRST_FIELD_SIZE_IS8; } + } + else if (pFieldStart[0].GetSize() == 8) + { + size = STRUCT_FIRST_FIELD_SIZE_IS8; + } - fieldType = pFieldStart[1].GetFieldType(); - if (pFieldStart[1].GetSize() > 8) + fieldType = pFieldStart[1].GetFieldType(); + if (pFieldStart[1].GetSize() > 8) + { + size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; + } + else if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + { + if (fieldType == ELEMENT_TYPE_R4) + { + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); + } + else if (fieldType == ELEMENT_TYPE_R8) + { + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); + } + else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) { size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; } - else if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) + else if (pFieldStart[1].GetSize() == 8) { - if (fieldType == ELEMENT_TYPE_R4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) + size |= STRUCT_SECOND_FIELD_SIZE_IS8; + } + + // Pass with two integer registers in `struct {int a, int b, float/double c}` cases + if ((size | STRUCT_FIRST_FIELD_SIZE_IS8 | STRUCT_FLOAT_FIELD_SECOND) == size) + { + size = STRUCT_NO_FLOAT_FIELD; + } + } + else if (fieldType == ELEMENT_TYPE_VALUETYPE) + { + const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); + NativeFieldCategory nfc = pNativeFieldDescs[1].GetCategory(); + + if (nfc == NativeFieldCategory::NESTED) + { + if (pNativeFieldDescs[1].GetNumElements() != 1) { size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; + goto _End_arg; } - // Pass with two integer registers in `struct {int a, int b, float/double c}` cases - if ((size | STRUCT_FIRST_FIELD_SIZE_IS8 | STRUCT_FLOAT_FIELD_SECOND) == size) + MethodTable* pMethodTable2 = pNativeFieldDescs[1].GetNestedNativeMethodTable(); + + if (!IsOnlyOneField(pMethodTable2)) { size = STRUCT_NO_FLOAT_FIELD; + goto _End_arg; } - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - NativeFieldCategory nfc = pNativeFieldDescs[1].GetCategory(); - if (nfc == NativeFieldCategory::NESTED) - { - if (pNativeFieldDescs[1].GetNumElements() != 1) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - - MethodTable* pMethodTable2 = pNativeFieldDescs[1].GetNestedNativeMethodTable(); - - if (!IsOnlyOneField(pMethodTable2)) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - - if ((GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable2) & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - if (pFieldStart[1].GetSize() == 4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (pFieldStart[1].GetSize() == 8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - } - else if (nfc == NativeFieldCategory::FLOAT) + if ((GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable2) & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) { if (pFieldStart[1].GetSize() == 4) { @@ -3957,6 +3930,17 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) size |= STRUCT_SECOND_FIELD_SIZE_IS8; } } + else if (nfc == NativeFieldCategory::FLOAT) + { + if (pFieldStart[1].GetSize() == 4) + { + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); + } + else if (pFieldStart[1].GetSize() == 8) + { + size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); + } + } else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) { size = STRUCT_NO_FLOAT_FIELD; @@ -3966,6 +3950,14 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) size |= STRUCT_SECOND_FIELD_SIZE_IS8; } } + else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) + { + size = STRUCT_NO_FLOAT_FIELD; + } + else if (pFieldStart[1].GetSize() == 8) + { + size |= STRUCT_SECOND_FIELD_SIZE_IS8; + } } } _End_arg: From 7f1b761596905a1b544b94157be3eefd9b2ad8f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Thu, 21 Mar 2024 16:18:13 +0100 Subject: [PATCH 04/18] Simplify managed branch of GetRiscV64PassStructInRegisterFlags --- src/coreclr/vm/methodtable.cpp | 280 ++++++++++----------------------- 1 file changed, 82 insertions(+), 198 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 02bbc5e77a6e10..431668cb1e601b 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3438,222 +3438,106 @@ int MethodTable::GetLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cl #endif #if defined(TARGET_RISCV64) -int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) + +//TODO: need to do the same for NativeLayoutInfo +static bool GetFlattenedFieldTypes(MethodTable* pMT, CorElementType types[2], int* typeIndex) { - TypeHandle th(cls); + int nFields = pMT->GetNumIntroducedInstanceFields(); + if (*typeIndex + nFields > 2) + return false; - if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) - return STRUCT_NO_FLOAT_FIELD; + assert(nFields == 1 || nFields == 2); - int size = STRUCT_NO_FLOAT_FIELD; + FieldDesc* fields = pMT->GetApproxFieldDescListRaw(); + if (nFields == 2 && fields[0].GetSize() > fields[1].GetOffset()) // overlapping fields + return false; - if (!th.IsTypeDesc()) + int elementTypeIndex = *typeIndex; + + for (int i = 0; i < nFields; ++i) { - MethodTable* pMethodTable = th.AsMethodTable(); + if (fields[i].GetSize() > TARGET_POINTER_SIZE) + return false; - DWORD numIntroducedFields = pMethodTable->GetNumIntroducedInstanceFields(); - if (numIntroducedFields == 1) + CorElementType type = fields[i].GetFieldType(); + if (CorTypeInfo::IsPrimitiveType_NoThrow(type)) { - FieldDesc *pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); + assert(*typeIndex < 2); + types[(*typeIndex)++] = type; + } + else if (type == ELEMENT_TYPE_VALUETYPE) + { + if (!GetFlattenedFieldTypes(fields[i].GetApproxFieldTypeHandleThrowing().GetMethodTable(), types, typeIndex)) + return false; + } + else + { + return false; + } + } - CorElementType fieldType = pFieldStart[0].GetFieldType(); + if (HasImpliedRepeatedFields(pMT)) // inline array or fixed buffer + { + assert(nFields == 1); - // InlineArray types and fixed buffer types have implied repeated fields. - // Checking if a type is an InlineArray type is cheap, so we'll do that first. - bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); + int nFieldsPerElement = *typeIndex - elementTypeIndex; + assert(nFieldsPerElement == 1 || nFieldsPerElement == 2); - if (hasImpliedRepeatedFields) - { - numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); - if (numIntroducedFields > 2) - { - goto _End_arg; - } + int nElements = pMT->GetNumInstanceFieldBytes() / fields[0].GetSize(); + if (nElements > 2) + return false; - if (fieldType == ELEMENT_TYPE_R4) - { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FLOAT_FIELD_ONLY_TWO; - } - goto _End_arg; - } - else if (fieldType == ELEMENT_TYPE_R8) - { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FIELD_TWO_DOUBLES; - } - goto _End_arg; - } - } + if (nElements == 2 && nFieldsPerElement == 1) + { + if (*typeIndex + 1 > 2) + return false; - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - if (fieldType == ELEMENT_TYPE_R4) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - pMethodTable = pFieldStart->GetApproxFieldTypeHandleThrowing().GetMethodTable(); - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); - } + assert(elementTypeIndex == 0); + assert(*typeIndex == 1); + types[(*typeIndex)++] = types[elementTypeIndex]; // duplicate the array element type } - else if (numIntroducedFields == 2) - { - FieldDesc *pFieldSecond; - FieldDesc *pFieldFirst = pMethodTable->GetApproxFieldDescListRaw(); - if (pFieldFirst->GetOffset() == 0) - { - pFieldSecond = pFieldFirst + 1; - } - else - { - pFieldSecond = pFieldFirst; - pFieldFirst = pFieldFirst + 1; - } - assert(pFieldFirst->GetOffset() == 0); + } + return true; +} - if (pFieldFirst->GetSize() > 8) - { - goto _End_arg; - } +int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) +{ + TypeHandle th(cls); - if (pFieldFirst->GetSize() > pFieldSecond->GetOffset()) - { - goto _End_arg; - } + if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) + return STRUCT_NO_FLOAT_FIELD; - CorElementType fieldType = pFieldFirst[0].GetFieldType(); - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - if (fieldType == ELEMENT_TYPE_R4) - { - size = STRUCT_FLOAT_FIELD_FIRST; - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = STRUCT_FIRST_FIELD_DOUBLE; - } - else if (pFieldFirst[0].GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_SIZE_IS8; - } + int size = STRUCT_NO_FLOAT_FIELD; - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - pMethodTable = pFieldFirst->GetApproxFieldTypeHandleThrowing().GetMethodTable(); - if (IsOnlyOneField(pMethodTable)) - { - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); - if ((size & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - size = pFieldFirst[0].GetSize() == 8 ? STRUCT_FIRST_FIELD_DOUBLE : STRUCT_FLOAT_FIELD_FIRST; - } - else if (size == STRUCT_NO_FLOAT_FIELD) - { - size = pFieldFirst[0].GetSize() == 8 ? STRUCT_FIRST_FIELD_SIZE_IS8: 0; - } - else - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - } - else - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - } - else if (pFieldFirst[0].GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_SIZE_IS8; - } + if (!th.IsTypeDesc()) + { + CorElementType types[2] = {ELEMENT_TYPE_END, ELEMENT_TYPE_END}; + int nFields = 0; + if (!GetFlattenedFieldTypes(th.AsMethodTable(), types, &nFields) || nFields == 0) + goto _End_arg; - fieldType = pFieldSecond[0].GetFieldType(); - if (pFieldSecond[0].GetSize() > 8) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - else if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - if (fieldType == ELEMENT_TYPE_R4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldSecond[0].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - pMethodTable = pFieldSecond[0].GetApproxFieldTypeHandleThrowing().GetMethodTable(); - if (IsOnlyOneField(pMethodTable)) - { - int size2 = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); - if ((size2 & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - if (pFieldSecond[0].GetSize() == 8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (size2 == STRUCT_NO_FLOAT_FIELD) - { - size |= pFieldSecond[0].GetSize() == 8 ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0; - } - else - { - size = STRUCT_NO_FLOAT_FIELD; - } - } - else - { - size = STRUCT_NO_FLOAT_FIELD; - } - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldSecond[0].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } + assert(nFields == 1 || nFields == 2); + #ifdef DEBUG + assert(CorTypeInfo::IsPrimitiveType_NoThrow(types[0])); + assert(CorTypeInfo::IsPrimitiveType_NoThrow(types[1])); + #endif + + size = + (CorTypeInfo::IsFloat_NoThrow(types[0]) ? STRUCT_FLOAT_FIELD_FIRST : 0) | + (CorTypeInfo::IsFloat_NoThrow(types[1]) ? STRUCT_FLOAT_FIELD_SECOND : 0) | + (CorTypeInfo::Size_NoThrow(types[0]) == 8 ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | + (CorTypeInfo::Size_NoThrow(types[1]) == 8 ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); + + int firstOrSecond = size & (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND); + if (firstOrSecond == (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND)) + { + size |= STRUCT_FLOAT_FIELD_ONLY_TWO; + size &= ~(STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND); + } + else if (firstOrSecond != 0 && nFields == 1) + { + size |= STRUCT_FLOAT_FIELD_ONLY_ONE; + size &= ~(STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND); } } else From 3117efe36fe45c9b3bd0b600d91805c2a8e3c768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Thu, 21 Mar 2024 17:37:46 +0100 Subject: [PATCH 05/18] Fix assert IsPrimitiveType for 2nd field --- src/coreclr/vm/methodtable.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 431668cb1e601b..a7021984223c2a 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3517,10 +3517,8 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) goto _End_arg; assert(nFields == 1 || nFields == 2); - #ifdef DEBUG assert(CorTypeInfo::IsPrimitiveType_NoThrow(types[0])); - assert(CorTypeInfo::IsPrimitiveType_NoThrow(types[1])); - #endif + assert(CorTypeInfo::IsPrimitiveType_NoThrow(types[1]) || nFields == 1); size = (CorTypeInfo::IsFloat_NoThrow(types[0]) ? STRUCT_FLOAT_FIELD_FIRST : 0) | From 2a715bf0bcc4b6d7e4d45f74b9381920e7ab8b02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 22 Mar 2024 09:17:02 +0100 Subject: [PATCH 06/18] Handle empty structs --- src/coreclr/vm/methodtable.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index a7021984223c2a..1bf32fe9234839 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3446,6 +3446,9 @@ static bool GetFlattenedFieldTypes(MethodTable* pMT, CorElementType types[2], in if (*typeIndex + nFields > 2) return false; + if (nFields == 0) + return true; + assert(nFields == 1 || nFields == 2); FieldDesc* fields = pMT->GetApproxFieldDescListRaw(); From d0093112fa712bed86ee6e27b0d60204f02d7723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Mon, 25 Mar 2024 08:58:36 +0100 Subject: [PATCH 07/18] Apply FIELD_SIZE_IS8 flags only when there's at least one float --- src/coreclr/vm/methodtable.cpp | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 1bf32fe9234839..61d367200efe6f 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3525,21 +3525,25 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) size = (CorTypeInfo::IsFloat_NoThrow(types[0]) ? STRUCT_FLOAT_FIELD_FIRST : 0) | - (CorTypeInfo::IsFloat_NoThrow(types[1]) ? STRUCT_FLOAT_FIELD_SECOND : 0) | - (CorTypeInfo::Size_NoThrow(types[0]) == 8 ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | - (CorTypeInfo::Size_NoThrow(types[1]) == 8 ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); + (CorTypeInfo::IsFloat_NoThrow(types[1]) ? STRUCT_FLOAT_FIELD_SECOND : 0); + + if (size == 0) + goto _End_arg; - int firstOrSecond = size & (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND); - if (firstOrSecond == (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND)) + if (size == (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND)) { - size |= STRUCT_FLOAT_FIELD_ONLY_TWO; - size &= ~(STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND); + assert(nFields == 2); + size = STRUCT_FLOAT_FIELD_ONLY_TWO; } - else if (firstOrSecond != 0 && nFields == 1) + else if (nFields == 1) { - size |= STRUCT_FLOAT_FIELD_ONLY_ONE; - size &= ~(STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND); + assert(size == STRUCT_FLOAT_FIELD_FIRST); + size = STRUCT_FLOAT_FIELD_ONLY_ONE; } + + size |= + (CorTypeInfo::Size_NoThrow(types[0]) == 8 ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | + (CorTypeInfo::Size_NoThrow(types[1]) == 8 ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); } else { From 774f21a3205b206d19c064a69d95d29f094994c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Mon, 25 Mar 2024 09:28:59 +0100 Subject: [PATCH 08/18] Handle empty array struct elements --- src/coreclr/vm/methodtable.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 61d367200efe6f..24d59a2696b678 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3483,16 +3483,19 @@ static bool GetFlattenedFieldTypes(MethodTable* pMT, CorElementType types[2], in { assert(nFields == 1); - int nFieldsPerElement = *typeIndex - elementTypeIndex; - assert(nFieldsPerElement == 1 || nFieldsPerElement == 2); + int nFlattenedFieldsPerElement = *typeIndex - elementTypeIndex; + if (nFlattenedFieldsPerElement == 0) + return true; + + assert(nFlattenedFieldsPerElement == 1 || nFlattenedFieldsPerElement == 2); int nElements = pMT->GetNumInstanceFieldBytes() / fields[0].GetSize(); if (nElements > 2) return false; - if (nElements == 2 && nFieldsPerElement == 1) + if (nElements == 2) { - if (*typeIndex + 1 > 2) + if (*typeIndex + nFlattenedFieldsPerElement > 2) return false; assert(elementTypeIndex == 0); From 959f1f4d5bc835dfe67b8fb2064a93d47ae02b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Mon, 25 Mar 2024 10:18:54 +0100 Subject: [PATCH 09/18] Enregister any field type <= 8 bytes, not just primitives; i.e. pointers and refs are also OK --- src/coreclr/vm/methodtable.cpp | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 24d59a2696b678..6080789d26dc13 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3459,20 +3459,17 @@ static bool GetFlattenedFieldTypes(MethodTable* pMT, CorElementType types[2], in for (int i = 0; i < nFields; ++i) { - if (fields[i].GetSize() > TARGET_POINTER_SIZE) - return false; - CorElementType type = fields[i].GetFieldType(); - if (CorTypeInfo::IsPrimitiveType_NoThrow(type)) - { - assert(*typeIndex < 2); - types[(*typeIndex)++] = type; - } - else if (type == ELEMENT_TYPE_VALUETYPE) + if (type == ELEMENT_TYPE_VALUETYPE) { if (!GetFlattenedFieldTypes(fields[i].GetApproxFieldTypeHandleThrowing().GetMethodTable(), types, typeIndex)) return false; } + else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) + { + assert(*typeIndex < 2); + types[(*typeIndex)++] = type; + } else { return false; @@ -3523,8 +3520,8 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) goto _End_arg; assert(nFields == 1 || nFields == 2); - assert(CorTypeInfo::IsPrimitiveType_NoThrow(types[0])); - assert(CorTypeInfo::IsPrimitiveType_NoThrow(types[1]) || nFields == 1); + assert(CorTypeInfo::Size_NoThrow(types[0]) <= TARGET_POINTER_SIZE); + assert(CorTypeInfo::Size_NoThrow(types[1]) <= TARGET_POINTER_SIZE || nFields == 1); size = (CorTypeInfo::IsFloat_NoThrow(types[0]) ? STRUCT_FLOAT_FIELD_FIRST : 0) | @@ -3545,8 +3542,8 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) } size |= - (CorTypeInfo::Size_NoThrow(types[0]) == 8 ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | - (CorTypeInfo::Size_NoThrow(types[1]) == 8 ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); + (CorTypeInfo::Size_NoThrow(types[0]) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | + (CorTypeInfo::Size_NoThrow(types[1]) == TARGET_POINTER_SIZE ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); } else { From 5c534305bc0487e04daacf4e6c7be6dcbf53ecb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Tue, 26 Mar 2024 09:28:47 +0100 Subject: [PATCH 10/18] Simplify native layout branch of GetRiscV64PassStructInRegisterFlags --- src/coreclr/vm/methodtable.cpp | 403 ++++++--------------------------- src/coreclr/vm/methodtable.h | 4 +- 2 files changed, 64 insertions(+), 343 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 6080789d26dc13..46bd40f5d9fe55 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -2798,7 +2798,7 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe #endif // defined(UNIX_AMD64_ABI_ITF) -#if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) +#if defined(TARGET_LOONGARCH64) bool MethodTable::IsOnlyOneField(MethodTable * pMT) { TypeHandle th(pMT); @@ -3439,10 +3439,13 @@ int MethodTable::GetLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cl #if defined(TARGET_RISCV64) -//TODO: need to do the same for NativeLayoutInfo -static bool GetFlattenedFieldTypes(MethodTable* pMT, CorElementType types[2], int* typeIndex) +static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, CorElementType types[2], int* typeIndex) { - int nFields = pMT->GetNumIntroducedInstanceFields(); + TypeHandle th(cls); + bool isManaged = !th.IsTypeDesc(); + MethodTable* pMT = isManaged ? th.AsMethodTable() : th.AsNativeValueType(); + int nFields = isManaged ? pMT->GetNumIntroducedInstanceFields() : pMT->GetNativeLayoutInfo()->GetNumFields(); + if (*typeIndex + nFields > 2) return false; @@ -3462,8 +3465,38 @@ static bool GetFlattenedFieldTypes(MethodTable* pMT, CorElementType types[2], in CorElementType type = fields[i].GetFieldType(); if (type == ELEMENT_TYPE_VALUETYPE) { - if (!GetFlattenedFieldTypes(fields[i].GetApproxFieldTypeHandleThrowing().GetMethodTable(), types, typeIndex)) - return false; + if (isManaged) + { + MethodTable* nested = fields[i].GetApproxFieldTypeHandleThrowing().GetMethodTable(); + if (!GetFlattenedFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) + return false; + } + else + { + const NativeFieldDescriptor& nativeField = pMT->GetNativeLayoutInfo()->GetNativeFieldDescriptors()[i]; + NativeFieldCategory category = nativeField.GetCategory(); + if (category == NativeFieldCategory::NESTED) + { + MethodTable* nested = nativeField.GetNestedNativeMethodTable(); + if (!GetFlattenedFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) + return false; + } + else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) + { + bool is8 = (fields[i].GetSize() == TARGET_POINTER_SIZE); + if (category == NativeFieldCategory::FLOAT) + type = is8 ? ELEMENT_TYPE_R8 : ELEMENT_TYPE_R4; + else + type = is8 ? ELEMENT_TYPE_I8 : ELEMENT_TYPE_I4; + + assert(*typeIndex < 2); + types[(*typeIndex)++] = type; + } + else + { + return false; + } + } } else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) { @@ -3510,348 +3543,38 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) return STRUCT_NO_FLOAT_FIELD; - int size = STRUCT_NO_FLOAT_FIELD; - - if (!th.IsTypeDesc()) - { - CorElementType types[2] = {ELEMENT_TYPE_END, ELEMENT_TYPE_END}; - int nFields = 0; - if (!GetFlattenedFieldTypes(th.AsMethodTable(), types, &nFields) || nFields == 0) - goto _End_arg; - - assert(nFields == 1 || nFields == 2); - assert(CorTypeInfo::Size_NoThrow(types[0]) <= TARGET_POINTER_SIZE); - assert(CorTypeInfo::Size_NoThrow(types[1]) <= TARGET_POINTER_SIZE || nFields == 1); + CorElementType types[2] = {ELEMENT_TYPE_END, ELEMENT_TYPE_END}; + int nFields = 0; + if (!GetFlattenedFieldTypes(cls, types, &nFields) || nFields == 0) + return STRUCT_NO_FLOAT_FIELD; - size = - (CorTypeInfo::IsFloat_NoThrow(types[0]) ? STRUCT_FLOAT_FIELD_FIRST : 0) | - (CorTypeInfo::IsFloat_NoThrow(types[1]) ? STRUCT_FLOAT_FIELD_SECOND : 0); + assert(nFields == 1 || nFields == 2); + assert(CorTypeInfo::Size_NoThrow(types[0]) <= TARGET_POINTER_SIZE); + assert(CorTypeInfo::Size_NoThrow(types[1]) <= TARGET_POINTER_SIZE || nFields == 1); - if (size == 0) - goto _End_arg; + int flags = + (CorTypeInfo::IsFloat_NoThrow(types[0]) ? STRUCT_FLOAT_FIELD_FIRST : 0) | + (CorTypeInfo::IsFloat_NoThrow(types[1]) ? STRUCT_FLOAT_FIELD_SECOND : 0); - if (size == (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND)) - { - assert(nFields == 2); - size = STRUCT_FLOAT_FIELD_ONLY_TWO; - } - else if (nFields == 1) - { - assert(size == STRUCT_FLOAT_FIELD_FIRST); - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } + if (flags == STRUCT_NO_FLOAT_FIELD) + return STRUCT_NO_FLOAT_FIELD; - size |= - (CorTypeInfo::Size_NoThrow(types[0]) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | - (CorTypeInfo::Size_NoThrow(types[1]) == TARGET_POINTER_SIZE ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); + if (flags == (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND)) + { + assert(nFields == 2); + flags = STRUCT_FLOAT_FIELD_ONLY_TWO; } - else + else if (nFields == 1) { - MethodTable* pMethodTable = th.AsNativeValueType(); - DWORD numIntroducedFields = pMethodTable->GetNativeLayoutInfo()->GetNumFields(); - FieldDesc *pFieldStart = nullptr; - - if (numIntroducedFields == 1) - { - pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); - - CorElementType fieldType = pFieldStart->GetFieldType(); - - // InlineArray types and fixed buffer types have implied repeated fields. - // Checking if a type is an InlineArray type is cheap, so we'll do that first. - bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(pMethodTable); - - if (hasImpliedRepeatedFields) - { - numIntroducedFields = pMethodTable->GetNumInstanceFieldBytes() / pFieldStart->GetSize(); - if (numIntroducedFields > 2) - { - goto _End_arg; - } - - if (fieldType == ELEMENT_TYPE_R4) - { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FLOAT_FIELD_ONLY_TWO; - } - goto _End_arg; - } - else if (fieldType == ELEMENT_TYPE_R8) - { - if (numIntroducedFields == 1) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - else if (numIntroducedFields == 2) - { - size = STRUCT_FIELD_TWO_DOUBLES; - } - goto _End_arg; - } - } - - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - if (fieldType == ELEMENT_TYPE_R4) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); - if (nfc == NativeFieldCategory::NESTED) - { - pMethodTable = pNativeFieldDescs->GetNestedNativeMethodTable(); - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); - return size; - } - else if (nfc == NativeFieldCategory::FLOAT) - { - if (pFieldStart->GetSize() == 4) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (pFieldStart->GetSize() == 8) - { - size = STRUCT_FLOAT_FIELD_ONLY_ONE | STRUCT_FIRST_FIELD_SIZE_IS8; - } - } - } - } - else if (numIntroducedFields == 2) - { - pFieldStart = pMethodTable->GetApproxFieldDescListRaw(); - - if (pFieldStart->GetSize() > 8) - { - goto _End_arg; - } - - if (pFieldStart->GetOffset() || !pFieldStart[1].GetOffset() || (pFieldStart[0].GetSize() > pFieldStart[1].GetOffset())) - { - goto _End_arg; - } - - CorElementType fieldType = pFieldStart[0].GetFieldType(); - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - if (fieldType == ELEMENT_TYPE_R4) - { - size = STRUCT_FLOAT_FIELD_FIRST; - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = STRUCT_FIRST_FIELD_DOUBLE; - } - else if (pFieldStart[0].GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_SIZE_IS8; - } - - fieldType = pFieldStart[1].GetFieldType(); - if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - if (fieldType == ELEMENT_TYPE_R4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - goto _End_arg; - } - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - - NativeFieldCategory nfc = pNativeFieldDescs->GetCategory(); - - if (nfc == NativeFieldCategory::NESTED) - { - if (pNativeFieldDescs->GetNumElements() != 1) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - - MethodTable* pMethodTable2 = pNativeFieldDescs->GetNestedNativeMethodTable(); - - if (!IsOnlyOneField(pMethodTable2)) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - - size = GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable2); - if ((size & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - if (pFieldStart->GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_DOUBLE; - } - else - { - size = STRUCT_FLOAT_FIELD_FIRST; - } - } - else if (pFieldStart->GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_SIZE_IS8; - } - else - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - } - else if (nfc == NativeFieldCategory::FLOAT) - { - if (pFieldStart[0].GetSize() == 4) - { - size = STRUCT_FLOAT_FIELD_FIRST; - } - else if (pFieldStart[0].GetSize() == 8) - { - _ASSERTE((pMethodTable->GetNativeSize() == 8) || (pMethodTable->GetNativeSize() == 16)); - size = STRUCT_FIRST_FIELD_DOUBLE; - } - } - else if (pFieldStart[0].GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_SIZE_IS8; - } - } - else if (pFieldStart[0].GetSize() == 8) - { - size = STRUCT_FIRST_FIELD_SIZE_IS8; - } - - fieldType = pFieldStart[1].GetFieldType(); - if (pFieldStart[1].GetSize() > 8) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - else if (CorTypeInfo::IsPrimitiveType_NoThrow(fieldType)) - { - if (fieldType == ELEMENT_TYPE_R4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (fieldType == ELEMENT_TYPE_R8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - - // Pass with two integer registers in `struct {int a, int b, float/double c}` cases - if ((size | STRUCT_FIRST_FIELD_SIZE_IS8 | STRUCT_FLOAT_FIELD_SECOND) == size) - { - size = STRUCT_NO_FLOAT_FIELD; - } - } - else if (fieldType == ELEMENT_TYPE_VALUETYPE) - { - const NativeFieldDescriptor *pNativeFieldDescs = pMethodTable->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - NativeFieldCategory nfc = pNativeFieldDescs[1].GetCategory(); - - if (nfc == NativeFieldCategory::NESTED) - { - if (pNativeFieldDescs[1].GetNumElements() != 1) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - - MethodTable* pMethodTable2 = pNativeFieldDescs[1].GetNestedNativeMethodTable(); - - if (!IsOnlyOneField(pMethodTable2)) - { - size = STRUCT_NO_FLOAT_FIELD; - goto _End_arg; - } - - if ((GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable2) & STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - if (pFieldStart[1].GetSize() == 4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (pFieldStart[1].GetSize() == 8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - } - else if (nfc == NativeFieldCategory::FLOAT) - { - if (pFieldStart[1].GetSize() == 4) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND) : (size | STRUCT_FLOAT_FIELD_SECOND); - } - else if (pFieldStart[1].GetSize() == 8) - { - size = size & STRUCT_FLOAT_FIELD_FIRST ? (size ^ STRUCT_MERGE_FIRST_SECOND_8) : (size | STRUCT_SECOND_FIELD_DOUBLE); - } - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - } - else if ((size & STRUCT_FLOAT_FIELD_FIRST) == 0) - { - size = STRUCT_NO_FLOAT_FIELD; - } - else if (pFieldStart[1].GetSize() == 8) - { - size |= STRUCT_SECOND_FIELD_SIZE_IS8; - } - } + assert(flags == STRUCT_FLOAT_FIELD_FIRST); + flags = STRUCT_FLOAT_FIELD_ONLY_ONE; } -_End_arg: - return size; + flags |= + (CorTypeInfo::Size_NoThrow(types[0]) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | + (CorTypeInfo::Size_NoThrow(types[1]) == TARGET_POINTER_SIZE ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); + + return flags; } #endif diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index e6c3c8c3aa2bec..3f05b695cb1a1e 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -824,14 +824,12 @@ class MethodTable // during object construction. void CheckRunClassInitAsIfConstructingThrowing(); -#if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - static bool IsOnlyOneField(MethodTable * pMT); #if defined(TARGET_LOONGARCH64) + static bool IsOnlyOneField(MethodTable * pMT); static int GetLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE clh); #elif defined(TARGET_RISCV64) static int GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE clh); #endif -#endif #if defined(UNIX_AMD64_ABI_ITF) // Builds the internal data structures and classifies struct eightbytes for Amd System V calling convention. From 0ac27cef816810ce492d4139fb1b69b13b0b2110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Tue, 26 Mar 2024 15:57:41 +0000 Subject: [PATCH 11/18] Rewrite native branch to look at only at native layout info --- src/coreclr/vm/methodtable.cpp | 134 +++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 57 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 46bd40f5d9fe55..ebe0e0a61ea81f 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3438,6 +3438,28 @@ int MethodTable::GetLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cl #endif #if defined(TARGET_RISCV64) +static bool HandleInlineArray(int elementTypeIndex, int nElements, CorElementType types[2], int* typeIndex) +{ + int nFlattenedFieldsPerElement = *typeIndex - elementTypeIndex; + if (nFlattenedFieldsPerElement == 0) + return true; + + assert(nFlattenedFieldsPerElement == 1 || nFlattenedFieldsPerElement == 2); + + if (nElements > 2) + return false; + + if (nElements == 2) + { + if (*typeIndex + nFlattenedFieldsPerElement > 2) + return false; + + assert(elementTypeIndex == 0); + assert(*typeIndex == 1); + types[(*typeIndex)++] = types[elementTypeIndex]; // duplicate the array element type + } + return true; +} static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, CorElementType types[2], int* typeIndex) { @@ -3454,83 +3476,81 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, CorElementType type assert(nFields == 1 || nFields == 2); - FieldDesc* fields = pMT->GetApproxFieldDescListRaw(); - if (nFields == 2 && fields[0].GetSize() > fields[1].GetOffset()) // overlapping fields - return false; - - int elementTypeIndex = *typeIndex; - - for (int i = 0; i < nFields; ++i) + // TODO: templatize isManaged and use if constexpr for differences when we migrate to C++17 + // because the logic for both branches is nearly the same. + if (isManaged) { - CorElementType type = fields[i].GetFieldType(); - if (type == ELEMENT_TYPE_VALUETYPE) + FieldDesc* fields = pMT->GetApproxFieldDescListRaw(); + if (nFields == 2 && fields[0].GetSize() > fields[1].GetOffset()) // overlapping fields + return false; + + int elementTypeIndex = *typeIndex; + for (int i = 0; i < nFields; ++i) { - if (isManaged) + CorElementType type = fields[i].GetFieldType(); + if (type == ELEMENT_TYPE_VALUETYPE) { MethodTable* nested = fields[i].GetApproxFieldTypeHandleThrowing().GetMethodTable(); if (!GetFlattenedFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) return false; } - else + else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) { - const NativeFieldDescriptor& nativeField = pMT->GetNativeLayoutInfo()->GetNativeFieldDescriptors()[i]; - NativeFieldCategory category = nativeField.GetCategory(); - if (category == NativeFieldCategory::NESTED) - { - MethodTable* nested = nativeField.GetNestedNativeMethodTable(); - if (!GetFlattenedFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) - return false; - } - else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) - { - bool is8 = (fields[i].GetSize() == TARGET_POINTER_SIZE); - if (category == NativeFieldCategory::FLOAT) - type = is8 ? ELEMENT_TYPE_R8 : ELEMENT_TYPE_R4; - else - type = is8 ? ELEMENT_TYPE_I8 : ELEMENT_TYPE_I4; - - assert(*typeIndex < 2); - types[(*typeIndex)++] = type; - } - else - { + if (*typeIndex >= 2) return false; - } + + types[(*typeIndex)++] = type; + } + else + { + return false; } } - else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) - { - assert(*typeIndex < 2); - types[(*typeIndex)++] = type; - } - else + + if (HasImpliedRepeatedFields(pMT)) // inline array or fixed buffer { - return false; + assert(nFields == 1); + int nElements = pMT->GetNumInstanceFieldBytes() / pMT->GetApproxFieldDescListRaw()->GetSize(); + if (!HandleInlineArray(elementTypeIndex, nElements, types, typeIndex)) + return false; } } - - if (HasImpliedRepeatedFields(pMT)) // inline array or fixed buffer + else // native layout { - assert(nFields == 1); + const NativeFieldDescriptor* fields = pMT->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); + if (nFields == 2 && fields[0].NativeSize() > fields[1].GetExternalOffset()) // overlapping fields + return false; - int nFlattenedFieldsPerElement = *typeIndex - elementTypeIndex; - if (nFlattenedFieldsPerElement == 0) - return true; + for (int i = 0; i < nFields; ++i) + { + NativeFieldCategory category = fields[i].GetCategory(); + if (category == NativeFieldCategory::NESTED) + { + int elementTypeIndex = *typeIndex; - assert(nFlattenedFieldsPerElement == 1 || nFlattenedFieldsPerElement == 2); + MethodTable* nested = fields[i].GetNestedNativeMethodTable(); + if (!GetFlattenedFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) + return false; - int nElements = pMT->GetNumInstanceFieldBytes() / fields[0].GetSize(); - if (nElements > 2) - return false; + // In native layout fixed arrays are marked as NESTED just like structs + int nElements = fields[i].GetNumElements(); + if (!HandleInlineArray(elementTypeIndex, nElements, types, typeIndex)) + return false; + } + else if (fields[i].NativeSize() <= TARGET_POINTER_SIZE) + { + if (*typeIndex >= 2) + return false; - if (nElements == 2) - { - if (*typeIndex + nFlattenedFieldsPerElement > 2) + bool is8 = (fields[i].NativeSize() == TARGET_POINTER_SIZE); + types[(*typeIndex)++] = (category == NativeFieldCategory::FLOAT) + ? (is8 ? ELEMENT_TYPE_R8 : ELEMENT_TYPE_R4) + : (is8 ? ELEMENT_TYPE_I8 : ELEMENT_TYPE_I4); + } + else + { return false; - - assert(elementTypeIndex == 0); - assert(*typeIndex == 1); - types[(*typeIndex)++] = types[elementTypeIndex]; // duplicate the array element type + } } } return true; From 98cd8c3958a5d8e7d5e5f054ccdf3d3e1d6cadf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Wed, 27 Mar 2024 09:32:46 +0100 Subject: [PATCH 12/18] Calculate flags already in GetFlattenedFieldTypes to avoid returning fake CorElementTypes from native branch --- src/coreclr/vm/methodtable.cpp | 43 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index ebe0e0a61ea81f..d06a6fbdcab914 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3438,7 +3438,7 @@ int MethodTable::GetLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cl #endif #if defined(TARGET_RISCV64) -static bool HandleInlineArray(int elementTypeIndex, int nElements, CorElementType types[2], int* typeIndex) +static bool HandleInlineArray(int elementTypeIndex, int nElements, int types[2], int* typeIndex) { int nFlattenedFieldsPerElement = *typeIndex - elementTypeIndex; if (nFlattenedFieldsPerElement == 0) @@ -3461,7 +3461,7 @@ static bool HandleInlineArray(int elementTypeIndex, int nElements, CorElementTyp return true; } -static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, CorElementType types[2], int* typeIndex) +static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* typeIndex) { TypeHandle th(cls); bool isManaged = !th.IsTypeDesc(); @@ -3499,7 +3499,10 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, CorElementType type if (*typeIndex >= 2) return false; - types[(*typeIndex)++] = type; + int retType = (CorTypeInfo::IsFloat_NoThrow(type) ? STRUCT_FLOAT_FIELD_FIRST : 0) | + (CorTypeInfo::Size_NoThrow(type) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0); + + types[(*typeIndex)++] = retType; } else { @@ -3542,10 +3545,10 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, CorElementType type if (*typeIndex >= 2) return false; - bool is8 = (fields[i].NativeSize() == TARGET_POINTER_SIZE); - types[(*typeIndex)++] = (category == NativeFieldCategory::FLOAT) - ? (is8 ? ELEMENT_TYPE_R8 : ELEMENT_TYPE_R4) - : (is8 ? ELEMENT_TYPE_I8 : ELEMENT_TYPE_I4); + int type = (category == NativeFieldCategory::FLOAT ? STRUCT_FLOAT_FIELD_FIRST : 0) | + (fields[i].NativeSize() == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0); + + types[(*typeIndex)++] = type; } else { @@ -3563,37 +3566,33 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) return STRUCT_NO_FLOAT_FIELD; - CorElementType types[2] = {ELEMENT_TYPE_END, ELEMENT_TYPE_END}; + int types[2] = {STRUCT_NO_FLOAT_FIELD, STRUCT_NO_FLOAT_FIELD}; int nFields = 0; if (!GetFlattenedFieldTypes(cls, types, &nFields) || nFields == 0) return STRUCT_NO_FLOAT_FIELD; assert(nFields == 1 || nFields == 2); - assert(CorTypeInfo::Size_NoThrow(types[0]) <= TARGET_POINTER_SIZE); - assert(CorTypeInfo::Size_NoThrow(types[1]) <= TARGET_POINTER_SIZE || nFields == 1); - int flags = - (CorTypeInfo::IsFloat_NoThrow(types[0]) ? STRUCT_FLOAT_FIELD_FIRST : 0) | - (CorTypeInfo::IsFloat_NoThrow(types[1]) ? STRUCT_FLOAT_FIELD_SECOND : 0); + static_assert((STRUCT_FLOAT_FIELD_SECOND | STRUCT_SECOND_FIELD_SIZE_IS8) + == (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FIRST_FIELD_SIZE_IS8) << 1, + "SECOND flags need to be FIRST shifted by 1"); + int flags = types[0] | (types[1] << 1); - if (flags == STRUCT_NO_FLOAT_FIELD) + static const int bothFloat = STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND; + if ((flags & bothFloat) == 0) return STRUCT_NO_FLOAT_FIELD; - if (flags == (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND)) + if ((flags & bothFloat) == bothFloat) { assert(nFields == 2); - flags = STRUCT_FLOAT_FIELD_ONLY_TWO; + flags ^= (bothFloat | STRUCT_FLOAT_FIELD_ONLY_TWO); // replace bothFloat with ONLY_TWO } else if (nFields == 1) { - assert(flags == STRUCT_FLOAT_FIELD_FIRST); - flags = STRUCT_FLOAT_FIELD_ONLY_ONE; + assert((flags & STRUCT_FLOAT_FIELD_FIRST) != 0); + flags ^= (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_ONLY_ONE); // replace FIRST with ONLY_ONE } - flags |= - (CorTypeInfo::Size_NoThrow(types[0]) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0) | - (CorTypeInfo::Size_NoThrow(types[1]) == TARGET_POINTER_SIZE ? STRUCT_SECOND_FIELD_SIZE_IS8 : 0); - return flags; } #endif From b846eff90bba6ccd28edafe006c4e491b053ec7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Wed, 27 Mar 2024 10:50:18 +0100 Subject: [PATCH 13/18] Ignore empty structs during field flattenting because RISC-V calling convention tells us to --- src/coreclr/vm/methodtable.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index d06a6fbdcab914..2d57ee160dc17e 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3467,26 +3467,20 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* bool isManaged = !th.IsTypeDesc(); MethodTable* pMT = isManaged ? th.AsMethodTable() : th.AsNativeValueType(); int nFields = isManaged ? pMT->GetNumIntroducedInstanceFields() : pMT->GetNativeLayoutInfo()->GetNumFields(); - - if (*typeIndex + nFields > 2) - return false; - if (nFields == 0) return true; - assert(nFields == 1 || nFields == 2); - // TODO: templatize isManaged and use if constexpr for differences when we migrate to C++17 // because the logic for both branches is nearly the same. if (isManaged) { FieldDesc* fields = pMT->GetApproxFieldDescListRaw(); - if (nFields == 2 && fields[0].GetSize() > fields[1].GetOffset()) // overlapping fields - return false; - int elementTypeIndex = *typeIndex; for (int i = 0; i < nFields; ++i) { + if (i > 0 && fields[i-1].GetOffset() + fields[i-1].GetSize() > fields[i].GetOffset()) + return false; // overlapping fields + CorElementType type = fields[i].GetFieldType(); if (type == ELEMENT_TYPE_VALUETYPE) { @@ -3521,11 +3515,11 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* else // native layout { const NativeFieldDescriptor* fields = pMT->GetNativeLayoutInfo()->GetNativeFieldDescriptors(); - if (nFields == 2 && fields[0].NativeSize() > fields[1].GetExternalOffset()) // overlapping fields - return false; - for (int i = 0; i < nFields; ++i) { + if (i > 0 && fields[i-1].GetExternalOffset() + fields[i-1].NativeSize() > fields[i].GetExternalOffset()) + return false; // overlapping fields + NativeFieldCategory category = fields[i].GetCategory(); if (category == NativeFieldCategory::NESTED) { From 50515a41595cc9e209f9537738153313e568c9cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Wed, 27 Mar 2024 15:50:12 +0100 Subject: [PATCH 14/18] Simplify crossgen2 GetRISCV64PassStructInRegisterFlags, make C++ and C# versions of this method look more alike --- .../tools/Common/JitInterface/CorInfoTypes.cs | 1 + .../RISCV64PassStructInRegister.cs | 282 ++++++------------ src/coreclr/vm/methodtable.cpp | 44 +-- 3 files changed, 121 insertions(+), 206 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index 2276c5cdc9a7cc..960b2e8e8113b3 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -1235,6 +1235,7 @@ public struct SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR // bit 5: `1` means the second field's size is 8. // // Note that bit 0 and 3 cannot both be set. + [Flags] public enum StructFloatFieldInfoFlags { STRUCT_NO_FLOAT_FIELD = 0x0, diff --git a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs index 8edac07cdfa1f9..7166b5a503366c 100644 --- a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs +++ b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs @@ -1,217 +1,129 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Diagnostics; using ILCompiler; using Internal.TypeSystem; +using static Internal.JitInterface.StructFloatFieldInfoFlags; namespace Internal.JitInterface { - internal static class RISCV64PassStructInRegister { - public static uint GetRISCV64PassStructInRegisterFlags(TypeDesc typeDesc) - { - FieldDesc firstField = null; - uint floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - int numIntroducedFields = 0; - foreach (FieldDesc field in typeDesc.GetFields()) - { - if (!field.IsStatic) - { - firstField ??= field; - numIntroducedFields++; - } - } + private const int + ENREGISTERED_PARAMTYPE_MAXSIZE = 16, + TARGET_POINTER_SIZE = 8; - if ((numIntroducedFields == 0) || (numIntroducedFields > 2) || (typeDesc.GetElementSize().AsInt > 16)) - { - return (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } + private static bool HandleInlineArray(int elementTypeIndex, int nElements, StructFloatFieldInfoFlags[] types, ref int typeIndex) + { + int nFlattenedFieldsPerElement = typeIndex - elementTypeIndex; + if (nFlattenedFieldsPerElement == 0) + return true; - MetadataType mdType = typeDesc as MetadataType; - Debug.Assert(mdType != null); + Debug.Assert(nFlattenedFieldsPerElement == 1 || nFlattenedFieldsPerElement == 2); - TypeDesc firstFieldElementType = firstField.FieldType; - int firstFieldSize = firstFieldElementType.GetElementSize().AsInt; - bool hasImpliedRepeatedFields = mdType.HasImpliedRepeatedFields(); + if (nElements > 2) + return false; - if (hasImpliedRepeatedFields) + if (nElements == 2) { - numIntroducedFields = typeDesc.GetElementSize().AsInt / firstFieldSize; - if (numIntroducedFields > 2) - { - return (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } + if (typeIndex + nFlattenedFieldsPerElement > 2) + return false; + + Debug.Assert(elementTypeIndex == 0); + Debug.Assert(typeIndex == 1); + types[typeIndex++] = types[elementTypeIndex]; // duplicate the array element type } + return true; + } - int fieldIndex = 0; - foreach (FieldDesc field in typeDesc.GetFields()) + private static bool FlattenFieldTypes(TypeDesc td, StructFloatFieldInfoFlags[] types, ref int typeIndex) + { + IEnumerable fields = td.GetFields(); + int nFields = 0; + int elementTypeIndex = typeIndex; + FieldDesc prevField = null; + foreach (FieldDesc field in fields) { - if (fieldIndex > 1) - { - return (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } - else if (field.IsStatic) - { + if (field.IsStatic) continue; - } - else if (fieldIndex == 1) + nFields++; + + if (prevField != null && prevField.Offset.AsInt + prevField.FieldType.GetElementSize().AsInt > field.Offset.AsInt) + return false; // overlapping fields + + prevField = field; + + TypeFlags category = field.FieldType.Category; + if (category == TypeFlags.ValueType) { - if (firstField.Offset.AsInt != 0 || field.Offset.AsInt == 0 || firstFieldSize > field.Offset.AsInt) - { - return (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } + TypeDesc nested = field.FieldType; + if (!FlattenFieldTypes(nested, types, ref typeIndex)) + return false; } + else if (field.FieldType.GetElementSize().AsInt <= TARGET_POINTER_SIZE) + { + if (typeIndex >= 2) + return false; - Debug.Assert(fieldIndex < numIntroducedFields); + StructFloatFieldInfoFlags type = + (category is TypeFlags.Single or TypeFlags.Double ? STRUCT_FLOAT_FIELD_FIRST : (StructFloatFieldInfoFlags)0) | + (field.FieldType.GetElementSize().AsInt == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : (StructFloatFieldInfoFlags)0); - switch (field.FieldType.Category) + types[typeIndex++] = type; + } + else { - case TypeFlags.Double: - { - if (numIntroducedFields == 1) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (fieldIndex == 0) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_FIRST_FIELD_DOUBLE; - } - else if ((floatFieldFlags & (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST) != 0) - { - floatFieldFlags ^= (uint)StructFloatFieldInfoFlags.STRUCT_MERGE_FIRST_SECOND_8; - } - else - { - floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_SECOND_FIELD_DOUBLE; - } - - // Pass with two integer registers in `struct {int a, int b, float/double c}` cases - if (fieldIndex == 1 && - (floatFieldFlags | - (uint)StructFloatFieldInfoFlags.STRUCT_FIRST_FIELD_SIZE_IS8 | - (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND) == - floatFieldFlags) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } - } - break; - - case TypeFlags.Single: - { - if (numIntroducedFields == 1) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_ONLY_ONE; - } - else if (fieldIndex == 0) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST; - } - else if ((floatFieldFlags & (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST) != 0) - { - floatFieldFlags ^= (uint)StructFloatFieldInfoFlags.STRUCT_MERGE_FIRST_SECOND; - } - else - { - floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND; - } - - // Pass with two integer registers in `struct {int a, int b, float/double c}` cases - if (fieldIndex == 1 && - (floatFieldFlags | - (uint)StructFloatFieldInfoFlags.STRUCT_FIRST_FIELD_SIZE_IS8 | - (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND) == - floatFieldFlags) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } - } - break; - - case TypeFlags.ValueType: - //case TypeFlags.Class: - //case TypeFlags.Array: - //case TypeFlags.SzArray: - { - uint floatFieldFlags2 = GetRISCV64PassStructInRegisterFlags(field.FieldType); - if (numIntroducedFields == 1) - { - floatFieldFlags = floatFieldFlags2; - } - else if (field.FieldType.GetElementSize().AsInt > 8) - { - return (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } - else if (fieldIndex == 0) - { - if ((floatFieldFlags2 & (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST; - } - if (field.FieldType.GetElementSize().AsInt == 8) - { - floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_FIRST_FIELD_SIZE_IS8; - } - } - else - { - Debug.Assert(fieldIndex == 1); - if ((floatFieldFlags2 & (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_ONLY_ONE) != 0) - { - floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND; - } - if (field.FieldType.GetElementSize().AsInt == 8) - { - floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_SECOND_FIELD_SIZE_IS8; - } - - floatFieldFlags2 = floatFieldFlags & ((uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST | (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND); - if (floatFieldFlags2 == 0) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } - else if (floatFieldFlags2 == ((uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST | (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND)) - { - floatFieldFlags ^= ((uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_ONLY_TWO | (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST | (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_SECOND); - } - } - } - break; - - default: - { - if (field.FieldType.GetElementSize().AsInt == 8) - { - if (numIntroducedFields > 1) - { - if (fieldIndex == 0) - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_FIRST_FIELD_SIZE_IS8; - } - else if ((floatFieldFlags & (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST) != 0) - { - floatFieldFlags |= (uint)StructFloatFieldInfoFlags.STRUCT_SECOND_FIELD_SIZE_IS8; - } - else - { - floatFieldFlags = (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } - } - } - else if (fieldIndex == 1) - { - floatFieldFlags = (floatFieldFlags & (uint)StructFloatFieldInfoFlags.STRUCT_FLOAT_FIELD_FIRST) > 0 ? floatFieldFlags : (uint)StructFloatFieldInfoFlags.STRUCT_NO_FLOAT_FIELD; - } - break; - } + return false; } + } - fieldIndex++; + if (nFields == 0) + return true; + + if ((td as MetadataType).HasImpliedRepeatedFields()) + { + Debug.Assert(nFields == 1); + int nElements = td.GetElementSize().AsInt / prevField.FieldType.GetElementSize().AsInt; + if (!HandleInlineArray(elementTypeIndex, nElements, types, ref typeIndex)) + return false; } + return true; + } + + public static uint GetRISCV64PassStructInRegisterFlags(TypeDesc td) + { + if (td.GetElementSize().AsInt > ENREGISTERED_PARAMTYPE_MAXSIZE) + return (uint)STRUCT_NO_FLOAT_FIELD; + + StructFloatFieldInfoFlags[] types = {STRUCT_NO_FLOAT_FIELD, STRUCT_NO_FLOAT_FIELD}; + int nFields = 0; + if (!FlattenFieldTypes(td, types, ref nFields) || nFields == 0) + return (uint)STRUCT_NO_FLOAT_FIELD; + + Debug.Assert(nFields == 1 || nFields == 2); + + Debug.Assert((uint)(STRUCT_FLOAT_FIELD_SECOND | STRUCT_SECOND_FIELD_SIZE_IS8) + == (uint)(STRUCT_FLOAT_FIELD_FIRST | STRUCT_FIRST_FIELD_SIZE_IS8) << 1, + "SECOND flags need to be FIRST shifted by 1"); + StructFloatFieldInfoFlags flags = types[0] | (StructFloatFieldInfoFlags)((uint)types[1] << 1); + + const StructFloatFieldInfoFlags bothFloat = STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_SECOND; + if ((flags & bothFloat) == 0) + return (uint)STRUCT_NO_FLOAT_FIELD; - return floatFieldFlags; + if ((flags & bothFloat) == bothFloat) + { + Debug.Assert(nFields == 2); + flags ^= (bothFloat | STRUCT_FLOAT_FIELD_ONLY_TWO); // replace bothFloat with ONLY_TWO + } + else if (nFields == 1) + { + Debug.Assert((flags & STRUCT_FLOAT_FIELD_FIRST) != 0); + flags ^= (STRUCT_FLOAT_FIELD_FIRST | STRUCT_FLOAT_FIELD_ONLY_ONE); // replace FIRST with ONLY_ONE + } + return (uint)flags; } } } diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 2d57ee160dc17e..6f26e69f376727 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3438,9 +3438,9 @@ int MethodTable::GetLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cl #endif #if defined(TARGET_RISCV64) -static bool HandleInlineArray(int elementTypeIndex, int nElements, int types[2], int* typeIndex) +static bool HandleInlineArray(int elementTypeIndex, int nElements, StructFloatFieldInfoFlags types[2], int& typeIndex) { - int nFlattenedFieldsPerElement = *typeIndex - elementTypeIndex; + int nFlattenedFieldsPerElement = typeIndex - elementTypeIndex; if (nFlattenedFieldsPerElement == 0) return true; @@ -3451,17 +3451,17 @@ static bool HandleInlineArray(int elementTypeIndex, int nElements, int types[2], if (nElements == 2) { - if (*typeIndex + nFlattenedFieldsPerElement > 2) + if (typeIndex + nFlattenedFieldsPerElement > 2) return false; assert(elementTypeIndex == 0); - assert(*typeIndex == 1); - types[(*typeIndex)++] = types[elementTypeIndex]; // duplicate the array element type + assert(typeIndex == 1); + types[typeIndex] = types[elementTypeIndex]; // duplicate the array element type } return true; } -static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* typeIndex) +static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlags types[2], int& typeIndex) { TypeHandle th(cls); bool isManaged = !th.IsTypeDesc(); @@ -3475,7 +3475,7 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* if (isManaged) { FieldDesc* fields = pMT->GetApproxFieldDescListRaw(); - int elementTypeIndex = *typeIndex; + int elementTypeIndex = typeIndex; for (int i = 0; i < nFields; ++i) { if (i > 0 && fields[i-1].GetOffset() + fields[i-1].GetSize() > fields[i].GetOffset()) @@ -3485,18 +3485,19 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* if (type == ELEMENT_TYPE_VALUETYPE) { MethodTable* nested = fields[i].GetApproxFieldTypeHandleThrowing().GetMethodTable(); - if (!GetFlattenedFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) + if (!FlattenFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) return false; } else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) { - if (*typeIndex >= 2) + if (typeIndex >= 2) return false; - int retType = (CorTypeInfo::IsFloat_NoThrow(type) ? STRUCT_FLOAT_FIELD_FIRST : 0) | - (CorTypeInfo::Size_NoThrow(type) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0); + StructFloatFieldInfoFlags retType = StructFloatFieldInfoFlags( + (CorTypeInfo::IsFloat_NoThrow(type) ? STRUCT_FLOAT_FIELD_FIRST : 0) | + (CorTypeInfo::Size_NoThrow(type) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0)); - types[(*typeIndex)++] = retType; + types[typeIndex++] = retType; } else { @@ -3507,7 +3508,7 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* if (HasImpliedRepeatedFields(pMT)) // inline array or fixed buffer { assert(nFields == 1); - int nElements = pMT->GetNumInstanceFieldBytes() / pMT->GetApproxFieldDescListRaw()->GetSize(); + int nElements = pMT->GetNumInstanceFieldBytes() / fields[0].GetSize(); if (!HandleInlineArray(elementTypeIndex, nElements, types, typeIndex)) return false; } @@ -3523,10 +3524,10 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* NativeFieldCategory category = fields[i].GetCategory(); if (category == NativeFieldCategory::NESTED) { - int elementTypeIndex = *typeIndex; + int elementTypeIndex = typeIndex; MethodTable* nested = fields[i].GetNestedNativeMethodTable(); - if (!GetFlattenedFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) + if (!FlattenFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) return false; // In native layout fixed arrays are marked as NESTED just like structs @@ -3536,13 +3537,14 @@ static bool GetFlattenedFieldTypes(CORINFO_CLASS_HANDLE cls, int types[2], int* } else if (fields[i].NativeSize() <= TARGET_POINTER_SIZE) { - if (*typeIndex >= 2) + if (typeIndex >= 2) return false; - int type = (category == NativeFieldCategory::FLOAT ? STRUCT_FLOAT_FIELD_FIRST : 0) | - (fields[i].NativeSize() == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0); + StructFloatFieldInfoFlags type = StructFloatFieldInfoFlags( + (category == NativeFieldCategory::FLOAT ? STRUCT_FLOAT_FIELD_FIRST : 0) | + (fields[i].NativeSize() == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0)); - types[(*typeIndex)++] = type; + types[typeIndex++] = type; } else { @@ -3560,9 +3562,9 @@ int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) return STRUCT_NO_FLOAT_FIELD; - int types[2] = {STRUCT_NO_FLOAT_FIELD, STRUCT_NO_FLOAT_FIELD}; + StructFloatFieldInfoFlags types[2] = {STRUCT_NO_FLOAT_FIELD, STRUCT_NO_FLOAT_FIELD}; int nFields = 0; - if (!GetFlattenedFieldTypes(cls, types, &nFields) || nFields == 0) + if (!FlattenFieldTypes(cls, types, nFields) || nFields == 0) return STRUCT_NO_FLOAT_FIELD; assert(nFields == 1 || nFields == 2); From 188e1f0864e6adab0303f3a6b2bfb7f399ec9e7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Thu, 28 Mar 2024 11:30:51 +0100 Subject: [PATCH 15/18] Remove early exit if nFields == 0 because it wasn't doing much, the loop won't do any work if there's no fields --- .../tools/Common/JitInterface/RISCV64PassStructInRegister.cs | 4 ---- src/coreclr/vm/methodtable.cpp | 4 ---- 2 files changed, 8 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs index 7166b5a503366c..f72beebd16372f 100644 --- a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs +++ b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs @@ -70,7 +70,6 @@ private static bool FlattenFieldTypes(TypeDesc td, StructFloatFieldInfoFlags[] t StructFloatFieldInfoFlags type = (category is TypeFlags.Single or TypeFlags.Double ? STRUCT_FLOAT_FIELD_FIRST : (StructFloatFieldInfoFlags)0) | (field.FieldType.GetElementSize().AsInt == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : (StructFloatFieldInfoFlags)0); - types[typeIndex++] = type; } else @@ -79,9 +78,6 @@ private static bool FlattenFieldTypes(TypeDesc td, StructFloatFieldInfoFlags[] t } } - if (nFields == 0) - return true; - if ((td as MetadataType).HasImpliedRepeatedFields()) { Debug.Assert(nFields == 1); diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 6f26e69f376727..2d3c602004074f 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3467,8 +3467,6 @@ static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlag bool isManaged = !th.IsTypeDesc(); MethodTable* pMT = isManaged ? th.AsMethodTable() : th.AsNativeValueType(); int nFields = isManaged ? pMT->GetNumIntroducedInstanceFields() : pMT->GetNativeLayoutInfo()->GetNumFields(); - if (nFields == 0) - return true; // TODO: templatize isManaged and use if constexpr for differences when we migrate to C++17 // because the logic for both branches is nearly the same. @@ -3496,7 +3494,6 @@ static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlag StructFloatFieldInfoFlags retType = StructFloatFieldInfoFlags( (CorTypeInfo::IsFloat_NoThrow(type) ? STRUCT_FLOAT_FIELD_FIRST : 0) | (CorTypeInfo::Size_NoThrow(type) == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0)); - types[typeIndex++] = retType; } else @@ -3543,7 +3540,6 @@ static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlag StructFloatFieldInfoFlags type = StructFloatFieldInfoFlags( (category == NativeFieldCategory::FLOAT ? STRUCT_FLOAT_FIELD_FIRST : 0) | (fields[i].NativeSize() == TARGET_POINTER_SIZE ? STRUCT_FIRST_FIELD_SIZE_IS8 : 0)); - types[typeIndex++] = type; } else From 4507267b59dbf3b55c4789d42f2b136bb571d72c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Mon, 8 Apr 2024 14:31:49 +0200 Subject: [PATCH 16/18] Return early from HasImpliedRepeatedFields. GetApproxFieldDescListRaw() is null on empty structs, which crashes pFieldStart->GetFieldType() --- src/coreclr/vm/methodtable.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 2d3c602004074f..4199918b9b3d71 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -2074,6 +2074,11 @@ namespace } DWORD numIntroducedFields = pMT->GetNumIntroducedInstanceFields(); + if (numIntroducedFields != 1) + { + return false; + } + FieldDesc *pFieldStart = pMT->GetApproxFieldDescListRaw(); CorElementType firstFieldElementType = pFieldStart->GetFieldType(); @@ -2084,8 +2089,7 @@ namespace // instead of adding additional padding at the end of a one-field structure. // We do this check here to save looking up the FixedBufferAttribute when loading the field // from metadata. - return numIntroducedFields == 1 - && ( CorTypeInfo::IsPrimitiveType_NoThrow(firstFieldElementType) + return (CorTypeInfo::IsPrimitiveType_NoThrow(firstFieldElementType) || firstFieldElementType == ELEMENT_TYPE_VALUETYPE) && (pFieldStart->GetOffset() == 0) && pMT->HasLayout() From e6c5de6785433c8820875c89cb2d2b89a9932f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Tue, 9 Apr 2024 14:09:33 +0200 Subject: [PATCH 17/18] Cleanup GetRiscV64PassStructInRegisterFlags call sites --- src/coreclr/vm/callingconvention.h | 16 ++-------------- src/coreclr/vm/jitinterface.cpp | 2 +- src/coreclr/vm/methodtable.cpp | 13 +++++-------- src/coreclr/vm/methodtable.h | 2 +- 4 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/coreclr/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index 12b935996b1a41..3e85ecd65947e0 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -1816,17 +1816,7 @@ int ArgIteratorTemplate::GetNextOffset() } else { - MethodTable* pMethodTable = nullptr; - - if (!thValueType.IsTypeDesc()) - pMethodTable = thValueType.AsMethodTable(); - else - { - _ASSERTE(thValueType.IsNativeValueType()); - pMethodTable = thValueType.AsNativeValueType(); - } - _ASSERTE(pMethodTable != nullptr); - flags = MethodTable::GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable); + flags = MethodTable::GetRiscV64PassStructInRegisterFlags(thValueType); if (flags & STRUCT_HAS_FLOAT_FIELDS_MASK) { cFPRegs = (flags & STRUCT_FLOAT_FIELD_ONLY_TWO) ? 2 : 1; @@ -2038,9 +2028,7 @@ void ArgIteratorTemplate::ComputeReturnFlags() if (size <= ENREGISTERED_RETURNTYPE_INTEGER_MAXSIZE) { assert(!thValueType.IsTypeDesc()); - - MethodTable *pMethodTable = thValueType.AsMethodTable(); - flags = (MethodTable::GetRiscV64PassStructInRegisterFlags((CORINFO_CLASS_HANDLE)pMethodTable) & 0xff) << RETURN_FP_SIZE_SHIFT; + flags = (MethodTable::GetRiscV64PassStructInRegisterFlags(thValueType) & 0xff) << RETURN_FP_SIZE_SHIFT; break; } #else diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 6a02a8c76b9ff6..64447437033199 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -9641,7 +9641,7 @@ uint32_t CEEInfo::getRISCV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) uint32_t size = STRUCT_NO_FLOAT_FIELD; #if defined(TARGET_RISCV64) - size = (uint32_t)MethodTable::GetRiscV64PassStructInRegisterFlags(cls); + size = (uint32_t)MethodTable::GetRiscV64PassStructInRegisterFlags(TypeHandle(cls)); #endif // TARGET_RISCV64 EE_TO_JIT_TRANSITION_LEAF(); diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 4199918b9b3d71..c80a4a8afa5a80 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3465,9 +3465,8 @@ static bool HandleInlineArray(int elementTypeIndex, int nElements, StructFloatFi return true; } -static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlags types[2], int& typeIndex) +static bool FlattenFieldTypes(TypeHandle th, StructFloatFieldInfoFlags types[2], int& typeIndex) { - TypeHandle th(cls); bool isManaged = !th.IsTypeDesc(); MethodTable* pMT = isManaged ? th.AsMethodTable() : th.AsNativeValueType(); int nFields = isManaged ? pMT->GetNumIntroducedInstanceFields() : pMT->GetNativeLayoutInfo()->GetNumFields(); @@ -3487,7 +3486,7 @@ static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlag if (type == ELEMENT_TYPE_VALUETYPE) { MethodTable* nested = fields[i].GetApproxFieldTypeHandleThrowing().GetMethodTable(); - if (!FlattenFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) + if (!FlattenFieldTypes(TypeHandle(nested), types, typeIndex)) return false; } else if (fields[i].GetSize() <= TARGET_POINTER_SIZE) @@ -3528,7 +3527,7 @@ static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlag int elementTypeIndex = typeIndex; MethodTable* nested = fields[i].GetNestedNativeMethodTable(); - if (!FlattenFieldTypes((CORINFO_CLASS_HANDLE)nested, types, typeIndex)) + if (!FlattenFieldTypes(TypeHandle(nested), types, typeIndex)) return false; // In native layout fixed arrays are marked as NESTED just like structs @@ -3555,16 +3554,14 @@ static bool FlattenFieldTypes(CORINFO_CLASS_HANDLE cls, StructFloatFieldInfoFlag return true; } -int MethodTable::GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE cls) +int MethodTable::GetRiscV64PassStructInRegisterFlags(TypeHandle th) { - TypeHandle th(cls); - if (th.GetSize() > ENREGISTERED_PARAMTYPE_MAXSIZE) return STRUCT_NO_FLOAT_FIELD; StructFloatFieldInfoFlags types[2] = {STRUCT_NO_FLOAT_FIELD, STRUCT_NO_FLOAT_FIELD}; int nFields = 0; - if (!FlattenFieldTypes(cls, types, nFields) || nFields == 0) + if (!FlattenFieldTypes(th, types, nFields) || nFields == 0) return STRUCT_NO_FLOAT_FIELD; assert(nFields == 1 || nFields == 2); diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 3f05b695cb1a1e..b933cf6c3cafdc 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -828,7 +828,7 @@ class MethodTable static bool IsOnlyOneField(MethodTable * pMT); static int GetLoongArch64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE clh); #elif defined(TARGET_RISCV64) - static int GetRiscV64PassStructInRegisterFlags(CORINFO_CLASS_HANDLE clh); + static int GetRiscV64PassStructInRegisterFlags(TypeHandle th); #endif #if defined(UNIX_AMD64_ABI_ITF) From ca936b4388d6d6004ffc476618511c845dec2fe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Thu, 18 Apr 2024 10:57:31 +0200 Subject: [PATCH 18/18] Stackalloc field types to avoid GC allocations --- .../Common/JitInterface/RISCV64PassStructInRegister.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs index f72beebd16372f..fae694f8768fc9 100644 --- a/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs +++ b/src/coreclr/tools/Common/JitInterface/RISCV64PassStructInRegister.cs @@ -15,7 +15,7 @@ private const int ENREGISTERED_PARAMTYPE_MAXSIZE = 16, TARGET_POINTER_SIZE = 8; - private static bool HandleInlineArray(int elementTypeIndex, int nElements, StructFloatFieldInfoFlags[] types, ref int typeIndex) + private static bool HandleInlineArray(int elementTypeIndex, int nElements, Span types, ref int typeIndex) { int nFlattenedFieldsPerElement = typeIndex - elementTypeIndex; if (nFlattenedFieldsPerElement == 0) @@ -38,7 +38,7 @@ private static bool HandleInlineArray(int elementTypeIndex, int nElements, Struc return true; } - private static bool FlattenFieldTypes(TypeDesc td, StructFloatFieldInfoFlags[] types, ref int typeIndex) + private static bool FlattenFieldTypes(TypeDesc td, Span types, ref int typeIndex) { IEnumerable fields = td.GetFields(); int nFields = 0; @@ -93,7 +93,9 @@ public static uint GetRISCV64PassStructInRegisterFlags(TypeDesc td) if (td.GetElementSize().AsInt > ENREGISTERED_PARAMTYPE_MAXSIZE) return (uint)STRUCT_NO_FLOAT_FIELD; - StructFloatFieldInfoFlags[] types = {STRUCT_NO_FLOAT_FIELD, STRUCT_NO_FLOAT_FIELD}; + Span types = stackalloc StructFloatFieldInfoFlags[] { + STRUCT_NO_FLOAT_FIELD, STRUCT_NO_FLOAT_FIELD + }; int nFields = 0; if (!FlattenFieldTypes(td, types, ref nFields) || nFields == 0) return (uint)STRUCT_NO_FLOAT_FIELD;