google / google/xls

codegen should not emit dangling signals for nested procs w/ combo next body

Open
#4,635 0 comments 0 reactions 0 assignees View on GitHub
codegen 🧦 sox
Dominant language
C++
Stars
1.9k
Forks
283
Avg merge
2d 10h
Merged PRs (30d)
135

Description

**Describe the bug**
When a proc resolves to a combinational module (either intentionally: pure forwarding procs, if I/O operation get optimized away, or if it has no state), codegen will still emit a `clk` and `rst` signals for the corresponding verilog module.

While top-level proc signals can easily be marked as unread, this is more of a problem for nested sub procs as their internal clock or reset signal would still be driven from the overall proc network connections but end up never being read, hence producing potential warning/error in downstream tools.

**To Reproduce**
Codegen the following proc:
```
proc PlusOne {
ns: chan in;
plusones: chan out;

config(ns: chan in, plusones: chan out) {
(ns, plusones)
}

init {}

next(state: ()) {
let (_, n) = recv(token(), ns);
send(token(), plusones, n + 1);
}
}
```
Result in the following verilog module:
```
module user_module(
input wire clk,
input wire [7:0] _ns_data,
input wire _ns_valid,
input wire _plusones_ready,
output wire _ns_ready,
output wire [7:0] _plusones_data,
output wire _plusones_valid
);
wire stage_outputs_valid_0;
wire [7:0] add_59;
assign stage_outputs_valid_0 = _ns_valid & _plusones_ready;
assign add_59 = _ns_data + 8'h01;
assign _ns_ready = stage_outputs_valid_0;
assign _plusones_data = add_59;
assign _plusones_valid = _ns_valid;
endmodule
```

**Expected behavior**
unused internal `clk` or `rst` signal would be not emited or marked as unread using a developer provided macro.

Contributor guide

Open the contributing guide

Research direction

Start in the codegen path for nested procs that resolve to combinational modules, using the PlusOne proc and its generated Verilog as the reproduction. Trace how clk and rst signals are added and connected, then verify that combinational nested procs no longer emit unused signals or mark them unread, while stateful procs retain the required signals.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.