BlackBoxで参照してたRTLのバグでハマった話 - ハードウェアの気になるあれこれ

通常運行に戻って、Chiselネタを。今回はこのつぶやきの話。

なーーーーーーーーーーーーんか、おかしいんだよなーーーーーって、思いながらも動いてるからいっか！！ってなって部分があったんだけど。。。。。
読み込んでるBlackBoxのメモリ推定記述のFF推定式にブロッキング代入が混じってるのが原因だった😱🤣😭

、、、、これブログに書くか、戒めとして😂😖
— だいにんぎょー@プリンの人 (@diningyo) December 25, 2019

BlackBoxのメモリ推定記述に仕込んだバグ

BlackBoxのメモリ推定記述に仕込んだバグ

やっと技書博の作業も落ち着いたし久々にRISC-Vの開発進めよーということで、dirvの作業を再開しました。ということでgithubを見に行ったらissuesが登録されていて、中身をみるとテストの結果がREADMEに書いてあるのと違ってますぜ！という話でした。

そういや今のmasterの状態ってUARTと繋いで動かした時のなので、テスト流し直してなかったなーという記憶が蘇り、デバッグをすることに。

そこで出くわしたのが、今回の話。いまのdirvの最新の状態でテストを流すとriscv-testsがキレイに全部落ちる。。。。で、単体で流して波形も取得してみるとおかしな動きが見えてます。そのおかしな動きというのが以下の波形。

f:id:diningyo-kpuku-jougeki:20191226002842p:plain — バグった波形

というか、これよくわからないけど、無理やり通してFPGAに持って行ったら動いたから、とりあえず「まあ、いっか！！後で直そ！」で放置になってました。

これだけだと、よくわからないので該当部分のChiselのコードも先に記載します。

  // qInst自体はレジスタ
  val qInst = RegInit(VecInit(Seq.fill(2)(0.U(cfg.dataBits.W))))

  val imemAddrReg = RegInit(cfg.initAddr.U)
  val fsm = RegInit(sIdle)

  when (qInstWren) {
    when (io.exu2ifu.updatePcReq) {
      imemAddrReg := io.exu2ifu.updatePc + 4.U
    } .otherwise {
      imemAddrReg := imemAddrReg + 4.U
    }
  }

  // Instruction FIFO
  val qInstIsFull = Wire(Bool())
  qInstWren := io.ifu2ext.r.get.valid && (!qInstIsFull)
  qInstRden := io.ifu2idu.valid && io.ifu2idu.ready

  when (qInstFlush) {
    qInst(0) := dirv.Constants.nop
  // qInstWrenが0x1の時にqInstWrPtrの示す”レジスタ”にr.get.bits.dataが格納される
  } .elsewhen (qInstWren) {
    qInst(qInstWrPtr) := io.ifu2ext.r.get.bits.data
  }

やってる事自体は複雑でもなくて、”ただ単にIFUから出したRISC-Vのプログラムのリード要求に対して、返ってきた命令をqInstに格納していく”というものです。で、”おかしい部分”ですが”io_ifu2ext_r_bits_dataがqInst_0/qInst_1に格納されるタイミングは本来1cycleずれるはずが、なぜか同じサイクルでqInstに反映されている”という点です。

f:id:diningyo-kpuku-jougeki:20191226003035p:plain — バグった波形

UARTの実装と動作確認した際にも同じような現象に出くわしていて、とりあえずUARTとdirvの結合試験環境でのテスト時に無理やりテストを通した結果riscv-testsがデグレしたという状態でした。

で、なんでなんだろーとコードを追っかけて行った所、原因はBlackBoxで読み込んでいるVerilogのメモリ推定記述に仕込んだバグでした。

そのバグってるコードがこちら↓。これはXilinxのFPGA向けのデュアルポートRAMの推論記述です。普通にVerologでRTLを書いてる人が見れば、すぐに気づけると思います。

    // imem side
    always @(posedge clk) begin
        if (rena) begin
            qa = mem[addra];
        end
    end

    // dmem side
    always @(posedge clk) begin
        // read
        if (renb) begin
            qb = mem[addrb];
        end

        // write
        if (wenb) begin
            for (i = 0; i < 4; i++) begin
                if (webb[i]) begin
                    mem[addrb][i*8 +: 8] <= datab[i*8 +: 8];
                end
            end
        end
    end

