Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Narrow cast fix #70518

Merged
merged 13 commits into from
Jun 16, 2022
Prev Previous commit
Next Next commit
Added fgOptimizeCastOnAssignment
  • Loading branch information
TIHan committed Jun 13, 2022
commit 367612c84c91daf5cf71da07d0381738a7d7a0db
3 changes: 2 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5587,7 +5587,7 @@ class Compiler
void fgMoveOpsLeft(GenTree* tree);
#endif

bool fgIsSafeToRemoveCastOnAssignment(GenTree* tree);
bool fgIsSafeToRemoveIntToIntCastOnAssignment(GenTree* tree);

bool fgIsCommaThrow(GenTree* tree, bool forFolding = false);

Expand Down Expand Up @@ -5719,6 +5719,7 @@ class Compiler
GenTree* fgMorphForRegisterFP(GenTree* tree);
GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac = nullptr);
GenTree* fgOptimizeCast(GenTreeCast* cast);
GenTree* fgOptimizeCastOnAssignment(GenTreeOp* asg);
GenTree* fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp);
GenTree* fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp);
#ifdef FEATURE_HW_INTRINSICS
Expand Down
31 changes: 9 additions & 22 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -712,22 +712,24 @@ bool Compiler::fgIsBlockCold(BasicBlock* blk)
}

//------------------------------------------------------------------------
// fgIsSafeToRemoveCastOnAssignment: Determine whether "tree" is an assignment whose
// right-hand side is a cast that can be removed.
// fgIsSafeToRemoveIntToIntCastOnAssignment: Determine whether "tree" is an assignment whose
// right-hand side is a cast that can be removed.
//
// Arguments:
// tree - The tree node under consideration
//
// Return Value:
// Returns true if the cast can be safely removed.
//
bool Compiler::fgIsSafeToRemoveCastOnAssignment(GenTree* tree)
bool Compiler::fgIsSafeToRemoveIntToIntCastOnAssignment(GenTree* tree)
{
assert(tree->OperIs(GT_ASG));

GenTree* const op1 = tree->gtGetOp1();
GenTree* const op2 = tree->gtGetOp2();

assert(op2->OperIs(GT_CAST));

GenTree* const effectiveOp1 = op1->gtEffectiveVal();

if (!effectiveOp1->OperIs(GT_IND, GT_LCL_VAR))
Expand All @@ -737,13 +739,6 @@ bool Compiler::fgIsSafeToRemoveCastOnAssignment(GenTree* tree)
!lvaGetDesc(effectiveOp1->AsLclVarCommon()->GetLclNum())->lvNormalizeOnLoad())
return false;

// If we are storing a small type, we might be able to omit a cast.
if (!varTypeIsSmall(effectiveOp1))
return false;

if (!op2->OperIs(GT_CAST))
return false;

if (op2->gtOverflow())
return false;

Expand All @@ -754,7 +749,7 @@ bool Compiler::fgIsSafeToRemoveCastOnAssignment(GenTree* tree)
var_types castToType = cast->CastToType();
var_types castFromType = cast->CastFromType();

if (!varTypeIsIntegral(effectiveOp1))
if (gtIsActiveCSE_Candidate(cast->CastOp()))
return false;

if (!varTypeIsIntegral(castToType))
TIHan marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -763,24 +758,16 @@ bool Compiler::fgIsSafeToRemoveCastOnAssignment(GenTree* tree)
if (!varTypeIsIntegral(castFromType))
TIHan marked this conversation as resolved.
Show resolved Hide resolved
return false;

if (!varTypeIsIntegral(effectiveOp1))
return false;

// If we are performing a narrowing cast and
// castType is larger or the same as op1's type
// then we can discard the cast.

if (!varTypeIsSmall(castToType))
return false;

if (genTypeSize(castToType) < genTypeSize(effectiveOp1))
return false;

// The VM types must be the same.

if (genActualType(castToType) != genActualType(effectiveOp1))
return false;

if (genActualType(castToType) != genActualType(castFromType))
return false;

return true;
}

Expand Down
77 changes: 75 additions & 2 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11214,9 +11214,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
lclVarTree->gtFlags |= GTF_VAR_DEF;
}

if (fgIsSafeToRemoveCastOnAssignment(tree))
if (op2->OperIs(GT_CAST))
{
tree->AsOp()->gtOp2 = op2 = op2->AsCast()->CastOp();
tree = fgOptimizeCastOnAssignment(tree->AsOp());

assert(tree->OperIs(GT_ASG));

op1 = tree->gtGetOp1();
op2 = tree->gtGetOp2();
}

fgAssignSetVarDef(tree);
Expand Down Expand Up @@ -12183,6 +12188,74 @@ GenTree* Compiler::fgOptimizeCast(GenTreeCast* cast)
return cast;
}

//------------------------------------------------------------------------
// fgOptimizeCastOnAssignment: Optimizes the supplied GT_ASG tree with a GT_CAST node.
//
// Arguments:
// tree - the cast tree to optimize
//
// Return Value:
// The optimized tree (that can have any shape).
//
GenTree* Compiler::fgOptimizeCastOnAssignment(GenTreeOp* asg)
{
assert(asg->OperIs(GT_ASG));
assert(asg->gtGetOp2()->OperIs(GT_CAST));

if (fgIsSafeToRemoveIntToIntCastOnAssignment(asg))
{
var_types castToType = asg->gtGetOp2()->AsCast()->CastToType();

// Removes the cast.
asg->AsOp()->gtOp2 = asg->gtGetOp2()->AsCast()->CastOp();

GenTree* effectiveOp2 = asg->gtGetOp2()->gtEffectiveVal();

// Normalizes the constant value.
if (effectiveOp2->IsIntegralConst())
{
GenTreeIntConCommon* effectiveConOp2 = effectiveOp2->AsIntConCommon();

ssize_t iconValue = effectiveConOp2->IconValue();
switch (castToType)
{
case TYP_UBYTE:
iconValue = UINT8(iconValue);
break;
case TYP_BYTE:
iconValue = INT8(iconValue);
break;
case TYP_USHORT:
iconValue = UINT16(iconValue);
break;
case TYP_SHORT:
iconValue = INT16(iconValue);
break;
case TYP_INT:
iconValue = INT32(iconValue);
break;
case TYP_UINT:
iconValue = UINT32(iconValue);
break;
case TYP_ULONG:
iconValue = UINT64(iconValue);
break;
case TYP_LONG:
iconValue = INT64(iconValue);
break;

default:
unreached();
}

asg->gtGetOp2()->ChangeType(castToType);
effectiveConOp2->SetIconValue(iconValue);
TIHan marked this conversation as resolved.
Show resolved Hide resolved
}
}

return asg;
}

//------------------------------------------------------------------------
// fgOptimizeEqualityComparisonWithConst: optimizes various EQ/NE(OP, CONST) patterns.
//
Expand Down