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

[arm64] JIT: Redundant zero/sign extensions after ldrX/ldrsX #62630

Merged
merged 14 commits into from
Jan 19, 2022
9 changes: 9 additions & 0 deletions src/coreclr/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,15 @@ class emitter
assert((ins != INS_invalid) && (ins < INS_count));
_idIns = ins;
}
bool idInsIs(instruction ins) const
{
return idIns() == ins;
}
template <typename... T>
bool idInsIs(instruction ins, T... rest) const
{
return idInsIs(ins) || idInsIs(rest...);
}

insFormat idInsFmt() const
{
Expand Down
19 changes: 17 additions & 2 deletions src/coreclr/jit/emitarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15614,6 +15614,11 @@ bool emitter::IsMovInstruction(instruction ins)
// mov Rx, Ry # <-- last instruction
// mov Ry, Rx # <-- current instruction can be omitted.
//
// 4. Move that does zero extension while previous instruction already did it
//
// ldr Wx, [Ry] # <-- ldr will clear upper 4 byte of Wx
// mov Wx, Wx # <-- clears upper 4 byte in Wx
//
// Arguments:
// ins - The current instruction
// size - Operand size of current instruction
Expand All @@ -15640,6 +15645,8 @@ bool emitter::IsRedundantMov(instruction ins, emitAttr size, regNumber dst, regN
return false;
}

const bool isFirstInstrInBlock = (emitCurIGinsCnt == 0) && ((emitCurIG->igFlags & IGF_EXTEND) == 0);

if (dst == src)
{
// A mov with a EA_4BYTE has the side-effect of clearing the upper bits
Expand All @@ -15655,10 +15662,18 @@ bool emitter::IsRedundantMov(instruction ins, emitAttr size, regNumber dst, regN
JITDUMP("\n -- suppressing mov because src and dst is same 16-byte register.\n");
return true;
}
else if (isGeneralRegisterOrSP(dst) && (size == EA_4BYTE))
{
// See if the previous instruction already cleared upper 4 bytes for us unintentionally
if (!isFirstInstrInBlock && (emitLastIns != nullptr) && (emitLastIns->idReg1() == dst) &&
(emitLastIns->idOpSize() == size) && emitLastIns->idInsIs(INS_ldr, INS_ldrh, INS_ldrb))
{
JITDUMP("\n -- suppressing mov because ldr already cleared upper 4 bytes\n");
return true;
}
}
}

bool isFirstInstrInBlock = (emitCurIGinsCnt == 0) && ((emitCurIG->igFlags & IGF_EXTEND) == 0);

if (!isFirstInstrInBlock && // Don't optimize if instruction is not the first instruction in IG.
(emitLastIns != nullptr) &&
(emitLastIns->idIns() == INS_mov) && // Don't optimize if last instruction was not 'mov'.
Expand Down
11 changes: 0 additions & 11 deletions src/coreclr/jit/lowerarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,6 @@ void Lowering::LowerCast(GenTree* tree)
GenTree* op1 = tree->AsOp()->gtOp1;
var_types dstType = tree->CastToType();
var_types srcType = genActualType(op1->TypeGet());
var_types tmpType = TYP_UNDEF;

if (varTypeIsFloating(srcType))
{
Expand All @@ -501,16 +500,6 @@ void Lowering::LowerCast(GenTree* tree)

assert(!varTypeIsSmall(srcType));

if (tmpType != TYP_UNDEF)
{
GenTree* tmp = comp->gtNewCastNode(tmpType, op1, tree->IsUnsigned(), tmpType);
tmp->gtFlags |= (tree->gtFlags & (GTF_OVERFLOW | GTF_EXCEPT));

tree->gtFlags &= ~GTF_UNSIGNED;
tree->AsOp()->gtOp1 = tmp;
BlockRange().InsertAfter(op1, tmp);
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead code

// Now determine if we have operands that should be contained.
ContainCheckCast(tree->AsCast());
}
Expand Down