On 07/01/11 19:18, Gabe Black wrote: > >> 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
I think part of the confusion was that I'd forgotten to add the x86 memhelpers to the change at all. I got rid of the intermediate headers anyway. Gabe _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
