Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Intrinsify Unsafe.Read/Write/Copy, handle struct BitCast #85562

Merged
merged 64 commits into from
Jul 23, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
1e54aa0
Intrinsify Unsafe.Read/Write, handle struct BitCast
MichalPetryka Apr 29, 2023
5e78c9c
Fix broken rebase
MichalPetryka Apr 29, 2023
93399c2
Handle small type extension
MichalPetryka Apr 29, 2023
8987b69
Fix formatting
MichalPetryka Apr 29, 2023
858f39c
Fix swapped operands
MichalPetryka Apr 30, 2023
a193350
Use right helper
MichalPetryka Apr 30, 2023
db54e97
Don't loose indir flags
MichalPetryka Apr 30, 2023
9c2ee9e
Fix build
MichalPetryka Apr 30, 2023
3e5003f
Fix missed deref
MichalPetryka Apr 30, 2023
b3e7e97
Also mask indir flags
MichalPetryka Apr 30, 2023
66f111b
Rename variables, fix structs
MichalPetryka Apr 30, 2023
a5bb60b
Fix build
MichalPetryka Apr 30, 2023
0922ea6
Merge upstream
MichalPetryka May 3, 2023
36ab91d
Update importer.cpp
MichalPetryka May 3, 2023
84a3edc
Resolve conflicts
MichalPetryka May 6, 2023
b911984
Fix 32bit bug, simplify code
MichalPetryka May 6, 2023
2982fd4
Fix merge
MichalPetryka May 6, 2023
feb0acc
Simplify code
MichalPetryka May 6, 2023
08e9ece
Update importercalls.cpp
MichalPetryka May 6, 2023
9204f80
Update importer.cpp
MichalPetryka May 6, 2023
36eecf9
Remove redundant nullchecks
MichalPetryka May 6, 2023
3ed32ad
Format code
MichalPetryka May 6, 2023
6cbaa45
Mark the local as having new uses
MichalPetryka May 8, 2023
6c758ee
Update targetosarch.h
MichalPetryka May 8, 2023
13ece70
Reword comments
MichalPetryka May 8, 2023
d68aed9
Update targetosarch.h
MichalPetryka May 8, 2023
873a914
Handle small types
MichalPetryka May 9, 2023
c9b274c
Reformat code
MichalPetryka May 9, 2023
290bf27
Commit missed changes
MichalPetryka May 9, 2023
3d77bb0
Fix check
MichalPetryka May 9, 2023
15aa641
Merge branch 'main' into bitcast-indir
MichalPetryka May 9, 2023
4ab78a0
Format code
MichalPetryka May 9, 2023
e1911ee
Add tests for small types and misalignment
MichalPetryka May 10, 2023
38f10fc
Fix a typo, add more tests
MichalPetryka May 10, 2023
b69aaa7
Adjust small type handling
MichalPetryka May 11, 2023
46c45cc
Fix typo
MichalPetryka May 11, 2023
0c245fb
Fix warning
MichalPetryka May 11, 2023
6ca55ac
Format code
MichalPetryka May 11, 2023
1d4191d
Merge branch 'main' into bitcast-indir
MichalPetryka May 15, 2023
455b468
Merge branch 'main' into bitcast-indir
MichalPetryka May 18, 2023
7544cff
Merge upstream
MichalPetryka Jun 5, 2023
3aed08b
Fix merge
MichalPetryka Jun 5, 2023
f68aec0
Fix merge
MichalPetryka Jun 5, 2023
23e48d2
Rename variables
MichalPetryka Jun 5, 2023
964ab38
Fix typo
MichalPetryka Jun 5, 2023
7253d9e
Format code
MichalPetryka Jun 5, 2023
81fd3c7
Merge upstream
MichalPetryka Jun 7, 2023
e653348
Add missing free
MichalPetryka Jun 7, 2023
86ff6d4
Merge upstream
MichalPetryka Jun 13, 2023
5111f6e
Add IL tests
MichalPetryka Jun 14, 2023
8174abd
Update BitCast.il
MichalPetryka Jun 14, 2023
b9160be
Fix IL test
MichalPetryka Jun 14, 2023
d864688
Intrinsify Copy too
MichalPetryka Jun 14, 2023
588d55e
Update UnsafeTests.cs
MichalPetryka Jul 10, 2023
674d605
Update UnsafeTests.cs
MichalPetryka Jul 10, 2023
43ebe78
Update UnsafeTests.cs
MichalPetryka Jul 10, 2023
8e52b2f
Update src/tests/JIT/Intrinsics/BitCast.il
MichalPetryka Jul 10, 2023
300951c
Add struct copy tests
MichalPetryka Jul 13, 2023
7b9afe4
Merge branch 'dotnet:main' into bitcast-indir
MichalPetryka Jul 13, 2023
ac25690
Merge branch 'dotnet:main' into bitcast-indir
MichalPetryka Jul 14, 2023
add3666
Merge branch 'dotnet:main' into bitcast-indir
MichalPetryka Jul 15, 2023
5754b95
Merge branch 'dotnet:main' into bitcast-indir
MichalPetryka Jul 15, 2023
88e8829
Merge branch 'dotnet:main' into bitcast-indir
MichalPetryka Jul 18, 2023
87950df
Merge branch 'dotnet:main' into bitcast-indir
MichalPetryka Jul 22, 2023
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
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4051,7 +4051,7 @@ class Compiler
const DebugInfo& di = DebugInfo(),
BasicBlock* block = nullptr);

