> On 2011-06-20 14:18:52, Nathan Binkert wrote: > > src/arch/alpha/isa/mem.isa, line 279 > > <http://reviews.m5sim.org/r/750/diff/1/?file=13025#file13025line279> > > > > I have a couple of issues: > > 1) Where did the byte swapping come from? I don't see it elsewhere. > > > > 2) We've never used typeof before. Should we start? In the future > > (c++0x), we'll have auto, but typeof is nonstandard. > > > > Would it be bad to do the following? > > > > Mem = htog(Mem) > > and then just use Mem as the parameter to that function? Updating in > > place could of course be sketchy if the value is not ephemeral. > > > > If it is, could we not simply use: > > uint%(mem_acc_size)d_t gMem = htog(Mem); > > > > There are other options involving templates too. > >
1) The byte swapping used to be done internal to the read/write functions since they knew what the type was and how to swap it. The readBytes and writeBytes functions don't, so that has to move up into the instructions. 2) I have no strong opinion about typeof. The reason I used that instead of modifying Mem in place was that I didn't know for sure whether Mem would be used after that as the code is now or in the future. I tried to update Mem in place in templates where there wouldn't be anything after it. A major motivation for this change is to make my previous change (which would be applied after this one) that simplifies operand types able to deal with signed and unsigned types in a (subjectively) less hacky way. I think those changes get rid of the mem_acc_size field all together since the parser wouldn't really know what the size is any more. I'd have to look at them again to be sure. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/750/#review1343 ----------------------------------------------------------- On 2011-06-20 11:43:52, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/750/ > ----------------------------------------------------------- > > (Updated 2011-06-20 11:43:52) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ISA: Use readBytes/writeBytes for all instruction level memory operations. > > > Diffs > ----- > > src/arch/alpha/isa/mem.isa f12d1cd32cc7 > src/arch/arm/isa/templates/mem.isa f12d1cd32cc7 > src/arch/mips/isa/formats/mem.isa f12d1cd32cc7 > src/arch/power/isa/formats/mem.isa f12d1cd32cc7 > src/arch/sparc/isa/formats/mem/swap.isa f12d1cd32cc7 > src/arch/sparc/isa/formats/mem/util.isa f12d1cd32cc7 > src/arch/x86/insts/microldstop.hh f12d1cd32cc7 > src/arch/x86/isa/microops/ldstop.isa f12d1cd32cc7 > > Diff: http://reviews.m5sim.org/r/750/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
