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.

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

riscv64: Add VecALUImm instruction format #6325

Merged
merged 6 commits into from
May 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
19 changes: 19 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,13 @@
(vs1 Reg)
(vstate VState))

(VecAluRRImm5
(op VecAluOpRRImm5)
(vd WritableReg)
(vs2 Reg)
(imm Imm5)
(vstate VState))

(VecSetState
(rd WritableReg)
(vstate VState))
Expand Down Expand Up @@ -697,6 +704,7 @@
(type OptionUimm5 (primitive OptionUimm5))
(type Imm12 (primitive Imm12))
(type UImm5 (primitive UImm5))
(type Imm5 (primitive Imm5))
(type Imm20 (primitive Imm20))
(type Imm3 (primitive Imm3))
(type BranchTarget (primitive BranchTarget))
Expand Down Expand Up @@ -1323,6 +1331,17 @@
(extern extractor imm12_from_u64 imm12_from_u64)


;; Imm5 Extractors

(decl imm5_from_u64 (Imm5) u64)
(extern extractor imm5_from_u64 imm5_from_u64)

;; Extractor that matches a `Value` equivalent to a replicated Imm5 on all lanes.
;; TODO: Try matching vconst here as well
(decl replicated_imm5 (Imm5) Value)
(extractor (replicated_imm5 n)
(def_inst (splat (iconst (u64_from_imm64 (imm5_from_u64 n))))))


;; Float Helpers

