From f9380e912b105030cee06544338d6068fb7e97e7 Mon Sep 17 00:00:00 2001 From: Sujin Kim Date: Fri, 31 Mar 2017 05:37:13 +0900 Subject: [PATCH] [Ryujit/ARM32] Implement NYI related with overflow for ARM (#10491) * Implement NYI(overflow checks) for ARM On last comment of #8496, the NYI message of overflow checks is printed after running the CodeGenBringUpTests. That was the message about temp register setup for overflow checks. It was referenced https://github.com/dotnet/coreclr/blob/master/src/jit/lsraarm64.cpp#L399 I think it doesn't make any problem even though writing it the same as arm64. * modifiy for coding convention * Implement NYI : Unimplmented GT_CAST:int <--> int with overflow I think it doesn't make any problem even though writing it the same as arm64. So I copied parts of CodeGen::genIntToIntCast() and modified some below codes. ``` if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, cmpSize)) ``` --> ``` if (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, INS_FLAGS_DONT_CARE)) ``` * Implement NYI : genLongToIntCast: overflow check I copied and pasted codes from codegenxarch.cpp. But It seemed be necessary that conditional execution values are changed by each architectures. So I used 'genJumpKindForOper' for getting the emitJumpKind value. The sample app has been checked to work well. * Modify the implementation of emitter::emitIns_valid_imm_for_cmp According to reference manual, I figured out CMP and ADD have different mechanisms on ARM unlike ARM64. So I defined "...for_cmp" function not just use "..for_add" in the function likes ARM64. --- src/jit/codegenarm.cpp | 120 ++++++++++++++++++++++++++++++++++++++++++++++--- src/jit/emitarm.cpp | 16 ++++++- src/jit/emitarm.h | 1 + src/jit/lsraarm.cpp | 22 ++++++++- 4 files changed, 150 insertions(+), 9 deletions(-) diff --git a/src/jit/codegenarm.cpp b/src/jit/codegenarm.cpp index e276b71..cd39cbf 100644 --- a/src/jit/codegenarm.cpp +++ b/src/jit/codegenarm.cpp @@ -610,10 +610,6 @@ void CodeGen::genCodeForTreeNode(GenTreePtr treeNode) // Cast is never contained (?) noway_assert(targetReg != REG_NA); - // Overflow conversions from float/double --> int types go through helper calls. - if (treeNode->gtOverflow() && !varTypeIsFloating(treeNode->gtOp.gtOp1)) - NYI("Unimplmented GT_CAST:int <--> int with overflow"); - if (varTypeIsFloating(targetType) && varTypeIsFloating(treeNode->gtOp.gtOp1)) { // Casts float/double <--> double/float @@ -2421,7 +2417,50 @@ void CodeGen::genLongToIntCast(GenTree* cast) if (cast->gtOverflow()) { - NYI("genLongToIntCast: overflow check"); + // + // Generate an overflow check for [u]long to [u]int casts: + // + // long -> int - check if the upper 33 bits are all 0 or all 1 + // + // ulong -> int - check if the upper 33 bits are all 0 + // + // long -> uint - check if the upper 32 bits are all 0 + // ulong -> uint - check if the upper 32 bits are all 0 + // + + if ((srcType == TYP_LONG) && (dstType == TYP_INT)) + { + BasicBlock* allOne = genCreateTempLabel(); + BasicBlock* success = genCreateTempLabel(); + + inst_RV_RV(INS_tst, loSrcReg, loSrcReg, TYP_INT, EA_4BYTE); + emitJumpKind JmpNegative = genJumpKindForOper(GT_LT, CK_LOGICAL); + inst_JMP(JmpNegative, allOne); + inst_RV_RV(INS_tst, hiSrcReg, hiSrcReg, TYP_INT, EA_4BYTE); + emitJumpKind jmpNotEqualL = genJumpKindForOper(GT_NE, CK_LOGICAL); + genJumpToThrowHlpBlk(jmpNotEqualL, SCK_OVERFLOW); + inst_JMP(EJ_jmp, success); + + genDefineTempLabel(allOne); + inst_RV_IV(INS_cmp, hiSrcReg, -1, EA_4BYTE); + emitJumpKind jmpNotEqualS = genJumpKindForOper(GT_NE, CK_SIGNED); + genJumpToThrowHlpBlk(jmpNotEqualS, SCK_OVERFLOW); + + genDefineTempLabel(success); + } + else + { + if ((srcType == TYP_ULONG) && (dstType == TYP_INT)) + { + inst_RV_RV(INS_tst, loSrcReg, loSrcReg, TYP_INT, EA_4BYTE); + emitJumpKind JmpNegative = genJumpKindForOper(GT_LT, CK_LOGICAL); + genJumpToThrowHlpBlk(JmpNegative, SCK_OVERFLOW); + } + + inst_RV_RV(INS_tst, hiSrcReg, hiSrcReg, TYP_INT, EA_4BYTE); + emitJumpKind jmpNotEqual = genJumpKindForOper(GT_NE, CK_LOGICAL); + genJumpToThrowHlpBlk(jmpNotEqual, SCK_OVERFLOW); + } } if (dstReg != loSrcReg) @@ -2483,7 +2522,76 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode) if (castInfo.requiresOverflowCheck) { - NYI_ARM("CodeGen::genIntToIntCast for OverflowCheck"); + 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)) + { + // cast to TYP_ULONG: + // We use a mov with size=EA_4BYTE + // which will zero out the upper bits + movSize = EA_4BYTE; + 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); + emit->emitIns_R_I(INS_tst, cmpSize, sourceReg, castInfo.typeMask); + 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 (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMax, INS_FLAGS_DONT_CARE)) + { + 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 (emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, INS_FLAGS_DONT_CARE)) + { + 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); + } + ins = INS_mov; } else // Non-overflow checking cast. { diff --git a/src/jit/emitarm.cpp b/src/jit/emitarm.cpp index f7943f2..77ee3cf 100644 --- a/src/jit/emitarm.cpp +++ b/src/jit/emitarm.cpp @@ -1380,7 +1380,7 @@ DONE: /***************************************************************************** * - * emitIns_valid_imm_for_add() returns true when the immediate 'imm' + * emitins_valid_imm_for_add() returns true when the immediate 'imm' * can be encoded using a single add or sub instruction. */ /*static*/ bool emitter::emitIns_valid_imm_for_add(int imm, insFlags flags) @@ -1396,6 +1396,20 @@ DONE: /***************************************************************************** * + * emitins_valid_imm_for_cmp() returns true if this 'imm' + * can be encoded as a input operand to an cmp instruction. + */ +/*static*/ bool emitter::emitIns_valid_imm_for_cmp(int imm, insFlags flags) +{ + if (isModImmConst(imm)) // funky arm immediate + return true; + if (isModImmConst(-imm)) // funky arm immediate via sub + return true; + return false; +} + +/***************************************************************************** + * * emitIns_valid_imm_for_add_sp() returns true when the immediate 'imm' * can be encoded in "add Rd,SP,i10". */ diff --git a/src/jit/emitarm.h b/src/jit/emitarm.h index 6c153a1..8ad1268 100644 --- a/src/jit/emitarm.h +++ b/src/jit/emitarm.h @@ -243,6 +243,7 @@ static bool emitIns_valid_imm_for_alu(int imm); static bool emitIns_valid_imm_for_mov(int imm); static bool emitIns_valid_imm_for_small_mov(regNumber reg, int imm, insFlags flags); static bool emitIns_valid_imm_for_add(int imm, insFlags flags); +static bool emitIns_valid_imm_for_cmp(int imm, insFlags flags); static bool emitIns_valid_imm_for_add_sp(int imm); void emitIns(instruction ins); diff --git a/src/jit/lsraarm.cpp b/src/jit/lsraarm.cpp index 3a2206b..26500b9 100644 --- a/src/jit/lsraarm.cpp +++ b/src/jit/lsraarm.cpp @@ -991,9 +991,27 @@ void Lowering::TreeNodeInfoInit(GenTree* tree) info->srcCount = 2; } - if (tree->gtOverflow()) + CastInfo castInfo; + + // Get information about the cast. + getCastDescription(tree, &castInfo); + + if (castInfo.requiresOverflowCheck) { - NYI_ARM("overflow checks"); + 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, INS_FLAGS_DONT_CARE); + bool canStoreMinValue = emitter::emitIns_valid_imm_for_cmp(castInfo.typeMin, INS_FLAGS_DONT_CARE); + + if (!canStoreMaxValue || !canStoreMinValue) + { + info->internalIntCount = 1; + } } } break; -- 2.7.4