はい、ということで原因ですが、お気づきになりましたでしょうか？そうです、Verilogのalwaysを使ったFF推定記述であるにも関わらずノンブロッキング代入が使われていないんです。。。

まっさかーーー、と思ってこの部分を修正してみた所、次の画像のように波形が変化し、無事に所望の動きとなりました。

    // imem side
    always @(posedge clk) begin
        if (rena) begin
            qa <= mem[addra]; // ノンブロッキング代入に変更
        end
    end

    // dmem side
    always @(posedge clk) begin
        // read
        if (renb) begin
            qb <= mem[addrb]; // ノンブロッキング代入に変更
        end

        // write
        if (wenb) begin
            for (i = 0; i < 4; i++) begin
                if (webb[i]) begin
                    mem[addrb][i*8 +: 8] <= datab[i*8 +: 8];
                end
            end
        end
    end

修正後の波形

f:id:diningyo-kpuku-jougeki:20191226002925p:plain — ノンブロッキング代入にしたらちゃんと動いた

あんまり深くは追ってないけど、このノンブロッキング代入の部分の変更を入れる前後のverilatorのソースに差が出てた（以下のようにqaって名前が見えるなど、思いっきり関係ありそうな部分）ので、この違いによって扱いが変わってるのは間違いない。

diff addi/VSimDtm.cpp addi_bug/VSimDtm.cpp
131d130
<     __Vdlyvset__SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_cmd_q__DOT__ram_addr__v0 = 0U;
132a132
>     __Vdlyvset__SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_cmd_q__DOT__ram_addr__v0 = 0U;
140a141,156
>     // ALWAYS at /home/diningyo/prj/risc-v/dirv/test_run_dir/isa/addi/RAM1RO1RW.v:45
>     if (vlTOPp->SimDtm__DOT__mem__DOT__m_dmem_brg__DOT__w_sram_read_req) {
>    vlTOPp->SimDtm__DOT__mem__DOT__m_ram__DOT__RAM1RO1RW_qb
>        = vlTOPp->SimDtm__DOT__mem__DOT__m_ram__DOT__RAM1RO1RW__DOT__mem
>        [(0xfffU & ((IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_dmem_brg__DOT__m_cmd_q_io_deq_bits_addr)
>            >> 2U))];
>     }
>     // ALWAYS at /home/diningyo/prj/risc-v/dirv/test_run_dir/isa/addi/RAM1RO1RW.v:38
>     if (vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg_io_sram_rden) {
>    vlTOPp->SimDtm__DOT__mem__DOT__m_ram__DOT__RAM1RO1RW_qa
>        = vlTOPp->SimDtm__DOT__mem__DOT__m_ram__DOT__RAM1RO1RW__DOT__mem
>        [(0xfffU & (((IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_cmd_q__DOT__maybe_full)
>              ? vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_cmd_q__DOT__ram_addr
>             [0U] : vlTOPp->SimDtm__DOT__dut__DOT__ifu_io_ifu2ext_c_bits_addr)
>            >> 2U))];
>     }
178a195,202
>     // ALWAYS at SimDtm.v:329
>     if (((IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT__maybe_full)
>      ? (IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT___T_3)
>      : (IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT___GEN_8))) {
>    __Vdlyvval__SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT__ram_data__v0
>        = vlTOPp->SimDtm__DOT__mem__DOT__m_ram__DOT__RAM1RO1RW_qa;
>    __Vdlyvset__SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT__ram_data__v0 = 1U;
>     }
438,445d461
<     // ALWAYS at SimDtm.v:329
<     if (((IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT__maybe_full)
<      ? (IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT___T_3)
<      : (IData)(vlTOPp->SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT___GEN_8))) {
<    __Vdlyvval__SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT__ram_data__v0
<        = vlTOPp->SimDtm__DOT__mem__DOT__m_ram__DOT__RAM1RO1RW_qa;
<    __Vdlyvset__SimDtm__DOT__mem__DOT__m_imem_brg__DOT__m_rd_q__DOT__ram_data__v0 = 1U;
<     }

Chiselにだけ目がいってたけど、ちゃんとVerilogのコードも確認しような！という、しょうもないミスでした。あと、もう少しVerilatorも見れるようにしないと、、、とも。とりあえず、所望の動きが得られるようになったのでdirvの開発を進められそう。