Expand Down
21 changes: 21 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ impl Inst {
Inst::VecSetState { .. } => None,

Inst::VecAluRRR { vstate, .. } |
Inst::VecAluRRImm5 { vstate, .. } |
// TODO: Unit-stride loads and stores only need the AVL to be correct, not
// the full vtype. A future optimization could be to decouple these two when
// updating vstate. This would allow us to avoid emitting a VecSetState in
Expand Down Expand Up @@ -2802,6 +2803,26 @@ impl MachInstEmit for Inst {
op.funct6(),
));
}
&Inst::VecAluRRImm5 {
op, vd, imm, vs2, ..
} => {
let vs2 = allocs.next(vs2);
let vd = allocs.next_writable(vd);

// This is the mask bit, we don't yet implement masking, so set it to 1, which means
// masking disabled.
let vm = 1;

sink.put4(encode_valu_imm(
op.opcode(),
vd.to_reg(),
op.funct3(),
imm,
vs2,
vm,
op.funct6(),
));
}
&Inst::VecSetState { rd, ref vstate } => {
let rd = allocs.next_writable(rd);

Expand Down
61 changes: 53 additions & 8 deletions cranelift/codegen/src/isa/riscv64/inst/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,37 @@
//! Some instructions especially in extensions have slight variations from
//! the base RISC-V specification.

use super::{UImm5, VType};
use super::{Imm5, UImm5, VType};
use crate::isa::riscv64::inst::reg_to_gpr_num;
use crate::isa::riscv64::lower::isle::generated_code::VecElementWidth;
use crate::Reg;

/// Encode an R-type instruction.
///
/// Layout:
/// 0-------6-7-------11-12------14-15------19-20------24-25-------31
/// | Opcode | rd | funct3 | rs1 | rs2 | funct7 |
pub fn encode_r_type(opcode: u32, rd: Reg, funct3: u32, rs1: Reg, rs2: Reg, funct7: u32) -> u32 {
fn encode_r_type_bits(opcode: u32, rd: u32, funct3: u32, rs1: u32, rs2: u32, funct7: u32) -> u32 {
let mut bits = 0;
bits |= opcode & 0b1111111;
bits |= reg_to_gpr_num(rd) << 7;
bits |= (rd & 0b11111) << 7;
bits |= (funct3 & 0b111) << 12;
bits |= reg_to_gpr_num(rs1) << 15;
bits |= reg_to_gpr_num(rs2) << 20;
bits |= (rs1 & 0b11111) << 15;
bits |= (rs2 & 0b11111) << 20;
bits |= (funct7 & 0b1111111) << 25;
bits
}

/// Encode an R-type instruction.
pub fn encode_r_type(opcode: u32, rd: Reg, funct3: u32, rs1: Reg, rs2: Reg, funct7: u32) -> u32 {
encode_r_type_bits(
opcode,
reg_to_gpr_num(rd),
funct3,
reg_to_gpr_num(rs1),
reg_to_gpr_num(rs2),
funct7,
)
}

/// Encodes a Vector ALU instruction.
///
/// Fields:
Expand All @@ -47,11 +57,46 @@ pub fn encode_valu(
vs2: Reg,
vm: u32,
funct6: u32,
) -> u32 {
// VALU is just VALUImm with the register in the immediate field.
let imm = Imm5::maybe_from_i8((reg_to_gpr_num(vs1) as i8) << 3 >> 3).unwrap();
encode_valu_imm(opcode, vd, funct3, imm, vs2, vm, funct6)
}

/// Encodes a Vector ALU+Imm instruction.
/// This is just a Vector ALU instruction with an immediate in the VS1 field.
///
/// Fields:
/// - opcode (7 bits)
/// - vd (5 bits)
/// - funct3 (3 bits)
/// - imm (5 bits)
/// - vs2 (5 bits)
/// - vm (1 bit)
/// - funct6 (6 bits)
///
/// See: https://github.com/riscv/riscv-v-spec/blob/master/valu-format.adoc
pub fn encode_valu_imm(
opcode: u32,
vd: Reg,
funct3: u32,
imm: Imm5,
vs2: Reg,
vm: u32,
funct6: u32,
) -> u32 {
let funct6 = funct6 & 0b111111;
let vm = vm & 0b1;
let funct7 = (funct6 << 1) | vm;
encode_r_type(opcode, vd, funct3, vs1, vs2, funct7)
let imm = (imm.bits() & 0b11111) as u32;
afonso360 marked this conversation as resolved.
Show resolved Hide resolved
encode_r_type_bits(
opcode,
reg_to_gpr_num(vd),
funct3,
imm,
reg_to_gpr_num(vs2),
funct7,
)
}

/// Encodes a Vector CFG Imm instruction.
Expand Down
28 changes: 28 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/imms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,34 @@ impl Display for UImm5 {
}
}

/// A Signed 5-bit immediate.
#[derive(Clone, Copy, Debug, PartialEq)]
pub struct Imm5 {
value: i8,
}

impl Imm5 {
/// Create an signed 5-bit immediate from an i8.
pub fn maybe_from_i8(value: i8) -> Option<Imm5> {
if value >= -16 && value <= 15 {
Some(Imm5 { value })
} else {
None
}
}

/// Bits for encoding.
pub fn bits(&self) -> u8 {
self.value as u8
}
}

impl Display for Imm5 {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
write!(f, "{}", self.value)
}
}

