Skip to content

Commit

Permalink
Import cgt.un(op, 0) as NE(op, 0) (dotnet#54539)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@SingleAccretion
SingleAccretion authored Jul 13, 2021
1 parent 62966bc commit 1c3d401
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 37 deletions.
29 changes: 19 additions & 10 deletions src/coreclr/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
64 changes: 37 additions & 27 deletions src/coreclr/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 1c3d401

Please sign in to comment.