> 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. > > > > Gabe Black wrote: > 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.
1) I saw it (I think in your other diff) 2) What is the type of Mem? I assume that the parser would have to know what the type of Mem was, no? - Nathan ----------------------------------------------------------- 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
