From f71f8e88e3cb70ded37eac8f62d093bee89e8dce Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 10 May 2022 12:24:13 -0700 Subject: [PATCH 01/10] Initial work for GT_CHK_DIV_BY_ZERO --- src/coreclr/jit/codegen.h | 1 + src/coreclr/jit/codegenarm64.cpp | 16 ++++++++++++++++ src/coreclr/jit/codegenarmarch.cpp | 4 ++++ src/coreclr/jit/gtlist.h | 4 ++++ 4 files changed, 25 insertions(+) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 722503cd5260c..f5a54cbc748df 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1383,6 +1383,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genCodeForBfiz(GenTreeOp* tree); void genCodeForAddEx(GenTreeOp* tree); void genCodeForCond(GenTreeOp* tree); + void genCodeForChkDivByZero(GenTreeOp* tree); #endif // TARGET_ARM64 #if defined(FEATURE_EH_FUNCLETS) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 5c1f8c64f15db..7ee27f346cf23 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -10243,4 +10243,20 @@ void CodeGen::genCodeForCond(GenTreeOp* tree) genProduceReg(tree); } +//------------------------------------------------------------------------ +// genCodeForChkDivByZero: Generates the code for checking +// if a value is zero to throw a div-by-zero exception. +// +// Arguments: +// tree - divide-by-zero check +// +void CodeGen::genCodeForChkDivByZero(GenTreeOp* tree) +{ + assert(tree->OperIs(GT_CHK_DIV_BY_ZERO)); + genConsumeOperands(tree->AsOp()); + + GetEmitter()->emitIns_R_I(INS_cmp, emitActualTypeSize(tree), tree->GetRegNum(), 0); + genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); +} + #endif // TARGET_ARM64 diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 4905aba6e4367..3992f1240b8b5 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -302,6 +302,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) break; #ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: + genCodeForChkDivByZero(treeNode->AsOp()); + break; + case GT_MADD: genCodeForMadd(treeNode->AsOp()); break; diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index d0aceb1ba07d2..2f20653508c14 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -80,6 +80,10 @@ GTNODE(ADDR , GenTreeOp ,0,GTK_UNOP|DBK_NOTLIR) // addre GTNODE(BOUNDS_CHECK , GenTreeBoundsChk ,0,GTK_BINOP|GTK_EXOP|GTK_NOVALUE) // a bounds check - for arrays/spans/SIMDs/HWINTRINSICs +#ifdef TARGET_ARM64 +GTNODE(CHK_DIV_BY_ZERO , GenTreeOp ,0,GTK_UNOP|GTK_EXOP|GTK_NOVALUE) // a divide-by-zero check - for integers +#endif + GTNODE(IND , GenTreeIndir ,0,GTK_UNOP) // Load indirection GTNODE(STOREIND , GenTreeStoreInd ,0,GTK_BINOP|GTK_NOVALUE) // Store indirection GTNODE(OBJ , GenTreeObj ,0,GTK_UNOP|GTK_EXOP) // Object that MAY have gc pointers, and thus includes the relevant gc layout info. From f4145dcf118039360e2697e9eb2ec79adf2e06f1 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 10 May 2022 18:55:15 -0700 Subject: [PATCH 02/10] Plumbing GT_CHK_DIV_BY_ZERO --- src/coreclr/jit/codegenarm64.cpp | 13 ++++++++----- src/coreclr/jit/compiler.h | 3 +++ src/coreclr/jit/gentree.cpp | 3 +++ src/coreclr/jit/gtlist.h | 2 +- src/coreclr/jit/importer.cpp | 15 +++++++++++++++ 5 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 7ee27f346cf23..cbc7311f8a071 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3129,8 +3129,9 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree) if (divisorOp->IsIntegralConst(0)) { + // TODO: Remove this code. // We unconditionally throw a divide by zero exception - genJumpToThrowHlpBlk(EJ_jmp, SCK_DIV_BY_ZERO); + //genJumpToThrowHlpBlk(EJ_jmp, SCK_DIV_BY_ZERO); // We still need to call genProduceReg genProduceReg(tree); @@ -3164,9 +3165,10 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree) } else // insert check for divison by zero { + // TODO: Remove this code. // Check if the divisor is zero throw a DivideByZeroException - emit->emitIns_R_I(INS_cmp, size, divisorReg, 0); - genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); + //emit->emitIns_R_I(INS_cmp, size, divisorReg, 0); + //genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); } if (checkDividend) @@ -3202,10 +3204,11 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree) // if (!divisorOp->IsCnsIntOrI()) { + // TODO: Remove this code. // divisorOp is not a constant, so it could be zero // - emit->emitIns_R_I(INS_cmp, size, divisorReg, 0); - genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); + //emit->emitIns_R_I(INS_cmp, size, divisorReg, 0); + //genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); } genCodeForBinary(tree); } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 7d79862592ffe..ec80b57beefbd 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -10720,6 +10720,9 @@ class GenTreeVisitor case GT_ARR_ADDR: case GT_KEEPALIVE: case GT_INC_SATURATE: +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: +#endif { GenTreeUnOp* const unOp = node->AsUnOp(); if (unOp->gtOp1 != nullptr) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a1d1c27dcbd6d..1c8b9ab581a4c 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -9120,6 +9120,9 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) case GT_PUTARG_SPLIT: #endif // FEATURE_ARG_SPLIT case GT_RETURNTRAP: +#if TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: +#endif m_edge = &m_node->AsUnOp()->gtOp1; assert(*m_edge != nullptr); m_advance = &GenTreeUseEdgeIterator::Terminate; diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index 2f20653508c14..7e05acfe8281f 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -81,7 +81,7 @@ GTNODE(ADDR , GenTreeOp ,0,GTK_UNOP|DBK_NOTLIR) // addre GTNODE(BOUNDS_CHECK , GenTreeBoundsChk ,0,GTK_BINOP|GTK_EXOP|GTK_NOVALUE) // a bounds check - for arrays/spans/SIMDs/HWINTRINSICs #ifdef TARGET_ARM64 -GTNODE(CHK_DIV_BY_ZERO , GenTreeOp ,0,GTK_UNOP|GTK_EXOP|GTK_NOVALUE) // a divide-by-zero check - for integers +GTNODE(CHK_DIV_BY_ZERO , GenTreeOp ,0,GTK_UNOP|GTK_NOVALUE) // a divide-by-zero check - for integers #endif GTNODE(IND , GenTreeIndir ,0,GTK_UNOP) // Load indirection diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 7860d7742e7c9..aab986f190b93 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -13500,6 +13500,21 @@ void Compiler::impImportBlockCode(BasicBlock* block) } } +#ifdef TARGET_ARM64 + if ((oper == GT_DIV || oper == GT_MOD) && varTypeIsIntOrI(type)) + { + unsigned tmpNum = lvaGrabTemp(true DEBUGARG("divisor expr")); + GenTree* divisorAsg = gtNewTempAssign(tmpNum, op2); + GenTree* divisor = gtNewLclvNode(tmpNum, op2->TypeGet()); + GenTree* chkDivByZero = + new (this, GT_CHK_DIV_BY_ZERO) GenTreeOp(GT_CHK_DIV_BY_ZERO, TYP_VOID, divisorAsg, nullptr); + + chkDivByZero->gtFlags |= GTF_SIDE_EFFECT | GTF_DONT_CSE; + + op2 = gtNewOperNode(GT_COMMA, divisor->TypeGet(), chkDivByZero, gtCloneExpr(divisor)); + } +#endif + // We can generate a TYP_FLOAT operation that has a TYP_DOUBLE operand // if (varTypeIsFloating(type) && varTypeIsFloating(op1->gtType) && varTypeIsFloating(op2->gtType)) From b8ef574b83ab99342e8e1dd9ed111855d32cb18f Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 11 May 2022 21:33:21 -0700 Subject: [PATCH 03/10] Handling a few more cases --- src/coreclr/jit/gentree.cpp | 14 ++++++++++++++ src/coreclr/jit/liveness.cpp | 3 +++ 2 files changed, 17 insertions(+) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 1c8b9ab581a4c..d344bc256eb96 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5099,6 +5099,13 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) costSz = 7; // jump to cold section break; +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: + costEx = 4; // cmp reg,reg and jae throw (not taken) + costSz = 7; // jump to cold section + break; +#endif + case GT_COMMA: /* Comma tosses the result of the left operand */ @@ -8340,6 +8347,13 @@ GenTree* Compiler::gtCloneExpr( copy->AsBoundsChk()->gtInxType = tree->AsBoundsChk()->gtInxType; break; +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: + copy = new (this, GT_CHK_DIV_BY_ZERO) + GenTreeOp(GT_CHK_DIV_BY_ZERO, tree->TypeGet(), tree->AsOp()->gtOp1, nullptr); + break; +#endif + case GT_LEA: { GenTreeAddrMode* addrModeOp = tree->AsAddrMode(); diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index e07fb4f95c97f..7d5c64934f914 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -2069,6 +2069,9 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR case GT_JMP: case GT_STOREIND: case GT_BOUNDS_CHECK: +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: +#endif case GT_STORE_OBJ: case GT_STORE_BLK: case GT_STORE_DYN_BLK: From 5d28e620625be18cbf5c24a471fedf14e5402340 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 12 May 2022 14:39:45 -0700 Subject: [PATCH 04/10] Plumbing GT_CHK_DIV_BY_ZERO --- src/coreclr/jit/codegenarm64.cpp | 31 ++++--------------------------- src/coreclr/jit/compiler.hpp | 3 +++ src/coreclr/jit/gentree.cpp | 6 ++++++ src/coreclr/jit/importer.cpp | 6 ++++-- src/coreclr/jit/morph.cpp | 7 +++++++ src/coreclr/jit/valuenum.cpp | 5 +++++ 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index e92da1ef4c16d..8e8ef26deb184 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3169,11 +3169,6 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree) if (divisorOp->IsIntegralConst(0)) { - // TODO: Remove this code. - // We unconditionally throw a divide by zero exception - //genJumpToThrowHlpBlk(EJ_jmp, SCK_DIV_BY_ZERO); - - // We still need to call genProduceReg genProduceReg(tree); } else // the divisor is not the constant zero @@ -3203,13 +3198,6 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree) checkDividend = false; // We statically know that the dividend is not -1 } } - else // insert check for divison by zero - { - // TODO: Remove this code. - // Check if the divisor is zero throw a DivideByZeroException - //emit->emitIns_R_I(INS_cmp, size, divisorReg, 0); - //genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); - } if (checkDividend) { @@ -3236,20 +3224,6 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree) } else // (tree->gtOper == GT_UDIV) { - // Only one possible exception - // (AnyVal / 0) => DivideByZeroException - // - // Note that division by the constant 0 was already checked for above by the - // op2->IsIntegralConst(0) check - // - if (!divisorOp->IsCnsIntOrI()) - { - // TODO: Remove this code. - // divisorOp is not a constant, so it could be zero - // - //emit->emitIns_R_I(INS_cmp, size, divisorReg, 0); - //genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); - } genCodeForBinary(tree); } } @@ -10228,8 +10202,11 @@ void CodeGen::genCodeForChkDivByZero(GenTreeOp* tree) assert(tree->OperIs(GT_CHK_DIV_BY_ZERO)); genConsumeOperands(tree->AsOp()); - GetEmitter()->emitIns_R_I(INS_cmp, emitActualTypeSize(tree), tree->GetRegNum(), 0); + GenTree* op1 = tree->gtGetOp1(); + + GetEmitter()->emitIns_R_I(INS_cmp, emitActualTypeSize(op1->TypeGet()), op1->GetRegNum(), 0); genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); + genProduceReg(op1); } #endif // TARGET_ARM64 diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index e610096100f25..0c90ca2ffdbb0 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4271,6 +4271,9 @@ void GenTree::VisitOperands(TVisitor visitor) case GT_RETURNTRAP: case GT_KEEPALIVE: case GT_INC_SATURATE: +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: +#endif visitor(this->AsUnOp()->gtOp1); return; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index fcfe6a8c0e424..971bd6e6bb053 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5902,6 +5902,9 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) case GT_BSWAP16: case GT_KEEPALIVE: case GT_INC_SATURATE: +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: +#endif if (operand == this->AsUnOp()->gtOp1) { *pUse = &this->AsUnOp()->gtOp1; @@ -6409,6 +6412,9 @@ bool GenTree::OperMayThrow(Compiler* comp) case GT_CKFINITE: case GT_INDEX: case GT_INDEX_ADDR: +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: +#endif return true; #ifdef FEATURE_HW_INTRINSICS diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 7de69cddba952..5f99fdb2bb9a1 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -14032,11 +14032,13 @@ void Compiler::impImportBlockCode(BasicBlock* block) GenTree* divisorAsg = gtNewTempAssign(tmpNum, op2); GenTree* divisor = gtNewLclvNode(tmpNum, op2->TypeGet()); GenTree* chkDivByZero = - new (this, GT_CHK_DIV_BY_ZERO) GenTreeOp(GT_CHK_DIV_BY_ZERO, TYP_VOID, divisorAsg, nullptr); + new (this, GT_CHK_DIV_BY_ZERO) GenTreeOp(GT_CHK_DIV_BY_ZERO, TYP_VOID, divisor, nullptr); chkDivByZero->gtFlags |= GTF_SIDE_EFFECT | GTF_DONT_CSE; - op2 = gtNewOperNode(GT_COMMA, divisor->TypeGet(), chkDivByZero, gtCloneExpr(divisor)); + op2 = + gtNewOperNode(GT_COMMA, divisor->TypeGet(), divisorAsg, + gtNewOperNode(GT_COMMA, divisor->TypeGet(), chkDivByZero, gtCloneExpr(divisor))); } #endif diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 816c093659f2d..97c79902e09c5 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11758,6 +11758,13 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) fgSetRngChkTarget(tree); break; +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: + assert(varTypeIsIntegralOrI(op1)); + // TODO: Add fgAddCodeRef for div by zero + break; +#endif + case GT_OBJ: case GT_BLK: case GT_IND: diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index b771b6285fddc..5c5e7a0c1755b 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10795,6 +10795,11 @@ void Compiler::fgValueNumberAddExceptionSet(GenTree* tree) fgValueNumberAddExceptionSetForBoundsCheck(tree); break; +#ifdef TARGET_ARM64 + case GT_CHK_DIV_BY_ZERO: + break; +#endif + case GT_LCLHEAP: // It is not necessary to model the StackOverflow exception for GT_LCLHEAP break; From aa3870d08c4842c1b7c93bcd1b4c964c44a52a4b Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 12 May 2022 14:49:22 -0700 Subject: [PATCH 05/10] Added UDIV and UMOD cases. Moved emitting the DIV_BY_ZERO block when handling the GT_CHK_DIV_BY_ZERO case in morph. --- src/coreclr/jit/importer.cpp | 5 +++-- src/coreclr/jit/morph.cpp | 6 +----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 5f99fdb2bb9a1..e777812ab38df 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -14025,8 +14025,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) } } -#ifdef TARGET_ARM64 - if ((oper == GT_DIV || oper == GT_MOD) && varTypeIsIntOrI(type)) +#if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) + if ((oper == GT_DIV || oper == GT_UDIV || oper == GT_MOD || oper == GT_UMOD) && varTypeIsIntOrI(type) && + (!op2->IsIntegralConst() || op2->IsIntegralConst(0))) { unsigned tmpNum = lvaGrabTemp(true DEBUGARG("divisor expr")); GenTree* divisorAsg = gtNewTempAssign(tmpNum, op2); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d292b5447ae73..573ea208a55b8 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11427,17 +11427,13 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) #endif if (!varTypeIsFloating(tree->gtType)) { - // Codegen for this instruction needs to be able to throw two exceptions: fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW); - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); } break; case GT_UDIV: #ifdef TARGET_LOONGARCH64 case GT_UMOD: #endif - // Codegen for this instruction needs to be able to throw one exception: - fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); break; #endif // defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) @@ -11541,7 +11537,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) #ifdef TARGET_ARM64 case GT_CHK_DIV_BY_ZERO: assert(varTypeIsIntegralOrI(op1)); - // TODO: Add fgAddCodeRef for div by zero + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); break; #endif From ebb3c8b6c628a90499c1b51baa7372c9d6d4be6c Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 12 May 2022 16:10:27 -0700 Subject: [PATCH 06/10] Fixed assert --- src/coreclr/jit/codegenarm64.cpp | 1 - src/coreclr/jit/importer.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 8e8ef26deb184..74880a5494a04 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -10206,7 +10206,6 @@ void CodeGen::genCodeForChkDivByZero(GenTreeOp* tree) GetEmitter()->emitIns_R_I(INS_cmp, emitActualTypeSize(op1->TypeGet()), op1->GetRegNum(), 0); genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); - genProduceReg(op1); } #endif // TARGET_ARM64 diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index e777812ab38df..ebe5548a19373 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -14035,7 +14035,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) GenTree* chkDivByZero = new (this, GT_CHK_DIV_BY_ZERO) GenTreeOp(GT_CHK_DIV_BY_ZERO, TYP_VOID, divisor, nullptr); - chkDivByZero->gtFlags |= GTF_SIDE_EFFECT | GTF_DONT_CSE; + chkDivByZero->gtFlags |= GTF_SIDE_EFFECT; op2 = gtNewOperNode(GT_COMMA, divisor->TypeGet(), divisorAsg, From f763d26c003323ed7bc322b046dc07180feb906c Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 12 May 2022 17:32:38 -0700 Subject: [PATCH 07/10] Fixed check --- src/coreclr/jit/importer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index ebe5548a19373..5bf1da2300229 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -14026,8 +14026,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) } #if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) - if ((oper == GT_DIV || oper == GT_UDIV || oper == GT_MOD || oper == GT_UMOD) && varTypeIsIntOrI(type) && - (!op2->IsIntegralConst() || op2->IsIntegralConst(0))) + if ((oper == GT_DIV || oper == GT_UDIV || oper == GT_MOD || oper == GT_UMOD) && + !varTypeIsFloating(type) && (!op2->IsIntegralConst() || op2->IsIntegralConst(0))) { unsigned tmpNum = lvaGrabTemp(true DEBUGARG("divisor expr")); GenTree* divisorAsg = gtNewTempAssign(tmpNum, op2); From d4d78d4a069200b3bb188cee1e41e3db577d203a Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 12 May 2022 17:38:07 -0700 Subject: [PATCH 08/10] Fixed check --- src/coreclr/jit/gentree.cpp | 4 ++-- src/coreclr/jit/morph.cpp | 2 +- src/coreclr/jit/valuenum.cpp | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 971bd6e6bb053..87b31fd36a3f7 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5101,8 +5101,8 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) #ifdef TARGET_ARM64 case GT_CHK_DIV_BY_ZERO: - costEx = 4; // cmp reg,reg and jae throw (not taken) - costSz = 7; // jump to cold section + costEx = 4; + costSz = 7; break; #endif diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 573ea208a55b8..ecc20740202d0 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11536,7 +11536,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) #ifdef TARGET_ARM64 case GT_CHK_DIV_BY_ZERO: - assert(varTypeIsIntegralOrI(op1)); + assert(!varTypeIsFloating(op1)); fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); break; #endif diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 5c5e7a0c1755b..037c99faaf45c 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10797,6 +10797,7 @@ void Compiler::fgValueNumberAddExceptionSet(GenTree* tree) #ifdef TARGET_ARM64 case GT_CHK_DIV_BY_ZERO: + // TODO: Add impl break; #endif From 67224b4a1088122e8742b5ab053e4e404b913ee6 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Thu, 12 May 2022 17:41:14 -0700 Subject: [PATCH 09/10] Revert LOONGARCH64 bit --- src/coreclr/jit/importer.cpp | 2 +- src/coreclr/jit/morph.cpp | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 5bf1da2300229..f9cd38feb765b 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -14025,7 +14025,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) } } -#if defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) +#if defined(TARGET_ARM64) if ((oper == GT_DIV || oper == GT_UDIV || oper == GT_MOD || oper == GT_UMOD) && !varTypeIsFloating(type) && (!op2->IsIntegralConst() || op2->IsIntegralConst(0))) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ecc20740202d0..437a147b766aa 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11427,13 +11427,17 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) #endif if (!varTypeIsFloating(tree->gtType)) { + // Codegen for this instruction needs to be able to throw two exceptions: fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_OVERFLOW); + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); } break; case GT_UDIV: #ifdef TARGET_LOONGARCH64 case GT_UMOD: #endif + // Codegen for this instruction needs to be able to throw one exception: + fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_DIV_BY_ZERO); break; #endif // defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) From 098a94ddce08e329757eb2da978b1a2a55f35a7a Mon Sep 17 00:00:00 2001 From: Will Smith Date: Mon, 6 Jun 2022 13:24:44 -0700 Subject: [PATCH 10/10] Do not create an extra comma if we do not need to --- src/coreclr/jit/importer.cpp | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 2cc5d411e77b6..8510c36238819 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -14017,17 +14017,31 @@ void Compiler::impImportBlockCode(BasicBlock* block) if ((oper == GT_DIV || oper == GT_UDIV || oper == GT_MOD || oper == GT_UMOD) && !varTypeIsFloating(type) && (!op2->IsIntegralConst() || op2->IsIntegralConst(0))) { - unsigned tmpNum = lvaGrabTemp(true DEBUGARG("divisor expr")); - GenTree* divisorAsg = gtNewTempAssign(tmpNum, op2); - GenTree* divisor = gtNewLclvNode(tmpNum, op2->TypeGet()); - GenTree* chkDivByZero = - new (this, GT_CHK_DIV_BY_ZERO) GenTreeOp(GT_CHK_DIV_BY_ZERO, TYP_VOID, divisor, nullptr); + if (fgIsSafeToClone(op2)) + { + GenTree* chkDivByZero = + new (this, GT_CHK_DIV_BY_ZERO) GenTreeOp(GT_CHK_DIV_BY_ZERO, TYP_VOID, gtClone(op2), nullptr); + + chkDivByZero->gtFlags |= GTF_SIDE_EFFECT; - chkDivByZero->gtFlags |= GTF_SIDE_EFFECT; + op2 = gtNewOperNode(GT_COMMA, op2->TypeGet(), chkDivByZero, op2); + } + else + { + TempInfo tempInfo = fgMakeTemp(op2); - op2 = - gtNewOperNode(GT_COMMA, divisor->TypeGet(), divisorAsg, - gtNewOperNode(GT_COMMA, divisor->TypeGet(), chkDivByZero, gtCloneExpr(divisor))); + GenTree* const divisorAsg = tempInfo.asg; + GenTree* const divisor = tempInfo.load; + + GenTree* chkDivByZero = + new (this, GT_CHK_DIV_BY_ZERO) GenTreeOp(GT_CHK_DIV_BY_ZERO, TYP_VOID, gtClone(divisor), nullptr); + + chkDivByZero->gtFlags |= GTF_SIDE_EFFECT; + + op2 = gtNewOperNode(GT_COMMA, divisor->TypeGet(), divisorAsg, + gtNewOperNode(GT_COMMA, divisor->TypeGet(), chkDivByZero, + divisor)); + } } #endif