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.

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

Updates #3

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Updates #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7dcbf5
add mnemonics to s4e-mac.core_desc
PhilippvK Jun 3, 2024
e2c98cd
use s4e prefix in s4e-mac.test.c
PhilippvK Jun 3, 2024
4b4561f
add more assembler tests
PhilippvK Jun 3, 2024
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
14 changes: 7 additions & 7 deletions s4e-mac.core_desc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,25 @@ InstructionSet X_S4E_MAC extends RV32I {
instructions {
RESET_ACC { // v-- funct7 v-- funct3
encoding: 7'd0 :: 10'b0 :: 3'd0 :: 5'b0 :: 7'b0001011;
assembly:"";
assembly: {"s4e.reset_acc", ""};
behavior: ACC = 0;
}

GET_ACC_LO {
encoding: 7'd1 :: 10'b0 :: 3'd0 :: rd[4:0] :: 7'b0001011;
assembly:"{name(rd)}";
assembly: {"s4e.get_acc_lo", "{name(rd)}"};
Copy link
Contributor

Choose a reason for hiding this comment

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

If I thought I understood any universal rule for how RISC-V instructions get named I'd be suggesting that a generation rule is placed elsewhere, but it would surely need the possibility of overriding per-instruction anyway.

As I remember it, the assembly statement was added to CoreDSL to support disassembly in the MINRES tooling and gets concatenated into a Python string (hence the "name" array of integer registers, supplemented in the FP instructions with the "f" array. If some directly addressable custom registers get added through an extension then some more generic way of labeling them will be needed). So an extension written using this approach would probably break the MINRES tooling - given how under-specified the "assembly" statement is this isn't an immediate issue, but maybe an additional "instruction" statement would be safer than extending what's there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed earlier. This mnemonic syntax is part of the CoreDSL spec and supported by the MINRES tooling. It is entirely optional but Seal5 can make use of it. The extensible compiler will not understand it but there is no need to fix this as an older commit of this repo can still be used.

Could we please merge this soon @thomasgoodfellow ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW the syntax change happened here: Minres/CoreDSL#80 (comment)

behavior: if (rd != 0) X[rd] = ACC[31:0];
}

GET_ACC_HI {
encoding: 7'd2 :: 10'b0 :: 3'd0 :: rd[4:0] :: 7'b0001011;
assembly:"{name(rd)}";
assembly: {"s4e.get_acc_hi", "{name(rd)}"};
behavior: if (rd != 0) X[rd] = ACC[63:32];
}

MACU_32 {
encoding: 7'd0 :: rs2[4:0] :: rs1[4:0] :: 3'd1 :: 5'b0 :: 7'b0001011;
assembly:"{name(rs1)}, {name(rs2)}";
assembly: {"s4e.macu_32", "{name(rs1)}, {name(rs2)}"};
behavior: {
unsigned<64> mul = X[rs1] * X[rs2];
unsigned<33> add = mul[31:0] + ACC[31:0];
Expand All @@ -36,7 +36,7 @@ InstructionSet X_S4E_MAC extends RV32I {

MACS_32 {
encoding: 7'd1 :: rs2[4:0] :: rs1[4:0] :: 3'd1 :: 5'b0 :: 7'b0001011;
assembly:"{name(rs1)}, {name(rs2)}";
assembly: {"s4e.macs_32", "{name(rs1)}, {name(rs2)}"};
behavior: {
signed<64> mul = ((signed) X[rs1]) * ((signed) X[rs2]);
signed<33> add = ((signed) mul[31:0]) + ((signed) ACC[31:0]);
Expand All @@ -46,7 +46,7 @@ InstructionSet X_S4E_MAC extends RV32I {

MACU_64 {
encoding: 7'd0 :: rs2[4:0] :: rs1[4:0] :: 3'd2 :: 5'b0 :: 7'b0001011;
assembly:"{name(rs1)}, {name(rs2)}";
assembly: {"s4e.macu_64", "{name(rs1)}, {name(rs2)}"};
behavior: {
unsigned<64> mul = X[rs1] * X[rs2];
unsigned<65> add = mul + ACC;
Expand All @@ -56,7 +56,7 @@ InstructionSet X_S4E_MAC extends RV32I {

MACS_64 {
encoding: 7'd1 :: rs2[4:0] :: rs1[4:0] :: 3'd2 :: 5'b0 :: 7'b0001011;
assembly:"{name(rs1)}, {name(rs2)}";
assembly: {"s4e.macs_64", "{name(rs1)}, {name(rs2)}"};
behavior: {
signed<64> mul = ((signed) X[rs1]) * ((signed) X[rs2]);
signed<65> add = mul + ((signed) ACC);
Expand Down
28 changes: 28 additions & 0 deletions s4e-mac.test-invalid.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# MC test of sample S4E MAC instruction extensions

# RUN: not llvm-mc -triple=riscv32 --mattr=+xs4emac %s 2>&1 \
# RUN: | FileCheck %s --check-prefixes=CHECK-ERROR

s4e.reset_acc 0
# CHECK-ERROR: invalid operand for instruction

s4e.reset_acc t1
# CHECK-ERROR: invalid operand for instruction

s4e.get_acc_lo 0
# CHECK-ERROR: invalid operand for instruction

s4e.get_acc_hi 0
# CHECK-ERROR: invalid operand for instruction

s4e.macu_32 s5, 0
# CHECK-ERROR: invalid operand for instruction

s4e.macs_32 0, a7
# CHECK-ERROR: invalid operand for instruction

s4e.macu_64 s5, 0
# CHECK-ERROR: invalid operand for instruction

s4e.macs_64 0, a7
# CHECK-ERROR: invalid operand for instruction
28 changes: 14 additions & 14 deletions s4e-mac.test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,26 @@ int main() {
// CHECK: b3 01 52 00 add x3, x4, x5
asm("add x3, x4, x5");

// CHECK: 0b 00 00 00 reset_acc
asm("reset_acc");
// CHECK: 0b 00 00 00 s4e.reset_acc
asm("s4e.reset_acc");

// CHECK: 0b 03 00 02 get_acc_lo x6
asm("get_acc_lo x6");
// CHECK: 0b 03 00 02 s4e.get_acc_lo x6
asm("s4e.get_acc_lo x6");

// CHECK: 8b 0c 00 04 get_acc_hi x25
asm("get_acc_hi x25");
// CHECK: 8b 0c 00 04 s4e.get_acc_hi x25
asm("s4e.get_acc_hi x25");

// CHECK: 0b 90 1a 01 macu_32 x21, x17
asm("macu_32 x21, x17");
// CHECK: 0b 90 1a 01 s4e.macu_32 x21, x17
asm("s4e.macu_32 x21, x17");

// CHECK: 0b 90 1a 03 macs_32 x21, x17
asm("macs_32 x21, x17");
// CHECK: 0b 90 1a 03 s4e.macs_32 x21, x17
asm("s4e.macs_32 x21, x17");

// CHECK: 0b a0 1a 01 macu_64 x21, x17
asm("macu_64 x21, x17");
// CHECK: 0b a0 1a 01 s4e.macu_64 x21, x17
asm("s4e.macu_64 x21, x17");

// CHECK: 0b a0 1a 03 macs_64 x21, x17
asm("macs_64 x21, x17");
// CHECK: 0b a0 1a 03 s4e.macs_64 x21, x17
asm("s4e.macs_64 x21, x17");

return 42;
}
41 changes: 41 additions & 0 deletions s4e-mac.test.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# MC test of sample S4E MAC instruction extensions

# RUN: llvm-mc -triple=riscv32 --mattr=+xs4emac -show-encoding %s \
# RUN: | FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INSTR
# RUN: not llvm-mc -triple riscv32 %s 2>&1 \
# RUN: | FileCheck -check-prefix=CHECK-NO-EXT %s

s4e.reset_acc
# CHECK-INSTR: s4e.reset_acc
# CHECK-ENCODING: [0x0b,0x00,0x00,0x00]
# CHECK-NO-EXT: instruction requires the following: 'XS4EMAC' (X_S4E_MAC Extension){{$}}

s4e.get_acc_lo t1
# CHECK-INSTR: s4e.get_acc_lo t1
# CHECK-ENCODING: [0x0b,0x03,0x00,0x02]
# CHECK-NO-EXT: instruction requires the following: 'XS4EMAC' (X_S4E_MAC Extension){{$}}

s4e.get_acc_hi s9
# CHECK-INSTR: s4e.get_acc_hi s9
# CHECK-ENCODING: [0x8b,0x0c,0x00,0x04]
# CHECK-NO-EXT: instruction requires the following: 'XS4EMAC' (X_S4E_MAC Extension){{$}}

s4e.macu_32 s5, a7
# CHECK-INSTR: s4e.macu_32 s5, a7
# CHECK-ENCODING: [0x0b,0x90,0x1a,0x01]
# CHECK-NO-EXT: instruction requires the following: 'XS4EMAC' (X_S4E_MAC Extension){{$}}

s4e.macs_32 s5, a7
# CHECK-INSTR: s4e.macs_32 s5, a7
# CHECK-ENCODING: [0x0b,0x90,0x1a,0x03]
# CHECK-NO-EXT: instruction requires the following: 'XS4EMAC' (X_S4E_MAC Extension){{$}}

s4e.macu_64 s5, a7
# CHECK-INSTR: s4e.macu_64 s5, a7
# CHECK-ENCODING: [0x0b,0xa0,0x1a,0x01]
# CHECK-NO-EXT: instruction requires the following: 'XS4EMAC' (X_S4E_MAC Extension){{$}}

s4e.macs_64 s5, a7
# CHECK-INSTR: s4e.macs_64 s5, a7
# CHECK-ENCODING: [0x0b,0xa0,0x1a,0x03]
# CHECK-NO-EXT: instruction requires the following: 'XS4EMAC' (X_S4E_MAC Extension){{$}}