Skip to content

Commit

Permalink
flwadd
Browse files Browse the repository at this point in the history
  • Loading branch information
tommydcjung committed Apr 20, 2021
1 parent 3acb35f commit 43f78e7
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 10 deletions.
2 changes: 1 addition & 1 deletion v/vanilla_bean/alu.v
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ always_comb
`RV32_AUIPC:
result_o = `RV32_signext_Uimm(op_i) + pc_plus4_i - 3'b100;

`RV32_ADDI, `RV32_ADD:
`RV32_ADDI, `RV32_ADD, `RV32_FLWADD:
begin
result_o = sum[31:0];
sub_not_add = 1'b0;
Expand Down
20 changes: 20 additions & 0 deletions v/vanilla_bean/bsg_vanilla_pkg.v
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ typedef struct packed {
logic is_byte_op; // Op is byte load/store
logic is_hex_op; // Op is hex load/store
logic is_load_unsigned; // Op is unsigned load
logic is_flwadd_op; // flwadd

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

update comment about for is_load_op and is_store_op -- apparently it is more nuanced than that

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

something about it being a load or store op with an immediate? Not sure.


// Branch & Jump
logic is_branch_op;
Expand Down Expand Up @@ -539,4 +540,23 @@ typedef struct packed {
`define RV32_FSQRT_S {7'b0101100, 5'b00000, 5'b?????, 3'b???, 5'b?????, 7'b1010011}



// NON-STANDARD RISC-V Instructions

// [FLWADD]
//
// Assembly format
// flwadd fd, rs2, 0(rs1)
//
// Semantic:
// fd = *rs1; rs1 = rs1 + rs2;
//
// Machine Format:
// rs1 rs2 rd opcode
// 0000000_?????_?????_111_?????_0000111
`define RV32_FLWADD_OP 7'b0000100

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

seems like we could squeeze this instruction inside the FPU map rather than taking an entire major opcode? the big opcode spaces are the most valuable.

`define RV32_FLWADD {7'b0000000, 5'b?????, 5'b?????, 3'b111, 5'b?????, `RV32_FLWADD_OP}



endpackage
15 changes: 12 additions & 3 deletions v/vanilla_bean/cl_decode.v
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ always_comb begin
`RV32_LUI_OP, `RV32_AUIPC_OP,
`RV32_JAL_OP, `RV32_JALR_OP,
`RV32_LOAD, `RV32_OP,
`RV32_OP_IMM, `RV32_AMO_OP: begin
`RV32_OP_IMM, `RV32_AMO_OP, `RV32_FLWADD_OP: begin

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

comment above:

// Op Writes RF -- register file write operation

-->
// Op Writes Integer RF -- register file write operation

decode_o.write_rd = 1'b1;
end
`RV32_OP_FP: begin
Expand All @@ -57,7 +57,7 @@ always_comb begin
`RV32_JALR_OP, `RV32_BRANCH,
`RV32_LOAD, `RV32_STORE,
`RV32_OP, `RV32_OP_IMM,
`RV32_AMO_OP: begin
`RV32_AMO_OP, `RV32_FLWADD_OP: begin

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

// declares if OP reads from first port of register file
-->
// declares if OP reads from first port of integer register file

decode_o.read_rs1 = 1'b1;
end
`RV32_OP_FP: begin
Expand Down Expand Up @@ -87,7 +87,7 @@ end
// declares if Op reads from second port of register file

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

of integer register file

always_comb begin
unique casez (instruction_i.op)
`RV32_BRANCH, `RV32_STORE, `RV32_OP: begin
`RV32_BRANCH, `RV32_STORE, `RV32_OP, `RV32_FLWADD_OP: begin
decode_o.read_rs2 = 1'b1;
end
`RV32_AMO_OP: begin
Expand All @@ -103,6 +103,7 @@ always_comb begin
end

// Load & Store
assign decode_o.is_flwadd_op = (instruction_i.op == `RV32_FLWADD_OP);
assign decode_o.is_load_op = (instruction_i.op == `RV32_LOAD) | (instruction_i.op == `RV32_LOAD_FP);
assign decode_o.is_store_op = (instruction_i.op == `RV32_STORE) | (instruction_i.op == `RV32_STORE_FP);

Expand Down Expand Up @@ -322,6 +323,14 @@ always_comb begin
decode_o.write_frd = 1'b1;
decode_o.is_fp_op = 1'b1;
end
// FLWADD
`RV32_FLWADD: begin
decode_o.read_frs1 = 1'b0;
decode_o.read_frs2 = 1'b0;
decode_o.read_frs3 = 1'b0;
decode_o.write_frd = 1'b1;
decode_o.is_fp_op = 1'b0;

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

add comment; what does is_fp_op mean and why does FLWADD not count as is_fp_op?

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

add comment for FSW as well and FLW as well.

end
default: begin
decode_o.read_frs1 = 1'b0;
decode_o.read_frs2 = 1'b0;
Expand Down
4 changes: 2 additions & 2 deletions v/vanilla_bean/lsu.v
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ module lsu

assign dmem_v_o = is_local_dmem_addr &
(exe_decode_i.is_load_op | exe_decode_i.is_store_op |
exe_decode_i.is_lr_op | exe_decode_i.is_lr_aq_op);
exe_decode_i.is_lr_op | exe_decode_i.is_lr_aq_op | exe_decode_i.is_flwadd_op);
assign dmem_w_o = exe_decode_i.is_store_op;
assign dmem_addr_o = mem_addr[2+:dmem_addr_width_lp];
assign dmem_data_o = store_data;
Expand Down Expand Up @@ -152,7 +152,7 @@ module lsu


assign remote_req_v_o = icache_miss_i |
((exe_decode_i.is_load_op | exe_decode_i.is_store_op | exe_decode_i.is_amo_op) & ~is_local_dmem_addr);
((exe_decode_i.is_load_op | exe_decode_i.is_store_op | exe_decode_i.is_amo_op | exe_decode_i.is_flwadd_op) & ~is_local_dmem_addr);

// reserve
// only valid on local DMEM
Expand Down
9 changes: 5 additions & 4 deletions v/vanilla_bean/vanilla_core.v
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ module vanilla_core
| id_r.decode.is_lr_aq_op
| id_r.decode.is_amo_op;

wire [data_width_p-1:0] mem_addr_op2 = is_amo_or_lr_op
wire [data_width_p-1:0] mem_addr_op2 = (is_amo_or_lr_op | id_r.decode.is_flwadd_op)
? '0
: (id_r.decode.is_store_op
? `RV32_signext_Simm(id_r.instruction)
Expand Down Expand Up @@ -1213,7 +1213,7 @@ module vanilla_core
wire id_rs2_non_zero = id_rs2 != '0;
wire id_rd_non_zero = id_rd != '0;
wire int_remote_load_in_exe = remote_req_in_exe & exe_r.decode.is_load_op & exe_r.decode.write_rd;

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

add comment to above line:
// importantly, an is_flwadd_op instruction does not cause is_load_op to be set high, so the fact that
// is_flwadd_op has exe_r.decode.write_rd high will not trigger int_remote_load_in_exe

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 20, 2021

Contributor

Seems like this pair of signals should be called int/float_scoreboard_remote_load_in_exe since the signal appears to be just about scoreboarding; when you do a load to $0, there is a remote load in exe, it just doesn't get scoreboarded?

This comment has been minimized.

Copy link
@tommydcjung

tommydcjung Apr 20, 2021

Author Contributor

x0 is handled in scoreboard.v

This comment has been minimized.

Copy link
@taylor-bsg

taylor-bsg Apr 22, 2021

Contributor

Yeah, but my point is that this signal can be false when you have a remote load in exe, so it is not correctly named.

wire float_remote_load_in_exe = remote_req_in_exe & exe_r.decode.is_load_op & exe_r.decode.write_frd;
wire float_remote_load_in_exe = remote_req_in_exe & (exe_r.decode.is_load_op | exe_r.decode.is_flwadd_op) & exe_r.decode.write_frd;
wire fdiv_fsqrt_in_fp_exe = fp_exe_r.fp_decode.is_fdiv_op | fp_exe_r.fp_decode.is_fsqrt_op;
wire remote_credit_pending = (out_credits_i != max_out_credits_p);

Expand Down Expand Up @@ -1286,7 +1286,8 @@ module vanilla_core
|id_r.decode.is_store_op
|id_r.decode.is_amo_op
|id_r.decode.is_lr_aq_op
|id_r.decode.is_lr_op);
|id_r.decode.is_lr_op
|id_r.decode.is_flwadd_op);

// stall_amo_rl
// If there is a remote request in EXE, there is a technically remote request pending, even if the credit counter has not yet been decremented.
Expand All @@ -1297,7 +1298,7 @@ module vanilla_core
// stall_remote_req
logic [lg_fwd_fifo_els_lp-1:0] remote_req_counter_r;
wire local_mem_op_restore = (lsu_dmem_v_lo & ~exe_r.decode.is_lr_op & ~exe_r.decode.is_lr_aq_op) & ~stall_all;
wire id_remote_req_op = (id_r.decode.is_load_op | id_r.decode.is_store_op | id_r.decode.is_amo_op | id_r.icache_miss);
wire id_remote_req_op = (id_r.decode.is_load_op | id_r.decode.is_store_op | id_r.decode.is_amo_op | id_r.icache_miss | id_r.decode.is_flwadd_op);
wire memory_op_issued = id_remote_req_op & ~flush & ~stall_id & ~stall_all;
wire [lg_fwd_fifo_els_lp-1:0] remote_req_available =
remote_req_counter_r +
Expand Down

1 comment on commit 43f78e7

@taylor-bsg
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice changes -- some feedback just to address nuances of some of the control signals; and also to reduce encoding space

Please sign in to comment.