From b204d0f9ac4a478d5ab258735d7f5430edadf8f3 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Thu, 13 Apr 2023 00:14:05 +0300 Subject: [PATCH 1/7] Add an assert --- src/coreclr/jit/rationalize.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index c2e0216f6a7dbc..5b0209ff04d0c9 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -40,11 +40,7 @@ void Rationalizer::RewriteIndir(LIR::Use& use) if (varTypeIsSIMD(indir)) { - if (indir->OperIs(GT_BLK)) - { - indir->SetOper(GT_IND); - } - + assert(indir->OperIs(GT_IND)); RewriteSIMDIndir(use); } } From 217d45be3cf01ccbe1803abe080fdf2d56bdca97 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 11 Apr 2023 21:11:08 +0300 Subject: [PATCH 2/7] Move gtNewStructVal/gtNewBlkIndir/gtNewIndir From compiler.hpp to gentree.cpp. Motivation: more methods are to come and the header file is very large already. --- src/coreclr/jit/compiler.h | 4 +- src/coreclr/jit/compiler.hpp | 54 ----------------- src/coreclr/jit/gentree.cpp | 114 ++++++++++++++++++++++++++--------- 3 files changed, 86 insertions(+), 86 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 21d88cabdc3b13..446334606ad854 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2516,8 +2516,6 @@ class Compiler GenTree* gtNewBitCastNode(var_types type, GenTree* arg); public: - GenTree* gtNewStructVal(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY); - GenTreeCall* gtNewCallNode(gtCallTypes callType, CORINFO_METHOD_HANDLE handle, var_types type, @@ -2833,6 +2831,8 @@ class Compiler GenTreeIndir* gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY); + GenTree* gtNewStructVal(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY); + GenTree* gtNewNullCheck(GenTree* addr, BasicBlock* basicBlock); var_types gtTypeForNullCheck(GenTree* tree); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 4957a1c30e65ee..923c659e14ae4b 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1177,60 +1177,6 @@ inline GenTreeMDArr* Compiler::gtNewMDArrLowerBound(GenTree* arrayOp, unsigned d return arrOp; } -//------------------------------------------------------------------------ -// gtNewBlkIndir: Create a struct indirection node. -// -// Arguments: -// layout - The struct layout -// addr - Address of the indirection -// indirFlags - Indirection flags -// -// Return Value: -// The created GT_BLK node. -// -inline GenTreeBlk* Compiler::gtNewBlkIndir(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags) -{ - assert((indirFlags & ~GTF_IND_FLAGS) == GTF_EMPTY); - - GenTreeBlk* blkNode = new (this, GT_BLK) GenTreeBlk(GT_BLK, layout->GetType(), addr, layout); - blkNode->gtFlags |= indirFlags; - blkNode->SetIndirExceptionFlags(this); - - if ((indirFlags & GTF_IND_INVARIANT) == 0) - { - blkNode->gtFlags |= GTF_GLOB_REF; - } - - if ((indirFlags & GTF_IND_VOLATILE) != 0) - { - blkNode->gtFlags |= GTF_ORDER_SIDEEFF; - } - - return blkNode; -} - -//------------------------------------------------------------------------------ -// gtNewIndir : Create an indirection node. -// -// Arguments: -// typ - Type of the node -// addr - Address of the indirection -// indirFlags - Indirection flags -// -// Return Value: -// The created GT_IND node. -// -inline GenTreeIndir* Compiler::gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags) -{ - assert((indirFlags & ~GTF_IND_FLAGS) == GTF_EMPTY); - - GenTree* indir = gtNewOperNode(GT_IND, typ, addr); - indir->gtFlags |= indirFlags; - indir->SetIndirExceptionFlags(this); - - return indir->AsIndir(); -} - //------------------------------------------------------------------------------ // gtNewNullCheck : Helper to create a null check node. // diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 3413b11ac7468d..6459a00b9820f1 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7798,6 +7798,90 @@ GenTreeField* Compiler::gtNewFieldAddrNode(var_types type, CORINFO_FIELD_HANDLE return fieldNode; } +//------------------------------------------------------------------------ +// gtNewStructVal: Return a node that represents a struct or block value +// +// Arguments: +// layout - The struct's layout +// addr - The struct's address +// indirFlags - Indirection flags +// +// Return Value: +// A "BLK" node, or "LCL_VAR" node if "addr" points to a local with a +// layout compatible with "layout". +// +GenTree* Compiler::gtNewStructVal(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags) +{ + assert((indirFlags & ~GTF_IND_FLAGS) == 0); + + if (((indirFlags & GTF_IND_VOLATILE) == 0) && addr->IsLclVarAddr()) + { + unsigned lclNum = addr->AsLclFld()->GetLclNum(); + LclVarDsc* varDsc = lvaGetDesc(lclNum); + if (!lvaIsImplicitByRefLocal(lclNum) && varTypeIsStruct(varDsc) && + ClassLayout::AreCompatible(layout, varDsc->GetLayout())) + { + return gtNewLclvNode(lclNum, varDsc->TypeGet()); + } + } + + return gtNewBlkIndir(layout, addr, indirFlags); +} + +//------------------------------------------------------------------------ +// gtNewBlkIndir: Create a struct indirection node. +// +// Arguments: +// layout - The struct layout +// addr - Address of the indirection +// indirFlags - Indirection flags +// +// Return Value: +// The created GT_BLK node. +// +GenTreeBlk* Compiler::gtNewBlkIndir(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags) +{ + assert((indirFlags & ~GTF_IND_FLAGS) == GTF_EMPTY); + + GenTreeBlk* blkNode = new (this, GT_BLK) GenTreeBlk(GT_BLK, layout->GetType(), addr, layout); + blkNode->gtFlags |= indirFlags; + blkNode->SetIndirExceptionFlags(this); + + if ((indirFlags & GTF_IND_INVARIANT) == 0) + { + blkNode->gtFlags |= GTF_GLOB_REF; + } + + if ((indirFlags & GTF_IND_VOLATILE) != 0) + { + blkNode->gtFlags |= GTF_ORDER_SIDEEFF; + } + + return blkNode; +} + +//------------------------------------------------------------------------------ +// gtNewIndir : Create an indirection node. +// +// Arguments: +// typ - Type of the node +// addr - Address of the indirection +// indirFlags - Indirection flags +// +// Return Value: +// The created GT_IND node. +// +GenTreeIndir* Compiler::gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags) +{ + assert((indirFlags & ~GTF_IND_FLAGS) == GTF_EMPTY); + + GenTree* indir = gtNewOperNode(GT_IND, typ, addr); + indir->gtFlags |= indirFlags; + indir->SetIndirExceptionFlags(this); + + return indir->AsIndir(); +} + /***************************************************************************** * * Create a node that will assign 'src' to 'dst'. @@ -7844,36 +7928,6 @@ GenTreeOp* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src) return asg; } -//------------------------------------------------------------------------ -// gtNewStructVal: Return a node that represents a struct or block value -// -// Arguments: -// layout - The struct's layout -// addr - The struct's address -// indirFlags - Indirection flags -// -// Return Value: -// An "OBJ/BLK" node, or "LCL_VAR" node if "addr" points to a local -// with a layout compatible with "layout". -// -GenTree* Compiler::gtNewStructVal(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags) -{ - assert((indirFlags & ~GTF_IND_FLAGS) == 0); - - if (((indirFlags & GTF_IND_VOLATILE) == 0) && addr->IsLclVarAddr()) - { - unsigned lclNum = addr->AsLclFld()->GetLclNum(); - LclVarDsc* varDsc = lvaGetDesc(lclNum); - if (!lvaIsImplicitByRefLocal(lclNum) && varTypeIsStruct(varDsc) && - ClassLayout::AreCompatible(layout, varDsc->GetLayout())) - { - return gtNewLclvNode(lclNum, varDsc->TypeGet()); - } - } - - return gtNewBlkIndir(layout, addr, indirFlags); -} - //------------------------------------------------------------------------ // FixupInitBlkValue: Fixup the init value for an initBlk operation // From 4c892cafaa24e0a97eb765e8e801512636f9c548 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Wed, 29 Mar 2023 00:46:31 +0300 Subject: [PATCH 3/7] Generalize "gtNewStructVal" to avoid diffs --- src/coreclr/jit/compiler.h | 8 +++++- src/coreclr/jit/gentree.cpp | 41 ++++++++++++++++----------- src/coreclr/jit/gentree.h | 5 ++++ src/coreclr/jit/importer.cpp | 20 +++++++------ src/coreclr/jit/importercalls.cpp | 4 +-- src/coreclr/jit/morph.cpp | 4 +-- src/coreclr/jit/morphblock.cpp | 2 +- src/coreclr/jit/simdashwintrinsic.cpp | 2 +- 8 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 446334606ad854..493c72751b5818 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2831,7 +2831,13 @@ class Compiler GenTreeIndir* gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY); - GenTree* gtNewStructVal(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY); + GenTree* gtNewLoadValueNode( + var_types type, ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY); + + GenTree* gtNewLoadValueNode(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY) + { + return gtNewLoadValueNode(layout->GetType(), layout, addr, indirFlags); + } GenTree* gtNewNullCheck(GenTree* addr, BasicBlock* basicBlock); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 6459a00b9820f1..6a9d8c5b55ae78 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7799,33 +7799,44 @@ GenTreeField* Compiler::gtNewFieldAddrNode(var_types type, CORINFO_FIELD_HANDLE } //------------------------------------------------------------------------ -// gtNewStructVal: Return a node that represents a struct or block value +// gtNewLoadValueNode: Return a node that represents a loaded value. // // Arguments: -// layout - The struct's layout -// addr - The struct's address +// type - Type to load +// layout - Struct layout to load +// addr - The address // indirFlags - Indirection flags // // Return Value: -// A "BLK" node, or "LCL_VAR" node if "addr" points to a local with a -// layout compatible with "layout". +// A "BLK/IND" node, or "LCL_VAR" if "addr" points to a compatible local. // -GenTree* Compiler::gtNewStructVal(ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags) +GenTree* Compiler::gtNewLoadValueNode(var_types type, ClassLayout* layout, GenTree* addr, GenTreeFlags indirFlags) { - assert((indirFlags & ~GTF_IND_FLAGS) == 0); + assert(((indirFlags & ~GTF_IND_FLAGS) == 0) && ((type != TYP_STRUCT) || (layout != nullptr))); if (((indirFlags & GTF_IND_VOLATILE) == 0) && addr->IsLclVarAddr()) { unsigned lclNum = addr->AsLclFld()->GetLclNum(); LclVarDsc* varDsc = lvaGetDesc(lclNum); - if (!lvaIsImplicitByRefLocal(lclNum) && varTypeIsStruct(varDsc) && - ClassLayout::AreCompatible(layout, varDsc->GetLayout())) + if (!lvaIsImplicitByRefLocal(lclNum) && (varDsc->TypeGet() == type) && + ((type != TYP_STRUCT) || ClassLayout::AreCompatible(layout, varDsc->GetLayout()))) { - return gtNewLclvNode(lclNum, varDsc->TypeGet()); + return gtNewLclvNode(lclNum, type); } } - return gtNewBlkIndir(layout, addr, indirFlags); + GenTree* node; + if (type == TYP_STRUCT) + { + node = gtNewBlkIndir(layout, addr, indirFlags); + } + else + { + node = gtNewIndir(type, addr, indirFlags); + node->gtFlags |= GTF_GLOB_REF; + } + + return node; } //------------------------------------------------------------------------ @@ -8154,7 +8165,7 @@ void GenTreeOp::CheckDivideByConstOptimized(Compiler* comp) // GenTree* Compiler::gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, bool isVolatile) { - assert(varTypeIsStruct(dst) && (dst->OperIsBlk() || dst->OperIsLocal() || dst->OperIs(GT_FIELD))); + assert(varTypeIsStruct(dst) && (dst->OperIsIndir() || dst->OperIsLocal() || dst->OperIs(GT_FIELD))); bool isCopyBlock = srcOrFillVal->TypeGet() == dst->TypeGet(); if (!isCopyBlock) // InitBlk @@ -15818,9 +15829,7 @@ GenTree* Compiler::gtNewTempAssign( } else if ((dstTyp == TYP_STRUCT) && (valTyp == TYP_INT)) { - // It could come from `ASG(struct, 0)` that was propagated to `RETURN struct(0)`, - // and now it is merging to a struct again. - assert(tmp == genReturnLocal); + assert(val->IsInitVal()); ok = true; } @@ -15853,7 +15862,7 @@ GenTree* Compiler::gtNewTempAssign( GenTree* asg; GenTree* dest = gtNewLclvNode(tmp, dstTyp); - if (val->IsConstInitVal()) + if (val->IsInitVal()) { asg = gtNewAssignNode(dest, val); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 61488498a36340..935b4b9a927efb 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1204,6 +1204,11 @@ struct GenTree return OperIsInitVal(OperGet()); } + bool IsInitVal() const + { + return IsIntegralConst(0) || OperIsInitVal(); + } + bool IsConstInitVal() const { return (gtOper == GT_CNS_INT) || (OperIsInitVal() && (gtGetOp1()->gtOper == GT_CNS_INT)); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index f718b2e1ca5da3..ff9abb78ba4047 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1173,8 +1173,12 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, BasicBlock* block /* = NULL */ ) { - GenTree* dst = gtNewStructVal(typGetObjLayout(structHnd), destAddr); - return impAssignStruct(dst, src, curLevel, pAfterStmt, di, block); + var_types type = src->TypeGet(); + ClassLayout* layout = (type == TYP_STRUCT) ? src->GetLayout(this) : nullptr; + GenTree* dst = gtNewLoadValueNode(type, layout, destAddr); + GenTree* store = impAssignStruct(dst, src, curLevel, pAfterStmt, di, block); + + return store; } //------------------------------------------------------------------------ @@ -10660,7 +10664,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) } op1 = impPopStack().val; - op1 = gtNewStructVal(layout, op1); + op1 = gtNewLoadValueNode(layout, op1); op2 = gtNewIconNode(0); op1 = gtNewBlkOpNode(op1, op2, (prefixFlags & PREFIX_VOLATILE) != 0); goto SPILL_APPEND; @@ -10692,8 +10696,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) } ClassLayout* layout = typGetBlkLayout(static_cast(op3->AsIntConCommon()->IconValue())); - op1 = gtNewStructVal(layout, op1, indirFlags); - op2 = opcode == CEE_INITBLK ? op2 : gtNewStructVal(layout, op2, indirFlags); + op1 = gtNewLoadValueNode(layout, op1, indirFlags); + op2 = opcode == CEE_INITBLK ? op2 : gtNewLoadValueNode(layout, op2, indirFlags); op1 = gtNewBlkOpNode(op1, op2, (indirFlags & GTF_IND_VOLATILE) != 0); } else @@ -10746,8 +10750,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = impPopStack().val; // Dest addr ClassLayout* layout = typGetObjLayout(resolvedToken.hClass); - op1 = gtNewStructVal(layout, op1); - op2 = gtNewStructVal(layout, op2); + op1 = gtNewLoadValueNode(layout, op1); + op2 = gtNewLoadValueNode(layout, op2); op1 = gtNewBlkOpNode(op1, op2, ((prefixFlags & PREFIX_VOLATILE) != 0)); goto SPILL_APPEND; } @@ -10772,7 +10776,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = impPopStack().val; // Ptr assertImp(varTypeIsStruct(op2)); - op1 = gtNewStructVal(typGetObjLayout(resolvedToken.hClass), op1, indirFlags); + op1 = gtNewLoadValueNode(typGetObjLayout(resolvedToken.hClass), op1, indirFlags); op1 = impAssignStruct(op1, op2, CHECK_SPILL_ALL); goto SPILL_APPEND; } diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 00928ccce1374f..d798687ef74510 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2124,11 +2124,11 @@ GenTree* Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig) ClassLayout* blkLayout = typGetBlkLayout(blkSize); GenTree* dstAddr = gtNewOperNode(GT_ADD, TYP_BYREF, arrayLocalNode, gtNewIconNode(dataOffset, TYP_I_IMPL)); - GenTree* dst = gtNewStructVal(blkLayout, dstAddr); + GenTree* dst = gtNewLoadValueNode(blkLayout, dstAddr); dst->gtFlags |= GTF_GLOB_REF; GenTree* srcAddr = gtNewIconHandleNode((size_t)initData, GTF_ICON_CONST_PTR); - GenTree* src = gtNewStructVal(blkLayout, srcAddr); + GenTree* src = gtNewLoadValueNode(blkLayout, srcAddr); #ifdef DEBUG src->gtGetOp1()->AsIntCon()->gtTargetHandle = THT_InitializeArrayIntrinsics; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index cc524e5421485b..7762dfde79b38d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -4729,7 +4729,7 @@ GenTree* Compiler::fgMorphExpandStackArgForVarArgs(GenTreeLclVarCommon* lclNode) GenTree* argNode; if (lclNode->TypeIs(TYP_STRUCT)) { - argNode = gtNewStructVal(lclNode->GetLayout(this), argAddr); + argNode = gtNewLoadValueNode(lclNode->GetLayout(this), argAddr); } else { @@ -4864,7 +4864,7 @@ GenTree* Compiler::fgMorphExpandImplicitByRefArg(GenTreeLclVarCommon* lclNode) { if (argNodeType == TYP_STRUCT) { - newArgNode = gtNewStructVal(argNodeLayout, addrNode); + newArgNode = gtNewLoadValueNode(argNodeLayout, addrNode); } else { diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index ad20697c2cc743..8a80825d7f8082 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -1624,7 +1624,7 @@ GenTree* Compiler::fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree) if ((size != 0) && FitsIn(size)) { ClassLayout* layout = typGetBlkLayout(static_cast(size)); - GenTree* dst = gtNewStructVal(layout, tree->Addr(), tree->gtFlags & GTF_IND_FLAGS); + GenTree* dst = gtNewLoadValueNode(layout, tree->Addr(), tree->gtFlags & GTF_IND_FLAGS); dst->gtFlags |= GTF_GLOB_REF; GenTree* src = tree->Data(); diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index 45f6c02fc09299..a2885a16f7875d 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -2172,7 +2172,7 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, // GTF_ALL_EFFECT for the dest. This is currently emulating the previous behavior of // block ops. - GenTree* dest = gtNewStructVal(typGetBlkLayout(simdSize), copyBlkDst); + GenTree* dest = gtNewLoadValueNode(typGetBlkLayout(simdSize), copyBlkDst); dest->gtType = simdType; dest->gtFlags |= GTF_GLOB_REF; From f322771c45f9af486f78ff98391a6be8a400138e Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 14 Apr 2023 00:11:45 +0300 Subject: [PATCH 4/7] Stop producing BLK --- src/coreclr/jit/compiler.h | 9 +- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/ee_il_dll.hpp | 26 +++++ src/coreclr/jit/gentree.cpp | 11 ++- src/coreclr/jit/importer.cpp | 135 +++++++++----------------- src/coreclr/jit/importercalls.cpp | 43 +++----- src/coreclr/jit/morphblock.cpp | 5 - src/coreclr/jit/simdashwintrinsic.cpp | 10 +- 8 files changed, 103 insertions(+), 138 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 493c72751b5818..2206b181ae48c8 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2839,6 +2839,11 @@ class Compiler return gtNewLoadValueNode(layout->GetType(), layout, addr, indirFlags); } + GenTree* gtNewLoadValueNode(var_types type, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY) + { + return gtNewLoadValueNode(type, nullptr, addr, indirFlags); + } + GenTree* gtNewNullCheck(GenTree* addr, BasicBlock* basicBlock); var_types gtTypeForNullCheck(GenTree* tree); @@ -2857,7 +2862,6 @@ class Compiler CORINFO_ACCESS_FLAGS access, CORINFO_FIELD_INFO* pFieldInfo, var_types lclTyp, - CORINFO_CLASS_HANDLE structType, GenTree* assg); GenTree* gtNewNothingNode(); @@ -10118,6 +10122,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX // Get the number of a layout for the specified class handle. unsigned typGetObjLayoutNum(CORINFO_CLASS_HANDLE classHandle); + var_types TypeHandleToVarType(CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout = nullptr); + var_types TypeHandleToVarType(CorInfoType jitType, CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout = nullptr); + //-------------------------- Global Compiler Data ------------------------------------ #ifdef DEBUG diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 923c659e14ae4b..3224ac68b6320e 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1074,7 +1074,7 @@ inline GenTreeIndexAddr* Compiler::gtNewArrayIndexAddr(GenTree* arra inline GenTreeIndir* Compiler::gtNewIndexIndir(GenTreeIndexAddr* indexAddr) { GenTreeIndir* index; - if (varTypeIsStruct(indexAddr->gtElemType)) + if (indexAddr->gtElemType == TYP_STRUCT) { index = gtNewBlkIndir(typGetObjLayout(indexAddr->gtStructElemClass), indexAddr); } diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index 0a9c248a861593..162d5479dff49a 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -296,6 +296,32 @@ inline var_types JitType2PreciseVarType(CorInfoType type) return ((var_types)preciseVarTypeMap[type]); }; +inline var_types Compiler::TypeHandleToVarType(CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout) +{ + CorInfoType jitType = info.compCompHnd->asCorInfoType(handle); + var_types type = TypeHandleToVarType(jitType, handle, pLayout); + + return type; +} + +inline var_types Compiler::TypeHandleToVarType(CorInfoType jitType, CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout) +{ + ClassLayout* layout = nullptr; + var_types type = JITtype2varType(jitType); + if (type == TYP_STRUCT) + { + layout = typGetObjLayout(handle); + type = layout->GetType(); + } + + if (pLayout != nullptr) + { + *pLayout = layout; + } + + return type; +} + inline CORINFO_CALLINFO_FLAGS combine(CORINFO_CALLINFO_FLAGS flag1, CORINFO_CALLINFO_FLAGS flag2) { return (CORINFO_CALLINFO_FLAGS)(flag1 | flag2); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 6a9d8c5b55ae78..d2e5ebc311ca52 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -15895,7 +15895,6 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr, CORINFO_ACCESS_FLAGS access, CORINFO_FIELD_INFO* pFieldInfo, var_types lclTyp, - CORINFO_CLASS_HANDLE structType, GenTree* assg) { assert(pFieldInfo->fieldAccessor == CORINFO_FIELD_INSTANCE_HELPER || @@ -15906,7 +15905,8 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr, GenTree* args[4]; size_t nArgs = 0; /* If we can't access it directly, we need to call a helper function */ - var_types helperType = TYP_BYREF; + var_types helperType = TYP_BYREF; + CORINFO_CLASS_HANDLE structType = pFieldInfo->structType; if (pFieldInfo->fieldAccessor == CORINFO_FIELD_INSTANCE_HELPER) { @@ -15916,7 +15916,6 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr, // helper needs pointer to struct, not struct itself if (pFieldInfo->helper == CORINFO_HELP_SETFIELDSTRUCT) { - assert(structType != nullptr); assg = impGetStructAddr(assg, structType, CHECK_SPILL_ALL, true); } else if (lclTyp == TYP_DOUBLE && assg->TypeGet() == TYP_FLOAT) @@ -16009,9 +16008,11 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr, // OK, now do the indirection if (access & CORINFO_ACCESS_GET) { - if (varTypeIsStruct(lclTyp)) + ClassLayout* layout; + lclTyp = TypeHandleToVarType(pFieldInfo->fieldType, structType, &layout); + if (lclTyp == TYP_STRUCT) { - result = gtNewBlkIndir(typGetObjLayout(structType), result); + result = gtNewBlkIndir(layout, result); } else { diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index ff9abb78ba4047..155be580ff37f9 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1402,7 +1402,7 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, CORINFO_CLASS_HANDLE str else #endif { - noway_assert(blockNode->OperIsBlk() || blockNode->OperIs(GT_FIELD)); + noway_assert(blockNode->OperIsIndir() || blockNode->OperIs(GT_FIELD)); // Sink the GT_COMMA below the blockNode addr. // That is GT_COMMA(op1, op2=blockNode) is transformed into @@ -4335,10 +4335,13 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT if (!(access & CORINFO_ACCESS_ADDRESS)) { + ClassLayout* layout; + lclTyp = TypeHandleToVarType(pFieldInfo->fieldType, pFieldInfo->structType, &layout); + // TODO-CQ: mark the indirections non-faulting. - if (varTypeIsStruct(lclTyp)) + if (lclTyp == TYP_STRUCT) { - op1 = gtNewBlkIndir(typGetObjLayout(pFieldInfo->structType), op1); + op1 = gtNewBlkIndir(layout, op1); } else { @@ -7203,7 +7206,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP(" %08X", resolvedToken.token); ldelemClsHnd = resolvedToken.hClass; - lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(ldelemClsHnd)); + lclTyp = TypeHandleToVarType(ldelemClsHnd); tiRetVal = verMakeTypeInfo(ldelemClsHnd); tiRetVal.NormaliseForStack(); goto ARR_LD; @@ -7283,7 +7286,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP(" %08X", resolvedToken.token); stelemClsHnd = resolvedToken.hClass; - lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(stelemClsHnd)); + lclTyp = TypeHandleToVarType(stelemClsHnd); if (lclTyp != TYP_REF) { @@ -9214,7 +9217,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CEE_LDFLDA: case CEE_LDSFLDA: { - bool isLoadAddress = (opcode == CEE_LDFLDA || opcode == CEE_LDSFLDA); bool isLoadStatic = (opcode == CEE_LDSFLD || opcode == CEE_LDSFLDA); @@ -9244,12 +9246,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) eeGetFieldInfo(&resolvedToken, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo); - // Figure out the type of the member. We always call canAccessField, so you always need this - // handle - CorInfoType ciType = fieldInfo.fieldType; - clsHnd = fieldInfo.structType; - - lclTyp = JITtype2varType(ciType); + // Note we avoid resolving the normalized (struct) type just yet; we may not need it (for ld[s]flda). + lclTyp = JITtype2varType(fieldInfo.fieldType); + clsHnd = fieldInfo.structType; if (compIsForInlining()) { @@ -9259,7 +9258,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CORINFO_FIELD_INSTANCE_ADDR_HELPER: case CORINFO_FIELD_STATIC_ADDR_HELPER: case CORINFO_FIELD_STATIC_TLS: - compInlineResult->NoteFatal(InlineObservation::CALLEE_LDFLD_NEEDS_HELPER); return; @@ -9274,8 +9272,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) break; } - if (!isLoadAddress && (fieldInfo.fieldFlags & CORINFO_FLG_FIELD_STATIC) && lclTyp == TYP_STRUCT && - clsHnd) + if (!isLoadAddress && (fieldInfo.fieldFlags & CORINFO_FLG_FIELD_STATIC) && (lclTyp == TYP_STRUCT)) { if ((info.compCompHnd->getTypeForPrimitiveValueClass(clsHnd) == CORINFO_TYPE_UNDEF) && !(info.compFlags & CORINFO_FLG_FORCEINLINE)) @@ -9301,7 +9298,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) } } - tiRetVal = verMakeTypeInfo(ciType, clsHnd); + tiRetVal = verMakeTypeInfo(fieldInfo.fieldType, clsHnd); if (isLoadAddress) { tiRetVal.MakeByRef(); @@ -9332,12 +9329,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) obj = nullptr; } - /* Preserve 'small' int types */ - if (!varTypeIsSmall(lclTyp)) - { - lclTyp = genActualType(lclTyp); - } - bool usesHelper = false; switch (fieldInfo.fieldAccessor) @@ -9406,9 +9397,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (!isLoadAddress) { - if (varTypeIsStruct(lclTyp)) + ClassLayout* layout; + lclTyp = TypeHandleToVarType(fieldInfo.fieldType, fieldInfo.structType, &layout); + if (lclTyp == TYP_STRUCT) { - op1 = gtNewBlkIndir(typGetObjLayout(fieldInfo.structType), op1, GTF_IND_NONFAULTING); + op1 = gtNewBlkIndir(layout, op1, GTF_IND_NONFAULTING); } else { @@ -9425,7 +9418,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CORINFO_FIELD_INSTANCE_HELPER: case CORINFO_FIELD_INSTANCE_ADDR_HELPER: op1 = gtNewRefCOMfield(obj, &resolvedToken, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo, lclTyp, - clsHnd, nullptr); + nullptr); usesHelper = true; break; @@ -9542,8 +9535,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) bool isStoreStatic = (opcode == CEE_STSFLD); - CORINFO_CLASS_HANDLE fieldClsHnd; // class of the field (if it's a ref type) - /* Get the CP_Fieldref index */ assertImp(sz == sizeof(unsigned)); @@ -9557,12 +9548,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) eeGetFieldInfo(&resolvedToken, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo); - // Figure out the type of the member. We always call canAccessField, so you always need this - // handle - CorInfoType ciType = fieldInfo.fieldType; - fieldClsHnd = fieldInfo.structType; - - lclTyp = JITtype2varType(ciType); + lclTyp = JITtype2varType(fieldInfo.fieldType); if (compIsForInlining()) { @@ -9575,7 +9561,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CORINFO_FIELD_INSTANCE_ADDR_HELPER: case CORINFO_FIELD_STATIC_ADDR_HELPER: case CORINFO_FIELD_STATIC_TLS: - compInlineResult->NoteFatal(InlineObservation::CALLEE_STFLD_NEEDS_HELPER); return; @@ -9640,12 +9625,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) obj = nullptr; } - /* Preserve 'small' int types */ - if (!varTypeIsSmall(lclTyp)) - { - lclTyp = genActualType(lclTyp); - } - switch (fieldInfo.fieldAccessor) { case CORINFO_FIELD_INSTANCE: @@ -9687,9 +9666,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = gtNewFieldAddrNode(TYP_I_IMPL, resolvedToken.hField, nullptr, fieldInfo.offset); op1->gtFlags |= GTF_FLD_TLS; // fgMorphExpandTlsField will handle the transformation. - if (varTypeIsStruct(lclTyp)) + ClassLayout* layout; + lclTyp = TypeHandleToVarType(fieldInfo.fieldType, fieldInfo.structType, &layout); + if (lclTyp == TYP_STRUCT) { - op1 = gtNewBlkIndir(typGetObjLayout(fieldInfo.structType), op1, GTF_IND_NONFAULTING); + op1 = gtNewBlkIndir(layout, op1, GTF_IND_NONFAULTING); } else { @@ -9705,7 +9686,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CORINFO_FIELD_INSTANCE_HELPER: case CORINFO_FIELD_INSTANCE_ADDR_HELPER: op1 = gtNewRefCOMfield(obj, &resolvedToken, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo, lclTyp, - clsHnd, op2); + op2); goto SPILL_APPEND; case CORINFO_FIELD_STATIC_TLS_MANAGED: @@ -9725,7 +9706,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) assert(!"Unexpected fieldAccessor"); } - if (lclTyp == TYP_STRUCT) + if (varTypeIsStruct(lclTyp)) { op1 = impAssignStruct(op1, op2, CHECK_SPILL_ALL); } @@ -10269,7 +10250,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) GenTree* boxPayloadOffset = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); GenTree* boxPayloadAddress = gtNewOperNode(GT_ADD, TYP_BYREF, op1, boxPayloadOffset); impPushOnStack(boxPayloadAddress, tiRetVal); - oper = GT_BLK; goto OBJ; } else @@ -10394,7 +10374,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) { // Normal unbox helper returns a TYP_BYREF. impPushOnStack(op1, tiRetVal); - oper = GT_BLK; goto OBJ; } @@ -10424,11 +10403,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) // Make sure the right type is placed on the operand type stack. impPushOnStack(op1, tiRetVal); - // Load the struct. - oper = GT_BLK; - assert(op1->gtType == TYP_BYREF); - goto OBJ; } else @@ -10647,19 +10622,12 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP(" %08X", resolvedToken.token); - lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass)); - - ClassLayout* layout = nullptr; - - if (lclTyp == TYP_STRUCT) - { - layout = typGetObjLayout(resolvedToken.hClass); - lclTyp = layout->GetType(); - } + ClassLayout* layout; + lclTyp = TypeHandleToVarType(resolvedToken.hClass, &layout); if (lclTyp != TYP_STRUCT) { - op2 = gtNewZeroConNode(genActualType(lclTyp)); + op2 = gtNewZeroConNode(lclTyp); goto STIND_VALUE; } @@ -10733,26 +10701,23 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP(" %08X", resolvedToken.token); - lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass)); + ClassLayout* layout; + lclTyp = TypeHandleToVarType(resolvedToken.hClass, &layout); if (lclTyp != TYP_STRUCT) { - op1 = impPopStack().val; // address to load from - - op1 = gtNewIndir(lclTyp, op1); - op1->gtFlags |= GTF_GLOB_REF; - - impPushOnStack(op1, typeInfo()); - goto STIND; + op2 = impPopStack().val; // address to load from + op2 = gtNewIndir(lclTyp, op2); + op2->gtFlags |= GTF_GLOB_REF; + goto STIND_VALUE; } op2 = impPopStack().val; // Src addr op1 = impPopStack().val; // Dest addr - ClassLayout* layout = typGetObjLayout(resolvedToken.hClass); - op1 = gtNewLoadValueNode(layout, op1); - op2 = gtNewLoadValueNode(layout, op2); - op1 = gtNewBlkOpNode(op1, op2, ((prefixFlags & PREFIX_VOLATILE) != 0)); + op1 = gtNewLoadValueNode(layout, op1); + op2 = gtNewLoadValueNode(layout, op2); + op1 = gtNewBlkOpNode(op1, op2, ((prefixFlags & PREFIX_VOLATILE) != 0)); goto SPILL_APPEND; } @@ -10764,19 +10729,19 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP(" %08X", resolvedToken.token); - lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass)); + ClassLayout* layout; + lclTyp = TypeHandleToVarType(resolvedToken.hClass, &layout); - if (lclTyp != TYP_STRUCT) + if (!varTypeIsStruct(lclTyp)) { goto STIND; } - GenTreeFlags indirFlags = impPrefixFlagsToIndirFlags(prefixFlags); - op2 = impPopStack().val; // Value - op1 = impPopStack().val; // Ptr - + op2 = impPopStack().val; // Value + op1 = impPopStack().val; // Ptr assertImp(varTypeIsStruct(op2)); - op1 = gtNewLoadValueNode(typGetObjLayout(resolvedToken.hClass), op1, indirFlags); + + op1 = gtNewLoadValueNode(layout, op1, impPrefixFlagsToIndirFlags(prefixFlags)); op1 = impAssignStruct(op1, op2, CHECK_SPILL_ALL); goto SPILL_APPEND; } @@ -10822,7 +10787,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CEE_LDOBJ: { - oper = GT_BLK; assertImp(sz == sizeof(unsigned)); _impResolveToken(CORINFO_TOKENKIND_Class); @@ -10830,7 +10794,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP(" %08X", resolvedToken.token); OBJ: - lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass)); + ClassLayout* layout; + lclTyp = TypeHandleToVarType(resolvedToken.hClass, &layout); tiRetVal = verMakeTypeInfo(resolvedToken.hClass); if (lclTyp != TYP_STRUCT) @@ -10838,14 +10803,10 @@ void Compiler::impImportBlockCode(BasicBlock* block) goto LDIND; } - GenTreeFlags indirFlags = impPrefixFlagsToIndirFlags(prefixFlags); - op1 = impPopStack().val; - + op1 = impPopStack().val; assertImp((genActualType(op1) == TYP_I_IMPL) || op1->TypeIs(TYP_BYREF)); - op1 = gtNewBlkIndir(typGetObjLayout(resolvedToken.hClass), op1, indirFlags); - op1->gtFlags |= GTF_EXCEPT; // TODO-1stClassStructs-Cleanup: delete this zero-diff quirk. - + op1 = gtNewBlkIndir(layout, op1, impPrefixFlagsToIndirFlags(prefixFlags)); impPushOnStack(op1, tiRetVal); break; } diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index d798687ef74510..b446a2e51da33d 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -5175,20 +5175,9 @@ GenTree* Compiler::impTransformThis(GenTree* thisPtr, GenTree* obj = thisPtr; assert(obj->TypeGet() == TYP_BYREF || obj->TypeGet() == TYP_I_IMPL); - obj = gtNewBlkIndir(typGetObjLayout(pConstrainedResolvedToken->hClass), obj); - - CorInfoType jitTyp = info.compCompHnd->asCorInfoType(pConstrainedResolvedToken->hClass); - if (impIsPrimitive(jitTyp)) - { - if (obj->OperIsBlk()) - { - obj->ChangeOperUnchecked(GT_IND); - obj->AsOp()->gtOp2 = nullptr; // must be zero for tree walkers - } - - obj->gtType = JITtype2varType(jitTyp); - assert(varTypeIsArithmetic(obj->gtType)); - } + ClassLayout* layout; + var_types objType = TypeHandleToVarType(pConstrainedResolvedToken->hClass, &layout); + obj = (objType == TYP_STRUCT) ? gtNewBlkIndir(layout, obj) : gtNewIndir(objType, obj); // This pushes on the dereferenced byref // This is then used immediately to box. @@ -8776,8 +8765,10 @@ GenTree* Compiler::impArrayAccessIntrinsic( return nullptr; } - CORINFO_CLASS_HANDLE arrElemClsHnd = nullptr; - var_types elemType = JITtype2varType(info.compCompHnd->getChildType(clsHnd, &arrElemClsHnd)); + CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE; + CorInfoType elemJitType = info.compCompHnd->getChildType(clsHnd, &elemClsHnd); + ClassLayout* elemLayout = nullptr; + var_types elemType = TypeHandleToVarType(elemJitType, elemClsHnd, &elemLayout); // For the ref case, we will only be able to inline if the types match // (verifier checks for this, we don't care for the nonverified case and the @@ -8822,18 +8813,9 @@ GenTree* Compiler::impArrayAccessIntrinsic( } } - unsigned arrayElemSize; - if (elemType == TYP_STRUCT) - { - assert(arrElemClsHnd); - arrayElemSize = info.compCompHnd->getClassSize(arrElemClsHnd); - } - else - { - arrayElemSize = genTypeSize(elemType); - } + unsigned arrayElemSize = (elemType == TYP_STRUCT) ? elemLayout->GetSize() : genTypeSize(elemType); - if ((unsigned char)arrayElemSize != arrayElemSize) + if (!FitsIn(arrayElemSize)) { // arrayElemSize would be truncated as an unsigned char. // This means the array element is too large. Don't do the optimization. @@ -8848,7 +8830,7 @@ GenTree* Compiler::impArrayAccessIntrinsic( { // Assignment of a struct is more work, and there are more gets than sets. // TODO-CQ: support SET (`a[i,j,k] = s`) for struct element arrays. - if (elemType == TYP_STRUCT) + if (varTypeIsStruct(elemType)) { JITDUMP("impArrayAccessIntrinsic: rejecting SET array intrinsic because elemType is TYP_STRUCT" " (implementation limitation)\n", @@ -8892,12 +8874,13 @@ GenTree* Compiler::impArrayAccessIntrinsic( if (intrinsicName != NI_Array_Address) { - if (varTypeIsStruct(elemType)) + if (elemType == TYP_STRUCT) { - arrElem = gtNewBlkIndir(typGetObjLayout(sig->retTypeClass), arrElem); + arrElem = gtNewBlkIndir(elemLayout, arrElem); } else { + // TODO-Bug: GTF_GLOB_REF is missing. arrElem = gtNewOperNode(GT_IND, elemType, arrElem); } } diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 8a80825d7f8082..9e78a49456991c 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -768,11 +768,6 @@ void MorphCopyBlockHelper::PrepareSrc() { assert(ClassLayout::AreCompatible(m_blockLayout, m_src->GetLayout(m_comp))); } - // TODO-1stClassStructs: produce simple "IND" nodes in importer. - else if (m_src->OperIsBlk()) - { - m_src->SetOper(GT_IND); - } } // TrySpecialCases: check special cases that require special transformations. diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index a2885a16f7875d..58236f1d992c3b 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -2166,15 +2166,7 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, if (copyBlkDst != nullptr) { assert(copyBlkSrc != nullptr); - - // At this point, we have a tree that we are going to store into a destination. - // TODO-1stClassStructs: This should be a simple store or assignment, and should not require - // GTF_ALL_EFFECT for the dest. This is currently emulating the previous behavior of - // block ops. - - GenTree* dest = gtNewLoadValueNode(typGetBlkLayout(simdSize), copyBlkDst); - - dest->gtType = simdType; + GenTree* dest = gtNewLoadValueNode(simdType, copyBlkDst); dest->gtFlags |= GTF_GLOB_REF; GenTree* retNode = gtNewBlkOpNode(dest, copyBlkSrc); From f36e51583ecba6bacdb70e5ffae2b80f9949de90 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 14 Apr 2023 01:14:14 +0300 Subject: [PATCH 5/7] Fix some regressions The IND(COMMA) -> COMMA(IND) transformation decanonicalizes array access as it is only performed on "uses" (i. e. not stores). Simply do not do this for SIMD types. Evidently, this unblocks some if conversions. --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 7762dfde79b38d..f78b23c963a8be 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9863,7 +9863,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA // Only do this optimization when we are in the global optimizer. Doing this after value numbering // could result in an invalid value number for the newly generated GT_IND node. // We skip INDs with GTF_DONT_CSE which is set if the IND is a location. - if (op1->OperIs(GT_COMMA) && fgGlobalMorph && ((tree->gtFlags & GTF_DONT_CSE) == 0)) + if (!varTypeIsSIMD(tree) && op1->OperIs(GT_COMMA) && fgGlobalMorph && ((tree->gtFlags & GTF_DONT_CSE) == 0)) { // Perform the transform IND(COMMA(x, ..., z)) -> COMMA(x, ..., IND(z)). GenTree* commaNode = op1; From f4ebfb4472e7735361c52d1187a30ba44b01b235 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 16 Apr 2023 01:36:15 +0300 Subject: [PATCH 6/7] Fix gtNewZeroCon --- src/coreclr/jit/flowgraph.cpp | 2 +- src/coreclr/jit/gentree.cpp | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 5e9eceb356fcc8..38b5e3bd821338 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -800,7 +800,7 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo } else if (helper == CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_NOCTOR_OPTIMIZED) { - result = gtNewHelperCallNode(helper, type, gtNewIconNode(typeIndex, TYP_UINT)); + result = gtNewHelperCallNode(helper, type, gtNewIconNode(typeIndex)); result->SetExpTLSFieldAccess(); } else diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d2e5ebc311ca52..5c49cdc72dcd95 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7061,6 +7061,7 @@ GenTreeQmark* Compiler::gtNewQmarkNode(var_types type, GenTree* cond, GenTreeCol GenTreeIntCon* Compiler::gtNewIconNode(ssize_t value, var_types type) { + assert(genActualType(type) == type); return new (this, GT_CNS_INT) GenTreeIntCon(type, value); } @@ -7389,7 +7390,8 @@ GenTree* Compiler::gtNewZeroConNode(var_types type) } #endif // FEATURE_SIMD - switch (genActualType(type)) + type = genActualType(type); + switch (type) { case TYP_INT: case TYP_REF: From bf368635dce6477e24cc1286abfc25fb2d030875 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sun, 16 Apr 2023 01:36:23 +0300 Subject: [PATCH 7/7] Fix regressions related to SIMD promotion Preserve the behavior of setting the "is used in a SIMD intrinsic" flag. Will bring additional positive diffs due to the "ldobj" change. --- src/coreclr/jit/gentree.cpp | 39 +++++++++--------------------------- src/coreclr/jit/importer.cpp | 7 +------ src/coreclr/jit/simd.cpp | 1 + 3 files changed, 12 insertions(+), 35 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 5c49cdc72dcd95..8858fbfe7d3b20 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7892,6 +7892,11 @@ GenTreeIndir* Compiler::gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags in indir->gtFlags |= indirFlags; indir->SetIndirExceptionFlags(this); + if ((indirFlags & GTF_IND_VOLATILE) != 0) + { + indir->gtFlags |= GTF_ORDER_SIDEEFF; + } + return indir->AsIndir(); } @@ -7917,9 +7922,8 @@ GenTreeOp* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src) dst->gtFlags |= GTF_DONT_CSE; #if defined(FEATURE_SIMD) && !defined(TARGET_X86) - // TODO-CQ: x86 Windows supports multi-reg returns but not SIMD multi-reg returns - - if (varTypeIsSIMD(dst->gtType)) + // TODO-Cleanup: enable on Windows x86. + if (varTypeIsSIMD(dst)) { // We want to track SIMD assignments as being intrinsics since they // are functionally SIMD `mov` instructions and are more efficient @@ -8227,21 +8231,7 @@ GenTree* Compiler::gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, bool isVo { // TODO-Cleanup: similar logic already exists in "gtNewAssignNode", // however, it is not enabled for x86. Fix that and delete this code. - GenTreeLclVarCommon* dstLclNode = nullptr; - - if (dst->OperIs(GT_LCL_VAR)) - { - dstLclNode = dst->AsLclVar(); - } - else if (dst->OperIsBlk() && dst->AsIndir()->Addr()->IsLclVarAddr()) - { - dstLclNode = dst->AsIndir()->Addr()->AsLclFld(); - } - - if ((dstLclNode != nullptr) && varTypeIsStruct(lvaGetDesc(dstLclNode))) - { - setLclRelatedToSIMDIntrinsic(dstLclNode); - } + SetOpLclRelatedToSIMDIntrinsic(dst); } #endif // FEATURE_SIMD @@ -18873,23 +18863,14 @@ FieldSeq::FieldSeq(CORINFO_FIELD_HANDLE fieldHnd, ssize_t offset, FieldKind fiel // as used by a SIMD intrinsic, and if so, set that local var appropriately. // // Arguments: -// op - The tree, to be an operand of a new GT_SIMD node, to check. +// op - The tree, to be an operand of a new SIMD-related node, to check. // void Compiler::SetOpLclRelatedToSIMDIntrinsic(GenTree* op) { - if (op == nullptr) - { - return; - } - - if (op->OperIs(GT_LCL_VAR)) + if ((op != nullptr) && op->OperIs(GT_LCL_VAR)) { setLclRelatedToSIMDIntrinsic(op); } - else if (op->OperIsBlk() && op->AsIndir()->Addr()->IsLclVarAddr()) - { - setLclRelatedToSIMDIntrinsic(op->AsIndir()->Addr()); - } } void GenTreeMultiOp::ResetOperandArray(size_t newOperandCount, diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 155be580ff37f9..7f8d4c2c57a6df 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -10798,15 +10798,10 @@ void Compiler::impImportBlockCode(BasicBlock* block) lclTyp = TypeHandleToVarType(resolvedToken.hClass, &layout); tiRetVal = verMakeTypeInfo(resolvedToken.hClass); - if (lclTyp != TYP_STRUCT) - { - goto LDIND; - } - op1 = impPopStack().val; assertImp((genActualType(op1) == TYP_I_IMPL) || op1->TypeIs(TYP_BYREF)); - op1 = gtNewBlkIndir(layout, op1, impPrefixFlagsToIndirFlags(prefixFlags)); + op1 = gtNewLoadValueNode(lclTyp, layout, op1, impPrefixFlagsToIndirFlags(prefixFlags)); impPushOnStack(op1, tiRetVal); break; } diff --git a/src/coreclr/jit/simd.cpp b/src/coreclr/jit/simd.cpp index 3b739b1f38853b..c715ad1e058e51 100644 --- a/src/coreclr/jit/simd.cpp +++ b/src/coreclr/jit/simd.cpp @@ -468,6 +468,7 @@ GenTree* Compiler::impSIMDPopStack(var_types type, bool expectAddr, CORINFO_CLAS tree = gtNewOperNode(GT_IND, type, tree); } + // TODO-ADDR: delete this. if (tree->OperIsIndir() && tree->AsIndir()->Addr()->IsLclVarAddr()) { GenTreeLclFld* lclAddr = tree->AsIndir()->Addr()->AsLclFld();