if a module assigns a value to a Signal within an finite state machine implemented as a migen.genlib.fsm.FSM, then generated Verilog will append an extra assignment to the always block to reset the generated register.

Implementing the state machine manually and not as an migen.genlib.fsm.FSM generates Verilog where the reset assignment is instead placed within an if (sys_reset) block.

If NextValue is used to assign to the signal this issue does not arise, but does heavily complicate the output, and add delays on the output which may not be desired.

My question is why is this assignment not within the reset block added, and can it be prevented whilst still using the FSM class. Is it a bug, or is it hinting at bad HDL design on my part?

Example:

Consider a module with two button inputs and a single LED output. One button should turn the LED on, and one turn it off. If I were to implement this with an FSM I might try:

from migen import *
from migen.fhdl import *

class FSMExample(Module):
    def __init__(self):
        # declare signals
        self.led = Signal()
        self.btn1 = Signal()
        self.btn2 = Signal()
        # define I/O
        self.ios = {self.led, self.btn1, self.btn2}
        self.fsm = FSM(reset_state="OFF")
        self.fsm.act("OFF",
            self.led.eq(0),
            If(self.btn1, NextState("ON"))
        )
        self.fsm.act("ON",
            self.led.eq(1),
            If(self.btn2, NextState("OFF"))
        )
        self.submodules += self.fsm

yet this generates (with verilog.convert(FSMExample(),FSMExample().ios) the following verilog:

module top(
        input led,
        input btn1,
        input btn2,
        input sys_clk,
        input sys_rst
);

reg fsmexample0_led;
reg fsmexample0_btn1 = 1'd0;
reg fsmexample0_btn2 = 1'd0;
reg state = 1'd0;
reg next_state;

// synthesis translate_off
reg dummy_s;
initial dummy_s <= 1'd0;
// synthesis translate_on


// synthesis translate_off
reg dummy_d;
// synthesis translate_on
always @(*) begin
        fsmexample0_led <= 1'd0;
        next_state <= 1'd0;
        next_state <= state;
        case (state)
                1'd1: begin
                        fsmexample0_led <= 1'd1;
                        if (fsmexample0_btn2) begin
                                next_state <= 1'd0;
                        end
                end
                default: begin
                        fsmexample0_led <= 1'd0;
                        if (fsmexample0_btn1) begin
                                next_state <= 1'd1;
                        end
                end
        endcase
// synthesis translate_off
        dummy_d <= dummy_s;
// synthesis translate_on
end

always @(posedge sys_clk) begin
        state <= next_state;
        if (sys_rst) begin
                state <= 1'd0;
        end
end

endmodule

Note the fsmexample0_led <= 1'd0; within the always block, resetting the LED even after btn2 is pressed.

Even manually implementing an FSM fixes this behaviour:

class FakeFSMExample(Module):
    def __init__(self):
        # declare signals
        self.led = Signal()
        self.btn1 = Signal()
        self.btn2 = Signal()
        self.state = Signal()
        self.next_state = Signal()
        # define I/O
        self.ios = {self.led, self.btn1, self.btn2}
        self.sync += self.state.eq(self.next_state)
        self.sync += If(~self.state,
            self.led.eq(0), 
            If(self.btn1, self.next_state.eq(1))
            ).Elif(self.state,
                self.led.eq(1), 
                If(self.btn2, self.next_state.eq(0))
            )

generates

module top(
        input led,
        input btn1,
        input btn2,
        input sys_clk,
        input sys_rst
);

reg fakefsmexample0_led = 1'd0;
reg fakefsmexample0_btn1 = 1'd0;
reg fakefsmexample0_btn2 = 1'd0;
reg fakefsmexample0_state = 1'd0;
reg fakefsmexample0_next_state = 1'd0;


always @(posedge sys_clk) begin
        fakefsmexample0_state <= fakefsmexample0_next_state;
        if ((~fakefsmexample0_state)) begin
                fakefsmexample0_led <= 1'd0;
                if (fakefsmexample0_btn1) begin
                        fakefsmexample0_next_state <= 1'd1;
                end
        end else begin
                if (fakefsmexample0_state) begin
                        fakefsmexample0_led <= 1'd1;
                        if (fakefsmexample0_btn2) begin
                                fakefsmexample0_next_state <= 1'd0;
                        end
                end
        end
        if (sys_rst) begin
                fakefsmexample0_led <= 1'd0;
                fakefsmexample0_state <= 1'd0;
                fakefsmexample0_next_state <= 1'd0;
        end
end

endmodule

where fakefsmexample0_led <= 1'd0 lives within an if (sys_rst) as desired.