Codegen: bug: failure for a proc with two RAMs that are accessed conditionally
- Dominant language
- C++
- Stars
- 1.9k
- Forks
- 283
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 135
Description
It is easy to write a proc which has two 1R1W RAMs and accesses them only when certain condition is met (using `send_if`, `recv_if`), that, if codegen is passed `ram_configurations` option, will fail Verilog code generation with a following error:
```
F0622 17:57:02.261463 2 codegen_main.cc:330] Check failed: ::absl::OkStatus() == (xls::RealMain(ir_path)) (OK vs. INVALID_ARGUMENT: Highest value in range fails condition of binary search.)
```
Initially I thought that this was a case of #1043. However, the solution described there, which is to increase number of pipeline stages, does not help here (even with e.g. 1000 stages the codegen still fails). The generation succeeds if we do not ask codegen to rewrite RAM interfaces - that is, it can generate Verilog with classic `data, vld, rdy` streams, but not with `rd_en, wr_en, rd_addr, rd_data, wr_addr, wr_data` buses.
From what I see, the issue happens when accesses to RAM are conditional. If the RAM is always accessed, e.g. you set `let cond = true;`, the codegen succeeds. The generation also succeeds when only one RAM is used and e.g. the read is unconditional but the write is conditional. Although most probably this depends on a specific datapath design, so it may be difficult to generalize based on these isolated observations..
@proppy @cdleary Please take a look at this if possible. Perhaps I'm missing something obvious here and it can be made to work with some extra codegen option? LZ4 encoder from #995 uses two RAMs in a way somewhat similar to what's described here and runs into this issue, which prevents us from generating Verilog module for it with SRAM-like memory interfaces.
---
DSLX proc to reproduce the issue:
```
import xls.examples.ram as ram
type RamWriteReq = ram::WriteReq;
type RamWriteResp = ram::WriteResp;
type RamReadReq = ram::ReadReq;
type RamReadResp = ram::ReadResp;
proc dpram {
o_write_req1: chan> out;
i_write_resp1: chan in;
o_read_req1: chan> out;
i_read_resp1: chan> in;
o_write_req2: chan> out;
i_write_resp2: chan in;
o_read_req2: chan> out;
i_read_resp2: chan> in;
init {u16:0}
config(
o_write_req1: chan> out,
i_write_resp1: chan in,
o_read_req1: chan> out,
i_read_resp1: chan> in,
o_write_req2: chan> out,
i_write_resp2: chan in,
o_read_req2: chan> out,
i_read_resp2: chan> in,
) {
(
o_write_req1, i_write_resp1,
o_read_req1, i_read_resp1,
o_write_req2, i_write_resp2,
o_read_req2, i_read_resp2,
)
}
next (tok: token, state: u16) {
let addr = state;
// - D=RAM1[addr]
// - D+=1
// - RAM2[addr]=D
// - addr++
// - if D > 100:
// - D=RAM2[addr]
// - D+=2
// - RAM1[addr]=D
// - addr++
let tok = send(tok, o_read_req1, ram::ReadWordReq(addr));
let (tok, resp) = recv(tok, i_read_resp1);
let d = resp.data;
//
let d = d + u8:1;
//
let tok = send(tok, o_write_req2, ram::WriteWordReq(addr, d));
let (tok, _) = recv(tok, i_write_resp2);
//
let addr = addr + u16:1;
//
// Calculate condition that controls whether below actions are executed
let cond = d > u8:100;
//
let tok = send_if(tok, o_read_req2, cond, ram::ReadWordReq(addr));
let (tok, resp) = recv_if(tok, i_read_resp2, cond, RamReadResp{
data: u8:0,
});
let d = resp.data;
//
let d = d + u8:2;
//
let tok = send_if(tok, o_write_req1, cond, ram::WriteWordReq(addr, d));
let (tok, _) = recv_if(tok, i_write_resp1, cond, RamWriteResp{});
//
let addr = if cond { addr + u16:1 } else { addr };
addr
}
}
```
Bazel rules:
```
xls_dslx_library(
name = "dpram_dslx",
srcs = [
"dpram.x"
],
deps = [
"//xls/examples:ram_dslx",
],
)
xls_dslx_test(
name = "dpram_dslx_test",
dslx_test_args = {
"compare": "none",
},
library = "dpram_dslx",
)
xls_dslx_ir(
name = "dpram_ir",
dslx_top = "dpram",
library = "dpram_dslx",
ir_file = "dpram_ir.ir",
)
xls_ir_opt_ir(
name = "dpram_opt_ir",
src = "dpram_ir.ir",
top = "__dpram__dpram_0_next",
)
xls_ir_verilog(
name = "dpram_verilog",
src = "dpram_opt_ir.opt.ir",
verilog_file = "dpram.v",
codegen_args = {
"module_name": "dpram",
"delay_model": "unit",
"pipeline_stages": "1000",
"reset": "rst",
"use_system_verilog": "false",
"ram_configurations": ",".join([
"ram1:1R1W:{rd_req}:{rd_resp}:{wr_req}:{wr_resp}".format(
rd_req = "dpram__o_read_req1",
rd_resp = "dpram__i_read_resp1",
wr_req = "dpram__o_write_req1",
wr_resp = "dpram__i_write_resp1",
),
"ram2:1R1W:{rd_req}:{rd_resp}:{wr_req}:{wr_resp}".format(
rd_req = "dpram__o_read_req2",
rd_resp = "dpram__i_read_resp2",
wr_req = "dpram__o_write_req2",
wr_resp = "dpram__i_write_resp2",
),
]),
},
)
```
Contributor guide
Assessment
This issue has not been assessed yet.