> On 2011-07-01 11:23:20, Steve Reinhardt wrote: > > Having a new, separate header for each ISA that does nothing but import the > > helpers from GenericISA into the specific ISA namespace seems overly > > complicated... do they really need to be in the ISA namespace to be used in > > the ISA definitions? Would it be so bad to just have these outside of a > > namespace in a header somewhere that just gets included directly where > > needed? > > > > Other than that admittedly nitpicky complaint, this looks great to me.
My reasoning there was that the naming would be the same and the implementation would be shared, but that the ISAs had to explicitly pick an implementation or provide one of their own. They aren't -all- just imports, the x86 version actually has a different implementation. I think it also has different arguments so conceivably it could be handled just through that, although then there's the danger of accidentally calling the normal one. I think those functions definitely belong in the ISA stuff somewhere since they're only used by instructions, so putting them in the generic ISA seems like the right thing. I guess the question is how they get brought in from there. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/750/#review1372 ----------------------------------------------------------- On 2011-06-29 04:03:58, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/750/ > ----------------------------------------------------------- > > (Updated 2011-06-29 04:03:58) > > > 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/main.isa 4adb1148ef73 > src/arch/alpha/isa/mem.isa 4adb1148ef73 > src/arch/alpha/memhelpers.hh PRE-CREATION > src/arch/arm/isa/includes.isa 4adb1148ef73 > src/arch/arm/isa/templates/mem.isa 4adb1148ef73 > src/arch/arm/memhelpers.hh PRE-CREATION > src/arch/generic/memhelpers.hh PRE-CREATION > src/arch/mips/isa/formats/mem.isa 4adb1148ef73 > src/arch/mips/isa/includes.isa 4adb1148ef73 > src/arch/mips/memhelpers.hh PRE-CREATION > src/arch/power/isa/formats/mem.isa 4adb1148ef73 > src/arch/power/isa/includes.isa 4adb1148ef73 > src/arch/power/memhelpers.hh PRE-CREATION > src/arch/sparc/isa/formats/mem/swap.isa 4adb1148ef73 > src/arch/sparc/isa/formats/mem/util.isa 4adb1148ef73 > src/arch/sparc/isa/includes.isa 4adb1148ef73 > src/arch/sparc/memhelpers.hh PRE-CREATION > src/arch/x86/insts/microldstop.hh 4adb1148ef73 > src/arch/x86/isa/includes.isa 4adb1148ef73 > src/arch/x86/isa/microops/ldstop.isa 4adb1148ef73 > > Diff: http://reviews.m5sim.org/r/750/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
