> On 2011-05-28 21:58:27, Steve Reinhardt wrote: > > The description is a little general; can you be more specific about what > > this change is doing? I see that you're getting rid of the size from the > > memory access operands and encoding that in the ctype instead, which seems > > fine. However it seems like you've gotten rid of a lot of the signed vs. > > unsigned code, and made everything look unsigned, and I don't see how that > > still works.
The reason I changed them to unsigned is that all the read/write functions have definitions generated for unsigned int operands but not signed ones. They were being forced to unsigned when the call was being made. All of the regressions passed with these changes and at the time I was convinced why this worked, but it's been about a month since then and I don't really remember the specifics. In SPARC and ARM, the Mem variables are being cast to an appropriate type in the assignment, and in Alpha there were apparently no signed Mem types. I'm guessing MIPS and Power don't have as extensive regressions so problems may have slipped through due to those instructions not being used or being used in a way that didn't expose a difference in behavior. The simple fix is to add casts to those too in the few places where it makes a difference, although there may be a reason that I'm not able to remember that it works as is. Generally speaking, signed vs. unsigned, size, and float vs. int information only makes sense if you're building up a few preselected types as the code is currently doing. By deferring more to the C++ type system, you can use whatever type you want and it should just work. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/655/#review1276 ----------------------------------------------------------- On 2011-04-25 03:03:53, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/655/ > ----------------------------------------------------------- > > (Updated 2011-04-25 03:03:53) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ISA parser: Simplify operand type handling. > > This change simplifies the code surrounding operand type handling and makes it > depend only on the ctype that goes with each operand type. Future changes will > allow defining operand types by their ctypes directly, convert the ISAs over > to that style of definition, and then remove support for the old style. These > changes are to make it easier to use non-builtin types like classes or > structures as the type for operands. > > > Diffs > ----- > > src/arch/alpha/isa/mem.isa de679a068dd8 > src/arch/arm/isa/insts/ldr.isa de679a068dd8 > src/arch/arm/isa/insts/mem.isa de679a068dd8 > src/arch/arm/isa/insts/str.isa de679a068dd8 > src/arch/arm/isa/templates/mem.isa de679a068dd8 > src/arch/isa_parser.py de679a068dd8 > src/arch/mips/isa/decoder.isa de679a068dd8 > src/arch/mips/isa/formats/mem.isa de679a068dd8 > src/arch/power/isa/decoder.isa de679a068dd8 > src/arch/power/isa/formats/mem.isa de679a068dd8 > src/arch/sparc/isa/decoder.isa de679a068dd8 > src/arch/sparc/isa/formats/mem/swap.isa de679a068dd8 > src/arch/sparc/isa/formats/mem/util.isa de679a068dd8 > > Diff: http://reviews.m5sim.org/r/655/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
