SpinalHDL / SpinalHDL

Scala based HDL
Other
1.68k stars 330 forks source link

Outer FSM `whenIsActive` statements always win vs. nested FSM `onEntry` statements for combinational logic #1528

Open thajohns opened 2 months ago

thajohns commented 2 months ago

If an FSM drives a combinational signal from the whenIsActive block of one of its states, and then transitions to a nested FSM state whose entry state has an onEntry block which also drives the combinational signal, then the outer FSM's statement always wins even if it appears earlier in the file.

For example:

case class BrokenFsm() extends Component {
  val io = new Bundle {
      val go = in Bool()
      val a = out Bool()
      val b = out Bool()
  }
  io.a := False
  io.b := False

  val fsm = new StateMachine() {
      val idle: State = new State with EntryPoint() {
          whenIsActive {
              io.b := False
              when (io.go) { goto(par) }
          }
      }
      val par = new StateFsm(
          new StateMachine {
              val nested = new State() with EntryPoint {
                  onEntry {
                      io.a := True
                      io.b := True
                  }
                  whenIsActive { exit() }

              }
          }
      ) {
          whenCompleted { goto(idle) }
      }
  }
}

In the above FSM, we expect to see io.a and io.b act the same way, since the second driver on io.b (the only one which io.a does not have) is redundant. However, io.b never goes high, but io.a does.

The generated Verilog also demonstrates the issue. The (correct) block produced for io.a is:

  always @(*) begin
    io_a = 1'b0;
    if(when_StateMachine_l253) begin
      io_a = 1'b1;
    end
  end

whereas the (incorrect) block for io.b is:

  always @(*) begin
    io_b = 1'b0;
    if(when_StateMachine_l253) begin
      io_b = 1'b1;
    end
    case(fsm_stateReg)
      fsm_enumDef_idle : begin
        io_b = 1'b0;
      end
      fsm_enumDef_par : begin
      end
      default : begin
      end
    endcase
  end

In the latter, the branch for the idle state comes after the branch for the onEntry state (the if above), which if I understand Verilog's semantics correctly means that it wins.

thajohns commented 2 months ago

On further testing, the issue also happens with registers.

Dolu1990 commented 2 months ago

Hi,

Yes right. Currently, the parrent statemachine first do childStateMachines.foreach(_.build()) and then build itself. Instead what should maybe be done, is to stage the child state machine build into multiple sub step called at different stages of the parrent FSM.