stanford-ppl / spatial-lang

Spatial: "Specify Parameterized Accelerators Through Inordinately Abstract Language"
MIT License
100 stars 12 forks source link

Time-critical Switch Conditions Busted with Retiming #251

Closed mattfel1 closed 6 years ago

mattfel1 commented 6 years ago

Apparently this has always been busted but we have never had a unit test that stressed it. I found out about this in PageRank_Bulk and will make a unit test to specifically target it. The implementation of PR was robust against this kind of thing and before partial retiming, we had a lucky bug where a filled fifo did not stall the pipeline. With new partial retiming, this filled fifo does the correct thing and stalls the pipeline.

Broken, with retiming: image

Working, without retiming: image

Offending code:

            val pagerank = Pipe.Reduce(Reg[X](0))(len by 1){i => 
              if (nearPages.empty) {
                println("page: " + page + ", local_page: " + local_page + " deq from far")
                local_farPages.deq() / local_farEdgeLens.deq().to[X]
              } else {
                val addr = nearPages.deq()
                println("page: " + page + ", local_page: " + local_page + " deq from near addr " + addr)
                local_pages(addr) / local_edgeLens(addr).to[X]
              }
            }{_+_}

The switchcase conditions have a 24 cycle delay, so the nearPages.empty retiming by 24 cycles happens outside of the enable for the reduce pipe

mattfel1 commented 6 years ago

Looks like we need to add fifo/filo checks in the initiation analyzer. We need to capture loops between a fifo status ops (full, empty, etc) and fifo modifications (deq, enq)