From af06641030247ab244034b6a5c51bc33bd1f74a1 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 3 Oct 2022 19:34:01 +0300 Subject: [PATCH 1/4] Fold simple SIMD indirs in local morph --- src/coreclr/jit/lclmorph.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 56dfa69e0f5d1..d17172e8f5f32 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -991,13 +991,6 @@ class LocalAddressVisitor final : public GenTreeVisitor LclVarDsc* varDsc = m_compiler->lvaGetDesc(val.LclNum()); - if (varTypeIsSIMD(varDsc)) - { - // TODO-ADDR: skip SIMD variables for now, fgMorphFieldAssignToSimdSetElement and - // others need to be updated to recognize LCL_FLDs. - return IndirTransform::None; - } - if (indir->TypeGet() != TYP_STRUCT) { if (varDsc->lvPromoted) @@ -1012,6 +1005,14 @@ class LocalAddressVisitor final : public GenTreeVisitor return IndirTransform::LclVar; } + if (varTypeIsSIMD(varDsc)) + { + // TODO-ADDR: skip SIMD variables for now, fgMorphFieldAssignToSimdSetElement and + // fgMorphFieldToSimdGetElement need to be updated to recognize LCL_FLDs or moved + // here. + return IndirTransform::None; + } + // Bool and ubyte are the same type. if ((indir->TypeIs(TYP_BOOL) && (varDsc->TypeGet() == TYP_UBYTE)) || (indir->TypeIs(TYP_UBYTE) && (varDsc->TypeGet() == TYP_BOOL))) From e492874f5f04315e1b8d016731281dcbedb7ff3e Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 3 Oct 2022 19:34:08 +0300 Subject: [PATCH 2/4] Fix various downstream bugs We should really delete the SIMD "InitObj" IR form, but that change had some unfortunate regressions to it, so, for now, just make it work reliably. --- src/coreclr/jit/compiler.h | 12 ------ src/coreclr/jit/rationalize.cpp | 66 +++++++-------------------------- src/coreclr/jit/valuenum.cpp | 19 +++++++++- 3 files changed, 30 insertions(+), 67 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 6cfcf3a506229..61df4ed727925 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8473,18 +8473,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX return (intrinsicId == SIMDIntrinsicEqual); } - // Returns base JIT type of a TYP_SIMD local. - // Returns CORINFO_TYPE_UNDEF if the local is not TYP_SIMD. - CorInfoType getBaseJitTypeOfSIMDLocal(GenTree* tree) - { - if (isSIMDTypeLocal(tree)) - { - return lvaGetDesc(tree->AsLclVarCommon())->GetSimdBaseJitType(); - } - - return CORINFO_TYPE_UNDEF; - } - bool isNumericsNamespace(const char* ns) { return strcmp(ns, "System.Numerics") == 0; diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 7e2b2376391f8..af1374f7b612c 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -56,27 +56,14 @@ void Rationalizer::RewriteIndir(LIR::Use& use) // Clear the `GTF_IND_ASG_LHS` flag, which overlaps with `GTF_IND_REQ_ADDR_IN_REG`. indir->gtFlags &= ~GTF_IND_ASG_LHS; - if (indir->OperIs(GT_IND)) + if (varTypeIsSIMD(indir)) { - if (varTypeIsSIMD(indir)) - { - RewriteSIMDIndir(use); - } - } - else if (indir->OperIs(GT_OBJ)) - { - assert((indir->TypeGet() == TYP_STRUCT) || !use.User()->OperIsInitBlkOp()); - if (varTypeIsSIMD(indir->TypeGet())) + if (indir->OperIs(GT_BLK, GT_OBJ)) { indir->SetOper(GT_IND); - RewriteSIMDIndir(use); } - } - else - { - assert(indir->OperIs(GT_BLK)); - // We should only see GT_BLK for TYP_STRUCT or for InitBlocks. - assert((indir->TypeGet() == TYP_STRUCT) || use.User()->OperIsInitBlkOp()); + + RewriteSIMDIndir(use); } } @@ -413,45 +400,18 @@ void Rationalizer::RewriteAssignment(LIR::Use& use) genTreeOps locationOp = location->OperGet(); - if (assignment->OperIsBlkOp()) + if (varTypeIsSIMD(location) && assignment->OperIsInitBlkOp()) { -#ifdef FEATURE_SIMD - if (varTypeIsSIMD(location) && assignment->OperIsInitBlkOp()) - { - if (location->OperIs(GT_LCL_VAR)) - { - var_types simdType = location->TypeGet(); - GenTree* initVal = assignment->AsOp()->gtOp2; - - CorInfoType simdBaseJitType = comp->getBaseJitTypeOfSIMDLocal(location); - if (simdBaseJitType == CORINFO_TYPE_UNDEF) - { - // Lie about the type if we don't know/have it. - simdBaseJitType = CORINFO_TYPE_FLOAT; - } - - if (initVal->IsIntegralConst(0)) - { - GenTree* zeroCon = comp->gtNewZeroConNode(simdType); - - assignment->gtOp2 = zeroCon; - value = zeroCon; + var_types simdType = location->TypeGet(); + GenTree* initVal = assignment->AsOp()->gtOp2; + GenTree* zeroCon = comp->gtNewZeroConNode(simdType); + noway_assert(initVal->IsIntegralConst(0)); // All SIMD InitBlks are zero inits. - BlockRange().InsertAfter(initVal, zeroCon); - BlockRange().Remove(initVal); - } - else - { - GenTreeSIMD* simdTree = comp->gtNewSIMDNode(simdType, initVal, SIMDIntrinsicInit, simdBaseJitType, - genTypeSize(simdType)); - assignment->gtOp2 = simdTree; - value = simdTree; + assignment->gtOp2 = zeroCon; + value = zeroCon; - BlockRange().InsertAfter(initVal, simdTree); - } - } - } -#endif // FEATURE_SIMD + BlockRange().InsertAfter(initVal, zeroCon); + BlockRange().Remove(initVal); } switch (locationOp) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 435aae46c8111..b42020fc10bdb 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8250,13 +8250,28 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) // Is the type being stored different from the type computed by the rhs? if (rhs->TypeGet() != lhs->TypeGet()) { - if (rhs->TypeGet() == TYP_REF) + if (tree->OperIsInitBlkOp()) + { + ValueNum initObjVN; + if (rhs->IsIntegralConst(0)) + { + initObjVN = lhs->TypeIs(TYP_STRUCT) ? vnStore->VNForZeroObj(lhs->GetLayout(this)) + : vnStore->VNZeroForType(lhs->TypeGet()); + } + else + { + initObjVN = vnStore->VNForExpr(compCurBB, lhs->TypeGet()); + } + + rhsVNPair.SetBoth(initObjVN); + } + else if (rhs->TypeGet() == TYP_REF) { // If we have an unsafe IL assignment of a TYP_REF to a non-ref (typically a TYP_BYREF) // then don't propagate this ValueNumber to the lhs, instead create a new unique VN. rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lhs->TypeGet())); } - else if (lhs->OperGet() != GT_BLK) + else { // This means that there is an implicit cast on the rhs value // We will add a cast function to reflect the possible narrowing of the rhs value From f288eeb4b48f60e7fbeaba61cd9dbd23f2ab39d9 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 18:21:25 +0300 Subject: [PATCH 3/4] Delete code --- src/coreclr/jit/assertionprop.cpp | 30 ------- src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/forwardsub.cpp | 19 ----- src/coreclr/jit/morph.cpp | 131 ++++-------------------------- src/coreclr/jit/rationalize.cpp | 4 +- 5 files changed, 16 insertions(+), 169 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 1126b930ba616..b49af5dbd7411 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1657,38 +1657,8 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, goto DONE_ASSERTION; } - // - // Copy Assertions - // - case GT_OBJ: - case GT_BLK: - { - // TODO-ADDR: delete once local morph folds SIMD-typed indirections. - // - GenTree* const addr = op2->AsIndir()->Addr(); - - if (addr->OperIs(GT_ADDR)) - { - GenTree* const base = addr->AsOp()->gtOp1; - - if (base->OperIs(GT_LCL_VAR) && varTypeIsStruct(base)) - { - ClassLayout* const varLayout = base->GetLayout(this); - ClassLayout* const objLayout = op2->GetLayout(this); - if (ClassLayout::AreCompatible(varLayout, objLayout)) - { - op2 = base; - goto IS_COPY; - } - } - } - - goto DONE_ASSERTION; - } - case GT_LCL_VAR: { - IS_COPY: // // Must either be an OAK_EQUAL or an OAK_NOT_EQUAL assertion // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 61df4ed727925..6bb3cc683ed15 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5756,7 +5756,6 @@ class Compiler GenTree* fgMorphMultiOp(GenTreeMultiOp* multiOp); GenTree* fgMorphConst(GenTree* tree); - GenTreeLclVar* fgMorphTryFoldObjAsLclVar(GenTreeObj* obj, bool destroyNodes = true); GenTreeOp* fgMorphCommutative(GenTreeOp* tree); GenTree* fgMorphCastedBitwiseOp(GenTreeOp* tree); diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index a2f9cc32cff30..e0ef7d77ed849 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -630,25 +630,6 @@ bool Compiler::fgForwardSubStatement(Statement* stmt) } } - // Optimization: - // - // If we are about to substitute GT_OBJ, see if we can simplify it first. - // Not doing so can lead to regressions... - // - // Hold off on doing this for call args for now (per issue #51569). - // Hold off on OBJ(GT_LCL_ADDR). - // - if (fwdSubNode->OperIs(GT_OBJ) && !fsv.IsCallArg() && fwdSubNode->gtGetOp1()->OperIs(GT_ADDR)) - { - const bool destroyNodes = false; - GenTree* const optTree = fgMorphTryFoldObjAsLclVar(fwdSubNode->AsObj(), destroyNodes); - if (optTree != nullptr) - { - JITDUMP(" [folding OBJ(ADDR(LCL...))]"); - fwdSubNode = optTree; - } - } - // Quirks: // // Don't substitute nodes "AddFinalArgsAndDetermineABIInfo" doesn't handle into struct args. diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 9bbd6fe7f9e3d..1402a34bd7152 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -1134,9 +1134,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) unsigned structSize = argObj->Size(); unsigned lastLoadSize = structSize % TARGET_POINTER_SIZE; - // TODO-ADDR: delete the "IsLocalAddrExpr" check once local morph transforms all such OBJs into - // local nodes. - if ((lastLoadSize != 0) && !isPow2(lastLoadSize) && (argObj->Addr()->IsLocalAddrExpr() == nullptr)) + if ((lastLoadSize != 0) && !isPow2(lastLoadSize)) { #ifdef TARGET_ARM // On ARM we don't expand split args larger than 16 bytes into field lists. @@ -3731,52 +3729,8 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) } #if FEATURE_MULTIREG_ARGS - // Examine 'arg' and setup argValue objClass and structSize - // - GenTree* argValue = argNode; // normally argValue will be arg, but see right below - ClassLayout* layout = nullptr; - unsigned structSize = 0; - - if (argNode->OperGet() == GT_OBJ) - { - GenTreeObj* argObj = argNode->AsObj(); - layout = argObj->GetLayout(); - structSize = layout->GetSize(); - - // If we have a GT_OBJ of a GT_ADDR then we set argValue to the child node of the GT_ADDR. - // TODO-ADDR: always perform this transformation in local morph and delete this code. - GenTree* addr = argObj->Addr(); - if (addr->OperGet() == GT_ADDR) - { - GenTree* location = addr->AsOp()->gtOp1; - - if (location->OperIsLocalRead()) - { - if (!location->OperIs(GT_LCL_VAR) || (location->TypeGet() != argObj->TypeGet()) || - !ClassLayout::AreCompatible(lvaGetDesc(location->AsLclVarCommon())->GetLayout(), layout)) - { - unsigned lclOffset = location->AsLclVarCommon()->GetLclOffs(); - - location->ChangeType(argObj->TypeGet()); - location->SetOper(GT_LCL_FLD); - location->AsLclFld()->SetLclOffs(lclOffset); - location->AsLclFld()->SetLayout(layout); - } - - argValue = location; - } - } - } - else if (argNode->TypeIs(TYP_STRUCT)) - { - assert(argNode->OperIsLocalRead()); - layout = argNode->AsLclVarCommon()->GetLayout(this); - structSize = layout->GetSize(); - } - else - { - structSize = genTypeSize(argNode); - } + ClassLayout* layout = argNode->TypeIs(TYP_STRUCT) ? argNode->GetLayout(this) : nullptr; + unsigned structSize = argNode->TypeIs(TYP_STRUCT) ? layout->GetSize() : genTypeSize(argNode); struct ArgElem { @@ -3795,7 +3749,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) } else { - assert(varTypeIsSIMD(argValue) && varTypeIsSIMD(arg->GetSignatureType())); + assert(varTypeIsSIMD(argNode) && varTypeIsSIMD(arg->GetSignatureType())); } if (arg->AbiInfo.IsHfaArg() && arg->AbiInfo.IsPassedInFloatRegisters()) @@ -3849,9 +3803,9 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) // However, if it comes from an unknown (arbitrary) address, we must fix up the // last element's type. // - if (!argValue->OperIs(GT_LCL_VAR, GT_LCL_FLD)) + if (!argNode->OperIs(GT_LCL_VAR, GT_LCL_FLD)) { - assert(argValue->OperIs(GT_OBJ)); + assert(argNode->OperIs(GT_OBJ)); unsigned lastElemExactSize = structSize - lastElem.Offset; @@ -3896,7 +3850,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) // by integer and float registers and it needs to include the padding here. assert(roundUp(structSize, TARGET_POINTER_SIZE) == roundUp(loadExtent, TARGET_POINTER_SIZE)); #else - if (argValue->IsLocal()) + if (argNode->IsLocal()) { assert((structSize <= loadExtent) && (loadExtent <= roundUp(structSize, TARGET_POINTER_SIZE))); } @@ -3908,15 +3862,15 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) #endif // DEBUG // We should still have a TYP_STRUCT - assert(varTypeIsStruct(argValue)); + assert(varTypeIsStruct(argNode)); GenTreeFieldList* newArg = nullptr; // Are we passing a struct LclVar? // - if (argValue->OperIs(GT_LCL_VAR)) + if (argNode->OperIs(GT_LCL_VAR)) { - GenTreeLclVarCommon* lclNode = argValue->AsLclVarCommon(); + GenTreeLclVarCommon* lclNode = argNode->AsLclVarCommon(); unsigned lclNum = lclNode->GetLclNum(); LclVarDsc* varDsc = lvaGetDesc(lclNum); @@ -3972,9 +3926,9 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) return argNode; } - if (argValue->OperIs(GT_LCL_VAR, GT_LCL_FLD)) + if (argNode->OperIs(GT_LCL_VAR, GT_LCL_FLD)) { - GenTreeLclVarCommon* lclNode = argValue->AsLclVarCommon(); + GenTreeLclVarCommon* lclNode = argNode->AsLclVarCommon(); unsigned lclNum = lclNode->GetLclNum(); LclVarDsc* varDsc = lvaGetDesc(lclNum); unsigned lclOffset = lclNode->GetLclOffs(); @@ -3992,9 +3946,9 @@ GenTree* Compiler::fgMorphMultiregStructArg(CallArg* arg) } else { - assert(argValue->OperIsIndir()); + assert(argNode->OperIsIndir()); - GenTree* baseAddr = argValue->AsIndir()->Addr(); + GenTree* baseAddr = argNode->AsIndir()->Addr(); var_types addrType = baseAddr->TypeGet(); // TODO-ADDR: make sure all such OBJs are transformed into TYP_STRUCT LCL_FLDs and delete this condition. @@ -8653,63 +8607,6 @@ GenTree* Compiler::fgMorphConst(GenTree* tree) return fgMorphTree(tree); } -//------------------------------------------------------------------------ -// fgMorphTryFoldObjAsLclVar: try to fold an Obj node as a LclVar. -// -// Arguments: -// obj - the obj node. -// destroyNodes -- destroy nodes that are optimized away -// -// Return value: -// GenTreeLclVar if the obj can be replaced by it, null otherwise. -// -GenTreeLclVar* Compiler::fgMorphTryFoldObjAsLclVar(GenTreeObj* obj, bool destroyNodes) -{ - if (opts.OptimizationEnabled()) - { - GenTree* op1 = obj->Addr(); - assert(!op1->OperIs(GT_LCL_VAR_ADDR) && "missed an opt opportunity"); - if (op1->OperIs(GT_ADDR)) - { - GenTreeUnOp* addr = op1->AsUnOp(); - GenTree* addrOp = addr->gtGetOp1(); - if (addrOp->TypeIs(obj->TypeGet()) && addrOp->OperIs(GT_LCL_VAR)) - { - GenTreeLclVar* lclVar = addrOp->AsLclVar(); - - ClassLayout* lclVarLayout = lvaGetDesc(lclVar)->GetLayout(); - ClassLayout* objLayout = obj->GetLayout(); - if (ClassLayout::AreCompatible(lclVarLayout, objLayout)) - { -#ifdef DEBUG - CORINFO_CLASS_HANDLE objClsHandle = obj->GetLayout()->GetClassHandle(); - assert(objClsHandle != NO_CLASS_HANDLE); - if (verbose) - { - CORINFO_CLASS_HANDLE lclClsHnd = gtGetStructHandle(lclVar); - printf("fold OBJ(ADDR(X)) [%06u] into X [%06u], ", dspTreeID(obj), dspTreeID(lclVar)); - printf("with %s handles\n", ((lclClsHnd == objClsHandle) ? "matching" : "different")); - } -#endif - // Keep the DONT_CSE flag in sync - // (as the addr always marks it for its op1) - lclVar->gtFlags &= ~GTF_DONT_CSE; - lclVar->gtFlags |= (obj->gtFlags & GTF_DONT_CSE); - - if (destroyNodes) - { - DEBUG_DESTROY_NODE(obj); - DEBUG_DESTROY_NODE(addr); - } - - return lclVar; - } - } - } - } - return nullptr; -} - //------------------------------------------------------------------------ // fgMorphLeaf: Fully morph a tree with no operands. // diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index af1374f7b612c..dad87564355ac 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -72,8 +72,8 @@ void Rationalizer::RewriteIndir(LIR::Use& use) // Arguments: // use - A use of a GT_IND node of SIMD type // -// TODO-1stClassStructs: These should be eliminated earlier, once we can handle -// lclVars in all the places that used to have GT_OBJ. +// TODO-ADDR: delete this once block morphing stops taking addresses of locals +// under COMMAs. // void Rationalizer::RewriteSIMDIndir(LIR::Use& use) { From 1394fc02fe8822ae4baf98756b60d3b483d540bb Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 25 Oct 2022 20:19:43 +0300 Subject: [PATCH 4/4] Fix args morphing Consider a promoted two-field HFA stack argument on ARM64. This argument needs to be processed by multi-reg morphing, so that it is transformed into a FIELD_LIST, otherwise it would end up independently promoted yet appear in the IR as a LCL_VAR. That was not happening because the number of stack slots consumed by such an argument is 1 (8 bytes), and the number of registers is zero (naturally). As the fix, use the more obviously correct check based on "structBaseType" to detect when we need to invoke multi-reg morphing. No diffs. --- src/coreclr/jit/gentree.h | 2 - src/coreclr/jit/morph.cpp | 94 +++++++++++++-------------------------- 2 files changed, 30 insertions(+), 66 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 70abd0a98ebb1..5a019bfcca96e 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4381,8 +4381,6 @@ struct CallArgABIInformation // argument type, but when a struct is passed as a scalar type, this is // that type. Note that if a struct is passed by reference, this will still // be the struct type. - // TODO-ARGS: Reconsider whether we need this, it does not make much sense - // to have this instead of using just the type of the arg node. var_types ArgType : 5; // True when the argument fills a register slot skipped due to alignment // requirements of previous arguments. diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1402a34bd7152..1eed2110dc660 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -3138,8 +3138,8 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // information about late arguments in CallArgs. // This information is used later to construct the late args - // Note that this name is a bit of a misnomer - it indicates that there are struct args - // that occupy more than a single slot that are passed by value (not necessarily in regs). + // Note that this name a misnomer - it indicates that there are struct args + // that are passed by value in more than one register or on stack. bool hasMultiregStructArgs = false; for (CallArg& arg : call->gtArgs.Args()) { @@ -3183,12 +3183,6 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) argx->gtType = TYP_I_IMPL; } - // Get information about this argument. - var_types hfaType = arg.AbiInfo.GetHfaType(); - bool isHfaArg = (hfaType != TYP_UNDEF); - bool passUsingFloatRegs = arg.AbiInfo.IsPassedInFloatRegisters(); - unsigned structSize = 0; - // Struct arguments may be morphed into a node that is not a struct type. // In such case the CallArgABIInformation keeps track of whether the original node (before morphing) // was a struct and the struct classification. @@ -3211,9 +3205,6 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) assert(originalSize == info.compCompHnd->getClassSize(arg.GetSignatureClassHandle())); - unsigned roundupSize = (unsigned)roundUp(originalSize, TARGET_POINTER_SIZE); - var_types structBaseType = arg.AbiInfo.ArgType; - // First, handle the case where the argument is passed by reference. if (arg.AbiInfo.PassedByRef) { @@ -3225,7 +3216,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) } else // This is passed by value. { - structSize = originalSize; + unsigned structSize = originalSize; unsigned passingSize = originalSize; // Check to see if we can transform this struct load (GT_OBJ) into a GT_IND of the appropriate size. @@ -3233,10 +3224,10 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // - In general, it can be done for power of 2 structs that fit in a single register. // - For ARM and ARM64 it must also be a non-HFA struct, or have a single field. // - This is irrelevant for X86, since structs are always passed by value on the stack. - - GenTree* lclVar = argObj->OperIs(GT_LCL_VAR) ? argObj : fgIsIndirOfAddrOfLocal(argObj); - bool argIsLocal = (lclVar != nullptr) || argObj->OperIs(GT_LCL_FLD); - bool canTransform = false; + // + var_types structBaseType = arg.AbiInfo.ArgType; + bool argIsLocal = argObj->OperIsLocalRead(); + bool canTransform = false; if (structBaseType != TYP_STRUCT) { @@ -3253,6 +3244,12 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) passingSize = genTypeSize(structBaseType); } } +#if !defined(TARGET_X86) + else + { + hasMultiregStructArgs = true; + } +#endif // !TARGET_X86 if (!canTransform) { @@ -3441,13 +3438,12 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // Here we don't need unsafe value cls check since the addr of temp is used only in mkrefany unsigned tmp = lvaGrabTemp(true DEBUGARG("by-value mkrefany struct argument")); lvaSetStruct(tmp, impGetRefAnyClass(), false); + lvaSetVarAddrExposed(tmp DEBUGARG(AddressExposedReason::TOO_CONSERVATIVE)); // Build the mkrefany as a comma node: // (tmp.ptr=argx),(tmp.type=handle) GenTreeLclFld* destPtrSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__dataPtr); GenTreeLclFld* destTypeSlot = gtNewLclFldNode(tmp, TYP_I_IMPL, OFFSETOF__CORINFO_TypedReference__type); - destPtrSlot->gtFlags |= GTF_VAR_DEF; - destTypeSlot->gtFlags |= GTF_VAR_DEF; GenTree* asgPtrSlot = gtNewAssignNode(destPtrSlot, argx->AsOp()->gtOp1); GenTree* asgTypeSlot = gtNewAssignNode(destTypeSlot, argx->AsOp()->gtOp2); @@ -3455,32 +3451,26 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) // Change the expression to "(tmp=val)" arg.SetEarlyNode(asg); - call->gtArgs.SetTemp(&arg, tmp); - lvaSetVarAddrExposed(tmp DEBUGARG(AddressExposedReason::TOO_CONSERVATIVE)); + hasMultiregStructArgs |= ((arg.AbiInfo.ArgType == TYP_STRUCT) && !arg.AbiInfo.PassedByRef); #endif // !TARGET_X86 } #if FEATURE_MULTIREG_ARGS - if (isStructArg) + if (!isStructArg) { - if (((arg.AbiInfo.NumRegs + arg.AbiInfo.GetStackSlotsNumber()) > 1) || - (isHfaArg && argx->TypeGet() == TYP_STRUCT)) +#ifdef TARGET_ARM + if ((arg.AbiInfo.ArgType == TYP_LONG) || (arg.AbiInfo.ArgType == TYP_DOUBLE)) { - hasMultiregStructArgs = true; + assert((arg.AbiInfo.NumRegs == 2) || (arg.AbiInfo.GetStackSlotsNumber() == 2)); } - } -#ifdef TARGET_ARM - else if ((arg.AbiInfo.ArgType == TYP_LONG) || (arg.AbiInfo.ArgType == TYP_DOUBLE)) - { - assert((arg.AbiInfo.NumRegs == 2) || (arg.AbiInfo.GetStackSlotsNumber() == 2)); - } + else #endif - else - { - // We must have exactly one register or slot. - assert(((arg.AbiInfo.NumRegs == 1) && (arg.AbiInfo.GetStackSlotsNumber() == 0)) || - ((arg.AbiInfo.NumRegs == 0) && (arg.AbiInfo.GetStackSlotsNumber() == 1))); + { + // We must have exactly one register or slot. + assert(((arg.AbiInfo.NumRegs == 1) && (arg.AbiInfo.GetStackSlotsNumber() == 0)) || + ((arg.AbiInfo.NumRegs == 0) && (arg.AbiInfo.GetStackSlotsNumber() == 1))); + } } #endif @@ -3611,38 +3601,14 @@ void Compiler::fgMorphMultiregStructArgs(GenTreeCall* call) for (CallArg& arg : call->gtArgs.Args()) { - GenTree*& argx = arg.GetLateNode() != nullptr ? arg.LateNodeRef() : arg.EarlyNodeRef(); - - if (!arg.AbiInfo.IsStruct) + if ((arg.AbiInfo.ArgType == TYP_STRUCT) && !arg.AbiInfo.PassedByRef) { - continue; - } + GenTree*& argx = (arg.GetLateNode() != nullptr) ? arg.LateNodeRef() : arg.EarlyNodeRef(); - unsigned size = (arg.AbiInfo.NumRegs + arg.AbiInfo.GetStackSlotsNumber()); - if ((size > 1) || (arg.AbiInfo.IsHfaArg() && argx->TypeGet() == TYP_STRUCT)) - { - foundStructArg = true; - if (varTypeIsStruct(argx) && !argx->OperIs(GT_FIELD_LIST)) + if (!argx->OperIs(GT_FIELD_LIST)) { - if (arg.AbiInfo.IsHfaRegArg()) - { - var_types hfaType = arg.AbiInfo.GetHfaType(); - unsigned structSize = - argx->TypeIs(TYP_STRUCT) ? argx->GetLayout(this)->GetSize() : genTypeSize(argx); - - if (structSize == genTypeSize(hfaType)) - { - if (argx->OperIs(GT_OBJ)) - { - argx->SetOper(GT_IND); - } - - argx->gtType = hfaType; - } - } - - GenTree* newArgx = fgMorphMultiregStructArg(&arg); - argx = newArgx; + argx = fgMorphMultiregStructArg(&arg); + foundStructArg = true; } } }