This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Implement a basic version of AArch64RedundantCopyElimination pass.
ClosedPublic

Authored by craig.topper on Jan 25 2022, 9:33 AM.

Details

Summary

Using AArch64's original implementation for reference, this patch
implements a pass to remove unneeded copies of X0. This pass runs
after register allocation and looks to see if a register is implied
to be 0 by a branch in the predecessor basic block.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 25 2022, 9:33 AM
craig.topper requested review of this revision.Jan 25 2022, 9:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 25 2022, 9:33 AM
liaolucy added inline comments.Jan 25 2022, 8:25 PM
llvm/lib/Target/RISCV/RISCVRedundantCopyElimination.cpp
12

W0 is typo?

96

select_cc also generates beq, I'm not sure should handle it?

craig.topper added inline comments.Jan 25 2022, 8:29 PM
llvm/lib/Target/RISCV/RISCVRedundantCopyElimination.cpp
12

Copy and pasted from AArch64. I'll fix. Thanks

96

This pass runs after register allocation so select_cc will already have been expanded into control flow before we here. I think all the test changes started as select_cc.

Fix copy/paste mistake

asb accepted this revision.Feb 3 2022, 1:36 AM

LGTM. It feels like this could perhaps be made target-independent with some additional hooks - a companion to or modification to isConstantPhysReg that indicates what the constant value is, and using analyzeBranchPredicate. Though extending it in some of the ways suggested in the TODOs would be more difficult, and as the pass itself as quite simple, the code added in terms of target hooks / interface modifications could quickly become larger than the pass itself.

This revision is now accepted and ready to land.Feb 3 2022, 1:36 AM

LGTM. It feels like this could perhaps be made target-independent with some additional hooks - a companion to or modification to isConstantPhysReg that indicates what the constant value is, and using analyzeBranchPredicate. Though extending it in some of the ways suggested in the TODOs would be more difficult, and as the pass itself as quite simple, the code added in terms of target hooks / interface modifications could quickly become larger than the pass itself.

I might look at this in the future. I started with the simpler version AArch64 started with so I didn't look as much at its current version which handles multiple branches, and constants. As noted in the TODOs, constants for RISCV will be harder. If the constant doesn't fit in an ADDI we might get luck and they'll be CSEd together. Then we would just need to check for register equality/inequality.

This revision was landed with ongoing or failed builds.Feb 4 2022, 10:44 AM
This revision was automatically updated to reflect the committed changes.