> On 2010-08-23 10:49:21, Gabe Black wrote: > > Does this actually work? I thought traceData was set at the end when the > > registers were written out, so I would expect setting it while the > > instruction is processing its data would just get overwritten at that > > point. A less hacky (ISA descriptions tend that way, take no offense) way > > to handle this is, I believe, is to change the value at the end of the > > operands lines when the operands are set up. That value is used for sorting > > the operands, and if I remember right that dictates the order they're > > written back and traceData is written. That may have unintended > > consequences, though, since there may be a dependence on the order they're > > written (there shouldn't be, but might be) and that may screw up the > > disassembly output. > > > > Ideally we'd have a mechanism that lets you select what register you want > > as a first level isa parser construct, maybe in the InstObjParams. That way > > you wouldn't need to play games with the sort order or try to strong arm > > the parser, and it wouldn't write all the registers back when only one will > > actually stick anyway. This is an instance where changing the parser is > > probably appropriate. > > > > If you've verified this patch actually does what you expect it to and > > you've decided changing the sort order won't work, then this change is > > mostly fine. One minor suggestion would be to change "if (traceVar != > > None):" to something more pythony (pythonish? eh, whatever) like "if > > traceVar:" or, if some subtle change in meaning makes that not work, "if > > traceVar is not None:". I'd still consider this approach a stop gap > > solution, though, and that we should make the parser handles multiple > > traceData items better.
Hmm.... it might not... I'll dig some... - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/234/#review304 ----------------------------------------------------------- On 2010-08-23 09:34:11, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/234/ > ----------------------------------------------------------- > > (Updated 2010-08-23 09:34:11) > > > Review request for Default. > > > Summary > ------- > > ARM: Exec traceflag prints out data for data instructions and some misc > > > Diffs > ----- > > src/arch/arm/isa/insts/data.isa 47d9409b2b7f > src/arch/arm/isa/insts/ldr.isa 47d9409b2b7f > src/arch/arm/isa/insts/misc.isa 47d9409b2b7f > > Diff: http://reviews.m5sim.org/r/234/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