impl Inst {
pub(crate) fn imm_min() -> i64 {
let imm20_max: i64 = (1 << 19) << 12;
Expand Down
16 changes: 16 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,10 @@ fn riscv64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
collector.reg_use(vs2);
collector.reg_def(vd);
}
&Inst::VecAluRRImm5 { vd, vs2, .. } => {
collector.reg_use(vs2);
collector.reg_def(vd);
}
&Inst::VecSetState { rd, .. } => {
collector.reg_def(rd);
}
Expand Down Expand Up @@ -1583,6 +1587,18 @@ impl Inst {
// This is noted in Section 10.1 of the RISC-V Vector spec.
format!("{} {},{},{} {}", op, vd_s, vs2_s, vs1_s, vstate)
}
&Inst::VecAluRRImm5 {
op,
vd,
imm,
vs2,
ref vstate,
} => {
let vs2_s = format_vec_reg(vs2, allocs);
let vd_s = format_vec_reg(vd.to_reg(), allocs);

format!("{} {},{},{} {}", op, vd_s, vs2_s, imm, vstate)
}
&Inst::VecSetState { rd, ref vstate } => {
let rd_s = format_reg(rd.to_reg(), allocs);
assert!(vstate.avl.is_static());
Expand Down
30 changes: 29 additions & 1 deletion cranelift/codegen/src/isa/riscv64/inst/vector.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::isa::riscv64::inst::EmitState;
use crate::isa::riscv64::lower::isle::generated_code::{
VecAMode, VecAluOpRRR, VecAvl, VecElementWidth, VecLmul, VecMaskMode, VecTailMode,
VecAMode, VecAluOpRRImm5, VecAluOpRRR, VecAvl, VecElementWidth, VecLmul, VecMaskMode,
VecTailMode,
};
use crate::Reg;
use core::fmt;
Expand Down Expand Up @@ -218,6 +219,7 @@ impl VecAluOpRRR {
0x57
}
pub fn funct3(&self) -> u32 {
// See: https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#101-vector-arithmetic-instruction-encoding
match self {
// OPIVV
VecAluOpRRR::Vadd
Expand Down Expand Up @@ -253,6 +255,32 @@ impl fmt::Display for VecAluOpRRR {
}
}

impl VecAluOpRRImm5 {
pub fn opcode(&self) -> u32 {
// Vector Opcode
0x57
}
pub fn funct3(&self) -> u32 {
// OPIVI
0b011
Copy link
Member

Choose a reason for hiding this comment

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

My curiosity has been piqued if you're willing to oblige -- according to this doc I see a bunch of V/X/I columns but I'm not sure how that translates to the encoding. Is there a different document that explains how these columns relate to the encoding?

Copy link
Contributor Author

@afonso360 afonso360 May 3, 2023

Choose a reason for hiding this comment

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

That table does not seem to be explained anywhere, which is weird. The closest thing is 10. Vector Arithmetic Instruction Formats which only really explains the format, but not the encoding in that table. The way I understand it is the following.

There are 3 columns, which correspond to the 3 columns for the minor opcodes at the top (i.e. OPIVV, OPIVX, OPMVX, etc..). The minor opcode corresponds to the funct3 field. (The exact encoding is specified here)

The minor opcodes correspond to the operands that instruction takes. OPIVV is for Vector/Vector instructions (i.e. vadd.vv), OPIVX is for Vector/X Register instructions (i.e. vadd.vx) and OPIVI is for Vector/Immediate instructions (i.e. vadd.vi) which is what is implemented in this PR.

I'm only planning on having 2 instruction formats, since OP*VV,OP*VX and OPFVF can share the same instruction format in cranelift. Just with an assert that we are using the correct regclass. OPIVI is the only special one here.

The big table displays the funct6 field. Take for example the vrsub instruction. It is in the leftmost column, and only has X and I formats. So we know that it only takes the minor opcode formats of OPIVX and OPIVI. vrsub itself has a funct6 of 0b000011 with both of those minor opcodes.

There are additional encoding spaces, at the bottom, but my understanding is that these are all OPIVI instructions and that encoding gets put in the immediate field (not 100% sure on this yet).

Copy link
Contributor Author

@afonso360 afonso360 May 3, 2023

Choose a reason for hiding this comment

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

I've also now put these into a VecOpCategory struct so that their encoding is somewhat centralized.

}
pub fn funct6(&self) -> u32 {
// See: https://github.com/riscv/riscv-v-spec/blob/master/inst-table.adoc
match self {
VecAluOpRRImm5::Vadd => 0b000000,
Copy link
Member

Choose a reason for hiding this comment

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

I don't know much about RISC-V, but should this AluOp be different than VecAluOpRRR or the same? The single data point of Vadd has the encodings the same, but I'm not sure if the pattern holds up elsewhere.

Copy link
Contributor Author

@afonso360 afonso360 May 3, 2023

Choose a reason for hiding this comment

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

Well, It is the same, but not all opcodes work on both formats. vadd works for both, but vsub does not have a vsub.vi format. It would be nice if we could share the funct6 mapping somehow, since it is exactly the same on both for the cases that do exist, I'm just not sure how.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok makes sense, in that case sounds good to me to have a minor amount of duplication 👍

}
}
}

impl fmt::Display for VecAluOpRRImm5 {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let mut s = format!("{self:?}");
s.make_ascii_lowercase();
s.push_str(".vi");
f.write_str(&s)
}
}

impl VecAMode {
pub fn get_base_register(&self) -> Reg {
match self {
Expand Down
17 changes: 17 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst_vector.isle
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@
(Vxor)
))

;; Register-Imm ALU Ops
(type VecAluOpRRImm5 (enum
(Vadd)
))



;; Vector Addressing Mode
Expand Down Expand Up @@ -128,6 +133,13 @@
(_ Unit (emit (MInst.VecAluRRR op vd vs2 vs1 vstate))))
vd))

