This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Implement support for the Zicbom and Zicboz extensions
ClosedPublic

Authored by asb on Jan 16 2022, 8:29 AM.

Details

Summary

Implements the ratified RISC-V Base Cache Management Operation ISA
Extensions: Zicbom and Zicboz, as described in
https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.pdf.

Zicbop is implemented in a separate patch due to it requiring a new ASM
operand type to be defined.

As discussed in the relevant issue in the upstream spec
https://github.com/riscv/riscv-CMOs/issues/47, the cbo.* instructions
use the format (rs1) or 0(rs1) for their operand, similar to the AMOs.

Diff Detail

Event Timeline

asb created this revision.Jan 16 2022, 8:29 AM
asb requested review of this revision.Jan 16 2022, 8:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 16 2022, 8:29 AM
eopXD added a subscriber: eopXD.Jan 16 2022, 8:30 AM
jrtc27 added inline comments.Jan 16 2022, 8:46 AM
llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
20

Ugh, this takes the encoding that people have been using for RV128I's LQ for years (riscvemu's implementation uses that), and we've been using that as a result for our 128-bit capability load/store instructions... guess we'll just have to be incompatible and stomp on this encoding downstream :(

llvm/test/MC/RISCV/rv32zicbom-invalid.s
18

(and copied to ...z/p-invalid.s)

jrtc27 added inline comments.Jan 16 2022, 8:49 AM
llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
20

Uh, just load; store is using the "obvious" encoding for SQ as only loads have to squeeze a sign bit in that reduces the number of width bits

asb updated this revision to Diff 403631.Jan 27 2022, 6:24 AM
asb marked an inline comment as done.

Address review comments and add test coverage to riscv-target-features.c, attributes.ll, and attribute-arch.s.

asb updated this revision to Diff 406440.Feb 7 2022, 6:48 AM
asb edited the summary of this revision. (Show Details)

Rebase, and move to using (rs1) or 0(rs1) for the operand, following discussion in https://github.com/riscv/riscv-CMOs/issues/47.

asb updated this revision to Diff 415498.Mar 15 2022, 10:43 AM

Rebased. Ping!

Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 10:43 AM
Herald added a subscriber: arichardson. · View Herald Transcript
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1603

There are many Zi* extentions, how about rename it to RISCVInstInfoZi.td

s added a subscriber: s.Mar 15 2022, 11:46 AM
kito-cheng accepted this revision.Mar 18 2022, 12:17 AM

LGTM, checked with spec, and I don't have no strong opinion on rename file, I think that could be another NFC patch is fine.

This revision is now accepted and ready to land.Mar 18 2022, 12:17 AM

Note: binutils side has been merged the patch from Tsukasa OI/a4lg, which support same syntax for cbo.* instruction as this patch, parentheses and 0 with parentheses.

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3b374308d3006407b9571e573e4ccce4e904a4c4
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=41d6ac5da655a2e78109848f2db47e53552fd61a

Reverse ping. Is anything blocking this being merged?

KYG added a subscriber: KYG.Jun 22 2022, 5:35 AM
This revision was landed with ongoing or failed builds.Jun 28 2022, 4:43 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 4:43 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
asb added a comment.Jun 28 2022, 4:44 AM

Reverse ping. Is anything blocking this being merged?

No, thanks for the ping - now landed.