Skip to content
New issue

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

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

Already on GitHub? # 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 all 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
5 changes: 5 additions & 0 deletions src/coreclr/inc/targetosarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ class TargetOS
class TargetArchitecture
{
public:
#ifdef TARGET_64BIT
static const bool Is64Bit = true;
#else
static const bool Is64Bit = false;
#endif
#ifdef TARGET_ARM
static const bool IsX86 = false;
static const bool IsX64 = false;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9647,9 +9647,9 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
{
chars += printf("[VAR_ITERATOR]");
}
if (tree->gtFlags & GTF_VAR_CLONED)
if (tree->gtFlags & GTF_VAR_MOREUSES)
{
chars += printf("[VAR_CLONED]");
chars += printf("[VAR_MOREUSES]");
}
if (!comp->lvaGetDesc(tree->AsLclVarCommon())->lvPromoted)
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4087,7 +4087,7 @@ class Compiler
BasicBlock* block = nullptr);
GenTree* impStoreStructPtr(GenTree* destAddr, GenTree* value, unsigned curLevel);

GenTree* impGetStructAddr(GenTree* structVal, unsigned curLevel, bool willDeref);
GenTree* impGetNodeAddr(GenTree* val, unsigned curLevel, GenTreeFlags* pDerefFlags);

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

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1632,7 +1632,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)

GenTree* argSingleUseNode = argInfo.argBashTmpNode;

if ((argSingleUseNode != nullptr) && !(argSingleUseNode->gtFlags & GTF_VAR_CLONED) && argIsSingleDef)
if ((argSingleUseNode != nullptr) && !(argSingleUseNode->gtFlags & GTF_VAR_MOREUSES) && argIsSingleDef)
{
// Change the temp in-place to the actual argument.
// We currently do not support this for struct arguments, so it must not be a GT_BLK.
Expand Down
20 changes: 12 additions & 8 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8781,7 +8781,7 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK)

