----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/884/#review1587 -----------------------------------------------------------
Seems reasonable, though it seems odd to have already pushed the change creating the variable and not just include it in this changeset. I was going to comment when I saw these two reviews that they should be merged, but I was too late. The related question that we need to resolve is: what is our naming policy for global constants? Seems like we had this discussion recently but I forgot what the outcome was and can't find anything relevant in my old email. Seems like we have other constants that are mixed-case with underscores (e.g., Full_System). Another argument is that the format of an identifier shouldn't depend on whether it's a const or not, so this should really be fullSystem or isFullSystem or something like that. This makes sense if we think that someday it won't be a constant... e.g., should it really be a per-System variable at some point? The use case of mixed-mode simulations isn't clear, but logically that's the granularity it could go to. Once we resolve the style issue and get the right name here, I'm OK with this. - Steve On 2011-09-26 02:18:26, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/884/ > ----------------------------------------------------------- > > (Updated 2011-09-26 02:18:26) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > SE/FS: Use the new FullSystem constant where possible. > > > Diffs > ----- > > src/arch/alpha/isa/decoder.isa cf26f9578cd0 > src/arch/alpha/isa/fp.isa cf26f9578cd0 > src/arch/alpha/isa/main.isa cf26f9578cd0 > src/arch/alpha/tlb.cc cf26f9578cd0 > src/arch/mips/faults.hh cf26f9578cd0 > src/arch/mips/faults.cc cf26f9578cd0 > src/arch/mips/isa/decoder.isa cf26f9578cd0 > src/arch/mips/isa/formats/control.isa cf26f9578cd0 > src/arch/mips/isa/formats/dsp.isa cf26f9578cd0 > src/arch/mips/isa/formats/fp.isa cf26f9578cd0 > src/arch/mips/isa/formats/unimp.isa cf26f9578cd0 > src/arch/mips/isa/includes.isa cf26f9578cd0 > src/arch/sparc/isa/base.isa cf26f9578cd0 > src/arch/sparc/isa/includes.isa cf26f9578cd0 > src/arch/x86/isa/decoder/one_byte_opcodes.isa cf26f9578cd0 > src/arch/x86/isa/decoder/two_byte_opcodes.isa cf26f9578cd0 > src/arch/x86/isa/includes.isa cf26f9578cd0 > > Diff: http://reviews.m5sim.org/r/884/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
