> On July 29, 2013, 1:56 p.m., Andreas Hansson wrote: > > I'm still seeing quite some stats differences for e.g.: > > build/ARM/tests/opt/quick/fs/10.linux-boot/arm/linux/realview-simple-timing > > > > I am not sure why and I'd like to understand before pushing this out. The > > regressions all use the DDR3 config with a 64 byte burst size, so I do not > > see why the behaviuor should change. Amin, any ideas? > > Andreas Hansson wrote: > My initial guess is that this is due to accesses that are unaligned and > were previously snooped in the write buffer regardless. The "fix" would be to > do write merging in the buffer, but let's leave that for a later patch. > > Amin Farmahini wrote: > I took a quick look at the stats diff and this is what I think is going > on: > In the new patch, before adding a packet to the read q, we check its > address and its SIZE against packets in the write q. See line 312 in the > patched simple_dram.cc. Previously, we did not check whether the size is > matching or not, which is clearly wrong. In the new patch, read packets are > serviced by wr q only if their size is equal or smaller than a write packet > in the wr q. > Now lets consider > build/ARM/tests/opt/quick/fs/10.linux-boot/arm/linux/realview-simple-timing. > In this case, most of read packets are 8 bytes, and most of write packets are > 4 bytes. So those read packets should NOT find a match in wr q. That explains > wr q behavior for most of packets, but for other packets such 64-byte > read/write packets and 4-byte read packets, one should dig in further. > > Amin Farmahini wrote: > Is this patch good to go? If any other changes are needed, please let me > know.
The short answer is yes. I've got it ready to go out. I merely want to reduce the size of changesets due to stats differences and so I'd like to push this out together with the write merging (and both of their stats updates). I hope that's ok. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1927/#review4555 ----------------------------------------------------------- On July 26, 2013, 7:18 a.m., Amin Farmahini wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1927/ > ----------------------------------------------------------- > > (Updated July 26, 2013, 7:18 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > This patch gets rid of bytesPerCacheLine parameter and makes the DRAM > configuration separate from cache line size. > Instead of bytesPerCacheLine, I define a parameter for DRAM called > burst_length. The burst_length parameter shows the length of a DRAM device > burst in bits. > Also, I replace lines_per_rowbuffer with device_rowbuffer_size to improve > code portablity. > > Updates: > - a burst length in beats for each memory type. > - an interface width for each memory type. > - the memory controller model is extended to reason about "system" packets vs > "dram" packets and assemble the responses properly. It means that system > packets larger than a full burst are split into multiple dram packets. > > > Diffs > ----- > > src/mem/simple_dram.cc UNKNOWN > src/mem/simple_dram.hh UNKNOWN > src/mem/SimpleDRAM.py UNKNOWN > > Diff: http://reviews.gem5.org/r/1927/diff/ > > > Testing > ------- > > Some short and fairly long tests on DDR3 and LPDRR3 with cache lines of 32, > 64, and 128 bytes. > > > Thanks, > > Amin Farmahini > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