FINISH_CLONING_LCL_NODE:
// Remember that the local node has been cloned. Below the flag will be set on 'copy' too.
tree->gtFlags |= GTF_VAR_CLONED;
tree->gtFlags |= GTF_VAR_MOREUSES;
copy->AsLclVarCommon()->SetSsaNum(tree->AsLclVarCommon()->GetSsaNum());
assert(!copy->AsLclVarCommon()->HasSsaName() || ((copy->gtFlags & GTF_VAR_DEF) == 0));
break;
Expand Down Expand Up @@ -8952,7 +8952,7 @@ GenTree* Compiler::gtCloneExpr(
else
{
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_CLONED;
tree->gtFlags |= GTF_VAR_MOREUSES;
copy = gtNewLclvNode(tree->AsLclVar()->GetLclNum(),
tree->gtType DEBUGARG(tree->AsLclVar()->gtLclILoffs));
copy->AsLclVarCommon()->SetSsaNum(tree->AsLclVarCommon()->GetSsaNum());
Expand All @@ -8967,7 +8967,7 @@ GenTree* Compiler::gtCloneExpr(
else
{
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_CLONED;
tree->gtFlags |= GTF_VAR_MOREUSES;
copy = new (this, GT_LCL_FLD)
GenTreeLclFld(GT_LCL_FLD, tree->TypeGet(), tree->AsLclFld()->GetLclNum(),
tree->AsLclFld()->GetLclOffs(), tree->AsLclFld()->GetLayout());
Expand Down Expand Up @@ -9032,14 +9032,14 @@ GenTree* Compiler::gtCloneExpr(
{
case GT_STORE_LCL_VAR:
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_CLONED;
tree->gtFlags |= GTF_VAR_MOREUSES;
copy = new (this, GT_STORE_LCL_VAR)
GenTreeLclVar(tree->TypeGet(), tree->AsLclVar()->GetLclNum(), tree->AsLclVar()->Data());
break;

case GT_STORE_LCL_FLD:
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_CLONED;
tree->gtFlags |= GTF_VAR_MOREUSES;
copy = new (this, GT_STORE_LCL_FLD)
GenTreeLclFld(tree->TypeGet(), tree->AsLclFld()->GetLclNum(), tree->AsLclFld()->GetLclOffs(),
tree->AsLclFld()->Data(), tree->AsLclFld()->GetLayout());
Expand Down Expand Up @@ -16074,7 +16074,9 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr,
// helper needs pointer to struct, not struct itself
if (pFieldInfo->helper == CORINFO_HELP_SETFIELDSTRUCT)
{
assg = impGetStructAddr(assg, CHECK_SPILL_ALL, true);
// TODO-Bug?: verify if flags matter here
GenTreeFlags indirFlags = GTF_EMPTY;
assg = impGetNodeAddr(assg, CHECK_SPILL_ALL, &indirFlags);
}
else if (lclTyp == TYP_DOUBLE && assg->TypeGet() == TYP_FLOAT)
{
Expand Down Expand Up @@ -16149,8 +16151,10 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr,
{
if (!varTypeIsStruct(lclTyp))
{
result = impGetStructAddr(result, CHECK_SPILL_ALL, true);
result = gtNewIndir(lclTyp, result);
// get the result as primitive type
GenTreeFlags indirFlags = GTF_EMPTY;
result = impGetNodeAddr(result, CHECK_SPILL_ALL, &indirFlags);
result = gtNewIndir(lclTyp, result, indirFlags);
}
}
else if (varTypeIsSmall(lclTyp))
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ enum GenTreeFlags : unsigned int
GTF_LIVENESS_MASK = GTF_VAR_DEF | GTF_VAR_USEASG | GTF_VAR_DEATH_MASK,

GTF_VAR_ITERATOR = 0x01000000, // GT_LCL_VAR -- this is a iterator reference in the loop condition
GTF_VAR_CLONED = 0x00800000, // GT_LCL_VAR -- this node has been cloned or is a clone
GTF_VAR_MOREUSES = 0x00800000, // GT_LCL_VAR -- this node has additonal uses, for example due to cloning
GTF_VAR_CONTEXT = 0x00400000, // GT_LCL_VAR -- this node is part of a runtime lookup
GTF_VAR_EXPLICIT_INIT = 0x00200000, // GT_LCL_VAR -- this node is an "explicit init" store. Valid until rationalization.

Expand Down Expand Up @@ -491,8 +491,8 @@ enum GenTreeFlags : unsigned int
GTF_IND_NONNULL = 0x00400000, // GT_IND -- the indirection never returns null (zero)
GTF_IND_INITCLASS = 0x00200000, // OperIsIndir() -- the indirection requires preceding static cctor

GTF_IND_FLAGS = GTF_IND_VOLATILE | GTF_IND_NONFAULTING | GTF_IND_UNALIGNED | GTF_IND_INVARIANT |
GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP | GTF_IND_INITCLASS,
GTF_IND_COPYABLE_FLAGS = GTF_IND_VOLATILE | GTF_IND_NONFAULTING | GTF_IND_UNALIGNED | GTF_IND_INITCLASS,
GTF_IND_FLAGS = GTF_IND_COPYABLE_FLAGS | GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP | GTF_IND_INVARIANT,

GTF_ADDRMODE_NO_CSE = 0x80000000, // GT_ADD/GT_MUL/GT_LSH -- Do not CSE this node only, forms complex
// addressing mode
Expand Down
76 changes: 48 additions & 28 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -807,8 +807,10 @@ GenTree* Compiler::impStoreStruct(GenTree* store,
WellKnownArg wellKnownArgType =
srcCall->ShouldHaveRetBufArg() ? WellKnownArg::RetBuffer : WellKnownArg::None;

GenTree* destAddr = impGetStructAddr(store, CHECK_SPILL_ALL, /* willDeref */ true);
NewCallArg newArg = NewCallArg::Primitive(destAddr).WellKnown(wellKnownArgType);
// TODO-Bug?: verify if flags matter here
GenTreeFlags indirFlags = GTF_EMPTY;
GenTree* destAddr = impGetNodeAddr(store, CHECK_SPILL_ALL, &indirFlags);
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 @@ -909,7 +911,9 @@ GenTree* Compiler::impStoreStruct(GenTree* store,
if (call->ShouldHaveRetBufArg())
{
// insert the return value buffer into the argument list as first byref parameter after 'this'
GenTree* destAddr = impGetStructAddr(store, CHECK_SPILL_ALL, /* willDeref */ true);
// TODO-Bug?: verify if flags matter here
GenTreeFlags indirFlags = GTF_EMPTY;
GenTree* destAddr = impGetNodeAddr(store, CHECK_SPILL_ALL, &indirFlags);
call->gtArgs.InsertAfterThisOrFirst(this,
NewCallArg::Primitive(destAddr).WellKnown(WellKnownArg::RetBuffer));

Expand All @@ -926,16 +930,17 @@ GenTree* Compiler::impStoreStruct(GenTree* store,
{
// 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(store, CHECK_SPILL_ALL, /* willDeref */ true);
GenTree* destAddrClone;
GenTreeFlags indirFlags = GTF_EMPTY;
GenTree* destAddr = impGetNodeAddr(store, CHECK_SPILL_ALL, &indirFlags);
GenTree* destAddrClone;
destAddr = impCloneExpr(destAddr, &destAddrClone, curLevel, pAfterStmt DEBUGARG("MKREFANY assignment"));

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

// Append the store of the pointer value.
// TODO-Bug: the pointer value can be a byref. Use its actual type here instead of TYP_I_IMPL.
GenTree* ptrFieldStore = gtNewStoreIndNode(TYP_I_IMPL, destAddr, src->AsOp()->gtOp1);
GenTree* ptrFieldStore = gtNewStoreIndNode(TYP_I_IMPL, destAddr, src->AsOp()->gtOp1, indirFlags);
if (pAfterStmt)
{
Statement* newStmt = gtNewStmt(ptrFieldStore, usedDI);
Expand Down Expand Up @@ -1020,51 +1025,59 @@ GenTree* Compiler::impStoreStructPtr(GenTree* destAddr, GenTree* value, unsigned
}

//------------------------------------------------------------------------
// impGetStructAddr: Get the address of a struct value / location.
// impGetNodeAddr: Get the address of a value.
//
// Arguments:
// structVal - The value in question
// curLevel - Stack level for spilling
// willDeref - Whether the caller will dereference the address
// val - The value in question
// curLevel - Stack level for spilling
// pDerefFlags - Flags to be used on dereference, nullptr when
// the address won't be dereferenced. Returned flags
// are included in the GTF_IND_COPYABLE_FLAGS mask.
//
// Return Value:
// In case "structVal" can represent locations (is an indirection/local),
// In case "val" represents a location (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, unsigned curLevel, bool willDeref)
GenTree* Compiler::impGetNodeAddr(GenTree* val, unsigned curLevel, GenTreeFlags* pDerefFlags)
{
assert(varTypeIsStruct(structVal));
switch (structVal->OperGet())
if (pDerefFlags != nullptr)
{
*pDerefFlags = GTF_EMPTY;
}
switch (val->OperGet())
{
case GT_BLK:
case GT_IND:
case GT_STOREIND:
case GT_STORE_BLK:
if (willDeref)
if (pDerefFlags != nullptr)
{
return structVal->AsIndir()->Addr();
*pDerefFlags = val->gtFlags & GTF_IND_COPYABLE_FLAGS;
return val->AsIndir()->Addr();
}
break;

case GT_LCL_VAR:
case GT_STORE_LCL_VAR:
return gtNewLclVarAddrNode(structVal->AsLclVar()->GetLclNum(), TYP_BYREF);
val->gtFlags |= GTF_VAR_MOREUSES;
return gtNewLclVarAddrNode(val->AsLclVar()->GetLclNum(), TYP_BYREF);

case GT_LCL_FLD:
case GT_STORE_LCL_FLD:
return gtNewLclAddrNode(structVal->AsLclFld()->GetLclNum(), structVal->AsLclFld()->GetLclOffs(), TYP_BYREF);
val->gtFlags |= GTF_VAR_MOREUSES;
return gtNewLclAddrNode(val->AsLclFld()->GetLclNum(), val->AsLclFld()->GetLclOffs(), TYP_BYREF);

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

default:
break;
}

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

// The 'return value' is now address of the temp itself.
return gtNewLclVarAddrNode(lclNum, TYP_BYREF);
Expand Down Expand Up @@ -3000,7 +3013,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, CHECK_SPILL_ALL, true);
// TODO-Bug?: verify if flags matter here
GenTreeFlags indirFlags = GTF_EMPTY;
objToBox = impGetNodeAddr(objToBox, CHECK_SPILL_ALL, &indirFlags);

static_assert_no_msg(OFFSETOF__CORINFO_NullableOfT__hasValue == 0);
impPushOnStack(gtNewIndir(TYP_BOOL, objToBox), typeInfo(TYP_INT));
Expand Down Expand Up @@ -3348,7 +3363,9 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
return;
}

op1 = gtNewHelperCallNode(boxHelper, TYP_REF, op2, impGetStructAddr(exprToBox, CHECK_SPILL_ALL, true));
// TODO-Bug?: verify if flags matter here
GenTreeFlags indirFlags = GTF_EMPTY;
op1 = gtNewHelperCallNode(boxHelper, TYP_REF, op2, impGetNodeAddr(exprToBox, CHECK_SPILL_ALL, &indirFlags));
}

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

JITDUMP("\n ... optimized to ...\n");
Expand Down Expand Up @@ -8908,7 +8925,9 @@ void Compiler::impImportBlockCode(BasicBlock* block)
BADCODE3("Unexpected opcode (has to be LDFLD)", ": %02X", (int)opcode);
}

obj = impGetStructAddr(obj, CHECK_SPILL_ALL, true);
// TODO-Bug?: verify if flags matter here
GenTreeFlags indirFlags = GTF_EMPTY;
obj = impGetNodeAddr(obj, CHECK_SPILL_ALL, &indirFlags);
}

op1 = gtNewFieldAddrNode(resolvedToken.hField, obj, fieldInfo.offset);
Expand Down Expand Up @@ -9661,12 +9680,13 @@ void Compiler::impImportBlockCode(BasicBlock* block)
else
{
// Get the address of the refany
op1 = impGetStructAddr(op1, CHECK_SPILL_ALL, /* willDeref */ true);
GenTreeFlags indirFlags = GTF_EMPTY;
op1 = impGetNodeAddr(op1, CHECK_SPILL_ALL, &indirFlags);

// 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, indirFlags);
}

// Convert native TypeHandle to RuntimeTypeHandle.
Expand Down
Loading