Bug in subword memory access in combinational memory port

[Deleted user]

Hi, I found a but in your combinational memory port. The lines that caused it are

        val data = Wire (UInt (32.W))
        // Mask the portion of the existing data so it can be or'd with the writedata
        when (io.pipeline.maskmode === 0.U) {
          data := readdata & ~(0xff.U << (offset * 8.U))
        } .otherwise {
          data := readdata & ~(0xffff.U << (offset * 8.U))
        }
        writedata := data | (io.pipeline.writedata << (offset * 8.U))
      } .otherwise {
        // Write the entire word
        writedata := io.pipeline.writedata
      }

Take one example:
If I attempt to perform a half-word write on a word's lower half
if the original word is 0x00000000
the register carrying the half word contains the value 0xffffffff
the
data would be 0x00000000
writedata would be 0x00000000 | 0xfffffffff which is 0xffffffff!
This means the upper half word is also overwritten.
This is because in

writedata := data | (io.pipeline.writedata << (offset * 8.U))

when you or two parts together, the io.pipeline.writedata doesn't have its upper half word zeroed, accidentally overwriting the upper half.
Here is a simple fix I'm proposing: change the code above to

        when (io.pipeline.maskmode === 0.U) {
          when(offset === 0.U) {
            writedata := Cat(readdata(31,8), io.pipeline.writedata(7,0))
          }.elsewhen(offset === 1.U) {
            writedata := Cat(readdata(31,16), Cat(io.pipeline.writedata(15,8), readdata(7,0)))
          }.elsewhen(offset === 2.U) {
            writedata := Cat(readdata(31,24), Cat(io.pipeline.writedata(23,16),readdata(15,0)))
          }.otherwise {
            writedata := Cat(io.pipeline.writedata(31,24), readdata(23,0))
          }
        } .otherwise {
          when (offset === 0.U) {
            writedata := Cat(readdata(31,16),io.pipeline.writedata(15,0))
          }.otherwise {
            writedata := Cat(io.pipeline.writedata(31,16), readdata(15,0))
          }
        }
      } .otherwise {
        // Write the entire word
        writedata := io.pipeline.writedata
      }

and you'll find subword access working.
It's kind of a ugly fix now. If you have something better, feel free to improve the code.
Also, just FYI, chisel has subword access support natively link. It would be much cleaner to use in your code. If someone's interested, I could submit a PR.

[powerjg]

Thanks for the bug report! That code has been bothering me for a while :).

I would be happy to merge a PR with either the code you posted here or using the native subword support. I'm a little surprised we didn't use that to begin with...

[Deleted user]

I created PR for this fix now, will submit another PR for native subword access next week and will add in tests for subword memory access.