This is an archive of the discontinued LLVM Phabricator instance.

[RISC-V] ADDI/ORI/XORI x, 0 should be as cheap as a move
ClosedPublic

Authored by arichardson on Aug 24 2020, 11:33 AM.

Details

Summary

The isTriviallyRematerializable hook is only called for instructions that are
tagged as isAsCheapAsAMove. Since ADDI 0 is used for "mv" it should definitely
be marked with "isAsCheapAsAMove". This change avoids one stack spill in most of
the atomic-rmw.ll tests functions. It also avoids stack spills in two of our
out-of-tree CHERI tests.
ORI/XORI with zero may or may not be the same as a move micro-architecturally,
but since we are already doing it for register == x0, we might as well
do the same if the immediate is zero.

Diff Detail

Event Timeline

arichardson created this revision.Aug 24 2020, 11:33 AM
arichardson requested review of this revision.Aug 24 2020, 11:34 AM

I think ORI and XORI should go together, as they're both moves when the immediate is 0. ADDI with 0 is special as the canonical move instruction, whereas ORI/XORI with 0 are not necessarily moves in microarchitectures, so I don't know whether they should be recognised here or not.

C.MV rd, rs is also ADD rd, x0, rs rather than ADDI rd, rs, 0 so it might make sense to match ADD with rs1 == x0.

Also include XOR

arichardson edited the summary of this revision. (Show Details)Aug 25 2020, 1:54 AM
arichardson retitled this revision from [RISC-V] ADDI/ORI x, 0 should be as cheap as a move to [RISC-V] ADDI/ORI/XORI x, 0 should be as cheap as a move.

merge if statement with return

I'll add a follow-up change for other instructions and for adding the isCopyInstrImpl() hook.

luismarques accepted this revision.Aug 25 2020, 2:28 AM

Looking at the AArch64 implementation, it seems there is precedent for this hook already including other instructions that are cheap but not necessarily recognized as actual moves by the microarchitecture (i.e. renames). So I guess they may not be, strictly speaking, as cheap as a move, but they are close enough. And that may very well be the point of the hook, otherwise they would just be non-canonical moves?

In sum, LGTM.

This revision is now accepted and ready to land.Aug 25 2020, 2:28 AM

Fix indentation by running clang-format

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp