> 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.
Is this patch good to go? If any other changes are needed, please let me know. - Amin ----------------------------------------------------------- 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
