From a2ba07ab7ebbd47fc673a1f4a7521a400badb298 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sun, 5 Aug 2018 12:16:00 +0300 Subject: [PATCH 1/7] Refactor genIntToIntCast on XARCH --- src/jit/codegen.h | 1 + src/jit/codegenxarch.cpp | 380 ++++++++++++++++----------------------- 2 files changed, 157 insertions(+), 224 deletions(-) diff --git a/src/jit/codegen.h b/src/jit/codegen.h index 9eb1ba397a79..82e89ab93049 100644 --- a/src/jit/codegen.h +++ b/src/jit/codegen.h @@ -789,6 +789,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genLongToIntCast(GenTree* treeNode); #endif + void genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg); void genIntToIntCast(GenTree* treeNode); void genFloatToFloatCast(GenTree* treeNode); void genFloatToIntCast(GenTree* treeNode); diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp index caccf4700bc4..b1723ebfaab0 100644 --- a/src/jit/codegenxarch.cpp +++ b/src/jit/codegenxarch.cpp @@ -6345,269 +6345,201 @@ void CodeGen::genLongToIntCast(GenTree* cast) #endif //------------------------------------------------------------------------ -// genIntToIntCast: Generate code for an integer cast -// This method handles integer overflow checking casts -// as well as ordinary integer casts. +// genIntCastOverflowCheck: Generate overflow checking code for an integer cast. // // Arguments: -// treeNode - The GT_CAST node +// cast - The GT_CAST node +// reg - The register containing the value to check // -// Return Value: -// None. +void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) +{ + const var_types srcType = genActualType(cast->gtGetOp1()->TypeGet()); + const bool srcUnsigned = cast->IsUnsigned(); + const unsigned srcSize = genTypeSize(srcType); + const var_types castType = cast->gtCastType; + const bool castUnsigned = varTypeIsUnsigned(castType); + const unsigned castSize = genTypeSize(castType); + + if (castSize >= srcSize) + { + // It's either a widening cast from a signed type to an unsigned + // type or a sign changing cast. + assert((castSize > srcSize) ? (!srcUnsigned && castUnsigned) : (srcUnsigned != castUnsigned)); + + // In both cases we just need to check if the value is positive. + getEmitter()->emitIns_R_R(INS_test, EA_SIZE(srcSize), reg, reg); + genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); + } +#ifdef _TARGET_64BIT_ + // We handled widening and sign changing casts so we're left with narrowing casts. + // Handle (U)LONG to (U)INT casts first as they're 64 bit target specific and there + // are some complications related to encoding of the large immediate values they need. + else if (castSize == 4) + { + assert(srcSize == 8); + + if (castUnsigned) // (U)LONG to UINT cast + { + // We need to check if the value is not greater than 0xFFFFFFFF but this value + // cannot be encoded in an immediate operand. Use a right shift to test if the + // upper 32 bits are zero. This requires a temporary register. + const regNumber tempReg = cast->GetSingleTempReg(); + assert(tempReg != reg); + getEmitter()->emitIns_R_R(INS_mov, EA_8BYTE, tempReg, reg); + getEmitter()->emitIns_R_I(INS_shr_N, EA_8BYTE, tempReg, 32); + genJumpToThrowHlpBlk(EJ_jne, SCK_OVERFLOW); + } + else if (srcUnsigned) // ULONG to INT cast + { + getEmitter()->emitIns_R_I(INS_cmp, EA_8BYTE, reg, INT32_MAX); + genJumpToThrowHlpBlk(EJ_ja, SCK_OVERFLOW); + } + else // LONG to INT cast + { + // We could combine this case with the unsigned case above but let's keep + // theses two separated to maintain the code structure similar to the ARM + // implementation that here requires a temporary register. + getEmitter()->emitIns_R_I(INS_cmp, EA_8BYTE, reg, INT32_MAX); + genJumpToThrowHlpBlk(EJ_jg, SCK_OVERFLOW); + + getEmitter()->emitIns_R_I(INS_cmp, EA_8BYTE, reg, INT32_MIN); + genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); + } + } +#endif + else // if (castSize < srcSize) + { + // This is a narrowing cast. We already handled (U)INT so what's left are small int types. + assert(castSize < 4); + + // This happens to allow for easy compututation of the min and max + // values of the castType without risk of integer overflow. + const int castNumBits = (castSize * 8) - (castUnsigned ? 0 : 1); + const int castMaxValue = (1 << castNumBits) - 1; + const int castMinValue = (castUnsigned | srcUnsigned) ? 0 : (-castMaxValue - 1); + + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMaxValue); + genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_ja : EJ_jg, SCK_OVERFLOW); + + if (castMinValue != 0) + { + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMinValue); + genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); + } + } +} + +//------------------------------------------------------------------------ +// genIntToIntCast: Generate code for an integer cast, with or without overflow check. +// +// Arguments: +// treeNode - The GT_CAST node // // Assumptions: // The treeNode is not a contained node and must have an assigned register. -// For a signed convert from byte, the source must be in a byte-addressable register. // Neither the source nor target type can be a floating point type. +// On x86 casts to (U)BYTE require that the source be in a byte register. // // TODO-XArch-CQ: Allow castOp to be a contained node without an assigned register. // TODO: refactor to use getCastDescription // void CodeGen::genIntToIntCast(GenTree* treeNode) { - assert(treeNode->OperGet() == GT_CAST); + GenTreeCast* cast = treeNode->AsCast(); + genConsumeOperands(cast); - GenTree* castOp = treeNode->gtCast.CastOp(); - var_types srcType = genActualType(castOp->TypeGet()); - noway_assert(genTypeSize(srcType) >= 4); - assert(genTypeSize(srcType) <= genTypeSize(TYP_I_IMPL)); + GenTree* src = cast->gtGetOp1(); - regNumber targetReg = treeNode->gtRegNum; - regNumber sourceReg = castOp->gtRegNum; - var_types dstType = treeNode->CastToType(); - bool isUnsignedDst = varTypeIsUnsigned(dstType); - bool isUnsignedSrc = varTypeIsUnsigned(srcType); + const var_types srcType = genActualType(src->TypeGet()); + const unsigned srcSize = genTypeSize(srcType); + const var_types castType = cast->gtCastType; + const unsigned castSize = genTypeSize(castType); + const var_types dstType = genActualType(cast->TypeGet()); + const unsigned dstSize = genTypeSize(dstType); - // if necessary, force the srcType to unsigned when the GT_UNSIGNED flag is set - if (!isUnsignedSrc && treeNode->IsUnsigned()) - { - srcType = genUnsignedType(srcType); - isUnsignedSrc = true; - } + assert((srcSize == 4) || (srcSize == genTypeSize(TYP_I_IMPL))); + assert((dstSize == 4) || (dstSize == genTypeSize(TYP_I_IMPL))); + assert(castSize <= dstSize); - bool requiresOverflowCheck = false; + const regNumber srcReg = src->gtRegNum; + const regNumber dstReg = cast->gtRegNum; - assert(genIsValidIntReg(targetReg)); - assert(genIsValidIntReg(sourceReg)); + assert(genIsValidIntReg(srcReg)); + assert(genIsValidIntReg(dstReg)); - instruction ins = INS_invalid; - emitAttr srcSize = EA_ATTR(genTypeSize(srcType)); - emitAttr dstSize = EA_ATTR(genTypeSize(dstType)); + instruction ins = INS_none; + emitAttr insSize; - if (srcSize < dstSize) + Lowering::CastInfo castInfo; + Lowering::getCastDescription(cast, &castInfo); + if (castInfo.requiresOverflowCheck) { -#ifdef _TARGET_X86_ - // dstType cannot be a long type on x86, such casts should have been decomposed. - // srcType cannot be a small type since it's the "actual type" of the cast operand. - // This means that widening casts do not actually occur on x86. - unreached(); -#else - // This is a widening cast from TYP_(U)INT to TYP_(U)LONG. - assert(dstSize == EA_8BYTE); - assert(srcSize == EA_4BYTE); + genIntCastOverflowCheck(cast, srcReg); - // When widening, overflows can only happen if the source type is signed and the - // destination type is unsigned. Since the overflow check ensures that the value - // is positive a cheaper mov instruction can be used instead of movsxd. - if (treeNode->gtOverflow() && !isUnsignedSrc && isUnsignedDst) - { - requiresOverflowCheck = true; - ins = INS_mov; - } - else +#ifdef _TARGET_64BIT_ + if ((srcType == TYP_INT) && !cast->IsUnsigned() && (castType == TYP_ULONG)) { - ins = isUnsignedSrc ? INS_mov : INS_movsxd; + // This is the only overflow checking cast that requires changing the + // source value (by zero extending), all others will copy the value as is. + ins = INS_mov; + insSize = EA_4BYTE; } #endif } - else + else // Non-overflow checking cast. { - // Narrowing cast, or sign-changing cast - noway_assert(srcSize >= dstSize); - - // Is this an Overflow checking cast? - if (treeNode->gtOverflow()) + if (castSize < 4) { - requiresOverflowCheck = true; - ins = INS_mov; - } - else - { - ins = ins_Move_Extend(dstType, false); - } - } - - noway_assert(ins != INS_invalid); - - genConsumeReg(castOp); - - if (requiresOverflowCheck) - { - ssize_t typeMin = 0; - ssize_t typeMax = 0; - ssize_t typeMask = 0; - bool needScratchReg = false; - bool signCheckOnly = false; - - /* Do we need to compare the value, or just check masks */ - - switch (dstType) - { - case TYP_BYTE: - typeMask = ssize_t((int)0xFFFFFF80); - typeMin = SCHAR_MIN; - typeMax = SCHAR_MAX; - break; - - case TYP_UBYTE: - typeMask = ssize_t((int)0xFFFFFF00L); - break; - - case TYP_SHORT: - typeMask = ssize_t((int)0xFFFF8000); - typeMin = SHRT_MIN; - typeMax = SHRT_MAX; - break; + assert((castSize == 1) || (castSize == 2)); - case TYP_USHORT: - typeMask = ssize_t((int)0xFFFF0000L); - break; - - case TYP_INT: - if (srcType == TYP_UINT) - { - signCheckOnly = true; - } - else - { - typeMask = ssize_t((int)0x80000000); - typeMin = INT_MIN; - typeMax = INT_MAX; - } - break; - - case TYP_UINT: - if (srcType == TYP_INT) - { - signCheckOnly = true; - } - else - { - needScratchReg = true; - } - break; - - case TYP_LONG: - noway_assert(srcType == TYP_ULONG); - signCheckOnly = true; - break; - - case TYP_ULONG: - noway_assert((srcType == TYP_LONG) || (srcType == TYP_INT)); - signCheckOnly = true; - break; - - default: - NO_WAY("Unknown type"); - return; - } - - if (signCheckOnly) - { - // We only need to check for a negative value in sourceReg - inst_RV_RV(INS_test, sourceReg, sourceReg, srcType, srcSize); - genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); + // Casting to a small type really means widening from that small type to INT/LONG. + ins = varTypeIsUnsigned(castType) ? INS_movzx : INS_movsx; + insSize = EA_ATTR(castSize); } - else +#ifdef _TARGET_64BIT_ + // castType cannot be a long type on 32 bit targets, such casts should have been decomposed. + // srcType cannot be a small type since it's the "actual type" of the cast operand. + // This means that widening casts do not occur on 32 bit targets. + else if (castSize > srcSize) { - // When we are converting from unsigned or to unsigned, we - // will only have to check for any bits set using 'typeMask' - if (isUnsignedSrc || isUnsignedDst) - { - if (needScratchReg) - { - regNumber tmpReg = treeNode->GetSingleTempReg(); - inst_RV_RV(INS_mov, tmpReg, sourceReg, TYP_LONG); // Move the 64-bit value to a writable temp reg - inst_RV_SH(INS_SHIFT_RIGHT_LOGICAL, srcSize, tmpReg, 32); // Shift right by 32 bits - genJumpToThrowHlpBlk(EJ_jne, SCK_OVERFLOW); // Throw if result shift is non-zero - } - else - { - noway_assert(typeMask != 0); - inst_RV_IV(INS_TEST, sourceReg, typeMask, srcSize); - genJumpToThrowHlpBlk(EJ_jne, SCK_OVERFLOW); - } - } - else - { - // For a narrowing signed cast - // - // We must check the value is in a signed range. - - // Compare with the MAX + // (U)INT to (U)LONG widening cast + assert((srcSize == 4) && (castSize == 8)); - noway_assert((typeMin != 0) && (typeMax != 0)); - - inst_RV_IV(INS_cmp, sourceReg, typeMax, srcSize); - genJumpToThrowHlpBlk(EJ_jg, SCK_OVERFLOW); - - // Compare with the MIN - - inst_RV_IV(INS_cmp, sourceReg, typeMin, srcSize); - genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); - } + ins = cast->IsUnsigned() ? INS_mov : INS_movsxd; + // We need a 32 bit MOV to zero out the upper 32 bit. MOVSXD ignores the size. + insSize = EA_4BYTE; } - - if (targetReg != sourceReg -#ifdef _TARGET_AMD64_ - // On amd64, we can hit this path for a same-register - // 4-byte to 8-byte widening conversion, and need to - // emit the instruction to set the high bits correctly. - || (dstSize == EA_8BYTE && srcSize == EA_4BYTE) -#endif // _TARGET_AMD64_ - ) - inst_RV_RV(ins, targetReg, sourceReg, srcType, srcSize); +#endif } - else // non-overflow checking cast + + if (ins == INS_none) { - // We may have code transformations that result in casts where srcType is the same as dstType. - // e.g. Bug 824281, in which a comma is split by the rationalizer, leaving an assignment of a - // long constant to a long lclVar. - if (srcType == dstType) - { - ins = INS_mov; - } + // If the instruction has not been selected yet it means we're dealing with + // a narrowing/same type/sign changing cast. + assert(castSize <= srcSize); + // Make sure the destination size is correct. This prevents unsupported casts + // such as LONG->INT->LONG, these would be classified as narrowing but in fact + // they're widening casts. It may be useful to allow such casts but that + // requires more work here and throughout the JIT. + assert(dstSize <= srcSize); - if (ins == INS_mov) - { - if (targetReg != sourceReg -#ifdef _TARGET_AMD64_ - // On amd64, 'mov' is the opcode used to zero-extend from - // 4 bytes to 8 bytes. - || (dstSize == EA_8BYTE && srcSize == EA_4BYTE) -#endif // _TARGET_AMD64_ - ) - { - inst_RV_RV(ins, targetReg, sourceReg, srcType, srcSize); - } - } -#ifdef _TARGET_AMD64_ - else if (ins == INS_movsxd) - { - inst_RV_RV(ins, targetReg, sourceReg, srcType, srcSize); - } -#endif // _TARGET_AMD64_ - else - { - noway_assert(ins == INS_movsx || ins == INS_movzx); - noway_assert(srcSize >= dstSize); + // This cast basically does nothing, even when narrowing it is the job of the + // consumer of this node to use the appropiate register size (32 or 64 bit) + // and not rely on the cast to set the upper 32 bits in a certain manner. + // Still, we will need to generate a MOV instruction if the source and destination + // registers are different. + ins = (srcReg != dstReg) ? INS_mov : INS_none; + // We can use either the destination size or the source size. Use the destination + // size as on x64 it may avoid the need for a REX prefix when casting LONG to INT. + insSize = EA_SIZE(dstSize); + } - /* Generate "mov targetReg, castOp->gtReg */ - inst_RV_RV(ins, targetReg, sourceReg, srcType, dstSize); - } + if (ins != INS_none) + { + getEmitter()->emitIns_R_R(ins, insSize, dstReg, srcReg); } - genProduceReg(treeNode); + genProduceReg(cast); } //------------------------------------------------------------------------ From 35e51c884f511a49f1bad6c5706e4fdf8e963ab3 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sun, 5 Aug 2018 12:27:48 +0300 Subject: [PATCH 2/7] Extract cast overflow check ARM codegen to genIntCastOverflowCheck --- src/jit/codegenarmarch.cpp | 184 +++++++++++++++++++++---------------- 1 file changed, 106 insertions(+), 78 deletions(-) diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp index eb53d2b0b71c..9152a3b35fa3 100644 --- a/src/jit/codegenarmarch.cpp +++ b/src/jit/codegenarmarch.cpp @@ -2849,6 +2849,110 @@ void CodeGen::genJmpMethod(GenTree* jmp) } } +//------------------------------------------------------------------------ +// genIntCastOverflowCheck: Generate overflow checking code for an integer cast. +// +// Arguments: +// cast - The GT_CAST node +// sourceReg - The register containing the value to check +// +void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber sourceReg) +{ + const var_types srcType = genActualType(cast->CastOp()->TypeGet()); + const var_types castType = cast->gtCastType; + + emitter* emit = getEmitter(); + + // For Long to Int conversion we will have a reserved integer register to hold the immediate mask + regNumber tmpReg = (cast->AvailableTempRegCount() == 0) ? REG_NA : cast->GetSingleTempReg(); + + Lowering::CastInfo castInfo; + Lowering::getCastDescription(cast, &castInfo); + + assert(castInfo.requiresOverflowCheck); + + emitAttr cmpSize = EA_ATTR(genTypeSize(srcType)); + + if (castInfo.signCheckOnly) + { + // We only need to check for a negative value in sourceReg + emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, 0); + emitJumpKind jmpLT = genJumpKindForOper(GT_LT, CK_SIGNED); + genJumpToThrowHlpBlk(jmpLT, SCK_OVERFLOW); + } + else if (castInfo.unsignedSource || castInfo.unsignedDest) + { + // When we are converting from/to unsigned, + // we only have to check for any bits set in 'typeMask' + + noway_assert(castInfo.typeMask != 0); +#if defined(_TARGET_ARM_) + if (arm_Valid_Imm_For_Instr(INS_tst, castInfo.typeMask, INS_FLAGS_DONT_CARE)) + { + emit->emitIns_R_I(INS_tst, cmpSize, sourceReg, castInfo.typeMask); + } + else + { + noway_assert(tmpReg != REG_NA); + instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMask); + emit->emitIns_R_R(INS_tst, cmpSize, sourceReg, tmpReg); + } +#elif defined(_TARGET_ARM64_) + emit->emitIns_R_I(INS_tst, cmpSize, sourceReg, castInfo.typeMask); +#endif // _TARGET_ARM* + emitJumpKind jmpNotEqual = genJumpKindForOper(GT_NE, CK_SIGNED); + genJumpToThrowHlpBlk(jmpNotEqual, SCK_OVERFLOW); + } + else + { + // For a narrowing signed cast + // + // We must check the value is in a signed range. + + // Compare with the MAX + + noway_assert((castInfo.typeMin != 0) && (castInfo.typeMax != 0)); + +#if defined(_TARGET_ARM_) + if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, INS_FLAGS_DONT_CARE)) +#elif defined(_TARGET_ARM64_) + if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, cmpSize)) +#endif // _TARGET_* + { + emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, castInfo.typeMax); + } + else + { + noway_assert(tmpReg != REG_NA); + instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMax); + emit->emitIns_R_R(INS_cmp, cmpSize, sourceReg, tmpReg); + } + + emitJumpKind jmpGT = genJumpKindForOper(GT_GT, CK_SIGNED); + genJumpToThrowHlpBlk(jmpGT, SCK_OVERFLOW); + +// Compare with the MIN + +#if defined(_TARGET_ARM_) + if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, INS_FLAGS_DONT_CARE)) +#elif defined(_TARGET_ARM64_) + if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, cmpSize)) +#endif // _TARGET_* + { + emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, castInfo.typeMin); + } + else + { + noway_assert(tmpReg != REG_NA); + instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMin); + emit->emitIns_R_R(INS_cmp, cmpSize, sourceReg, tmpReg); + } + + emitJumpKind jmpLT = genJumpKindForOper(GT_LT, CK_SIGNED); + genJumpToThrowHlpBlk(jmpLT, SCK_OVERFLOW); + } +} + //------------------------------------------------------------------------ // genIntToIntCast: Generate code for an integer cast // @@ -2880,9 +2984,6 @@ void CodeGen::genIntToIntCast(GenTree* treeNode) regNumber targetReg = treeNode->gtRegNum; regNumber sourceReg = castOp->gtRegNum; - // For Long to Int conversion we will have a reserved integer register to hold the immediate mask - regNumber tmpReg = (treeNode->AvailableTempRegCount() == 0) ? REG_NA : treeNode->GetSingleTempReg(); - assert(genIsValidIntReg(targetReg)); assert(genIsValidIntReg(sourceReg)); @@ -2894,16 +2995,14 @@ void CodeGen::genIntToIntCast(GenTree* treeNode) if (castInfo.requiresOverflowCheck) { + genIntCastOverflowCheck(treeNode->AsCast(), sourceReg); + bool movRequired = (sourceReg != targetReg); emitAttr movSize = emitActualTypeSize(dstType); - emitAttr cmpSize = EA_ATTR(genTypeSize(srcType)); if (castInfo.signCheckOnly) { // We only need to check for a negative value in sourceReg - emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, 0); - emitJumpKind jmpLT = genJumpKindForOper(GT_LT, CK_SIGNED); - genJumpToThrowHlpBlk(jmpLT, SCK_OVERFLOW); noway_assert(genTypeSize(srcType) == 4 || genTypeSize(srcType) == 8); // This is only interesting case to ensure zero-upper bits. if ((srcType == TYP_INT) && (dstType == TYP_ULONG)) @@ -2915,77 +3014,6 @@ void CodeGen::genIntToIntCast(GenTree* treeNode) movRequired = true; } } - else if (castInfo.unsignedSource || castInfo.unsignedDest) - { - // When we are converting from/to unsigned, - // we only have to check for any bits set in 'typeMask' - - noway_assert(castInfo.typeMask != 0); -#if defined(_TARGET_ARM_) - if (arm_Valid_Imm_For_Instr(INS_tst, castInfo.typeMask, INS_FLAGS_DONT_CARE)) - { - emit->emitIns_R_I(INS_tst, cmpSize, sourceReg, castInfo.typeMask); - } - else - { - noway_assert(tmpReg != REG_NA); - instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMask); - emit->emitIns_R_R(INS_tst, cmpSize, sourceReg, tmpReg); - } -#elif defined(_TARGET_ARM64_) - emit->emitIns_R_I(INS_tst, cmpSize, sourceReg, castInfo.typeMask); -#endif // _TARGET_ARM* - emitJumpKind jmpNotEqual = genJumpKindForOper(GT_NE, CK_SIGNED); - genJumpToThrowHlpBlk(jmpNotEqual, SCK_OVERFLOW); - } - else - { - // For a narrowing signed cast - // - // We must check the value is in a signed range. - - // Compare with the MAX - - noway_assert((castInfo.typeMin != 0) && (castInfo.typeMax != 0)); - -#if defined(_TARGET_ARM_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, INS_FLAGS_DONT_CARE)) -#elif defined(_TARGET_ARM64_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, cmpSize)) -#endif // _TARGET_* - { - emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, castInfo.typeMax); - } - else - { - noway_assert(tmpReg != REG_NA); - instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMax); - emit->emitIns_R_R(INS_cmp, cmpSize, sourceReg, tmpReg); - } - - emitJumpKind jmpGT = genJumpKindForOper(GT_GT, CK_SIGNED); - genJumpToThrowHlpBlk(jmpGT, SCK_OVERFLOW); - -// Compare with the MIN - -#if defined(_TARGET_ARM_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, INS_FLAGS_DONT_CARE)) -#elif defined(_TARGET_ARM64_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, cmpSize)) -#endif // _TARGET_* - { - emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, castInfo.typeMin); - } - else - { - noway_assert(tmpReg != REG_NA); - instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMin); - emit->emitIns_R_R(INS_cmp, cmpSize, sourceReg, tmpReg); - } - - emitJumpKind jmpLT = genJumpKindForOper(GT_LT, CK_SIGNED); - genJumpToThrowHlpBlk(jmpLT, SCK_OVERFLOW); - } if (movRequired) { From c41a0e34a5ea9d850b09f5d5ac3a1a27d1485df5 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sun, 5 Aug 2018 12:44:27 +0300 Subject: [PATCH 3/7] Refactor genIntToIntCast on ARM --- src/jit/codegen.h | 2 +- src/jit/codegenarmarch.cpp | 145 +++++++++++++++++-------------------- src/jit/codegenlinear.cpp | 2 +- src/jit/codegenxarch.cpp | 7 +- 4 files changed, 71 insertions(+), 85 deletions(-) diff --git a/src/jit/codegen.h b/src/jit/codegen.h index 82e89ab93049..7942166e01c0 100644 --- a/src/jit/codegen.h +++ b/src/jit/codegen.h @@ -790,7 +790,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX #endif void genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg); - void genIntToIntCast(GenTree* treeNode); + void genIntToIntCast(GenTreeCast* cast); void genFloatToFloatCast(GenTree* treeNode); void genFloatToIntCast(GenTree* treeNode); void genIntToFloatCast(GenTree* treeNode); diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp index 9152a3b35fa3..b7be7a2576da 100644 --- a/src/jit/codegenarmarch.cpp +++ b/src/jit/codegenarmarch.cpp @@ -2954,126 +2954,113 @@ void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber sourceReg) } //------------------------------------------------------------------------ -// genIntToIntCast: Generate code for an integer cast +// genIntToIntCast: Generate code for an integer cast, with or without overflow check. // // Arguments: -// treeNode - The GT_CAST node -// -// Return Value: -// None. +// cast - The GT_CAST node // // Assumptions: -// The treeNode must have an assigned register. -// For a signed convert from byte, the source must be in a byte-addressable register. +// The cast node is not a contained node and must have an assigned register. // Neither the source nor target type can be a floating point type. // // TODO-ARM64-CQ: Allow castOp to be a contained node without an assigned register. // -void CodeGen::genIntToIntCast(GenTree* treeNode) +void CodeGen::genIntToIntCast(GenTreeCast* cast) { - assert(treeNode->OperGet() == GT_CAST); + genConsumeOperands(cast); - GenTree* castOp = treeNode->gtCast.CastOp(); - emitter* emit = getEmitter(); - - var_types dstType = treeNode->CastToType(); - var_types srcType = genActualType(castOp->TypeGet()); + GenTree* src = cast->gtGetOp1(); - assert(genTypeSize(srcType) <= genTypeSize(TYP_I_IMPL)); + const var_types srcType = genActualType(src->TypeGet()); + const unsigned srcSize = genTypeSize(srcType); + const var_types castType = cast->gtCastType; + const unsigned castSize = genTypeSize(castType); + const var_types dstType = genActualType(cast->TypeGet()); + const unsigned dstSize = genTypeSize(dstType); - regNumber targetReg = treeNode->gtRegNum; - regNumber sourceReg = castOp->gtRegNum; + assert((srcSize == 4) || (srcSize == genTypeSize(TYP_I_IMPL))); + assert((dstSize == 4) || (dstSize == genTypeSize(TYP_I_IMPL))); + assert(castSize <= dstSize); - assert(genIsValidIntReg(targetReg)); - assert(genIsValidIntReg(sourceReg)); + const regNumber srcReg = src->gtRegNum; + const regNumber dstReg = cast->gtRegNum; - genConsumeReg(castOp); - Lowering::CastInfo castInfo; + assert(genIsValidIntReg(srcReg)); + assert(genIsValidIntReg(dstReg)); - // Get information about the cast. - Lowering::getCastDescription(treeNode, &castInfo); + instruction ins = INS_none; + emitAttr insSize; + Lowering::CastInfo castInfo; + Lowering::getCastDescription(cast, &castInfo); if (castInfo.requiresOverflowCheck) { - genIntCastOverflowCheck(treeNode->AsCast(), sourceReg); - - bool movRequired = (sourceReg != targetReg); - emitAttr movSize = emitActualTypeSize(dstType); + genIntCastOverflowCheck(cast, srcReg); - if (castInfo.signCheckOnly) - { - // We only need to check for a negative value in sourceReg - noway_assert(genTypeSize(srcType) == 4 || genTypeSize(srcType) == 8); - // This is only interesting case to ensure zero-upper bits. - if ((srcType == TYP_INT) && (dstType == TYP_ULONG)) - { - // cast to TYP_ULONG: - // We use a mov with size=EA_4BYTE - // which will zero out the upper bits - movSize = EA_4BYTE; - movRequired = true; - } - } - - if (movRequired) +#ifdef _TARGET_64BIT_ + if ((srcType == TYP_INT) && !cast->IsUnsigned() && (castType == TYP_ULONG)) { - emit->emitIns_R_R(INS_mov, movSize, targetReg, sourceReg); + // This is the only overflow checking cast that requires changing the + // source value (by zero extending), all others will copy the value as is. + ins = INS_mov; + insSize = EA_4BYTE; } +#endif } else // Non-overflow checking cast. { - const unsigned srcSize = genTypeSize(srcType); - const unsigned dstSize = genTypeSize(dstType); - instruction ins; - emitAttr insSize; - - if (dstSize < 4) + if (castSize < 4) { + assert((castSize == 1) || (castSize == 2)); + // Casting to a small type really means widening from that small type to INT/LONG. - ins = ins_Move_Extend(dstType, true); - insSize = emitActualTypeSize(treeNode->TypeGet()); + ins = ins_Move_Extend(castType, true); + insSize = EA_ATTR(dstSize); } #ifdef _TARGET_64BIT_ - // dstType cannot be a long type on 32 bit targets, such casts should have been decomposed. + // castType cannot be a long type on 32 bit targets, such casts should have been decomposed. // srcType cannot be a small type since it's the "actual type" of the cast operand. // This means that widening casts do not occur on 32 bit targets. - else if (dstSize > srcSize) + else if (castSize > srcSize) { // (U)INT to (U)LONG widening cast - assert((srcSize == 4) && (dstSize == 8)); - // Make sure the node type has the same size as the destination type. - assert(genTypeSize(treeNode->TypeGet()) == dstSize); + assert((srcSize == 4) && (castSize == 8)); - ins = treeNode->IsUnsigned() ? INS_mov : INS_sxtw; + ins = cast->IsUnsigned() ? INS_mov : INS_sxtw; // SXTW requires EA_8BYTE but MOV requires EA_4BYTE in order to zero out the upper 32 bits. insSize = (ins == INS_sxtw) ? EA_8BYTE : EA_4BYTE; } #endif - else - { - // Sign changing cast or narrowing cast - assert(dstSize <= srcSize); - // Note that narrowing casts are possible only on 64 bit targets. - assert(srcSize <= genTypeSize(TYP_I_IMPL)); - // Make sure the node type has the same size as the destination type. - assert(genTypeSize(treeNode->TypeGet()) == dstSize); + } - // This cast basically does nothing, even when narrowing it is the job of the - // consumer of this node to use the appropiate register size (32 or 64 bit) - // and not rely on the cast to set the upper 32 bits in a certain manner. - // Still, we will need to generate a MOV instruction if the source and target - // registers are different. - ins = (sourceReg != targetReg) ? INS_mov : INS_none; - insSize = EA_SIZE(dstSize); - } + if (ins == INS_none) + { + // If the instruction has not been selected yet it means we're dealing with + // a narrowing/same type/sign changing cast. + assert(castSize <= srcSize); + // Make sure the destination size is correct. This prevents unsupported casts + // such as LONG->INT->LONG, these would be classified as narrowing but in fact + // they're widening casts. It may be useful to allow such casts but that + // requires more work here and throughout the JIT. + assert(dstSize <= srcSize); - if (ins != INS_none) - { - emit->emitIns_R_R(ins, insSize, targetReg, sourceReg); - } + // This cast basically does nothing, even when narrowing it is the job of the + // consumer of this node to use the appropiate register size (32 or 64 bit) + // and not rely on the cast to set the upper 32 bits in a certain manner. + // Still, we will need to generate a MOV instruction if the source and destination + // registers are different. + ins = (srcReg != dstReg) ? INS_mov : INS_none; + // We can use either the destination size or the source size. Use the destination + // size to be consistent with x64 where this may avoid the need for a REX prefix. + insSize = EA_SIZE(dstSize); } - genProduceReg(treeNode); + if (ins != INS_none) + { + getEmitter()->emitIns_R_R(ins, insSize, dstReg, srcReg); + } + + genProduceReg(cast); } //------------------------------------------------------------------------ diff --git a/src/jit/codegenlinear.cpp b/src/jit/codegenlinear.cpp index 9962a627f285..bf229305c660 100644 --- a/src/jit/codegenlinear.cpp +++ b/src/jit/codegenlinear.cpp @@ -1943,7 +1943,7 @@ void CodeGen::genCodeForCast(GenTreeOp* tree) else { // Casts int <--> int - genIntToIntCast(tree); + genIntToIntCast(tree->AsCast()); } // The per-case functions call genProduceReg() } diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp index b1723ebfaab0..352f27bb6d2f 100644 --- a/src/jit/codegenxarch.cpp +++ b/src/jit/codegenxarch.cpp @@ -6433,19 +6433,18 @@ void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) // genIntToIntCast: Generate code for an integer cast, with or without overflow check. // // Arguments: -// treeNode - The GT_CAST node +// cast - The GT_CAST node // // Assumptions: -// The treeNode is not a contained node and must have an assigned register. +// The cast node is not a contained node and must have an assigned register. // Neither the source nor target type can be a floating point type. // On x86 casts to (U)BYTE require that the source be in a byte register. // // TODO-XArch-CQ: Allow castOp to be a contained node without an assigned register. // TODO: refactor to use getCastDescription // -void CodeGen::genIntToIntCast(GenTree* treeNode) +void CodeGen::genIntToIntCast(GenTreeCast* cast) { - GenTreeCast* cast = treeNode->AsCast(); genConsumeOperands(cast); GenTree* src = cast->gtGetOp1(); From 763477bcae7b2de2264bba1b43cb1c11eb4877fd Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sun, 5 Aug 2018 16:36:03 +0300 Subject: [PATCH 4/7] Improve ARM genIntCastOverflowCheck --- src/jit/codegenarmarch.cpp | 139 ++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 73 deletions(-) diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp index b7be7a2576da..c59edda0d57c 100644 --- a/src/jit/codegenarmarch.cpp +++ b/src/jit/codegenarmarch.cpp @@ -2854,102 +2854,95 @@ void CodeGen::genJmpMethod(GenTree* jmp) // // Arguments: // cast - The GT_CAST node -// sourceReg - The register containing the value to check +// reg - The register containing the value to check // -void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber sourceReg) +void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) { - const var_types srcType = genActualType(cast->CastOp()->TypeGet()); - const var_types castType = cast->gtCastType; - - emitter* emit = getEmitter(); - - // For Long to Int conversion we will have a reserved integer register to hold the immediate mask - regNumber tmpReg = (cast->AvailableTempRegCount() == 0) ? REG_NA : cast->GetSingleTempReg(); - - Lowering::CastInfo castInfo; - Lowering::getCastDescription(cast, &castInfo); - - assert(castInfo.requiresOverflowCheck); + const var_types srcType = genActualType(cast->gtGetOp1()->TypeGet()); + const bool srcUnsigned = cast->IsUnsigned(); + const unsigned srcSize = genTypeSize(srcType); + const var_types castType = cast->gtCastType; + const bool castUnsigned = varTypeIsUnsigned(castType); + const unsigned castSize = genTypeSize(castType); - emitAttr cmpSize = EA_ATTR(genTypeSize(srcType)); - - if (castInfo.signCheckOnly) + if (castSize >= srcSize) { - // We only need to check for a negative value in sourceReg - emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, 0); - emitJumpKind jmpLT = genJumpKindForOper(GT_LT, CK_SIGNED); - genJumpToThrowHlpBlk(jmpLT, SCK_OVERFLOW); + // It's either a widening cast from a signed type to an unsigned + // type or a sign changing cast. + assert((castSize > srcSize) ? (!srcUnsigned && castUnsigned) : (srcUnsigned != castUnsigned)); + + // In both cases we just need to check if the value is positive. + getEmitter()->emitIns_R_I(INS_cmp, EA_ATTR(srcSize), reg, 0); + genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); } - else if (castInfo.unsignedSource || castInfo.unsignedDest) +#ifdef _TARGET_64BIT_ + // We handled widening and sign changing casts so we're left with narrowing casts. + // Handle (U)LONG to (U)INT casts first as they're 64 bit target specific and there + // are some complications related to encoding of the large immediate values they need. + else if (castSize == 4) { - // When we are converting from/to unsigned, - // we only have to check for any bits set in 'typeMask' + assert(srcSize == 8); - noway_assert(castInfo.typeMask != 0); -#if defined(_TARGET_ARM_) - if (arm_Valid_Imm_For_Instr(INS_tst, castInfo.typeMask, INS_FLAGS_DONT_CARE)) + if (castUnsigned) // (U)LONG to UINT cast { - emit->emitIns_R_I(INS_tst, cmpSize, sourceReg, castInfo.typeMask); + // We need to check if the value is not greater than 0xFFFFFFFF but this value + // cannot be encoded in the immediate operand of CMP. Use TST instead to check + // if the upper 32 bits are zero. + getEmitter()->emitIns_R_I(INS_tst, EA_8BYTE, reg, 0xFFFFFFFF00000000LL); + genJumpToThrowHlpBlk(EJ_ne, SCK_OVERFLOW); } - else + else if (srcUnsigned) // ULONG to INT cast { - noway_assert(tmpReg != REG_NA); - instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMask); - emit->emitIns_R_R(INS_tst, cmpSize, sourceReg, tmpReg); + // We need to check if the value is not greater than 0x7FFFFFFF but this value + // cannot be encoded in the immediate operand of CMP. Use TST instead to check + // if the upper 33 bits are zero. + getEmitter()->emitIns_R_I(INS_tst, EA_8BYTE, reg, 0xFFFFFFFF80000000LL); + genJumpToThrowHlpBlk(EJ_ne, SCK_OVERFLOW); + } + else // LONG to INT cast + { + const regNumber tempReg = cast->GetSingleTempReg(); + assert(tempReg != reg); + instGen_Set_Reg_To_Imm(EA_8BYTE, tempReg, INT32_MAX); + getEmitter()->emitIns_R_R(INS_cmp, EA_8BYTE, reg, tempReg); + genJumpToThrowHlpBlk(EJ_gt, SCK_OVERFLOW); + instGen_Set_Reg_To_Imm(EA_8BYTE, tempReg, INT32_MIN); + getEmitter()->emitIns_R_R(INS_cmp, EA_8BYTE, reg, tempReg); + genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); } -#elif defined(_TARGET_ARM64_) - emit->emitIns_R_I(INS_tst, cmpSize, sourceReg, castInfo.typeMask); -#endif // _TARGET_ARM* - emitJumpKind jmpNotEqual = genJumpKindForOper(GT_NE, CK_SIGNED); - genJumpToThrowHlpBlk(jmpNotEqual, SCK_OVERFLOW); } - else +#endif + else // if (castSize < srcSize) { - // For a narrowing signed cast - // - // We must check the value is in a signed range. - - // Compare with the MAX + // This is a narrowing cast. We already handled (U)INT so what's left are small int types. + assert(castSize < 4); - noway_assert((castInfo.typeMin != 0) && (castInfo.typeMax != 0)); + // This happens to allow for easy compututation of the min and max + // values of the castType without risk of integer overflow. + const int castNumBits = (castSize * 8) - (castUnsigned ? 0 : 1); + const int castMaxValue = (1 << castNumBits) - 1; + const int castMinValue = (castUnsigned | srcUnsigned) ? 0 : (-castMaxValue - 1); -#if defined(_TARGET_ARM_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, INS_FLAGS_DONT_CARE)) -#elif defined(_TARGET_ARM64_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, cmpSize)) -#endif // _TARGET_* + // These values cannot be encoded in the immediate operand of CMP, + // replace (x > max) with (x >= max + 1) where max + 1 can be encoded. + // We could do this for all max values but on ARM32 "cmp r0, 255" + // is better than "cmp r0, 256" because it has a shorter encoding. + if ((castMaxValue == 32767) || (castMaxValue == 65535)) { - emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, castInfo.typeMax); + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMaxValue + 1); + genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_hs : EJ_ge, SCK_OVERFLOW); } else { - noway_assert(tmpReg != REG_NA); - instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMax); - emit->emitIns_R_R(INS_cmp, cmpSize, sourceReg, tmpReg); + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMaxValue); + genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_hi : EJ_gt, SCK_OVERFLOW); } - emitJumpKind jmpGT = genJumpKindForOper(GT_GT, CK_SIGNED); - genJumpToThrowHlpBlk(jmpGT, SCK_OVERFLOW); - -// Compare with the MIN - -#if defined(_TARGET_ARM_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, INS_FLAGS_DONT_CARE)) -#elif defined(_TARGET_ARM64_) - if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, cmpSize)) -#endif // _TARGET_* + if (castMinValue != 0) { - emit->emitIns_R_I(INS_cmp, cmpSize, sourceReg, castInfo.typeMin); + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMinValue); + genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); } - else - { - noway_assert(tmpReg != REG_NA); - instGen_Set_Reg_To_Imm(cmpSize, tmpReg, castInfo.typeMin); - emit->emitIns_R_R(INS_cmp, cmpSize, sourceReg, tmpReg); - } - - emitJumpKind jmpLT = genJumpKindForOper(GT_LT, CK_SIGNED); - genJumpToThrowHlpBlk(jmpLT, SCK_OVERFLOW); } } From 0959db249a11d5eda4f4f4166707e8e1a925169e Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sun, 5 Aug 2018 10:33:27 +0300 Subject: [PATCH 5/7] Cleanup LinearScan::BuildCast --- src/jit/lsra.h | 2 +- src/jit/lsraarm.cpp | 72 ++--------------------------------------- src/jit/lsraarm64.cpp | 48 ++------------------------- src/jit/lsraarmarch.cpp | 40 +++++++++++++++++++++++ src/jit/lsraxarch.cpp | 53 ++++++++++++------------------ 5 files changed, 66 insertions(+), 149 deletions(-) diff --git a/src/jit/lsra.h b/src/jit/lsra.h index eddeee91b459..ca772bdfeef4 100644 --- a/src/jit/lsra.h +++ b/src/jit/lsra.h @@ -1578,7 +1578,7 @@ class LinearScan : public LinearScanInterface int BuildStoreLoc(GenTreeLclVarCommon* tree); int BuildIndir(GenTreeIndir* indirTree); int BuildGCWriteBarrier(GenTree* tree); - int BuildCast(GenTree* tree); + int BuildCast(GenTreeCast* cast); #if defined(_TARGET_XARCH_) // returns true if the tree can use the read-modify-write memory instruction form diff --git a/src/jit/lsraarm.cpp b/src/jit/lsraarm.cpp index 00496687c69a..980b2a3d6493 100644 --- a/src/jit/lsraarm.cpp +++ b/src/jit/lsraarm.cpp @@ -287,77 +287,9 @@ int LinearScan::BuildNode(GenTree* tree) break; case GT_CAST: - { assert(dstCount == 1); - - // Non-overflow casts to/from float/double are done using SSE2 instructions - // and that allow the source operand to be either a reg or memop. Given the - // fact that casts from small int to float/double are done as two-level casts, - // the source operand is always guaranteed to be of size 4 or 8 bytes. - var_types castToType = tree->CastToType(); - GenTree* castOp = tree->gtCast.CastOp(); - var_types castOpType = castOp->TypeGet(); - if (tree->gtFlags & GTF_UNSIGNED) - { - castOpType = genUnsignedType(castOpType); - } - - if (varTypeIsLong(castOpType)) - { - assert((castOp->OperGet() == GT_LONG) && castOp->isContained()); - } - - // FloatToIntCast needs a temporary register - if (varTypeIsFloating(castOpType) && varTypeIsIntOrI(tree)) - { - buildInternalFloatRegisterDefForNode(tree, RBM_ALLFLOAT); - setInternalRegsDelayFree = true; - } - - Lowering::CastInfo castInfo; - - // Get information about the cast. - Lowering::getCastDescription(tree, &castInfo); - - if (castInfo.requiresOverflowCheck) - { - var_types srcType = castOp->TypeGet(); - emitAttr cmpSize = EA_ATTR(genTypeSize(srcType)); - - // If we cannot store data in an immediate for instructions, - // then we will need to reserve a temporary register. - - if (!castInfo.signCheckOnly) // In case of only sign check, temp regs are not needeed. - { - if (castInfo.unsignedSource || castInfo.unsignedDest) - { - // check typeMask - bool canStoreTypeMask = emitter::emitIns_valid_imm_for_alu(castInfo.typeMask); - if (!canStoreTypeMask) - { - buildInternalIntRegisterDefForNode(tree); - } - } - else - { - // For comparing against the max or min value - bool canStoreMaxValue = - emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, INS_FLAGS_DONT_CARE); - bool canStoreMinValue = - emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, INS_FLAGS_DONT_CARE); - - if (!canStoreMaxValue || !canStoreMinValue) - { - buildInternalIntRegisterDefForNode(tree); - } - } - } - } - srcCount = BuildOperandUses(castOp); - buildInternalRegisterUses(); - BuildDef(tree); - } - break; + srcCount = BuildCast(tree->AsCast()); + break; case GT_JTRUE: srcCount = 0; diff --git a/src/jit/lsraarm64.cpp b/src/jit/lsraarm64.cpp index 13b45a943e55..76b56b368896 100644 --- a/src/jit/lsraarm64.cpp +++ b/src/jit/lsraarm64.cpp @@ -335,53 +335,9 @@ int LinearScan::BuildNode(GenTree* tree) #endif // FEATURE_HW_INTRINSICS case GT_CAST: - { - // TODO-ARM64-CQ: Int-To-Int conversions - castOp cannot be a memory op and must have an assigned - // register. - // see CodeGen::genIntToIntCast() - - // Non-overflow casts to/from float/double are done using SSE2 instructions - // and that allow the source operand to be either a reg or memop. Given the - // fact that casts from small int to float/double are done as two-level casts, - // the source operand is always guaranteed to be of size 4 or 8 bytes. - var_types castToType = tree->CastToType(); - GenTree* castOp = tree->gtCast.CastOp(); - var_types castOpType = castOp->TypeGet(); - if (tree->gtFlags & GTF_UNSIGNED) - { - castOpType = genUnsignedType(castOpType); - } - - // Some overflow checks need a temp reg - - Lowering::CastInfo castInfo; - // Get information about the cast. - Lowering::getCastDescription(tree, &castInfo); - - if (castInfo.requiresOverflowCheck) - { - var_types srcType = castOp->TypeGet(); - emitAttr cmpSize = EA_ATTR(genTypeSize(srcType)); - - // If we cannot store the comparisons in an immediate for either - // comparing against the max or min value, then we will need to - // reserve a temporary register. - - bool canStoreMaxValue = emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, cmpSize); - bool canStoreMinValue = emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, cmpSize); - - if (!canStoreMaxValue || !canStoreMinValue) - { - buildInternalIntRegisterDefForNode(tree); - } - } - BuildUse(tree->gtGetOp1()); - srcCount = 1; - buildInternalRegisterUses(); assert(dstCount == 1); - BuildDef(tree); - } - break; + srcCount = BuildCast(tree->AsCast()); + break; case GT_NEG: case GT_NOT: diff --git a/src/jit/lsraarmarch.cpp b/src/jit/lsraarmarch.cpp index 87c0991ed518..aee569861522 100644 --- a/src/jit/lsraarmarch.cpp +++ b/src/jit/lsraarmarch.cpp @@ -716,4 +716,44 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode) return srcCount; } +//------------------------------------------------------------------------ +// BuildCast: Set the NodeInfo for a GT_CAST. +// +// Arguments: +// cast - The GT_CAST node +// +// Return Value: +// The number of sources consumed by this node. +// +int LinearScan::BuildCast(GenTreeCast* cast) +{ + GenTree* src = cast->gtGetOp1(); + + const var_types srcType = genActualType(src->TypeGet()); + const var_types castType = cast->gtCastType; + +#ifdef _TARGET_ARM_ + assert(!varTypeIsLong(srcType) || (src->OperIs(GT_LONG) && src->isContained())); + + // Floating point to integer casts requires a temporary register. + if (varTypeIsFloating(srcType) && !varTypeIsFloating(castType)) + { + buildInternalFloatRegisterDefForNode(cast, RBM_ALLFLOAT); + setInternalRegsDelayFree = true; + } +#else + // Overflow checking cast from TYP_LONG to TYP_INT requires a temporary register to + // store the min and max immediate values that cannot be encoded in the CMP instruction. + if (cast->gtOverflow() && varTypeIsLong(srcType) && !cast->IsUnsigned() && (castType == TYP_INT)) + { + buildInternalIntRegisterDefForNode(cast); + } +#endif + + int srcCount = BuildOperandUses(src); + buildInternalRegisterUses(); + BuildDef(cast); + return srcCount; +} + #endif // _TARGET_ARMARCH_ diff --git a/src/jit/lsraxarch.cpp b/src/jit/lsraxarch.cpp index c64ea11ac330..b131123c423b 100644 --- a/src/jit/lsraxarch.cpp +++ b/src/jit/lsraxarch.cpp @@ -354,7 +354,8 @@ int LinearScan::BuildNode(GenTree* tree) #endif // FEATURE_HW_INTRINSICS case GT_CAST: - srcCount = BuildCast(tree); + assert(dstCount == 1); + srcCount = BuildCast(tree->AsCast()); break; case GT_BITCAST: @@ -2684,52 +2685,40 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree) // BuildCast: Set the NodeInfo for a GT_CAST. // // Arguments: -// tree - The node of interest +// cast - The GT_CAST node // // Return Value: // The number of sources consumed by this node. // -int LinearScan::BuildCast(GenTree* tree) +int LinearScan::BuildCast(GenTreeCast* cast) { - // TODO-XArch-CQ: Int-To-Int conversions - castOp cannot be a memory op and must have an assigned register. - // see CodeGen::genIntToIntCast() - - // Non-overflow casts to/from float/double are done using SSE2 instructions - // and that allow the source operand to be either a reg or memop. Given the - // fact that casts from small int to float/double are done as two-level casts, - // the source operand is always guaranteed to be of size 4 or 8 bytes. - var_types castToType = tree->CastToType(); - GenTree* castOp = tree->gtCast.CastOp(); - var_types castOpType = castOp->TypeGet(); - regMaskTP candidates = RBM_NONE; + GenTree* src = cast->gtGetOp1(); - if (tree->gtFlags & GTF_UNSIGNED) - { - castOpType = genUnsignedType(castOpType); - } + const var_types srcType = genActualType(src->TypeGet()); + const var_types castType = cast->gtCastType; + regMaskTP candidates = RBM_NONE; #ifdef _TARGET_X86_ - if (varTypeIsByte(castToType)) + if (varTypeIsByte(castType)) { candidates = allByteRegs(); } -#endif // _TARGET_X86_ - // some overflow checks need a temp reg: - // - GT_CAST from INT64/UINT64 to UINT32 - RefPosition* internalDef = nullptr; - if (tree->gtOverflow() && (castToType == TYP_UINT)) + assert(!varTypeIsLong(srcType) || (src->OperIs(GT_LONG) && src->isContained())); +#else + // Overflow checking cast from TYP_(U)LONG to TYP_UINT requires a temporary + // register to extract the upper 32 bits of the 64 bit source register. + if (cast->gtOverflow() && varTypeIsLong(srcType) && (castType == TYP_UINT)) { - if (genTypeSize(castOpType) == 8) - { - // Here we don't need internal register to be different from targetReg, - // rather require it to be different from operand's reg. - buildInternalIntRegisterDefForNode(tree); - } + // Here we don't need internal register to be different from targetReg, + // rather require it to be different from operand's reg. + buildInternalIntRegisterDefForNode(cast); } - int srcCount = BuildOperandUses(castOp, candidates); +#endif + + int srcCount = BuildOperandUses(src, candidates); buildInternalRegisterUses(); - BuildDef(tree, candidates); + BuildDef(cast, candidates); return srcCount; } From 43c5f23bf1b3d697ebe7178c612717a194cffdfe Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Sun, 5 Aug 2018 19:53:50 +0300 Subject: [PATCH 6/7] Get rid of Lowering::getCastDescription --- src/jit/codegenarmarch.cpp | 6 +- src/jit/codegenxarch.cpp | 7 +-- src/jit/gentree.cpp | 40 +++++++++++++ src/jit/gentree.h | 3 + src/jit/lower.cpp | 115 ------------------------------------- src/jit/lower.h | 17 ------ 6 files changed, 49 insertions(+), 139 deletions(-) diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp index c59edda0d57c..b3a2ee2a35bc 100644 --- a/src/jit/codegenarmarch.cpp +++ b/src/jit/codegenarmarch.cpp @@ -2858,6 +2858,8 @@ void CodeGen::genJmpMethod(GenTree* jmp) // void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) { + assert(cast->IsOverflowCheckRequired()); + const var_types srcType = genActualType(cast->gtGetOp1()->TypeGet()); const bool srcUnsigned = cast->IsUnsigned(); const unsigned srcSize = genTypeSize(srcType); @@ -2984,9 +2986,7 @@ void CodeGen::genIntToIntCast(GenTreeCast* cast) instruction ins = INS_none; emitAttr insSize; - Lowering::CastInfo castInfo; - Lowering::getCastDescription(cast, &castInfo); - if (castInfo.requiresOverflowCheck) + if (cast->IsOverflowCheckRequired()) { genIntCastOverflowCheck(cast, srcReg); diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp index 352f27bb6d2f..555c55028db6 100644 --- a/src/jit/codegenxarch.cpp +++ b/src/jit/codegenxarch.cpp @@ -6353,6 +6353,8 @@ void CodeGen::genLongToIntCast(GenTree* cast) // void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) { + assert(cast->IsOverflowCheckRequired()); + const var_types srcType = genActualType(cast->gtGetOp1()->TypeGet()); const bool srcUnsigned = cast->IsUnsigned(); const unsigned srcSize = genTypeSize(srcType); @@ -6441,7 +6443,6 @@ void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) // On x86 casts to (U)BYTE require that the source be in a byte register. // // TODO-XArch-CQ: Allow castOp to be a contained node without an assigned register. -// TODO: refactor to use getCastDescription // void CodeGen::genIntToIntCast(GenTreeCast* cast) { @@ -6469,9 +6470,7 @@ void CodeGen::genIntToIntCast(GenTreeCast* cast) instruction ins = INS_none; emitAttr insSize; - Lowering::CastInfo castInfo; - Lowering::getCastDescription(cast, &castInfo); - if (castInfo.requiresOverflowCheck) + if (cast->IsOverflowCheckRequired()) { genIntCastOverflowCheck(cast, srcReg); diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 94152eae5955..3504aee79f73 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -17940,3 +17940,43 @@ regNumber GenTree::ExtractTempReg(regMaskTP mask /* = (regMaskTP)-1 */) gtRsvdRegs &= ~tempRegMask; return genRegNumFromMask(tempRegMask); } + +//------------------------------------------------------------------------ +// IsOverflowCheckRequired: Check if this cast node requires an overflow check. +// +// Notes: +// Unlike gtOverflow, this function returns true only for casts that actually +// require an overflow check: +// - narrowing casts (e.g. TYP_INT -> TYP_SHORT, TYP_LONG -> TYP_INT) +// - sign changing casts (e.g. TYP_INT -> TYP_UINT, TYP_ULONG -> TYP_LONG) +// - widening from signed to unsigned casts (only TYP_INT -> TYP_ULONG) +// +bool GenTreeCast::IsOverflowCheckRequired() const +{ + if (!gtOverflow()) + { + return false; + } + + const var_types srcType = genActualType(gtGetOp1()->TypeGet()); + const bool srcUnsigned = IsUnsigned(); + const unsigned srcSize = genTypeSize(srcType); + const var_types castType = gtCastType; + const bool castUnsigned = varTypeIsUnsigned(castType); + const unsigned castSize = genTypeSize(castType); + + if (castSize > srcSize) + { + // Overflow check required for widening from signed to unsigned. + return !srcUnsigned && castUnsigned; + } + + if (castSize == srcSize) + { + // Overflow check required for any sign changing cast. + return srcUnsigned != castUnsigned; + } + + // Overflow check required for any narrowing cast. + return true; +} diff --git a/src/jit/gentree.h b/src/jit/gentree.h index ef86c5d8a3ff..24f52f4cc470 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -2901,6 +2901,9 @@ struct GenTreeCast : public GenTreeOp { } #endif + + // Returns true for casts that require an overflow check. + bool IsOverflowCheckRequired() const; }; // GT_BOX nodes are place markers for boxed values. The "real" tree diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index 0d0f4804d2c5..fb5fab276314 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -5519,121 +5519,6 @@ bool Lowering::NodesAreEquivalentLeaves(GenTree* tree1, GenTree* tree2) } } -/** - * Get common information required to handle a cast instruction - */ -void Lowering::getCastDescription(GenTree* treeNode, CastInfo* castInfo) -{ - // Intialize castInfo - memset(castInfo, 0, sizeof(*castInfo)); - - GenTree* castOp = treeNode->gtCast.CastOp(); - - var_types dstType = treeNode->CastToType(); - var_types srcType = genActualType(castOp->TypeGet()); - - castInfo->unsignedDest = varTypeIsUnsigned(dstType); - castInfo->unsignedSource = varTypeIsUnsigned(srcType); - - // If necessary, force the srcType to unsigned when the GT_UNSIGNED flag is set. - if (!castInfo->unsignedSource && (treeNode->gtFlags & GTF_UNSIGNED) != 0) - { - srcType = genUnsignedType(srcType); - castInfo->unsignedSource = true; - } - - if (treeNode->gtOverflow() && - (genTypeSize(srcType) >= genTypeSize(dstType) || (srcType == TYP_INT && dstType == TYP_ULONG))) - { - castInfo->requiresOverflowCheck = true; - } - - if (castInfo->requiresOverflowCheck) - { - ssize_t typeMin = 0; - ssize_t typeMax = 0; - ssize_t typeMask = 0; - bool signCheckOnly = false; - - // Do we need to compare the value, or just check masks - switch (dstType) - { - default: - assert(!"unreachable: getCastDescription"); - break; - - case TYP_BYTE: - typeMask = ssize_t((int)0xFFFFFF80); - typeMin = SCHAR_MIN; - typeMax = SCHAR_MAX; - break; - - case TYP_UBYTE: - typeMask = ssize_t((int)0xFFFFFF00L); - break; - - case TYP_SHORT: - typeMask = ssize_t((int)0xFFFF8000); - typeMin = SHRT_MIN; - typeMax = SHRT_MAX; - break; - - case TYP_USHORT: - typeMask = ssize_t((int)0xFFFF0000L); - break; - - case TYP_INT: - if (srcType == TYP_UINT) - { - signCheckOnly = true; - } - else - { -#ifdef _TARGET_64BIT_ - typeMask = 0xFFFFFFFF80000000LL; -#else - typeMask = 0x80000000; -#endif - typeMin = INT_MIN; - typeMax = INT_MAX; - } - break; - - case TYP_UINT: - if (srcType == TYP_INT) - { - signCheckOnly = true; - } - else - { -#ifdef _TARGET_64BIT_ - typeMask = 0xFFFFFFFF00000000LL; -#else - typeMask = 0x00000000; -#endif - } - break; - - case TYP_LONG: - signCheckOnly = true; - break; - - case TYP_ULONG: - signCheckOnly = true; - break; - } - - if (signCheckOnly) - { - castInfo->signCheckOnly = true; - } - - castInfo->typeMax = typeMax; - castInfo->typeMin = typeMin; - castInfo->typeMask = typeMask; - } -} - //------------------------------------------------------------------------ // Containment Analysis //------------------------------------------------------------------------ diff --git a/src/jit/lower.h b/src/jit/lower.h index 656e1773245f..c78e26be2cca 100644 --- a/src/jit/lower.h +++ b/src/jit/lower.h @@ -30,23 +30,6 @@ class Lowering : public Phase } virtual void DoPhase() override; - // If requiresOverflowCheck is false, all other values will be unset - struct CastInfo - { - bool requiresOverflowCheck; // Will the cast require an overflow check - bool unsignedSource; // Is the source unsigned - bool unsignedDest; // is the dest unsigned - - // All other fields are only meaningful if requiresOverflowCheck is set. - - ssize_t typeMin; // Lowest storable value of the dest type - ssize_t typeMax; // Highest storable value of the dest type - ssize_t typeMask; // For converting from/to unsigned - bool signCheckOnly; // For converting between unsigned/signed int - }; - - static void getCastDescription(GenTree* treeNode, CastInfo* castInfo); - // This variant of LowerRange is called from outside of the main Lowering pass, // so it creates its own instance of Lowering to do so. void LowerRange(BasicBlock* block, LIR::ReadOnlyRange& range) From 38a5f91a3e11ecf4d7441a5ce05bb94b6edaf9c3 Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Tue, 7 Aug 2018 22:16:51 +0300 Subject: [PATCH 7/7] Make cast classification arch independent --- src/jit/codegen.h | 72 ++++++++++++- src/jit/codegenarmarch.cpp | 210 +++++++++++++------------------------ src/jit/codegenlinear.cpp | 118 +++++++++++++++++++++ src/jit/codegenxarch.cpp | 189 +++++++++++---------------------- src/jit/gentree.cpp | 40 ------- src/jit/gentree.h | 3 - 6 files changed, 326 insertions(+), 306 deletions(-) diff --git a/src/jit/codegen.h b/src/jit/codegen.h index 7942166e01c0..0523d53b0188 100644 --- a/src/jit/codegen.h +++ b/src/jit/codegen.h @@ -789,7 +789,77 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genLongToIntCast(GenTree* treeNode); #endif - void genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg); + struct GenIntCastDesc + { + enum CheckKind + { + CHECK_NONE, + CHECK_SMALL_INT_RANGE, + CHECK_POSITIVE, +#ifdef _TARGET_64BIT_ + CHECK_UINT_RANGE, + CHECK_POSITIVE_INT_RANGE, + CHECK_INT_RANGE, +#endif + }; + + enum ExtendKind + { + COPY, + ZERO_EXTEND_SMALL_INT, + SIGN_EXTEND_SMALL_INT, +#ifdef _TARGET_64BIT_ + ZERO_EXTEND_INT, + SIGN_EXTEND_INT, +#endif + }; + + private: + CheckKind m_checkKind; + unsigned m_checkSrcSize; + int m_checkSmallIntMin; + int m_checkSmallIntMax; + ExtendKind m_extendKind; + unsigned m_extendSrcSize; + + public: + GenIntCastDesc(GenTreeCast* cast); + + CheckKind CheckKind() const + { + return m_checkKind; + } + + unsigned CheckSrcSize() const + { + assert(m_checkKind != CHECK_NONE); + return m_checkSrcSize; + } + + int CheckSmallIntMin() const + { + assert(m_checkKind == CHECK_SMALL_INT_RANGE); + return m_checkSmallIntMin; + } + + int CheckSmallIntMax() const + { + assert(m_checkKind == CHECK_SMALL_INT_RANGE); + return m_checkSmallIntMax; + } + + ExtendKind ExtendKind() const + { + return m_extendKind; + } + + unsigned ExtendSrcSize() const + { + return m_extendSrcSize; + } + }; + + void genIntCastOverflowCheck(GenTreeCast* cast, const GenIntCastDesc& desc, regNumber reg); void genIntToIntCast(GenTreeCast* cast); void genFloatToFloatCast(GenTree* treeNode); void genFloatToIntCast(GenTree* treeNode); diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp index b3a2ee2a35bc..746636a69dc4 100644 --- a/src/jit/codegenarmarch.cpp +++ b/src/jit/codegenarmarch.cpp @@ -2854,54 +2854,36 @@ void CodeGen::genJmpMethod(GenTree* jmp) // // Arguments: // cast - The GT_CAST node +// desc - The cast description // reg - The register containing the value to check // -void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) +void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, const GenIntCastDesc& desc, regNumber reg) { - assert(cast->IsOverflowCheckRequired()); - - const var_types srcType = genActualType(cast->gtGetOp1()->TypeGet()); - const bool srcUnsigned = cast->IsUnsigned(); - const unsigned srcSize = genTypeSize(srcType); - const var_types castType = cast->gtCastType; - const bool castUnsigned = varTypeIsUnsigned(castType); - const unsigned castSize = genTypeSize(castType); - - if (castSize >= srcSize) + switch (desc.CheckKind()) { - // It's either a widening cast from a signed type to an unsigned - // type or a sign changing cast. - assert((castSize > srcSize) ? (!srcUnsigned && castUnsigned) : (srcUnsigned != castUnsigned)); + case GenIntCastDesc::CHECK_POSITIVE: + getEmitter()->emitIns_R_I(INS_cmp, EA_ATTR(desc.CheckSrcSize()), reg, 0); + genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); + break; - // In both cases we just need to check if the value is positive. - getEmitter()->emitIns_R_I(INS_cmp, EA_ATTR(srcSize), reg, 0); - genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); - } #ifdef _TARGET_64BIT_ - // We handled widening and sign changing casts so we're left with narrowing casts. - // Handle (U)LONG to (U)INT casts first as they're 64 bit target specific and there - // are some complications related to encoding of the large immediate values they need. - else if (castSize == 4) - { - assert(srcSize == 8); - - if (castUnsigned) // (U)LONG to UINT cast - { + case GenIntCastDesc::CHECK_UINT_RANGE: // We need to check if the value is not greater than 0xFFFFFFFF but this value // cannot be encoded in the immediate operand of CMP. Use TST instead to check // if the upper 32 bits are zero. getEmitter()->emitIns_R_I(INS_tst, EA_8BYTE, reg, 0xFFFFFFFF00000000LL); genJumpToThrowHlpBlk(EJ_ne, SCK_OVERFLOW); - } - else if (srcUnsigned) // ULONG to INT cast - { + break; + + case GenIntCastDesc::CHECK_POSITIVE_INT_RANGE: // We need to check if the value is not greater than 0x7FFFFFFF but this value // cannot be encoded in the immediate operand of CMP. Use TST instead to check // if the upper 33 bits are zero. getEmitter()->emitIns_R_I(INS_tst, EA_8BYTE, reg, 0xFFFFFFFF80000000LL); genJumpToThrowHlpBlk(EJ_ne, SCK_OVERFLOW); - } - else // LONG to INT cast + break; + + case GenIntCastDesc::CHECK_INT_RANGE: { const regNumber tempReg = cast->GetSingleTempReg(); assert(tempReg != reg); @@ -2912,39 +2894,38 @@ void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) getEmitter()->emitIns_R_R(INS_cmp, EA_8BYTE, reg, tempReg); genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); } - } + break; #endif - else // if (castSize < srcSize) - { - // This is a narrowing cast. We already handled (U)INT so what's left are small int types. - assert(castSize < 4); - - // This happens to allow for easy compututation of the min and max - // values of the castType without risk of integer overflow. - const int castNumBits = (castSize * 8) - (castUnsigned ? 0 : 1); - const int castMaxValue = (1 << castNumBits) - 1; - const int castMinValue = (castUnsigned | srcUnsigned) ? 0 : (-castMaxValue - 1); - // These values cannot be encoded in the immediate operand of CMP, - // replace (x > max) with (x >= max + 1) where max + 1 can be encoded. - // We could do this for all max values but on ARM32 "cmp r0, 255" - // is better than "cmp r0, 256" because it has a shorter encoding. - if ((castMaxValue == 32767) || (castMaxValue == 65535)) - { - getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMaxValue + 1); - genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_hs : EJ_ge, SCK_OVERFLOW); - } - else + default: { - getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMaxValue); - genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_hi : EJ_gt, SCK_OVERFLOW); - } + assert(desc.CheckKind() == GenIntCastDesc::CHECK_SMALL_INT_RANGE); + const int castMaxValue = desc.CheckSmallIntMax(); + const int castMinValue = desc.CheckSmallIntMin(); - if (castMinValue != 0) - { - getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMinValue); - genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); + // Values greater than 255 cannot be encoded in the immediate operand of CMP. + // Replace (x > max) with (x >= max + 1) where max + 1 (a power of 2) can be + // encoded. We could do this for all max values but on ARM32 "cmp r0, 255" + // is better than "cmp r0, 256" because it has a shorter encoding. + if (castMaxValue > 255) + { + assert((castMaxValue == 32767) || (castMaxValue == 65535)); + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(desc.CheckSrcSize()), reg, castMaxValue + 1); + genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_hs : EJ_ge, SCK_OVERFLOW); + } + else + { + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(desc.CheckSrcSize()), reg, castMaxValue); + genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_hi : EJ_gt, SCK_OVERFLOW); + } + + if (castMinValue != 0) + { + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(desc.CheckSrcSize()), reg, castMinValue); + genJumpToThrowHlpBlk(EJ_lt, SCK_OVERFLOW); + } } + break; } } @@ -2962,95 +2943,54 @@ void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) // void CodeGen::genIntToIntCast(GenTreeCast* cast) { - genConsumeOperands(cast); - - GenTree* src = cast->gtGetOp1(); + genConsumeRegs(cast->gtGetOp1()); - const var_types srcType = genActualType(src->TypeGet()); - const unsigned srcSize = genTypeSize(srcType); - const var_types castType = cast->gtCastType; - const unsigned castSize = genTypeSize(castType); - const var_types dstType = genActualType(cast->TypeGet()); - const unsigned dstSize = genTypeSize(dstType); - - assert((srcSize == 4) || (srcSize == genTypeSize(TYP_I_IMPL))); - assert((dstSize == 4) || (dstSize == genTypeSize(TYP_I_IMPL))); - assert(castSize <= dstSize); - - const regNumber srcReg = src->gtRegNum; + const regNumber srcReg = cast->gtGetOp1()->gtRegNum; const regNumber dstReg = cast->gtRegNum; assert(genIsValidIntReg(srcReg)); assert(genIsValidIntReg(dstReg)); - instruction ins = INS_none; - emitAttr insSize; + GenIntCastDesc desc(cast); - if (cast->IsOverflowCheckRequired()) + if (desc.CheckKind() != GenIntCastDesc::CHECK_NONE) { - genIntCastOverflowCheck(cast, srcReg); - -#ifdef _TARGET_64BIT_ - if ((srcType == TYP_INT) && !cast->IsUnsigned() && (castType == TYP_ULONG)) - { - // This is the only overflow checking cast that requires changing the - // source value (by zero extending), all others will copy the value as is. - ins = INS_mov; - insSize = EA_4BYTE; - } -#endif + genIntCastOverflowCheck(cast, desc, srcReg); } - else // Non-overflow checking cast. + + if ((desc.ExtendKind() != GenIntCastDesc::COPY) || (srcReg != dstReg)) { - if (castSize < 4) - { - assert((castSize == 1) || (castSize == 2)); + instruction ins; + unsigned insSize; - // Casting to a small type really means widening from that small type to INT/LONG. - ins = ins_Move_Extend(castType, true); - insSize = EA_ATTR(dstSize); - } -#ifdef _TARGET_64BIT_ - // castType cannot be a long type on 32 bit targets, such casts should have been decomposed. - // srcType cannot be a small type since it's the "actual type" of the cast operand. - // This means that widening casts do not occur on 32 bit targets. - else if (castSize > srcSize) + switch (desc.ExtendKind()) { - // (U)INT to (U)LONG widening cast - assert((srcSize == 4) && (castSize == 8)); - - ins = cast->IsUnsigned() ? INS_mov : INS_sxtw; - // SXTW requires EA_8BYTE but MOV requires EA_4BYTE in order to zero out the upper 32 bits. - insSize = (ins == INS_sxtw) ? EA_8BYTE : EA_4BYTE; - } + case GenIntCastDesc::ZERO_EXTEND_SMALL_INT: + ins = (desc.ExtendSrcSize() == 1) ? INS_uxtb : INS_uxth; + insSize = 4; + break; + case GenIntCastDesc::SIGN_EXTEND_SMALL_INT: + ins = (desc.ExtendSrcSize() == 1) ? INS_sxtb : INS_sxth; + insSize = 4; + break; +#ifdef _TARGET_64BIT_ + case GenIntCastDesc::ZERO_EXTEND_INT: + ins = INS_mov; + insSize = 4; + break; + case GenIntCastDesc::SIGN_EXTEND_INT: + ins = INS_sxtw; + insSize = 8; + break; #endif - } - - if (ins == INS_none) - { - // If the instruction has not been selected yet it means we're dealing with - // a narrowing/same type/sign changing cast. - assert(castSize <= srcSize); - // Make sure the destination size is correct. This prevents unsupported casts - // such as LONG->INT->LONG, these would be classified as narrowing but in fact - // they're widening casts. It may be useful to allow such casts but that - // requires more work here and throughout the JIT. - assert(dstSize <= srcSize); - - // This cast basically does nothing, even when narrowing it is the job of the - // consumer of this node to use the appropiate register size (32 or 64 bit) - // and not rely on the cast to set the upper 32 bits in a certain manner. - // Still, we will need to generate a MOV instruction if the source and destination - // registers are different. - ins = (srcReg != dstReg) ? INS_mov : INS_none; - // We can use either the destination size or the source size. Use the destination - // size to be consistent with x64 where this may avoid the need for a REX prefix. - insSize = EA_SIZE(dstSize); - } + default: + assert(desc.ExtendKind() == GenIntCastDesc::COPY); + ins = INS_mov; + insSize = desc.ExtendSrcSize(); + break; + } - if (ins != INS_none) - { - getEmitter()->emitIns_R_R(ins, insSize, dstReg, srcReg); + getEmitter()->emitIns_R_R(ins, EA_ATTR(insSize), dstReg, srcReg); } genProduceReg(cast); diff --git a/src/jit/codegenlinear.cpp b/src/jit/codegenlinear.cpp index bf229305c660..4d9fe4eb55ba 100644 --- a/src/jit/codegenlinear.cpp +++ b/src/jit/codegenlinear.cpp @@ -1948,6 +1948,124 @@ void CodeGen::genCodeForCast(GenTreeOp* tree) // The per-case functions call genProduceReg() } +CodeGen::GenIntCastDesc::GenIntCastDesc(GenTreeCast* cast) +{ + const var_types srcType = genActualType(cast->gtGetOp1()->TypeGet()); + const bool srcUnsigned = cast->IsUnsigned(); + const unsigned srcSize = genTypeSize(srcType); + const var_types castType = cast->gtCastType; + const bool castUnsigned = varTypeIsUnsigned(castType); + const unsigned castSize = genTypeSize(castType); + const var_types dstType = genActualType(cast->TypeGet()); + const unsigned dstSize = genTypeSize(dstType); + const bool overflow = cast->gtOverflow(); + + assert((srcSize == 4) || (srcSize == genTypeSize(TYP_I_IMPL))); + assert((dstSize == 4) || (dstSize == genTypeSize(TYP_I_IMPL))); + + assert(dstSize == genTypeSize(genActualType(castType))); + + if (castSize < 4) // Cast to small int type + { + if (overflow) + { + m_checkKind = CHECK_SMALL_INT_RANGE; + m_checkSrcSize = srcSize; + // Since these are small int types we can compute the min and max + // values of the castType without risk of integer overflow. + const int castNumBits = (castSize * 8) - (castUnsigned ? 0 : 1); + m_checkSmallIntMax = (1 << castNumBits) - 1; + m_checkSmallIntMin = (castUnsigned | srcUnsigned) ? 0 : (-m_checkSmallIntMax - 1); + + m_extendKind = COPY; + m_extendSrcSize = dstSize; + } + else + { + m_checkKind = CHECK_NONE; + + // Casting to a small type really means widening from that small type to INT/LONG. + m_extendKind = castUnsigned ? ZERO_EXTEND_SMALL_INT : SIGN_EXTEND_SMALL_INT; + m_extendSrcSize = castSize; + } + } +#ifdef _TARGET_64BIT_ + // castType cannot be (U)LONG on 32 bit targets, such casts should have been decomposed. + // srcType cannot be a small int type since it's the "actual type" of the cast operand. + // This means that widening casts do not occur on 32 bit targets. + else if (castSize > srcSize) // (U)INT to (U)LONG widening cast + { + assert((srcSize == 4) && (castSize == 8)); + + if (overflow && !srcUnsigned && castUnsigned) + { + // Widening from INT to ULONG, check if the value is positive + m_checkKind = CHECK_POSITIVE; + m_checkSrcSize = 4; + + // This is the only overflow checking cast that requires changing the + // source value (by zero extending), all others copy the value as is. + assert((srcType == TYP_INT) && (castType == TYP_ULONG)); + m_extendKind = ZERO_EXTEND_INT; + m_extendSrcSize = 4; + } + else + { + m_checkKind = CHECK_NONE; + + m_extendKind = srcUnsigned ? ZERO_EXTEND_INT : SIGN_EXTEND_INT; + m_extendSrcSize = 4; + } + } + else if (castSize < srcSize) // (U)LONG to (U)INT narrowing cast + { + assert((srcSize == 8) && (castSize == 4)); + + if (overflow) + { + if (castUnsigned) // (U)LONG to UINT cast + { + m_checkKind = CHECK_UINT_RANGE; + } + else if (srcUnsigned) // ULONG to INT cast + { + m_checkKind = CHECK_POSITIVE_INT_RANGE; + } + else // LONG to INT cast + { + m_checkKind = CHECK_INT_RANGE; + } + + m_checkSrcSize = 8; + } + else + { + m_checkKind = CHECK_NONE; + } + + m_extendKind = COPY; + m_extendSrcSize = 4; + } +#endif + else // if (castSize == srcSize) // Sign changing or same type cast + { + assert(castSize == srcSize); + + if (overflow && (srcUnsigned != castUnsigned)) + { + m_checkKind = CHECK_POSITIVE; + m_checkSrcSize = srcSize; + } + else + { + m_checkKind = CHECK_NONE; + } + + m_extendKind = COPY; + m_extendSrcSize = srcSize; + } +} + #if !defined(_TARGET_64BIT_) //------------------------------------------------------------------------ // genStoreLongLclVar: Generate code to store a non-enregistered long lclVar diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp index 555c55028db6..ca8e1417ec6e 100644 --- a/src/jit/codegenxarch.cpp +++ b/src/jit/codegenxarch.cpp @@ -6349,38 +6349,20 @@ void CodeGen::genLongToIntCast(GenTree* cast) // // Arguments: // cast - The GT_CAST node +// desc - The cast description // reg - The register containing the value to check // -void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) +void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, const GenIntCastDesc& desc, regNumber reg) { - assert(cast->IsOverflowCheckRequired()); - - const var_types srcType = genActualType(cast->gtGetOp1()->TypeGet()); - const bool srcUnsigned = cast->IsUnsigned(); - const unsigned srcSize = genTypeSize(srcType); - const var_types castType = cast->gtCastType; - const bool castUnsigned = varTypeIsUnsigned(castType); - const unsigned castSize = genTypeSize(castType); - - if (castSize >= srcSize) + switch (desc.CheckKind()) { - // It's either a widening cast from a signed type to an unsigned - // type or a sign changing cast. - assert((castSize > srcSize) ? (!srcUnsigned && castUnsigned) : (srcUnsigned != castUnsigned)); + case GenIntCastDesc::CHECK_POSITIVE: + getEmitter()->emitIns_R_R(INS_test, EA_SIZE(desc.CheckSrcSize()), reg, reg); + genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); + break; - // In both cases we just need to check if the value is positive. - getEmitter()->emitIns_R_R(INS_test, EA_SIZE(srcSize), reg, reg); - genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); - } #ifdef _TARGET_64BIT_ - // We handled widening and sign changing casts so we're left with narrowing casts. - // Handle (U)LONG to (U)INT casts first as they're 64 bit target specific and there - // are some complications related to encoding of the large immediate values they need. - else if (castSize == 4) - { - assert(srcSize == 8); - - if (castUnsigned) // (U)LONG to UINT cast + case GenIntCastDesc::CHECK_UINT_RANGE: { // We need to check if the value is not greater than 0xFFFFFFFF but this value // cannot be encoded in an immediate operand. Use a right shift to test if the @@ -6391,43 +6373,37 @@ void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) getEmitter()->emitIns_R_I(INS_shr_N, EA_8BYTE, tempReg, 32); genJumpToThrowHlpBlk(EJ_jne, SCK_OVERFLOW); } - else if (srcUnsigned) // ULONG to INT cast - { + break; + + case GenIntCastDesc::CHECK_POSITIVE_INT_RANGE: getEmitter()->emitIns_R_I(INS_cmp, EA_8BYTE, reg, INT32_MAX); genJumpToThrowHlpBlk(EJ_ja, SCK_OVERFLOW); - } - else // LONG to INT cast - { - // We could combine this case with the unsigned case above but let's keep - // theses two separated to maintain the code structure similar to the ARM - // implementation that here requires a temporary register. + break; + + case GenIntCastDesc::CHECK_INT_RANGE: getEmitter()->emitIns_R_I(INS_cmp, EA_8BYTE, reg, INT32_MAX); genJumpToThrowHlpBlk(EJ_jg, SCK_OVERFLOW); - getEmitter()->emitIns_R_I(INS_cmp, EA_8BYTE, reg, INT32_MIN); genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); - } - } + break; #endif - else // if (castSize < srcSize) - { - // This is a narrowing cast. We already handled (U)INT so what's left are small int types. - assert(castSize < 4); - // This happens to allow for easy compututation of the min and max - // values of the castType without risk of integer overflow. - const int castNumBits = (castSize * 8) - (castUnsigned ? 0 : 1); - const int castMaxValue = (1 << castNumBits) - 1; - const int castMinValue = (castUnsigned | srcUnsigned) ? 0 : (-castMaxValue - 1); + default: + { + assert(desc.CheckKind() == GenIntCastDesc::CHECK_SMALL_INT_RANGE); + const int castMaxValue = desc.CheckSmallIntMax(); + const int castMinValue = desc.CheckSmallIntMin(); - getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMaxValue); - genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_ja : EJ_jg, SCK_OVERFLOW); + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(desc.CheckSrcSize()), reg, castMaxValue); + genJumpToThrowHlpBlk((castMinValue == 0) ? EJ_ja : EJ_jg, SCK_OVERFLOW); - if (castMinValue != 0) - { - getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(srcSize), reg, castMinValue); - genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); + if (castMinValue != 0) + { + getEmitter()->emitIns_R_I(INS_cmp, EA_SIZE(desc.CheckSrcSize()), reg, castMinValue); + genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW); + } } + break; } } @@ -6446,95 +6422,54 @@ void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, regNumber reg) // void CodeGen::genIntToIntCast(GenTreeCast* cast) { - genConsumeOperands(cast); - - GenTree* src = cast->gtGetOp1(); + genConsumeRegs(cast->gtGetOp1()); - const var_types srcType = genActualType(src->TypeGet()); - const unsigned srcSize = genTypeSize(srcType); - const var_types castType = cast->gtCastType; - const unsigned castSize = genTypeSize(castType); - const var_types dstType = genActualType(cast->TypeGet()); - const unsigned dstSize = genTypeSize(dstType); - - assert((srcSize == 4) || (srcSize == genTypeSize(TYP_I_IMPL))); - assert((dstSize == 4) || (dstSize == genTypeSize(TYP_I_IMPL))); - assert(castSize <= dstSize); - - const regNumber srcReg = src->gtRegNum; + const regNumber srcReg = cast->gtGetOp1()->gtRegNum; const regNumber dstReg = cast->gtRegNum; assert(genIsValidIntReg(srcReg)); assert(genIsValidIntReg(dstReg)); - instruction ins = INS_none; - emitAttr insSize; + GenIntCastDesc desc(cast); - if (cast->IsOverflowCheckRequired()) + if (desc.CheckKind() != GenIntCastDesc::CHECK_NONE) { - genIntCastOverflowCheck(cast, srcReg); - -#ifdef _TARGET_64BIT_ - if ((srcType == TYP_INT) && !cast->IsUnsigned() && (castType == TYP_ULONG)) - { - // This is the only overflow checking cast that requires changing the - // source value (by zero extending), all others will copy the value as is. - ins = INS_mov; - insSize = EA_4BYTE; - } -#endif + genIntCastOverflowCheck(cast, desc, srcReg); } - else // Non-overflow checking cast. + + if ((desc.ExtendKind() != GenIntCastDesc::COPY) || (srcReg != dstReg)) { - if (castSize < 4) - { - assert((castSize == 1) || (castSize == 2)); + instruction ins; + unsigned insSize; - // Casting to a small type really means widening from that small type to INT/LONG. - ins = varTypeIsUnsigned(castType) ? INS_movzx : INS_movsx; - insSize = EA_ATTR(castSize); - } -#ifdef _TARGET_64BIT_ - // castType cannot be a long type on 32 bit targets, such casts should have been decomposed. - // srcType cannot be a small type since it's the "actual type" of the cast operand. - // This means that widening casts do not occur on 32 bit targets. - else if (castSize > srcSize) + switch (desc.ExtendKind()) { - // (U)INT to (U)LONG widening cast - assert((srcSize == 4) && (castSize == 8)); - - ins = cast->IsUnsigned() ? INS_mov : INS_movsxd; - // We need a 32 bit MOV to zero out the upper 32 bit. MOVSXD ignores the size. - insSize = EA_4BYTE; - } + case GenIntCastDesc::ZERO_EXTEND_SMALL_INT: + ins = INS_movzx; + insSize = desc.ExtendSrcSize(); + break; + case GenIntCastDesc::SIGN_EXTEND_SMALL_INT: + ins = INS_movsx; + insSize = desc.ExtendSrcSize(); + break; +#ifdef _TARGET_64BIT_ + case GenIntCastDesc::ZERO_EXTEND_INT: + ins = INS_mov; + insSize = 4; + break; + case GenIntCastDesc::SIGN_EXTEND_INT: + ins = INS_movsxd; + insSize = 4; + break; #endif - } - - if (ins == INS_none) - { - // If the instruction has not been selected yet it means we're dealing with - // a narrowing/same type/sign changing cast. - assert(castSize <= srcSize); - // Make sure the destination size is correct. This prevents unsupported casts - // such as LONG->INT->LONG, these would be classified as narrowing but in fact - // they're widening casts. It may be useful to allow such casts but that - // requires more work here and throughout the JIT. - assert(dstSize <= srcSize); - - // This cast basically does nothing, even when narrowing it is the job of the - // consumer of this node to use the appropiate register size (32 or 64 bit) - // and not rely on the cast to set the upper 32 bits in a certain manner. - // Still, we will need to generate a MOV instruction if the source and destination - // registers are different. - ins = (srcReg != dstReg) ? INS_mov : INS_none; - // We can use either the destination size or the source size. Use the destination - // size as on x64 it may avoid the need for a REX prefix when casting LONG to INT. - insSize = EA_SIZE(dstSize); - } + default: + assert(desc.ExtendKind() == GenIntCastDesc::COPY); + ins = INS_mov; + insSize = desc.ExtendSrcSize(); + break; + } - if (ins != INS_none) - { - getEmitter()->emitIns_R_R(ins, insSize, dstReg, srcReg); + getEmitter()->emitIns_R_R(ins, EA_ATTR(insSize), dstReg, srcReg); } genProduceReg(cast); diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 3504aee79f73..94152eae5955 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -17940,43 +17940,3 @@ regNumber GenTree::ExtractTempReg(regMaskTP mask /* = (regMaskTP)-1 */) gtRsvdRegs &= ~tempRegMask; return genRegNumFromMask(tempRegMask); } - -//------------------------------------------------------------------------ -// IsOverflowCheckRequired: Check if this cast node requires an overflow check. -// -// Notes: -// Unlike gtOverflow, this function returns true only for casts that actually -// require an overflow check: -// - narrowing casts (e.g. TYP_INT -> TYP_SHORT, TYP_LONG -> TYP_INT) -// - sign changing casts (e.g. TYP_INT -> TYP_UINT, TYP_ULONG -> TYP_LONG) -// - widening from signed to unsigned casts (only TYP_INT -> TYP_ULONG) -// -bool GenTreeCast::IsOverflowCheckRequired() const -{ - if (!gtOverflow()) - { - return false; - } - - const var_types srcType = genActualType(gtGetOp1()->TypeGet()); - const bool srcUnsigned = IsUnsigned(); - const unsigned srcSize = genTypeSize(srcType); - const var_types castType = gtCastType; - const bool castUnsigned = varTypeIsUnsigned(castType); - const unsigned castSize = genTypeSize(castType); - - if (castSize > srcSize) - { - // Overflow check required for widening from signed to unsigned. - return !srcUnsigned && castUnsigned; - } - - if (castSize == srcSize) - { - // Overflow check required for any sign changing cast. - return srcUnsigned != castUnsigned; - } - - // Overflow check required for any narrowing cast. - return true; -} diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 24f52f4cc470..ef86c5d8a3ff 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -2901,9 +2901,6 @@ struct GenTreeCast : public GenTreeOp { } #endif - - // Returns true for casts that require an overflow check. - bool IsOverflowCheckRequired() const; }; // GT_BOX nodes are place markers for boxed values. The "real" tree