> On May 20, 2012, 3:22 p.m., Steve Reinhardt wrote: > > Creating the arch/generic files just to get rid of them two changesets > > later seems like serious overkill to me. Why not just copy the decoder > > class into per-ISA versions now, then modify them in the other cset? IMO > > that makes what's going on clearer than transitioning through this > > intermediate state. I won't fight it if you want to keep things this way > > though. > > Steve Reinhardt wrote: > I don't mean to sound too critical here... I can certainly see how this > order of things would come about as you're working on it. Just pointing out > that in retrospect it's kind of a roundabout path from beginning to end.
I don't disagree that it's a little strange. There are several reasons things ended up that way. First, I originally had a fairly different design for how this would work, and I tried to reuse bits of my earlier attempt rather than reimplement everything. Second, I found I was spending a lot of my gem5 time each weekend rediscovering what the heck I was doing last weekend, and that didn't leave a lot of time for new work. I wanted to get to a stable point and get this bit in place, and didn't want to go back rearchitecting things to make the history smoother since that would have made everything take even longer. I tried to keep the history fairly clean and making sense, and I'm pretty happy with how it ended up. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1192/#review2755 ----------------------------------------------------------- On May 15, 2012, 5:55 a.m., Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1192/ > ----------------------------------------------------------- > > (Updated May 15, 2012, 5:55 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9006:aede609c2ff0 > --------------------------- > Decode: Make the Decoder class defined per ISA. > > > Diffs > ----- > > src/arch/SConscript f681719e2e99 > src/arch/alpha/decoder.hh PRE-CREATION > src/arch/alpha/remote_gdb.cc f681719e2e99 > src/arch/arm/decoder.hh PRE-CREATION > src/arch/arm/remote_gdb.cc f681719e2e99 > src/arch/generic/SConscript PRE-CREATION > src/arch/generic/decoder.hh PRE-CREATION > src/arch/generic/decoder.cc PRE-CREATION > src/arch/mips/decoder.hh PRE-CREATION > src/arch/mips/remote_gdb.cc f681719e2e99 > src/arch/power/decoder.hh PRE-CREATION > src/arch/sparc/decoder.hh PRE-CREATION > src/arch/x86/decoder.hh PRE-CREATION > src/cpu/SConscript f681719e2e99 > src/cpu/checker/thread_context.hh f681719e2e99 > src/cpu/decode.hh f681719e2e99 > src/cpu/decode.cc f681719e2e99 > src/cpu/inorder/cpu.hh f681719e2e99 > src/cpu/inorder/cpu.cc f681719e2e99 > src/cpu/inorder/resources/fetch_unit.hh f681719e2e99 > src/cpu/inorder/thread_context.hh f681719e2e99 > src/cpu/legiontrace.cc f681719e2e99 > src/cpu/o3/fetch.hh f681719e2e99 > src/cpu/o3/thread_context.hh f681719e2e99 > src/cpu/simple/base.hh f681719e2e99 > src/cpu/simple_thread.hh f681719e2e99 > src/cpu/thread_context.hh f681719e2e99 > > Diff: http://reviews.gem5.org/r/1192/diff/ > > > Testing > ------- > > > Thanks, > > Gabe Black > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
