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.

Sign up for GitHub

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: Eliminate redundant opposite mov #38179

Merged
merged 13 commits into from
Jun 30, 2020
Merged

ARM64: Eliminate redundant opposite mov #38179

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
31f9cca
Added IsOppositeOfPrevMov()
kunalspathak Jun 19, 2020
dcf463e
review comments
kunalspathak Jun 20, 2020
172cb9d
Refactor code into IsRedundantMov()
kunalspathak Jun 22, 2020
de0f711
dont optimize beyong IG boundary
kunalspathak Jun 23, 2020
a9cbba2
add check to not cross IG boundary
kunalspathak Jun 23, 2020
0d4e8e1
Add more checks:
kunalspathak Jun 25, 2020
18a68b8
fix the check
kunalspathak Jun 26, 2020
f94951e
review comments
kunalspathak Jun 26, 2020
1023b61
Add check to ensure if src/dst are both scalar or both vector
kunalspathak Jun 26, 2020
54dc37d
minor comment
kunalspathak Jun 27, 2020
1e650fd
Add check for insFormat
kunalspathak Jun 29, 2020
412c4b8
fix the typo
kunalspathak Jun 30, 2020
b5882c5
minor comments
kunalspathak Jun 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/coreclr/src/jit/emitarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4025,6 +4025,11 @@ void emitter::emitIns_R_R(
}
}

if (IsOppositeOfPrevMov(ins, reg1, reg2))
Copy link
Member

Choose a reason for hiding this comment

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

Seems like you might need to worry about the EA_4BYTE case here too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. I think I will try to move the one above reg1 == reg2 inside the new IsOppositeOfPrevMov so we have all the checks in one place.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, generalize to "is unnecessary mov" and catch all 3 cases:

mov   RX, RX

mov RX, RY
mov RY, RX

mov RX, RY
mov RX, RY

Copy link
Member Author

Choose a reason for hiding this comment

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

Given that we want to check OptimizationEnabled() before doing this optimization and mov RX, RX was earlier done without this check, will it be fine to make this optimization only when opts is enabled?

{
return;
}

// Check for the 'mov' aliases for the vector registers
if (isVectorRegister(reg1))
{
Expand Down Expand Up @@ -14882,4 +14887,35 @@ emitter::insExecutionCharacteristics emitter::getInsExecutionCharacteristics(ins

#endif // defined(DEBUG) || defined(LATE_DISASM)

//----------------------------------------------------------------------------------------
// IsOppositeOfPrevMov:
// Check if previous instruction was a mov where dst was current src and src was current dst.
//
// Arguments:
// ins - The current instruction
// dst - The current destination
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
// src - The current source
//
// Return Value:
// true if previous instruction moved from current dst to src.
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved

bool emitter::IsOppositeOfPrevMov(instruction ins, regNumber dst, regNumber src)
{
assert(ins == INS_mov);

if (emitLastIns != nullptr && emitLastIns->idIns() == INS_mov)
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
{
// Check if we did same move in prev instruction except dst/src were switched.
regNumber prevDst = emitLastIns->idReg1();
regNumber prevSrc = emitLastIns->idReg2();

if ((prevDst == src) && (prevSrc == dst))
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
{
JITDUMP("\n -- suppressing mov as previous instruction did opposite mov.\n", dst, src,
this->emitComp->info.compMethodName, prevDst, prevSrc);
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
}
return false;
}
#endif // defined(TARGET_ARM64)
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/emitarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ void emitDispExtendReg(regNumber reg, insOpts opt, ssize_t imm);
void emitDispAddrRI(regNumber reg, insOpts opt, ssize_t imm);
void emitDispAddrRRExt(regNumber reg1, regNumber reg2, insOpts opt, bool isScaled, emitAttr size);

bool IsOppositeOfPrevMov(instruction ins, regNumber dst, regNumber src);

void emitDispIns(instrDesc* id,
bool isNew,
bool doffs,
Expand Down