From f686b5fca7940723e6f2bc966a43490902b2e8bb Mon Sep 17 00:00:00 2001 From: sivarv Date: Thu, 9 Feb 2017 21:31:04 -0800 Subject: [PATCH] Promote struct containing fields of struct of a single pointer-size field. --- src/jit/lclvars.cpp | 75 +++++++++++++++++++++++++++++++++++++++++++---------- src/jit/lower.cpp | 36 +++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 13 deletions(-) diff --git a/src/jit/lclvars.cpp b/src/jit/lclvars.cpp index b4e4cc6..61cf8a1 100644 --- a/src/jit/lclvars.cpp +++ b/src/jit/lclvars.cpp @@ -1522,15 +1522,8 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, pFieldInfo->fldOffset = (BYTE)fldOffset; pFieldInfo->fldOrdinal = ordinal; CorInfoType corType = info.compCompHnd->getFieldType(pFieldInfo->fldHnd, &pFieldInfo->fldTypeHnd); - var_types varType = JITtype2varType(corType); - pFieldInfo->fldType = varType; - unsigned size = genTypeSize(varType); - pFieldInfo->fldSize = size; - - if (varTypeIsGC(varType)) - { - containsGCpointers = true; - } + pFieldInfo->fldType = JITtype2varType(corType); + pFieldInfo->fldSize = genTypeSize(pFieldInfo->fldType); #ifdef FEATURE_SIMD // Check to see if this is a SIMD type. @@ -1542,8 +1535,7 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, var_types simdBaseType = getBaseTypeAndSizeOfSIMDType(pFieldInfo->fldTypeHnd, &simdSize); if (simdBaseType != TYP_UNKNOWN) { - varType = getSIMDTypeForSize(simdSize); - pFieldInfo->fldType = varType; + pFieldInfo->fldType = getSIMDTypeForSize(simdSize); pFieldInfo->fldSize = simdSize; } } @@ -1551,8 +1543,60 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, if (pFieldInfo->fldSize == 0) { - // Non-primitive struct field. Don't promote. - return; + // Size of TYP_BLK, TYP_FUNC, TYP_VOID and TYP_STRUCT is zero. + // Early out if field type is other than TYP_STRUCT. + // This is a defensive check as we don't expect a struct to have + // fields of TYP_BLK, TYP_FUNC or TYP_VOID. + if (pFieldInfo->fldType != TYP_STRUCT) + { + return; + } + + // Non-primitive struct field. + // Try to promote structs of single field of scalar types aligned at their + // natural boundary. + + // Do Not promote if the struct field in turn has more than one field. + if (info.compCompHnd->getClassNumInstanceFields(pFieldInfo->fldTypeHnd) != 1) + { + return; + } + + // Do not promote if the single field is not aligned at its natural boundary within + // the struct field. + CORINFO_FIELD_HANDLE fHnd = info.compCompHnd->getFieldInClass(pFieldInfo->fldTypeHnd, 0); + unsigned fOffset = info.compCompHnd->getFieldOffset(fHnd); + if (fOffset != 0) + { + return; + } + + CORINFO_CLASS_HANDLE cHnd; + CorInfoType fieldCorType = info.compCompHnd->getFieldType(fHnd, &cHnd); + var_types fieldVarType = JITtype2varType(fieldCorType); + unsigned fieldSize = genTypeSize(fieldVarType); + + // Do not promote if either not a primitive type or size equal to ptr size on + // target or a struct containing a single floating-point field. + // + // TODO-PERF: Structs containing a single floating-point field on Amd64 + // needs to be passed in integer registers. Right now LSRA doesn't support + // passing of floating-point LCL_VARS in integer registers. Enabling promotion + // of such structs results in an assert in lsra right now. + // + // TODO-PERF: Right now promotion is confined to struct containing a ptr sized + // field (int/uint/ref/byref on 32-bits and long/ulong/ref/byref on 64-bits). + // Though this would serve the purpose of promoting Span containing ByReference, + // this can be extended to other primitive types as long as they are aligned at their + // natural boundary. + if (fieldSize == 0 || fieldSize != TARGET_POINTER_SIZE || varTypeIsFloating(fieldVarType)) + { + return; + } + + // Retype the field as the type of the single field of the struct + pFieldInfo->fldType = fieldVarType; + pFieldInfo->fldSize = fieldSize; } if ((pFieldInfo->fldOffset % pFieldInfo->fldSize) != 0) @@ -1563,6 +1607,11 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, return; } + if (varTypeIsGC(pFieldInfo->fldType)) + { + containsGCpointers = true; + } + // The end offset for this field should never be larger than our structSize. noway_assert(fldOffset + pFieldInfo->fldSize <= structSize); diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index 0316a34..0b4d5a7 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -971,6 +971,42 @@ GenTreePtr Lowering::NewPutArg(GenTreeCall* call, GenTreePtr arg, fgArgTabEntryP assert(!varTypeIsSIMD(arg)); numRefs = comp->info.compCompHnd->getClassGClayout(arg->gtObj.gtClass, gcLayout); putArg->AsPutArgStk()->setGcPointers(numRefs, gcLayout); + +#ifdef _TARGET_X86_ + // On x86 VM lies about the type of a struct containing a pointer sized + // integer field by returning the type of its field as the type of struct. + // Such struct can be passed in a register depending its position in + // parameter list. VM does this unwrapping only one level and therefore + // a type like Struct Foo { Struct Bar { int f}} awlays needs to be + // passed on stack. Also, VM doesn't lie about type of such a struct + // when it is a field of another struct. That is VM doesn't lie about + // the type of Foo.Bar + // + // Promotion of structs containing structs of single field can promote + // such a struct. Say Foo.Bar field is getting passed as a parameter + // to a call, Since it is a TYP_STRUCT, as per x86 ABI it should always + // be passed on stack. Therefore GenTree node under a PUTARG_STK could + // be GT_OBJ(GT_LCL_VAR_ADDR(v1)), where local v1 could be a promoted + // field standing for Foo.Bar. Note that the type of v1 will be the + // type of field of Foo.Bar.f when Foo is promoted. That is v1 + // will be a scalar type. In this case we need to pass v1 on stack + // instead of in a register. + // + // TODO-PERF: replace GT_OBJ(GT_LCL_VAR_ADDR(v1)) with v1 if v1 is + // a scalar type and the width of GT_OBJ matches the type size of v1. + // Note that this cannot be done till call node arguments are morphed + // because we should not lose the fact that the type of argument is + // a struct so that the arg gets correctly marked to be passed on stack. + GenTree* objOp1 = arg->gtGetOp1(); + if (objOp1->OperGet() == GT_LCL_VAR_ADDR) + { + unsigned lclNum = objOp1->AsLclVarCommon()->GetLclNum(); + if (comp->lvaTable[lclNum].lvType != TYP_STRUCT) + { + comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_VMNeedsStackAddr)); + } + } +#endif // _TARGET_X86_ } } #endif // FEATURE_PUT_STRUCT_ARG_STK -- 2.7.4