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

[BUG] Genus unsynthesizable error on rising/falling edge interrupt field #113

Closed
1 task done
BertVerrycken opened this issue May 25, 2024 · 2 comments
Closed
1 task done
Labels
bug Something isn't working

Comments

@BertVerrycken
Copy link

BertVerrycken commented May 25, 2024

Describe the bug

When defining a field inside an interrupt status register as a negedge intr, the peakrdl generated always_ff code creates
a non-blocking assignment outside the clocked if then else. The same issue is also observed for posedge intr fields.

Peakrdl version: 1.1.0
Python version: 3.11.5
OS: RedHat 8.9
Genus error: CDFG-364: unsynthesizable process

RDL file:

reg stat_irq_reg_t {
  name="irq_stat_reg";

  default hw=w;  // HW can Set int only
  default sw=rw; // SW can clear
  default woclr; // Clear is via writing a 1

  field {
    name="empty";
    desc="Fifo empty";
    negedge intr;
  } empty[0:0] = 1'b1;
};

addrmap test_reg{
  name = "test_addrmap";
  stat_irq_reg_t             irq_status_0;  // IRQ status and clear
};

command: peakrdl regblock test1.rdl -o test1

Peakrdl generated code in test_reg.sv in dir test1:

    always_ff @(posedge clk) begin
        if(rst) begin
            field_storage.irq_status_0.empty.value <= 1'h1;
        end else if(field_combo.irq_status_0.empty.load_next) begin
            field_storage.irq_status_0.empty.value <= field_combo.irq_status_0.empty.next;
        end
        field_storage.irq_status_0.empty.next_q <= hwif_in.irq_status_0.empty.next;
    end

The next_q assignment is outside the if rst else clock statement. The simulation tool does
not complain about it, but the synthesis tool does. Even if it is legal verilog, the best way
to implement this, is to have next_q assigned a reset value and do the assignment in
the end else (clocked) part. Like this (I have not verified the behaviour, just to look at
correcting the statement and bring it in the if else to make it synthesizable):

    always_ff @(posedge clk) begin
        if(rst) begin
            field_storage.irq_status_0.empty.value <= 1'h1;
            field_storage.irq_status_0.empty.next_q <= 1'h1;
        end
        else begin
           field_storage.irq_status_0.empty.next_q <= hwif_in.irq_status_0.empty.next;
           if(field_combo.irq_status_0.empty.load_next) begin
              field_storage.irq_status_0.empty.value <= field_combo.irq_status_0.empty.next;
          end
      end
    end

Expected behavior

I expect the falling/rising edge extra flop (next_q) to have a reset value and to be assigned in the clocked part.

@BertVerrycken BertVerrycken changed the title [BUG] Genus unsynthesizable error on rising edge interrupt field [BUG] Genus unsynthesizable error on rising/falling edge interrupt field May 25, 2024
@amykyta3 amykyta3 added the bug Something isn't working label Dec 15, 2024
@amykyta3
Copy link
Member

Strange that Genus complains about that as unsynthesizable.
That is a perfectly valid and common way to describe an assignment to a storage element that does not have a synchronous reset.

Even so, this would end up causing issues with asynchronous resets, and it is probably best to explicitly reset this storage element anyways.

amykyta3 added a commit that referenced this issue Dec 20, 2024
@amykyta3
Copy link
Member

Implemented in v0.23.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants