Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix for bugs related to clasifying System.TypedReference - issue #2854. #2860

Merged
merged 1 commit into from
Feb 2, 2016
Merged

Fix for bugs related to clasifying System.TypedReference - issue #2854. #2860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 51 additions & 3 deletions src/inc/corinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,26 @@ enum SystemVClassificationType : unsigned __int8
SystemVClassificationTypeMemory = 3,
SystemVClassificationTypeInteger = 4,
SystemVClassificationTypeIntegerReference = 5,
SystemVClassificationTypeSSE = 6,
SystemVClassificationTypeIntegerByRef = 6,
SystemVClassificationTypeSSE = 7,
// SystemVClassificationTypeSSEUp = Unused, // Not supported by the CLR.
// SystemVClassificationTypeX87 = Unused, // Not supported by the CLR.
// SystemVClassificationTypeX87Up = Unused, // Not supported by the CLR.
// SystemVClassificationTypeComplexX87 = Unused, // Not supported by the CLR.
SystemVClassificationTypeMAX = 7,

// Internal flags - never returned outside of the classification implementation.
Copy link
Member

Choose a reason for hiding this comment

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

What does this comment line mean? As per my understanding struct classification code is returning SystemVClassificationTypeTypedReference flag for TypedReference, no?

Copy link
Author

Choose a reason for hiding this comment

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

No. The classification returns to through the Jit interface only the integral types and the SSE type. This enum value is used internally by the classification algorithm - it is a counterpart to the VM fields' ELEMENT_TYPE_TYPEDBYREF.


// This value represents a very special type with two eightbytes.
Copy link
Member

Choose a reason for hiding this comment

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

Nit - Two eight bytes (space between eight and bytes)
Suggestion is to replace this sentence with: "TypedReference is a struct with two fields"

To eight bytes - this is true only on 64-bit targets. on 32-bit targets IntPtr is of size 4 bytes.

Copy link
Author

Choose a reason for hiding this comment

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

"eightbyte" is a term used in the classification specification. The term refers to the classification meaning of the word.
Clarified that only the first eightbyte type is changed. There is no classification for 32 bit targets. The whole classification concept is relevant to AMD64 System V ABI only.

// First ByRef, second Integer (platform int).
// The VM has a special Elem type for this type - ELEMENT_TYPE_TYPEDBYREF.
// This is the classification counterpart for that element type. It is used to detect
// the special TypedReference type and specialize its classification.
// This type is represented as a struct with two fields. The classification needs to do
// special handling of it since the source/methadata type of the fieds is IntPtr.
// The VM changes the first to ByRef. The second is left as IntPtr (TYP_I_IMPL really). The classification needs to match this and
// special handling is warranted (similar thing is done in the getGCLayout function for this type).
SystemVClassificationTypeTypedReference = 8,
SystemVClassificationTypeMAX = 9,
};

// Represents classification information for a struct.
Expand All @@ -298,6 +312,7 @@ struct SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR
unsigned __int8 eightByteSizes[CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS]; // The size of the eightbytes (an eightbyte could include padding. This represents the no padding size of the eightbyte).
unsigned __int8 eightByteOffsets[CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS]; // The start offset of the eightbytes (in bytes).

// Members

//------------------------------------------------------------------------
// CopyFrom: Copies a struct classification into this one.
Expand All @@ -318,7 +333,40 @@ struct SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR
}
}

// Members
//------------------------------------------------------------------------
// IsIntegralSlot: Returns whether the eightbyte at slotIndex is of integral type.
//
// Arguments:
// 'slotIndex' the slot number we are determining if it is of integral type.
//
// Return value:
// returns true if we the eightbyte at index slotIndex is of integral type.
//

bool IsIntegralSlot(unsigned slotIndex) const
{
return ((eightByteClassifications[slotIndex] == SystemVClassificationTypeInteger) ||
(eightByteClassifications[slotIndex] == SystemVClassificationTypeIntegerReference) ||
(eightByteClassifications[slotIndex] == SystemVClassificationTypeIntegerByRef));
}

//------------------------------------------------------------------------
// IsSseSlot: Returns whether the eightbyte at slotIndex is SSE type.
Copy link
Member

Choose a reason for hiding this comment

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

Please elaborate this comment line what is an SSE type with an example.

Copy link
Author

Choose a reason for hiding this comment

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

Added a reference to the AMD64 System V ABI spec. Please refer to it it has examples and definitions on what SSE classification means.

//
// Arguments:
// 'slotIndex' the slot number we are determining if it is of SSE type.
//
// Return value:
// returns true if we the eightbyte at index slotIndex is of SSE type.
//
// Follows the rules of the AMD64 System V ABI specification at www.x86-64.org/documentation/abi.pdf.
// Please reffer to it for definitions/examples.
//
bool IsSseSlot(unsigned slotIndex) const
{
return (eightByteClassifications[slotIndex] == SystemVClassificationTypeSSE);
}

private:
void Initialize()
{
Expand Down
6 changes: 2 additions & 4 deletions src/jit/codegencommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3940,10 +3940,8 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg,

regArgNum = genMapRegNumToRegArgNum(regNum, regType);

if ((!doingFloat &&
((structDesc.eightByteClassifications[slotCounter] == SystemVClassificationTypeInteger) ||
(structDesc.eightByteClassifications[slotCounter] == SystemVClassificationTypeIntegerReference))) ||
(doingFloat && structDesc.eightByteClassifications[slotCounter] == SystemVClassificationTypeSSE))
if ((!doingFloat && (structDesc.IsIntegralSlot(slotCounter))) ||
(doingFloat && (structDesc.IsSseSlot(slotCounter))))
{
Copy link

Choose a reason for hiding this comment

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

Such an improvement in readability - thanks!

// Store the reg for the first slot.
if (slots == 0)
Expand Down
28 changes: 19 additions & 9 deletions src/jit/codegenlinear.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,16 +182,26 @@

void genJmpMethod(GenTreePtr jmp);

#if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
void genGetStructTypeSizeOffset(const SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR& structDesc,
var_types* type0,
var_types* type1,
emitAttr* size0,
emitAttr* size1,
unsigned __int8* offset0,
unsigned __int8* offset1);

bool genStoreRegisterReturnInLclVar(GenTreePtr treeNode);

// Deals with codegen for muti-register struct returns.
bool isStructReturn(GenTreePtr treeNode);
void genStructReturn(GenTreePtr treeNode);

// Codegen for GT_RETURN.
void genReturn(GenTreePtr treeNode);

#if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
void getStructTypeOffset(const SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR& structDesc,
var_types* type0,
var_types* type1,
unsigned __int8* offset0,
unsigned __int8* offset1);

void getStructReturnRegisters(var_types type0,
var_types type1,
regNumber* retRegPtr0,
regNumber* retRegPtr1);
#endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)

void genLclHeap(GenTreePtr tree);
Expand Down
Loading