From 1c3d401d329c3305ca9b8fdbc36cb4d5ceba1a63 Mon Sep 17 00:00:00 2001 From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Tue, 13 Jul 2021 19:39:21 +0300 Subject: [PATCH] Import `cgt.un(op, 0)` as `NE(op, 0)` (#54539) * Some minor code modernization Use "genActualType" directly as it is now a templated method. Don't create casts to TYP_ULONG - they are identical to casts to TYP_LONG. TYP_ULONG is only relevant for checked casts. Add a TODO on addressing the duplicated logic that upcasts to native int from int32 on 64 bit. Use modern comments. Zero diffs. * Normalize GT_UN(op, 0) early in importer Normalizing this idiom helps downstream optimizations. * Solve most of the regressions In morph, when narrowing the AND operand, only insert casts if necessary - prefer to use "optNarrowTree". Otherwise we end up with redundant register shuffling. --- src/coreclr/jit/importer.cpp | 29 ++++++++++------ src/coreclr/jit/morph.cpp | 64 +++++++++++++++++++++--------------- 2 files changed, 56 insertions(+), 37 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index d49a12ffd13dd0..c30e9a177846e6 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -13075,27 +13075,36 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = impPopStack().val; op1 = impPopStack().val; + // Recognize the IL idiom of CGT_UN(op1, 0) and normalize + // it so that downstream optimizations don't have to. + if ((opcode == CEE_CGT_UN) && op2->IsIntegralConst(0)) + { + oper = GT_NE; + uns = false; + } + #ifdef TARGET_64BIT - if (varTypeIsI(op1->TypeGet()) && (genActualType(op2->TypeGet()) == TYP_INT)) + // TODO-Casts: create a helper that upcasts int32 -> native int when necessary. + // See also identical code in impGetByRefResultType and STSFLD import. + if (varTypeIsI(op1) && (genActualType(op2) == TYP_INT)) { - op2 = gtNewCastNode(TYP_I_IMPL, op2, uns, uns ? TYP_U_IMPL : TYP_I_IMPL); + op2 = gtNewCastNode(TYP_I_IMPL, op2, uns, TYP_I_IMPL); } - else if (varTypeIsI(op2->TypeGet()) && (genActualType(op1->TypeGet()) == TYP_INT)) + else if (varTypeIsI(op2) && (genActualType(op1) == TYP_INT)) { - op1 = gtNewCastNode(TYP_I_IMPL, op1, uns, uns ? TYP_U_IMPL : TYP_I_IMPL); + op1 = gtNewCastNode(TYP_I_IMPL, op1, uns, TYP_I_IMPL); } #endif // TARGET_64BIT - assertImp(genActualType(op1->TypeGet()) == genActualType(op2->TypeGet()) || - (varTypeIsI(op1->TypeGet()) && varTypeIsI(op2->TypeGet())) || - (varTypeIsFloating(op1->gtType) && varTypeIsFloating(op2->gtType))); + assertImp(genActualType(op1) == genActualType(op2) || (varTypeIsI(op1) && varTypeIsI(op2)) || + (varTypeIsFloating(op1) && varTypeIsFloating(op2))); - /* Create the comparison node */ + // Create the comparison node. op1 = gtNewOperNode(oper, TYP_INT, op1, op2); - /* TODO: setting both flags when only one is appropriate */ - if (opcode == CEE_CGT_UN || opcode == CEE_CLT_UN) + // TODO: setting both flags when only one is appropriate. + if (uns) { op1->gtFlags |= GTF_RELOP_NAN_UN | GTF_UNSIGNED; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f0bc076f2356e1..354b1e4a357491 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12465,44 +12465,54 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) noway_assert(op1->TypeGet() == TYP_LONG && op1->OperGet() == GT_AND); - /* Is the result of the mask effectively an INT ? */ - - GenTree* andMask; - andMask = op1->AsOp()->gtOp2; - if (andMask->gtOper != GT_CNS_NATIVELONG) - { - goto COMPARE; - } - if ((andMask->AsIntConCommon()->LngValue() >> 32) != 0) + // The transform below cannot preserve VNs. + if (fgGlobalMorph) { - goto COMPARE; - } + // Is the result of the mask effectively an INT ? - /* Now we know that we can cast AsOp()->gtOp1 of AND to int */ + GenTree* andMask = op1->AsOp()->gtOp2; + + if (andMask->gtOper != GT_CNS_NATIVELONG) + { + goto COMPARE; + } + if ((andMask->AsIntConCommon()->LngValue() >> 32) != 0) + { + goto COMPARE; + } - op1->AsOp()->gtOp1 = gtNewCastNode(TYP_INT, op1->AsOp()->gtOp1, false, TYP_INT); + // Now we narrow AsOp()->gtOp1 of AND to int. + if (optNarrowTree(op1->AsOp()->gtGetOp1(), TYP_LONG, TYP_INT, ValueNumPair(), false)) + { + optNarrowTree(op1->AsOp()->gtGetOp1(), TYP_LONG, TYP_INT, ValueNumPair(), true); + } + else + { + op1->AsOp()->gtOp1 = gtNewCastNode(TYP_INT, op1->AsOp()->gtGetOp1(), false, TYP_INT); + } - /* now replace the mask node (AsOp()->gtOp2 of AND node) */ + // now replace the mask node (AsOp()->gtOp2 of AND node). - noway_assert(andMask == op1->AsOp()->gtOp2); + noway_assert(andMask == op1->AsOp()->gtOp2); - ival1 = (int)andMask->AsIntConCommon()->LngValue(); - andMask->SetOper(GT_CNS_INT); - andMask->gtType = TYP_INT; - andMask->AsIntCon()->gtIconVal = ival1; + ival1 = (int)andMask->AsIntConCommon()->LngValue(); + andMask->SetOper(GT_CNS_INT); + andMask->gtType = TYP_INT; + andMask->AsIntCon()->gtIconVal = ival1; - /* now change the type of the AND node */ + // now change the type of the AND node. - op1->gtType = TYP_INT; + op1->gtType = TYP_INT; - /* finally we replace the comparand */ + // finally we replace the comparand. - ival2 = (int)cns2->AsIntConCommon()->LngValue(); - cns2->SetOper(GT_CNS_INT); - cns2->gtType = TYP_INT; + ival2 = (int)cns2->AsIntConCommon()->LngValue(); + cns2->SetOper(GT_CNS_INT); + cns2->gtType = TYP_INT; - noway_assert(cns2 == op2); - cns2->AsIntCon()->gtIconVal = ival2; + noway_assert(cns2 == op2); + cns2->AsIntCon()->gtIconVal = ival2; + } goto COMPARE;