;; Helper for emitting `MInst.VecAluRRImm5` instructions.
(decl vec_alu_rr_imm5 (VecAluOpRRImm5 Reg Imm5 VState) Reg)
(rule (vec_alu_rr_imm5 op vs2 imm vstate)
(let ((vd WritableReg (temp_writable_reg $I8X16))
(_ Unit (emit (MInst.VecAluRRImm5 op vd vs2 imm vstate))))
vd))

;; Helper for emitting `MInst.VecLoad` instructions.
(decl vec_load (VecElementWidth VecAMode MemFlags VState) Reg)
(rule (vec_load eew from flags vstate)
Expand All @@ -146,6 +158,11 @@
(rule (rv_vadd_vv vs2 vs1 vstate)
(vec_alu_rrr (VecAluOpRRR.Vadd) vs2 vs1 vstate))

;; Helper for emitting the `vadd.vi` instruction.
(decl rv_vadd_vi (Reg Imm5 VState) Reg)
(rule (rv_vadd_vi vs2 imm vstate)
(vec_alu_rr_imm5 (VecAluOpRRImm5.Vadd) vs2 imm vstate))

;; Helper for emitting the `vsub.vv` instruction.
(decl rv_vsub_vv (Reg Reg VState) Reg)
(rule (rv_vsub_vv vs2 vs1 vstate)
Expand Down
6 changes: 6 additions & 0 deletions cranelift/codegen/src/isa/riscv64/lower.isle
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@
(rule 8 (lower (has_type (ty_vec_fits_in_register ty) (iadd x y)))
(rv_vadd_vv x y ty))

(rule 9 (lower (has_type (ty_vec_fits_in_register ty) (iadd x (replicated_imm5 y))))
(rv_vadd_vi x y ty))

(rule 10 (lower (has_type (ty_vec_fits_in_register ty) (iadd (replicated_imm5 x) y)))
(rv_vadd_vi y x ty))

;;; Rules for `uadd_overflow_trap` ;;;;;;;;;;;;;
(rule
(lower (has_type (fits_in_64 ty) (uadd_overflow_trap x y tc)))
Expand Down
4 changes: 4 additions & 0 deletions cranelift/codegen/src/isa/riscv64/lower/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ impl generated_code::Context for RV64IsleContext<'_, '_, MInst, Riscv64Backend>
Imm12::maybe_from_u64(arg0)
}
#[inline]
fn imm5_from_u64(&mut self, arg0: u64) -> Option<Imm5> {
Imm5::maybe_from_i8(arg0 as i64 as i8)
afonso360 marked this conversation as resolved.
Show resolved Hide resolved
}
#[inline]
fn writable_zero_reg(&mut self) -> WritableReg {
writable_zero_reg()
}
Expand Down
Loading