----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/234/#review304 -----------------------------------------------------------
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. - Gabe 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
