> On 2011-12-20 01:16:59, Gabe Black wrote: > > src/cpu/checker/cpu.cc, line 332 > > <http://reviews.m5sim.org/r/910/diff/3/?file=16278#file16278line332> > > > > No {}s, indentation. > > Steve Reinhardt wrote: > I know we just had this discussion recently... as I said then, I > distinctly recall a period in time where we decided that we should *always* > have braces even around single-statement blocks, for consistency and to make > it easier to add new lines to the block (even temporarily, e.g. for > debugging) without having to add the braces. Obviously that policy didn't > make it into the official style guide, and I was always somewhat ambivalent > about it anyway, so I'm not trying to revive it. However, it's equally true > that the converse of that policy is not in the style guide, i.e., there's no > requirement to *not* have braces around single-statement blocks either. So > IMO Geoff should be free to disregard those specific comments of yours. > (Though there are a few places where the brace does need to be moved to the > preceding line if it's not deleted.) > > If we want to have a discussion about whether or not we should amend the > style guide to specifically cover single-statement blocks, we should do that > in a separate thread. I'm fine with leaving it at the coder's discretion > though. > > Gabe Black wrote: > We did, and then too I said that's not how I remember it. I can't find > those emails, but we were talking about insisting the brackets were *not* > used except when the code wouldn't fit on a single line. My position was that > it shouldn't be mandatory to drop the brackets, and there didn't seem to be > opposition to that. > > As far as what we do now, I'm still fine with leaving it at the coders > discretion and it isn't in the style guide. Geoff wouldn't *have* to remove > the {}s, but I think it looks better.
I'm not surprised you don't remember the original policy, since I believe it was before your time. Through the magic of gmail I found the thread where we originally decided to require braces on single-line blocks: Nate and Steve Raasch were opposed, Erik and Lisa were in favor, and I was in the middle, but leaned enough toward Erik & Lisa to be the deciding factor. This was September 12, 2003. Apparently none of our on-line list archives go back that far anymore. Apparently Nate eventually forgot that he was on the losing side of that one, and by that point Erik wasn't around to hound people (and Lisa's not the hounding type), so it fell by the wayside. There are a couple of threads from late 2008 where we basically had the same discussion we're having now: http://www.mail-archive.com/[email protected]/msg01365.html http://www.mail-archive.com/[email protected]/msg02026.html I've updated the style guide to reflect what I believe are the current rules: http://gem5.org/Coding_Style#Braces If we want to discuss further, we should do it on the list (or break new ground and use the wiki talk page). I probably should have put this on the list instead of typing it into reviewboard as well, but now that I've done it I'm not sure it's worth switching. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/910/#review1764 ----------------------------------------------------------- On 2011-12-13 13:25:24, Geoffrey Blake wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/910/ > ----------------------------------------------------------- > > (Updated 2011-12-13 13:25:24) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5 > > Brings the CheckerCPU back to a functioning state allowing FS and SE mode > checking of the O3CPU. These changes have only been tested with the > ARM ISA. Other ISAs will potentially require modification. > > > Diffs > ----- > > configs/common/FSConfig.py c1ab57ea8805 > src/arch/arm/isa.cc c1ab57ea8805 > src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805 > src/arch/arm/isa/insts/misc.isa c1ab57ea8805 > src/arch/arm/table_walker.hh c1ab57ea8805 > src/arch/arm/table_walker.cc c1ab57ea8805 > src/arch/arm/tlb.hh c1ab57ea8805 > src/arch/arm/tlb.cc c1ab57ea8805 > src/arch/arm/utility.cc c1ab57ea8805 > src/cpu/BaseCPU.py c1ab57ea8805 > src/cpu/CheckerCPU.py c1ab57ea8805 > src/cpu/DummyChecker.py PRE-CREATION > src/cpu/SConscript c1ab57ea8805 > src/cpu/base.cc c1ab57ea8805 > src/cpu/base_dyn_inst.hh c1ab57ea8805 > src/cpu/base_dyn_inst_impl.hh c1ab57ea8805 > src/cpu/checker/cpu.hh c1ab57ea8805 > src/cpu/checker/cpu.cc c1ab57ea8805 > src/cpu/checker/cpu_impl.hh c1ab57ea8805 > src/cpu/checker/thread_context.hh c1ab57ea8805 > src/cpu/dummy_checker_builder.cc PRE-CREATION > src/cpu/o3/O3CPU.py c1ab57ea8805 > src/cpu/o3/O3Checker.py c1ab57ea8805 > src/cpu/o3/checker_builder.cc c1ab57ea8805 > src/cpu/o3/commit_impl.hh c1ab57ea8805 > src/cpu/o3/cpu.hh c1ab57ea8805 > src/cpu/o3/cpu.cc c1ab57ea8805 > src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805 > src/cpu/o3/fetch_impl.hh c1ab57ea8805 > src/cpu/o3/iew_impl.hh c1ab57ea8805 > src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805 > src/cpu/o3/thread_context.hh c1ab57ea8805 > src/cpu/o3/thread_context_impl.hh c1ab57ea8805 > src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805 > src/cpu/simple/base.hh c1ab57ea8805 > src/cpu/simple/base.cc c1ab57ea8805 > src/cpu/simple_thread.hh c1ab57ea8805 > src/cpu/thread_context.hh c1ab57ea8805 > src/dev/Device.py c1ab57ea8805 > src/dev/isa_fake.hh c1ab57ea8805 > src/dev/isa_fake.cc c1ab57ea8805 > > Diff: http://reviews.m5sim.org/r/910/diff > > > Testing > ------- > > Successfully runs gzip in SE mode. Successfully boots linux kernel in FS > mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs > to test checker's capabilities. > > > Thanks, > > Geoffrey > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