GenTree* impGetStructAddr(GenTree* structVal, CORINFO_CLASS_HANDLE structHnd, unsigned curLevel, bool willDeref);
GenTree* impGetNodeAddr(GenTree* val, CORINFO_CLASS_HANDLE typeHnd, unsigned curLevel, GenTreeFlags* derefFlags);

var_types impNormStructType(CORINFO_CLASS_HANDLE structHnd, CorInfoType* simdBaseJitType = nullptr);

Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15937,7 +15937,9 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr,
// helper needs pointer to struct, not struct itself
if (pFieldInfo->helper == CORINFO_HELP_SETFIELDSTRUCT)
{
assg = impGetStructAddr(assg, structType, CHECK_SPILL_ALL, true);
// TODO: deal with flags
MichalPetryka marked this conversation as resolved.
Show resolved Hide resolved
GenTreeFlags flags = GTF_EMPTY;
MichalPetryka marked this conversation as resolved.
Show resolved Hide resolved
assg = impGetNodeAddr(assg, structType, CHECK_SPILL_ALL, &flags);
}
else if (lclTyp == TYP_DOUBLE && assg->TypeGet() == TYP_FLOAT)
{
Expand Down Expand Up @@ -16013,8 +16015,9 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr,
if (!varTypeIsStruct(lclTyp))
{
// get the result as primitive type
result = impGetStructAddr(result, structType, CHECK_SPILL_ALL, true);
result = gtNewIndir(lclTyp, result);
GenTreeFlags flags = GTF_EMPTY;
result = impGetNodeAddr(result, structType, CHECK_SPILL_ALL, &flags);
result = gtNewIndir(lclTyp, result, flags);
}
}
else if (varTypeIsIntegral(lclTyp) && genTypeSize(lclTyp) < genTypeSize(TYP_INT))
Expand Down
86 changes: 52 additions & 34 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -957,8 +957,10 @@ GenTree* Compiler::impAssignStruct(GenTree* dest,
WellKnownArg wellKnownArgType =
srcCall->ShouldHaveRetBufArg() ? WellKnownArg::RetBuffer : WellKnownArg::None;

GenTree* destAddr = impGetStructAddr(dest, srcCall->gtRetClsHnd, CHECK_SPILL_ALL, /* willDeref */ true);
NewCallArg newArg = NewCallArg::Primitive(destAddr).WellKnown(wellKnownArgType);
// TODO: deal with flags
GenTreeFlags flags = GTF_EMPTY;
GenTree* destAddr = impGetNodeAddr(dest, srcCall->gtRetClsHnd, CHECK_SPILL_ALL, &flags);
NewCallArg newArg = NewCallArg::Primitive(destAddr).WellKnown(wellKnownArgType);

#if !defined(TARGET_ARM)
// Unmanaged instance methods on Windows or Unix X86 need the retbuf arg after the first (this) parameter
Expand Down Expand Up @@ -1059,7 +1061,9 @@ GenTree* Compiler::impAssignStruct(GenTree* dest,
if (call->ShouldHaveRetBufArg())
{
// insert the return value buffer into the argument list as first byref parameter after 'this'
GenTree* destAddr = impGetStructAddr(dest, call->gtRetClsHnd, CHECK_SPILL_ALL, /* willDeref */ true);
// TODO: deal with flags
GenTreeFlags flags = GTF_EMPTY;
GenTree* destAddr = impGetNodeAddr(dest, call->gtRetClsHnd, CHECK_SPILL_ALL, &flags);
call->gtArgs.InsertAfterThisOrFirst(this,
NewCallArg::Primitive(destAddr).WellKnown(WellKnownArg::RetBuffer));

Expand All @@ -1076,19 +1080,20 @@ GenTree* Compiler::impAssignStruct(GenTree* dest,
{
// Since we are assigning the result of a GT_MKREFANY, "destAddr" must point to a refany.
// TODO-CQ: we can do this without address-exposing the local on the LHS.
GenTree* destAddr = impGetStructAddr(dest, impGetRefAnyClass(), CHECK_SPILL_ALL, /* willDeref */ true);
GenTree* destAddrClone;
GenTreeFlags flags = GTF_EMPTY;
GenTree* destAddr = impGetNodeAddr(dest, impGetRefAnyClass(), CHECK_SPILL_ALL, &flags);
GenTree* destAddrClone;
destAddr = impCloneExpr(destAddr, &destAddrClone, NO_CLASS_HANDLE, curLevel,
pAfterStmt DEBUGARG("MKREFANY assignment"));

assert(OFFSETOF__CORINFO_TypedReference__dataPtr == 0);
assert(destAddr->gtType == TYP_I_IMPL || destAddr->gtType == TYP_BYREF);

GenTree* ptrSlot = gtNewIndir(TYP_I_IMPL, destAddr);
GenTree* ptrSlot = gtNewIndir(TYP_I_IMPL, destAddr, flags);
GenTreeIntCon* typeFieldOffset = gtNewIconNode(OFFSETOF__CORINFO_TypedReference__type, TYP_I_IMPL);

GenTree* typeSlot =
gtNewIndir(TYP_I_IMPL, gtNewOperNode(GT_ADD, destAddr->gtType, destAddrClone, typeFieldOffset));
gtNewIndir(TYP_I_IMPL, gtNewOperNode(GT_ADD, destAddr->gtType, destAddrClone, typeFieldOffset), flags);

// append the assign of the pointer value
GenTree* asg = gtNewAssignNode(ptrSlot, src->AsOp()->gtOp1);
Expand Down Expand Up @@ -1182,44 +1187,49 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr,
}

//------------------------------------------------------------------------
// impGetStructAddr: Get the address of a struct value.
// impGetNodeAddr: Get the address of a value.
//
// Arguments:
// structVal - The value in question
// structHnd - The struct handle for "structVal"
// curLevel - Stack level for spilling
// willDeref - Whether the caller will dereference the address
// val - The value in question
// typeHnd - The type handle for "val"
// curLevel - Stack level for spilling
// derefFlags - Flags to be used on dereference, nullptr when
// the address won't be dereferenced
//
// Return Value:
// In case "structVal" can represent locations (is an indirection/local),
// In case "val" can represent locations (is an indirection/local),
// will return its address. Otherwise, address of a temporary assigned
// the value of "structVal" will be returned.
// the value of "val" will be returned.
//
GenTree* Compiler::impGetStructAddr(GenTree* structVal,
CORINFO_CLASS_HANDLE structHnd,
unsigned curLevel,
bool willDeref)
GenTree* Compiler::impGetNodeAddr(GenTree* val,
CORINFO_CLASS_HANDLE typeHnd,
unsigned curLevel,
GenTreeFlags* derefFlags)
{
assert(varTypeIsStruct(structVal));
switch (structVal->OperGet())
switch (val->OperGet())
{
case GT_BLK:
case GT_IND:
if (willDeref)
if (derefFlags != nullptr)
{
return structVal->AsIndir()->Addr();
*derefFlags |= (val->gtFlags & GTF_IND_FLAGS);
return val->AsIndir()->Addr();
}
break;

case GT_LCL_VAR:
return gtNewLclVarAddrNode(structVal->AsLclVar()->GetLclNum(), TYP_BYREF);
return gtNewLclVarAddrNode(val->AsLclVar()->GetLclNum(), TYP_BYREF);

case GT_LCL_FLD:
return gtNewLclAddrNode(structVal->AsLclFld()->GetLclNum(), structVal->AsLclFld()->GetLclOffs(), TYP_BYREF);
return gtNewLclAddrNode(val->AsLclFld()->GetLclNum(), val->AsLclFld()->GetLclOffs(), TYP_BYREF);

case GT_FIELD:
{
GenTreeField* fieldNode = structVal->AsField();
if (derefFlags != nullptr)
{
*derefFlags |= (val->gtFlags & GTF_IND_FLAGS);
}
GenTreeField* fieldNode = val->AsField();
GenTreeField* fieldAddr =
new (this, GT_FIELD_ADDR) GenTreeField(GT_FIELD_ADDR, TYP_BYREF, fieldNode->GetFldObj(),
fieldNode->gtFldHnd, fieldNode->gtFldOffset);
Expand All @@ -1231,15 +1241,15 @@ GenTree* Compiler::impGetStructAddr(GenTree* structVal,
}

case GT_COMMA:
impAppendTree(structVal->AsOp()->gtGetOp1(), curLevel, impCurStmtDI);
return impGetStructAddr(structVal->AsOp()->gtGetOp2(), structHnd, curLevel, willDeref);
impAppendTree(val->AsOp()->gtGetOp1(), curLevel, impCurStmtDI);
return impGetNodeAddr(val->AsOp()->gtGetOp2(), typeHnd, curLevel, derefFlags);

default:
break;
}

unsigned lclNum = lvaGrabTemp(true DEBUGARG("location for address-of(RValue)"));
impAssignTempGen(lclNum, structVal, structHnd, curLevel);
impAssignTempGen(lclNum, val, typeHnd, curLevel);

// The 'return value' is now address of the temp itself.
return gtNewLclVarAddrNode(lclNum, TYP_BYREF);
Expand Down Expand Up @@ -3272,7 +3282,9 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken,
GenTree* objToBox = impPopStack().val;

// Spill struct to get its address (to access hasValue field)
objToBox = impGetStructAddr(objToBox, nullableCls, CHECK_SPILL_ALL, true);
// TODO: deal with flags
GenTreeFlags flags = GTF_EMPTY;
objToBox = impGetNodeAddr(objToBox, nullableCls, CHECK_SPILL_ALL, &flags);

impPushOnStack(gtNewFieldRef(TYP_BOOL, hasValueFldHnd, objToBox, 0),
typeInfo(TI_INT));
Expand Down Expand Up @@ -3625,7 +3637,9 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
return;
}

op1 = gtNewHelperCallNode(boxHelper, TYP_REF, op2, impGetStructAddr(exprToBox, operCls, CHECK_SPILL_ALL, true));
// TODO: deal with flags
GenTreeFlags flags = GTF_EMPTY;
op1 = gtNewHelperCallNode(boxHelper, TYP_REF, op2, impGetNodeAddr(exprToBox, operCls, CHECK_SPILL_ALL, &flags));
}

/* Push the result back on the stack, */
Expand Down Expand Up @@ -8293,7 +8307,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
}
else
{
op1 = impGetStructAddr(op1, clsHnd, CHECK_SPILL_ALL, false);
op1 = impGetNodeAddr(op1, clsHnd, CHECK_SPILL_ALL, nullptr);
}

JITDUMP("\n ... optimized to ...\n");
Expand Down Expand Up @@ -9256,7 +9270,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
{
BADCODE("top of stack must be a value type");
}
obj = impGetStructAddr(obj, objType, CHECK_SPILL_ALL, true);

// TODO: deal with flags
GenTreeFlags flags = GTF_EMPTY;
obj = impGetNodeAddr(obj, objType, CHECK_SPILL_ALL, &flags);
}

if (isLoadAddress)
Expand Down Expand Up @@ -10012,12 +10029,13 @@ void Compiler::impImportBlockCode(BasicBlock* block)
else
{
// Get the address of the refany
op1 = impGetStructAddr(op1, impGetRefAnyClass(), CHECK_SPILL_ALL, /* willDeref */ true);
GenTreeFlags flags = GTF_EMPTY;
op1 = impGetNodeAddr(op1, impGetRefAnyClass(), CHECK_SPILL_ALL, &flags);

// Fetch the type from the correct slot
op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1,
gtNewIconNode(OFFSETOF__CORINFO_TypedReference__type, TYP_I_IMPL));
op1 = gtNewIndir(TYP_BYREF, op1);
op1 = gtNewIndir(TYP_BYREF, op1, flags);
}

// Convert native TypeHandle to RuntimeTypeHandle.
Expand Down
90 changes: 38 additions & 52 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4037,40 +4037,34 @@ GenTree* Compiler::impSRCSUnsafeIntrinsic(NamedIntrinsic intrinsic,
return nullptr;
}

CorInfoType fromJitType = info.compCompHnd->asCorInfoType(fromTypeHnd);
var_types fromType = JITtype2varType(fromJitType);
ClassLayout* fromLayout = nullptr;
var_types fromType = TypeHandleToVarType(fromTypeHnd, &fromLayout);

CorInfoType toJitType = info.compCompHnd->asCorInfoType(toTypeHnd);
var_types toType = JITtype2varType(toJitType);
ClassLayout* toLayout = nullptr;
var_types toType = TypeHandleToVarType(toTypeHnd, &toLayout);

bool involvesStructType = false;

if (fromType == TYP_STRUCT)
if (fromLayout != nullptr && toLayout != nullptr && ClassLayout::AreCompatible(fromLayout, toLayout))
{
involvesStructType = true;

if (toType == TYP_STRUCT)
{
ClassLayout* fromLayout = typGetObjLayout(fromTypeHnd);
ClassLayout* toLayout = typGetObjLayout(toTypeHnd);

if (ClassLayout::AreCompatible(fromLayout, toLayout))
{
// Handle compatible struct layouts where we can simply return op1
return impPopStack().val;
}
}
}
else if (toType == TYP_STRUCT)
{
involvesStructType = true;
// Handle compatible struct layouts where we can simply return op1
return impPopStack().val;
}

if (involvesStructType)
if (varTypeIsStruct(fromType) || varTypeIsStruct(toType))
{
// TODO-CQ: Handle this by getting the address of `op1` and then dereferencing
// that as TTo, much as `Unsafe.As<TFrom, TTo>(ref op1)` would work.
return nullptr;
GenTree* addr;
GenTreeFlags flags = GTF_EMPTY;
GenTree* val = impPopStack().val;
if (val->OperIsIndir() && (fromSize != val->AsIndir()->Size()))
{
unsigned lclNum = lvaGrabTemp(true DEBUGARG("bitcast small type extension"));
impAssignTempGen(lclNum, val, fromTypeHnd, CHECK_SPILL_ALL);
addr = gtNewLclVarAddrNode(lclNum, TYP_BYREF);
}
else
{
addr = impGetNodeAddr(val, fromTypeHnd, CHECK_SPILL_ALL, &flags);
}
return gtNewLoadValueNode(toType, toLayout, addr, flags);
}

if (varTypeIsFloating(fromType))
Expand Down Expand Up @@ -4300,26 +4294,21 @@ GenTree* Compiler::impSRCSUnsafeIntrinsic(NamedIntrinsic intrinsic,
}

case NI_SRCS_UNSAFE_Read:
{
assert(sig->sigInst.methInstCount == 1);

// ldarg.0
// ldobj !!T
// ret

return nullptr;
}

case NI_SRCS_UNSAFE_ReadUnaligned:
{
assert(sig->sigInst.methInstCount == 1);

// ldarg.0
// unaligned. 0x1
// if NI_SRCS_UNSAFE_ReadUnaligned: unaligned. 0x1
// ldobj !!T
// ret

return nullptr;
CORINFO_CLASS_HANDLE typeHnd = sig->sigInst.methInst[0];
ClassLayout* layout = nullptr;
var_types type = TypeHandleToVarType(typeHnd, &layout);
GenTreeFlags flags = intrinsic == NI_SRCS_UNSAFE_ReadUnaligned ? GTF_IND_UNALIGNED : GTF_EMPTY;

return gtNewLoadValueNode(type, layout, impPopStack().val, flags);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}

case NI_SRCS_UNSAFE_SizeOf:
Expand Down Expand Up @@ -4410,28 +4399,25 @@ GenTree* Compiler::impSRCSUnsafeIntrinsic(NamedIntrinsic intrinsic,
}

case NI_SRCS_UNSAFE_Write:
case NI_SRCS_UNSAFE_WriteUnaligned:
{
assert(sig->sigInst.methInstCount == 1);

// ldarg.0
// ldarg.1
// if NI_SRCS_UNSAFE_WriteUnaligned: unaligned. 0x01
// stobj !!T
// ret

return nullptr;
}

case NI_SRCS_UNSAFE_WriteUnaligned:
MichalPetryka marked this conversation as resolved.
Show resolved Hide resolved
{
assert(sig->sigInst.methInstCount == 1);
GenTree* op1 = impPopStack().val;
GenTree* op2 = impPopStack().val;

// ldarg.0
// ldarg.1
// unaligned. 0x01
// stobj !!T
// ret
CORINFO_CLASS_HANDLE typeHnd = sig->sigInst.methInst[0];
ClassLayout* layout = nullptr;
var_types type = TypeHandleToVarType(typeHnd, &layout);
GenTreeFlags flags = intrinsic == NI_SRCS_UNSAFE_WriteUnaligned ? GTF_IND_UNALIGNED : GTF_EMPTY;

return nullptr;
return gtNewAssignNode(gtNewLoadValueNode(type, layout, op2, flags), op1);
MichalPetryka marked this conversation as resolved.
Show resolved Hide resolved
}

default:
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/jit/importervectorization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,9 @@ GenTree* Compiler::impSpanEqualsOrStartsWith(bool startsWith, CORINFO_SIG_INFO*
if (unrolled != nullptr)
{
// We succeeded, fill the placeholders:
impAssignTempGen(spanObjRef, impGetStructAddr(spanObj, spanCls, CHECK_SPILL_NONE, true));
// TODO: deal with flags
GenTreeFlags flags = GTF_EMPTY;
impAssignTempGen(spanObjRef, impGetNodeAddr(spanObj, spanCls, CHECK_SPILL_NONE, &flags));
impAssignTempGen(spanDataTmp, spanData);
if (unrolled->OperIs(GT_QMARK))
{
Expand Down