Skip to content

[RISCV] Add CompressPat for all cases in isCopyInstrImpl #136875

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

Closed
wants to merge 1 commit into from
Closed

[RISCV] Add CompressPat for all cases in isCopyInstrImpl #136875

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 26 additions & 4 deletions llvm/lib/Target/RISCV/RISCVInstrInfoC.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,11 +1012,15 @@ let Predicates = [HasStdExtCOrZca] in {
def : CompressPat<(JALR X0, GPRNoX0:$rs1, 0),
(C_JR GPRNoX0:$rs1)>;
let isCompressOnly = true in {
def : CompressPat<(ADD GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
def : CompressPat<(ADD GPRNoX0:$rs1, GPRNoX0:$rs2, X0),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
foreach Inst=[ADD, OR, XOR] in {
def : CompressPat<(Inst GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
def : CompressPat<(Inst GPRNoX0:$rs1, GPRNoX0:$rs2, X0),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
}
def : CompressPat<(SUB GPRNoX0:$rs1, GPRNoX0:$rs2, X0),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
} // isCompressOnly = true
def : CompressPat<(ADDI GPRNoX0:$rs1, GPRNoX0:$rs2, 0),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
def : CompressPat<(EBREAK), (C_EBREAK)>;
Expand Down Expand Up @@ -1053,3 +1057,21 @@ let Predicates = [HasStdExtCOrZca, IsRV64] in {
def : CompressPat<(SD GPR:$rs2, SPMem:$rs1, uimm9_lsb000:$imm),
(C_SDSP GPR:$rs2, SPMem:$rs1, uimm9_lsb000:$imm)>;
} // Predicates = [HasStdExtCOrZca, IsRV64]

let isCompressOnly = true, Predicates = [HasStdExtCOrZca, HasStdExtZba] in {
def : CompressPat<(SH1ADD GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
Copy link
Collaborator

Choose a reason for hiding this comment

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

SHXADD is available on a single ALU on SiFive x280, but ADD/ADDI are available on both ALUs. x280 supports C so this patch would compress, but I could envision other CPUs that support SHXADD with lower performance and don't support C. Should we canonicalize a copy SHXADD to ADD earlier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the kind trade-off I wondered about. I oversimplified a bit in my summary.

On the one hand: Although canonicalising earlier is theoretically better for this reason, it also seem defensible to optimise for existing real-world and likely future hardware where unless something big changes, it's hard to imagine it doesn't have RVC support. Especially as this is more of a micro-optimisation that we're mainly motivated to do to keep output clean and non-surprising for anyone doing perf analysis.

On the other hand: there's an attraction and probably minor engineering advantage to keeping differences between compressed/non-compressed as minimal as possible. And if non-C hardware did appear, our future selves would be thankful.

Overall I'm still torn...

(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
def : CompressPat<(SH2ADD GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
def : CompressPat<(SH3ADD GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
} // isCompressOnly = true, Predicates = [HasStdExtCOrZca, HasStdExtZba]

let isCompressOnly = true, Predicates = [HasStdExtCOrZca, HasStdExtZba, IsRV64] in {
def : CompressPat<(SH1ADD_UW GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
def : CompressPat<(SH2ADD_UW GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
def : CompressPat<(SH3ADD_UW GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
(C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
} // isCompressOnly = true, Predicates = [HasStdExtCOrZca, HasStdExtZba, IsRV64]
Loading