From 100f3ea6aed26316d4cd3e1b90bc7e8dffe1cf56 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 26 Jun 2018 09:14:05 -0700 Subject: [PATCH 1/7] JIT: improve handling of small struct returns The jit was retyping all calls with small struct returns as power of two sized ints when generating code for linux x64 and arm32/arm64. When the results of those calls were assigned to fields the jit would use overly wide stores which could corrupt neighboring fields. The fix is to keep better track of the smallest power of two enclosing int type size for the struct. If this exactly matches the struct size then the the result of the call can be used in an arbitrary context. If the enclosing type is larger, the call's result must be copied to a temp and then the temp can be reinterpreted as the struct for subsequent uses. --- src/jit/compiler.cpp | 20 ++++++++++---------- src/jit/importer.cpp | 6 ++++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 868e0ce6f5c8..8417009a050b 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -1004,28 +1004,30 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, assert(structSize > 0); #ifdef UNIX_AMD64_ABI - // An 8-byte struct may need to be returned in a floating point register // So we always consult the struct "Classifier" routine // SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc; eeGetSystemVAmd64PassStructInRegisterDescriptor(clsHnd, &structDesc); - // If we have one eightByteCount then we can set 'useType' based on that if (structDesc.eightByteCount == 1) { - // Set 'useType' to the type of the first eightbyte item - useType = GetEightByteType(structDesc, 0); - assert(structDesc.passedInRegisters == true); + if (structDesc.eightByteClassifications[0] == SystemVClassificationTypeSSE) + { + // If this is returned as a floating type, use that. + // Otherwise, we'll use the general case - we don't want to use the "EightByteType" + // directly, because it returns `TYP_INT` for any integral type <= 4 bytes, and + // we need to preserve small types. + useType = GetEightByteType(structDesc, 0); + } } -#else // not UNIX_AMD64 +#endif // UNIX_AMD64_ABI // The largest primitive type is 8 bytes (TYP_DOUBLE) // so we can skip calling getPrimitiveTypeForStruct when we // have a struct that is larger than that. - // - if (structSize <= sizeof(double)) + if ((useType == TYP_UNKNOWN) && (structSize <= sizeof(double))) { // We set the "primitive" useType based upon the structSize // and also examine the clsHnd to see if it is an HFA of count one @@ -1034,8 +1036,6 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, useType = getPrimitiveTypeForStruct(structSize, clsHnd, /*isVararg=*/false); } -#endif // UNIX_AMD64_ABI - #ifdef _TARGET_64BIT_ // Note this handles an odd case when FEATURE_MULTIREG_RET is disabled and HFAs are enabled // diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 1b2e35771810..29dd3d975d9d 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -8555,6 +8555,9 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE re assert(varTypeIsStruct(info.compRetType)); assert(info.compRetBuffArg == BAD_VAR_NUM); + JITDUMP("\nimpFixupStructReturnType: retyping\n"); + DISPTREE(op); + #if defined(_TARGET_XARCH_) #ifdef UNIX_AMD64_ABI @@ -8750,6 +8753,9 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE re op->gtType = info.compRetNativeType; + JITDUMP("\nimpFixupStructReturnType: result of retyping is\n"); + DISPTREE(op); + return op; } From bdc5aacda5564aacd9b0a554e2576ea63e0dcb76 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 20 Jun 2018 20:29:38 -0700 Subject: [PATCH 2/7] Extend fix to handle struct that are not exact integer type sizes. --- src/jit/compiler.cpp | 29 +++++++++++++++++++++++------ src/jit/compiler.h | 2 ++ src/jit/gentree.cpp | 4 ++++ src/jit/gentree.h | 9 +++++++++ src/jit/importer.cpp | 9 ++++++++- src/jit/lclvars.cpp | 3 ++- 6 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 8417009a050b..7998b69d240b 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -1018,7 +1018,8 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, // Otherwise, we'll use the general case - we don't want to use the "EightByteType" // directly, because it returns `TYP_INT` for any integral type <= 4 bytes, and // we need to preserve small types. - useType = GetEightByteType(structDesc, 0); + useType = GetEightByteType(structDesc, 0); + howToReturnStruct = SPK_PrimitiveType; } } @@ -1032,8 +1033,24 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, // We set the "primitive" useType based upon the structSize // and also examine the clsHnd to see if it is an HFA of count one // - // The ABI for struct returns in varArg methods, is same as the normal case, so pass false for isVararg + // The ABI for struct returns in varArg methods, is same as the normal case, + // so pass false for isVararg useType = getPrimitiveTypeForStruct(structSize, clsHnd, /*isVararg=*/false); + + if (useType != TYP_UNKNOWN) + { + if (structSize == genTypeSize(useType)) + { + // Currently: 1, 2, 4, or 8 byte structs + howToReturnStruct = SPK_PrimitiveType; + } + else + { + // Currently: 3, 5, 6, or 7 byte structs + assert(structSize <= genTypeSize(useType)); + howToReturnStruct = SPK_EnclosingType; + } + } } #ifdef _TARGET_64BIT_ @@ -1048,16 +1065,16 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, // if ((FEATURE_MULTIREG_RET == 0) && (useType == TYP_UNKNOWN) && (structSize == (2 * sizeof(float))) && IsHfa(clsHnd)) { - useType = TYP_I_IMPL; + useType = TYP_I_IMPL; + howToReturnStruct = SPK_PrimitiveType; } #endif // Did we change this struct type into a simple "primitive" type? - // if (useType != TYP_UNKNOWN) { - // Yes, we should use the "primitive" type in 'useType' - howToReturnStruct = SPK_PrimitiveType; + // If so, we should have already set howToReturnStruct, too. + assert(howToReturnStruct != SPK_Unknown); } else // We can't replace the struct with a "primitive" type { diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 2e8d27d71a0d..63b0d9fb65b6 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -4223,6 +4223,8 @@ class Compiler { SPK_Unknown, // Invalid value, never returned SPK_PrimitiveType, // The struct is passed/returned using a primitive type. + SPK_EnclosingType, // Like SPK_Primitive type, but used for return types that + // are not a power of two byte size. SPK_ByValue, // The struct is passed/returned by value (using the ABI rules) // for ARM64 and UNIX_X64 in multiple registers. (when all of the // parameters registers are used, then the stack will be used) diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index e1bc249cdfca..bba58cbecf8f 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -17679,6 +17679,10 @@ void ReturnTypeDesc::InitializeStructReturnType(Compiler* comp, CORINFO_CLASS_HA switch (howToReturnStruct) { + case Compiler::SPK_EnclosingType: + m_isEnclosingType = true; + __fallthrough; + case Compiler::SPK_PrimitiveType: { assert(returnType != TYP_UNKNOWN); diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 4baed55b6b5c..e59b79d9c8c5 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -3089,6 +3089,7 @@ struct ReturnTypeDesc { private: var_types m_regType[MAX_RET_REG_COUNT]; + bool m_isEnclosingType; #ifdef DEBUG bool m_inited; @@ -3114,6 +3115,7 @@ struct ReturnTypeDesc { m_regType[i] = TYP_UNKNOWN; } + m_isEnclosingType = false; #ifdef DEBUG m_inited = false; #endif @@ -3206,6 +3208,13 @@ struct ReturnTypeDesc return result; } + // True if this value is returned in integer register + // that is larger than the type itself. + bool IsEnclosingType() const + { + return m_isEnclosingType; + } + // Get ith ABI return register regNumber GetABIReturnReg(unsigned idx); diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 29dd3d975d9d..895181a5c139 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -8465,7 +8465,14 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN if (retRegCount == 1) { // struct returned in a single register - call->gtReturnType = retTypeDesc->GetReturnRegType(0); + // retype iff struct size exactly matches integer type size. + if (retTypeDesc->IsEnclosingType()) + { + } + else + { + call->gtReturnType = retTypeDesc->GetReturnRegType(0); + } } else { diff --git a/src/jit/lclvars.cpp b/src/jit/lclvars.cpp index b36fcad94164..eb6545f8e944 100644 --- a/src/jit/lclvars.cpp +++ b/src/jit/lclvars.cpp @@ -141,7 +141,8 @@ void Compiler::lvaInitTypeRef() Compiler::structPassingKind howToReturnStruct; var_types returnType = getReturnTypeForStruct(retClsHnd, &howToReturnStruct); - if (howToReturnStruct == SPK_PrimitiveType) + // We can safely widen the return type for enclosed structs. + if ((howToReturnStruct == SPK_PrimitiveType) || (howToReturnStruct == SPK_EnclosingType)) { assert(returnType != TYP_UNKNOWN); assert(!varTypeIsStruct(returnType)); From 44287dce57fd95b9fad5a56abeab23a5f7302c24 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 21 Jun 2018 17:22:14 -0700 Subject: [PATCH 3/7] handle short structs via temps really --- src/jit/compiler.h | 4 ++++ src/jit/importer.cpp | 52 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 63b0d9fb65b6..87079a81d0b1 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -1691,6 +1691,10 @@ class Compiler GenTree* impAssignMultiRegTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE hClass); #endif // FEATURE_MULTIREG_RET +#ifdef UNIX_AMD64_ABI + GenTree* impAssignSmallStructTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE hClass); +#endif + #ifdef ARM_SOFTFP bool isSingleFloat32Struct(CORINFO_CLASS_HANDLE hClass); #endif // ARM_SOFTFP diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 895181a5c139..63cc5c60e4ba 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -8464,14 +8464,14 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN { if (retRegCount == 1) { - // struct returned in a single register - // retype iff struct size exactly matches integer type size. + // struct returned in a single register, so retype call + call->gtReturnType = retTypeDesc->GetReturnRegType(0); + + // if struct size does not matche a supported integer + // type size, ensure return writes to a suitable temp. if (retTypeDesc->IsEnclosingType()) { - } - else - { - call->gtReturnType = retTypeDesc->GetReturnRegType(0); + return impAssignSmallStructTypeToVar(call, retClsHnd); } } else @@ -15660,7 +15660,47 @@ void Compiler::impMarkLclDstNotPromotable(unsigned tmpNum, GenTree* src, CORINFO } #endif // _TARGET_ARM_ +#ifdef UNIX_AMD64_ABI +//------------------------------------------------------------------------ +// impAssignSmallStructTypeToVar: ensure calls that return small structs whose +// sizes are not supported integral type sizes return values to temps. +// +// Arguments: +// op -- call returning a small struct in a register +// hClass -- class handle for struct +// +// Returns: +// Tree with reference to struct local to use as call return value. +// +// Remarks: +// The call will be spilled into a preceding statement. +// Currently handles struct returns for 3, 5, 6, and 7 byte structs. + +GenTree* Compiler::impAssignSmallStructTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE hClass) +{ + unsigned tmpNum = lvaGrabTemp(true DEBUGARG("Return value temp for small struct return.")); + impAssignTempGen(tmpNum, op, hClass, (unsigned)CHECK_SPILL_ALL); + GenTree* ret = gtNewLclvNode(tmpNum, lvaTable[tmpNum].lvType); + + // TODO-1stClassStructs: Handle constant propagation and CSE-ing of small struct returns. + ret->gtFlags |= GTF_DONT_CSE; + + return ret; +} +#endif // UNIX_AMD64_ABI + #if FEATURE_MULTIREG_RET +//------------------------------------------------------------------------ +// impAssignMultiRegTypeToVar: ensure calls that return structs in multiple +// registers return values to suitable temps. +// +// Arguments: +// op -- call returning a struct in a registers +// hClass -- class handle for struct +// +// Returns: +// Tree with reference to struct local to use as call return value. + GenTree* Compiler::impAssignMultiRegTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE hClass) { unsigned tmpNum = lvaGrabTemp(true DEBUGARG("Return value temp for multireg return.")); From 352b6459d5f2fb01ca914ab9c65917aa22ce4db4 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 21 Jun 2018 20:02:58 -0700 Subject: [PATCH 4/7] Handle inline candidate cases Defer retyping inline candidates and tail call candidates. Then handle deferred updates for cases where calls don't get inlined. --- src/jit/flowgraph.cpp | 109 +++++++++++++++++++++++++++++++++++------- src/jit/importer.cpp | 25 ++++++---- 2 files changed, 108 insertions(+), 26 deletions(-) diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index a852fb315f64..34443bb3ca28 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -22324,30 +22324,105 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr } } -#if FEATURE_MULTIREG_RET - - // Did we record a struct return class handle above? +#if FEATURE_MULTIREG_RET || defined(UNIX_AMD64_ABI) + // If an inline was rejected and the call returns a struct, we may + // have deferred some work when importing call for cases where the + // struct is returned in register(s). + // + // See the bail-out clauses in impFixupCallStructReturn for inline + // candidates. // + // Do the deferred work now. if (retClsHnd != NO_CLASS_HANDLE) { - // Is this a type that is returned in multiple registers? - // if so we need to force into into a form we accept. - // i.e. LclVar = call() - // - if (comp->IsMultiRegReturnedType(retClsHnd)) + structPassingKind howToReturnStruct; + var_types returnType = comp->getReturnTypeForStruct(retClsHnd, &howToReturnStruct); + GenTree* parent = data->parent; + + switch (howToReturnStruct) { - GenTree* parent = data->parent; - // See assert below, we only look one level above for an asg parent. - if (parent->gtOper == GT_ASG) + // Is this a type that is returned in multiple registers + // or a via a primitve type that is larger than the struct type? + // if so we need to force into into a form we accept. + // i.e. LclVar = call() + case SPK_ByValue: + case SPK_ByValueAsHfa: { - // Either lhs is a call V05 = call(); or lhs is addr, and asg becomes a copyBlk. - comp->fgAttachStructInlineeToAsg(parent, tree, retClsHnd); + // See assert below, we only look one level above for an asg parent. + if (parent->gtOper == GT_ASG) + { + // Either lhs is a call V05 = call(); or lhs is addr, and asg becomes a copyBlk. + comp->fgAttachStructInlineeToAsg(parent, tree, retClsHnd); + } + else + { + // Just assign the inlinee to a variable to keep it simple. + tree->ReplaceWith(comp->fgAssignStructInlineeToVar(tree, retClsHnd), comp); + } } - else + break; + + case SPK_EnclosingType: { - // Just assign the inlinee to a variable to keep it simple. - tree->ReplaceWith(comp->fgAssignStructInlineeToVar(tree, retClsHnd), comp); + // For enclosing type returns, we must return the call value to a temp since + // the return type is larger than the struct type. + if (!tree->IsCall()) + { + break; + } + + GenTreeCall* call = tree->AsCall(); + + assert(call->gtReturnType == TYP_STRUCT); + + if (call->gtReturnType != TYP_STRUCT) + { + break; + } + + JITDUMP("\nCall returns small struct via enclosing type, retyping. Before:\n"); + DISPTREE(call); + + // Create new struct typed temp for return value + const unsigned tmpNum = + comp->lvaGrabTemp(true DEBUGARG("small struct return temp for rejected inline")); + comp->lvaSetStruct(tmpNum, retClsHnd, false); + GenTree* assign = comp->gtNewTempAssign(tmpNum, call); + + // Modify assign tree and call return types to the primitive return type + call->gtReturnType = returnType; + call->gtType = returnType; + assign->gtType = returnType; + + // Modify the temp reference in the assign as a primitive reference via GT_LCL_FLD + GenTree* tempAsPrimitive = assign->gtOp.gtOp1; + assert(tempAsPrimitive->gtOper == GT_LCL_VAR); + tempAsPrimitive->gtType = returnType; + tempAsPrimitive->ChangeOper(GT_LCL_FLD); + + // Return temp as value of call tree via comma + GenTree* tempAsStruct = comp->gtNewLclvNode(tmpNum, TYP_STRUCT); + GenTree* comma = comp->gtNewOperNode(GT_COMMA, TYP_STRUCT, assign, tempAsStruct); + parent->ReplaceOperand(pTree, comma); + + JITDUMP("\nAfter:\n"); + DISPTREE(comma); } + break; + + case SPK_PrimitiveType: + // We should have already retyped the call as a primitive type + // when we first imported the call + break; + + case SPK_ByReference: + // We should have already added the return buffer + // when we first imported the call + break; + + default: + noway_assert(!"Unexpected struct passing kind"); + break; } } @@ -22373,7 +22448,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr } #endif // defined(DEBUG) -#endif // FEATURE_MULTIREG_RET +#endif // FEATURE_MULTIREG_RET || defined(UNIX_AMD64_ABI) return WALK_CONTINUE; } diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 63cc5c60e4ba..6937ef5a63f6 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -8464,14 +8464,23 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN { if (retRegCount == 1) { - // struct returned in a single register, so retype call - call->gtReturnType = retTypeDesc->GetReturnRegType(0); - - // if struct size does not matche a supported integer - // type size, ensure return writes to a suitable temp. + // See if the struct size is smaller than the return + // type size... if (retTypeDesc->IsEnclosingType()) { - return impAssignSmallStructTypeToVar(call, retClsHnd); + // If we know for sure this call will remain a call, + // retype and return value via a suitable temp. + if ((!call->CanTailCall()) && (!call->IsInlineCandidate())) + { + call->gtReturnType = retTypeDesc->GetReturnRegType(0); + return impAssignSmallStructTypeToVar(call, retClsHnd); + } + } + else + { + // Return type is same size as struct, so we can + // simply retype the call. + call->gtReturnType = retTypeDesc->GetReturnRegType(0); } } else @@ -8735,9 +8744,7 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE re } else { - assert(info.compRetNativeType == op->gtCall.gtReturnType); - - // Don't change the gtType of the node just yet, it will get changed later. + // Don't change the gtType of the call just yet, it will get changed later. return op; } } From 18301868973ecd009abefda82d6525fca74ba25e Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 25 Jun 2018 11:53:25 -0700 Subject: [PATCH 5/7] review feedback --- src/jit/compiler.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 7998b69d240b..5e8088d2c476 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -1012,12 +1012,12 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, if (structDesc.eightByteCount == 1) { + assert(structSize <= sizeof(double)); + if (structDesc.eightByteClassifications[0] == SystemVClassificationTypeSSE) { // If this is returned as a floating type, use that. - // Otherwise, we'll use the general case - we don't want to use the "EightByteType" - // directly, because it returns `TYP_INT` for any integral type <= 4 bytes, and - // we need to preserve small types. + // Otherwise, leave as TYP_UNKONWN and we'll sort things out below. useType = GetEightByteType(structDesc, 0); howToReturnStruct = SPK_PrimitiveType; } @@ -1025,6 +1025,9 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, #endif // UNIX_AMD64_ABI + // Check for cases where a small struct is returned in a register + // via a primitive type. + // // The largest primitive type is 8 bytes (TYP_DOUBLE) // so we can skip calling getPrimitiveTypeForStruct when we // have a struct that is larger than that. @@ -1047,7 +1050,7 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, else { // Currently: 3, 5, 6, or 7 byte structs - assert(structSize <= genTypeSize(useType)); + assert(structSize < genTypeSize(useType)); howToReturnStruct = SPK_EnclosingType; } } From aabba68c554cd19e89bb4a7bae7958c0611b5433 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 25 Jun 2018 11:41:13 -0700 Subject: [PATCH 6/7] Extend support for small struct cases to windows Arm32/Arm64. Unlike Windows x64, these ABIs return 3 (and 5,6,7) byte structs in a register. So make the necessary transformations for those cases too. Note the actual behavior change is triggered by the code in `getPrimitiveTypeForStruct` so there are no ifdefs at the transform points here. --- src/jit/compiler.h | 5 ++--- src/jit/flowgraph.cpp | 9 +++++++-- src/jit/importer.cpp | 22 +++++++++++++++++++--- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 87079a81d0b1..eb92395f7abf 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -1691,9 +1691,7 @@ class Compiler GenTree* impAssignMultiRegTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE hClass); #endif // FEATURE_MULTIREG_RET -#ifdef UNIX_AMD64_ABI GenTree* impAssignSmallStructTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE hClass); -#endif #ifdef ARM_SOFTFP bool isSingleFloat32Struct(CORINFO_CLASS_HANDLE hClass); @@ -4228,7 +4226,8 @@ class Compiler SPK_Unknown, // Invalid value, never returned SPK_PrimitiveType, // The struct is passed/returned using a primitive type. SPK_EnclosingType, // Like SPK_Primitive type, but used for return types that - // are not a power of two byte size. + // require a primitive type temp that is larger than the struct size. + // Currently used for structs of size 3, 5, 6, or 7 bytes. SPK_ByValue, // The struct is passed/returned by value (using the ABI rules) // for ARM64 and UNIX_X64 in multiple registers. (when all of the // parameters registers are used, then the stack will be used) diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index 34443bb3ca28..197af3100b53 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -22324,7 +22324,6 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr } } -#if FEATURE_MULTIREG_RET || defined(UNIX_AMD64_ABI) // If an inline was rejected and the call returns a struct, we may // have deferred some work when importing call for cases where the // struct is returned in register(s). @@ -22341,6 +22340,9 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr switch (howToReturnStruct) { + +#if FEATURE_MULTIREG_RET + // Is this a type that is returned in multiple registers // or a via a primitve type that is larger than the struct type? // if so we need to force into into a form we accept. @@ -22362,6 +22364,8 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr } break; +#endif // FEATURE_MULTIREG_RET + case SPK_EnclosingType: { // For enclosing type returns, we must return the call value to a temp since @@ -22426,6 +22430,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr } } +#if FEATURE_MULTIREG_RET #if defined(DEBUG) // Make sure we don't have a tree like so: V05 = (, , , retExpr); @@ -22448,7 +22453,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr } #endif // defined(DEBUG) -#endif // FEATURE_MULTIREG_RET || defined(UNIX_AMD64_ABI) +#endif // FEATURE_MULTIREG_RET return WALK_CONTINUE; } diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 6937ef5a63f6..74f48083d20d 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -8523,7 +8523,25 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN else { assert(returnType != TYP_UNKNOWN); - call->gtReturnType = returnType; + + // See if the struct size is smaller than the return + // type size... + if (howToReturnStruct == SPK_EnclosingType) + { + // If we know for sure this call will remain a call, + // retype and return value via a suitable temp. + if ((!call->CanTailCall()) && (!call->IsInlineCandidate())) + { + call->gtReturnType = returnType; + return impAssignSmallStructTypeToVar(call, retClsHnd); + } + } + else + { + // Return type is same size as struct, so we can + // simply retype the call. + call->gtReturnType = returnType; + } // ToDo: Refactor this common code sequence into its own method as it is used 4+ times if ((returnType == TYP_LONG) && (compLongUsed == false)) @@ -15667,7 +15685,6 @@ void Compiler::impMarkLclDstNotPromotable(unsigned tmpNum, GenTree* src, CORINFO } #endif // _TARGET_ARM_ -#ifdef UNIX_AMD64_ABI //------------------------------------------------------------------------ // impAssignSmallStructTypeToVar: ensure calls that return small structs whose // sizes are not supported integral type sizes return values to temps. @@ -15694,7 +15711,6 @@ GenTree* Compiler::impAssignSmallStructTypeToVar(GenTree* op, CORINFO_CLASS_HAND return ret; } -#endif // UNIX_AMD64_ABI #if FEATURE_MULTIREG_RET //------------------------------------------------------------------------ From eecd311e7c76d09408bce890649657201fc539cc Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 26 Jun 2018 09:17:21 -0700 Subject: [PATCH 7/7] Add test cases for 6 byte structs showing the various situations the jit must handle: callee is not an inline candidate, is an inline candidate and gets inlined, or inline candidate that does not get inlined. In the case where the callee gets inlined, handle the transitive cases where the callee's returne value itself comes from a call. Add a 3 byte test case to get coverage on arm32. Add new tests to the arm32/arm64 test lists. --- tests/arm/Tests.lst | 64 ++++++++++++++++ tests/arm64/Tests.lst | 64 ++++++++++++++++ .../JitBlue/GitHub_18522/GitHub_18522.cs | 39 ++++++++++ .../JitBlue/GitHub_18522/GitHub_18522.csproj | 34 +++++++++ .../JitBlue/GitHub_18522/GitHub_18522_1.cs | 52 +++++++++++++ .../GitHub_18522/GitHub_18522_1.csproj | 34 +++++++++ .../JitBlue/GitHub_18522/GitHub_18522_2.cs | 54 ++++++++++++++ .../GitHub_18522/GitHub_18522_2.csproj | 34 +++++++++ .../JitBlue/GitHub_18522/GitHub_18522_3.cs | 66 +++++++++++++++++ .../GitHub_18522/GitHub_18522_3.csproj | 34 +++++++++ .../JitBlue/GitHub_18522/GitHub_18522_4.cs | 62 ++++++++++++++++ .../GitHub_18522/GitHub_18522_4.csproj | 34 +++++++++ .../JitBlue/GitHub_18522/GitHub_18522_5.cs | 74 +++++++++++++++++++ .../GitHub_18522/GitHub_18522_5.csproj | 34 +++++++++ .../JitBlue/GitHub_18522/GitHub_18522_6.cs | 61 +++++++++++++++ .../GitHub_18522/GitHub_18522_6.csproj | 34 +++++++++ .../JitBlue/GitHub_18522/GitHub_18522_7.cs | 67 +++++++++++++++++ .../GitHub_18522/GitHub_18522_7.csproj | 34 +++++++++ 18 files changed, 875 insertions(+) create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.csproj create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.csproj create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.csproj create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.csproj create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.csproj create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.csproj create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.csproj create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.csproj diff --git a/tests/arm/Tests.lst b/tests/arm/Tests.lst index 9e96a1eb994d..db556779e9dc 100644 --- a/tests/arm/Tests.lst +++ b/tests/arm/Tests.lst @@ -94740,3 +94740,67 @@ MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 +[GitHub_18522.cmd_11903] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522\GitHub_18522.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_1.cmd_11904] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_1\GitHub_18522_1.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_1 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_2.cmd_11905] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_2\GitHub_18522_2.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_2 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_3.cmd_11906] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_3\GitHub_18522_3.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_3 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_4.cmd_11907] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_4\GitHub_18522_4.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_4 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_5.cmd_11908] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_5\GitHub_18522_5.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_5 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_6.cmd_11909] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_6\GitHub_18522_6.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_6 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_7.cmd_11910] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_7\GitHub_18522_7.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_7 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + diff --git a/tests/arm64/Tests.lst b/tests/arm64/Tests.lst index a9a732053a00..4d7592873474 100644 --- a/tests/arm64/Tests.lst +++ b/tests/arm64/Tests.lst @@ -94763,3 +94763,67 @@ Expected=0 MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 + +[GitHub_18522.cmd_12223] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522\GitHub_18522.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_1.cmd_12224] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_1\GitHub_18522_1.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_1 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_2.cmd_12225] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_2\GitHub_18522_2.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_2 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_3.cmd_12226] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_3\GitHub_18522_3.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_3 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_4.cmd_12227] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_4\GitHub_18522_4.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_4 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_5.cmd_12228] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_5\GitHub_18522_5.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_5 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_6.cmd_12229] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_6\GitHub_18522_6.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_6 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18522_7.cmd_12230] +RelativePath=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_7\GitHub_18522_7.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18522\GitHub_18522_7 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.cs new file mode 100644 index 000000000000..8cbcc6e5b8b1 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.cs @@ -0,0 +1,39 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +// Based on a program generated by Fuzzlyn + +struct S0 +{ + public ushort F0; +} + +struct S1 +{ + public S0 F0; + public ushort F1; +} + +public class GitHub_18522 +{ + static S1 s_36; + + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M113, and so + // inadvertently overwriting the F1 field of s_36 on return from + // the (inlined) call. + public static int Main() + { + s_36.F1 = 0xAA; + s_36.F0 = M113(); + return (s_36.F1 == 0xAA ? 100 : 0); + } + + static S0 M113() + { + return new S0(); + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.cs new file mode 100644 index 000000000000..a5726451fb6c --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.cs @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +// Based on +// Original generated by Fuzzlyn on 2018-06-20 00:58:58 +// Seed: 11049252875418439527 +// Reduced from 97.5 KiB to 0.5 KiB +// Debug: Outputs -1 +// Release: Outputs -65536 + +struct S0 +{ + public sbyte F0; + public char F1; + public ushort F2; +} + +struct S1 +{ + public short F0; + public S0 F1; + public S0 F2; + public S0 F3; + public int F4; + public S1(int f4): this() + { + F4 = f4; + } +} + +public class GitHub_18522_1 +{ + static S1 s_6; + static S1[] s_13 = new S1[]{new S1(-1)}; + public static int Main() + { + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M16, and so + // inadvertently overwriting the F4 field of s_13[0] on return + // from the (inlined) call. + s_13[0].F3 = M16(); + return s_13[0].F4 == -1 ? 100 : 0; + } + + static S0 M16() + { + return s_6.F3; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_1.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.cs new file mode 100644 index 000000000000..ab4090d0d369 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.cs @@ -0,0 +1,54 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +// Based on +// Original generated by Fuzzlyn on 2018-06-20 00:58:58 +// Seed: 11049252875418439527 +// Reduced from 97.5 KiB to 0.5 KiB +// Debug: Outputs -1 +// Release: Outputs -65536 + +struct S0 +{ + public sbyte F0; + public char F1; + public ushort F2; +} + +struct S1 +{ + public short F0; + public S0 F1; + public S0 F2; + public S0 F3; + public int F4; + public S1(int f4): this() + { + F4 = f4; + } +} + +public class GitHub_18522_2 +{ + static S1 s_6; + static S1[] s_13 = new S1[]{new S1(-1)}; + public static int Main() + { + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M16, and so + // inadvertently overwriting the F4 field of s_13[0] on return + // from the (not inlined) call. + s_13[0].F3 = M16(); + return s_13[0].F4 == -1 ? 100 : 0; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static S0 M16() + { + return s_6.F3; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_2.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.cs new file mode 100644 index 000000000000..9d6960cfcb76 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.cs @@ -0,0 +1,66 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +// Based on +// Original generated by Fuzzlyn on 2018-06-20 00:58:58 +// Seed: 11049252875418439527 +// Reduced from 97.5 KiB to 0.5 KiB +// Debug: Outputs -1 +// Release: Outputs -65536 + +struct S0 +{ + public sbyte F0; + public char F1; + public ushort F2; +} + +struct S1 +{ + public short F0; + public S0 F1; + public S0 F2; + public S0 F3; + public int F4; + public S1(int f4): this() + { + F4 = f4; + } +} + +public class GitHub_18522_3 +{ + static S1 s_6; + static S1[] s_13 = new S1[]{new S1(-1)}; + public static int Main() + { + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M16, and so + // inadvertently overwriting the F4 field of s_13[0] on return + // from the call (which was an inline candidate, but not inlined). + s_13[0].F3 = M16(); + return s_13[0].F4 == -1 ? 100 : 0; + } + + static S0 M16() + { + // This bit of code is intended to allow M16 to be an + // inline candidate that ultimately does not get inlined. + short x = 0; + for (int i = 0; i < 10; i++) + { + for (int j = 0; j < 10; j++) + { + x++; + } + } + s_6.F0 = x; + + // Actual interesting part + return s_6.F3; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_3.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.cs new file mode 100644 index 000000000000..b6f54dc61507 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +// Based on +// Original generated by Fuzzlyn on 2018-06-20 00:58:58 +// Seed: 11049252875418439527 +// Reduced from 97.5 KiB to 0.5 KiB +// Debug: Outputs -1 +// Release: Outputs -65536 + +struct S0 +{ + public sbyte F0; + public char F1; + public ushort F2; +} + +struct S1 +{ + public short F0; + public S0 F1; + public S0 F2; + public S0 F3; + public int F4; + public S1(int f4): this() + { + F4 = f4; + } +} + +public class GitHub_18522_4 +{ + static S1 s_6; + static S1[] s_13 = new S1[]{new S1(-1)}; + public static int Main() + { + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M16, and so + // inadvertently overwriting the F4 field of s_13[0] on return + // from the call. + // + // Here we make sure we properly handle an inline call that + // resolves to a noinline call. + s_13[0].F3 = W(); + return s_13[0].F4 == -1 ? 100 : 0; + } + + static S0 W() + { + return M16(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static S0 M16() + { + return s_6.F3; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_4.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.cs new file mode 100644 index 000000000000..fdb2a72dc9ca --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.cs @@ -0,0 +1,74 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +// Based on +// Original generated by Fuzzlyn on 2018-06-20 00:58:58 +// Seed: 11049252875418439527 +// Reduced from 97.5 KiB to 0.5 KiB +// Debug: Outputs -1 +// Release: Outputs -65536 + +struct S0 +{ + public sbyte F0; + public char F1; + public ushort F2; +} + +struct S1 +{ + public short F0; + public S0 F1; + public S0 F2; + public S0 F3; + public int F4; + public S1(int f4): this() + { + F4 = f4; + } +} + +public class GitHub_18522_5 +{ + static S1 s_6; + static S1[] s_13 = new S1[]{new S1(-1)}; + public static int Main() + { + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M16, and so + // inadvertently overwriting the F4 field of s_13[0] on return + // from the call. + // + // Here we make sure we properly handle an inlined call that + // resolves to a rejected inline candidate. + s_13[0].F3 = W(); + return s_13[0].F4 == -1 ? 100 : 0; + } + + static S0 W() + { + return M16(); + } + + static S0 M16() + { + // This bit of code is intended to allow M16 to be an + // inline candidate that ultimately does not get inlined. + short x = 0; + for (int i = 0; i < 10; i++) + { + for (int j = 0; j < 10; j++) + { + x++; + } + } + s_6.F0 = x; + + // Actual interesting part + return s_6.F3; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_5.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.cs new file mode 100644 index 000000000000..5a3990f509ae --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.cs @@ -0,0 +1,61 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +// Based on +// Original generated by Fuzzlyn on 2018-06-20 00:58:58 +// Seed: 11049252875418439527 +// Reduced from 97.5 KiB to 0.5 KiB +// Debug: Outputs -1 +// Release: Outputs -65536 + +struct S0 +{ + public sbyte F0; + public char F1; + public ushort F2; +} + +struct S1 +{ + public short F0; + public S0 F1; + public S0 F2; + public S0 F3; + public int F4; + public S1(int f4): this() + { + F4 = f4; + } +} + +public class GitHub_18522_6 +{ + static S1 s_6; + static S1[] s_13 = new S1[]{new S1(-1)}; + public static int Main() + { + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M16, and so + // inadvertently overwriting the F4 field of s_13[0] on return + // from the call. + // + // Here we make sure we properly handle an inlined call that + // resolves to another inlined call. + s_13[0].F3 = W(); + return s_13[0].F4 == -1 ? 100 : 0; + } + + static S0 W() + { + return M16(); + } + + static S0 M16() + { + return s_6.F3; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_6.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.cs b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.cs new file mode 100644 index 000000000000..e59d3ded8b19 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +// Based on +// Original generated by Fuzzlyn on 2018-06-20 00:58:58 +// Seed: 11049252875418439527 +// Reduced from 97.5 KiB to 0.5 KiB +// Debug: Outputs -1 +// Release: Outputs -65536 + +// Similar to other variants but using a 3 byte struct instead of 6. + +struct S0 +{ + public byte F0; + public byte F1; + public byte F2; +} + +struct S1 +{ + public S0 F3; + public sbyte F4; + public short F0; + public S1(sbyte f4): this() + { + F4 = f4; + } +} + +public class GitHub_18522_7 +{ + static S1 s_6; + static S1[] s_13 = new S1[]{new S1(-1)}; + public static int Main() + { + // When generating code for the x64 SysV ABI, the jit was + // incorrectly typing the return type from M16, and so + // inadvertently overwriting the F4 field of s_13[0] on return + // from the call. + // + // Here we make sure we properly handle the failed inline case. + s_13[0].F3 = M16(); + return s_13[0].F4 == -1 ? 100 : 0; + } + + static S0 M16() + { + // This bit of code is intended to allow M16 to be an + // inline candidate that ultimately does not get inlined. + short x = 0; + for (int i = 0; i < 10; i++) + { + for (int j = 0; j < 10; j++) + { + x++; + } + } + s_6.F0 = x; + + return s_6.F3; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.csproj new file mode 100644 index 000000000000..95aba995a255 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_18522/GitHub_18522_7.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + +