> On 2011-09-12 08:53:52, Steve Reinhardt wrote: > > I agree with Ali that this and the other byteswap.hh change should be > > combined. > > > > Also it seems odd that, combining these two patches, you're getting rid of > > ByteOrderDiffers since it's not used, but you're adding a GuestByteOrder > > constant that's also not (yet) used... is there a specific place where > > you're needing this? > > Gabe Black wrote: > Ok, I'll merge them. The primary reason they're separate is that I > discovered I needed the new constant first and created that change. Then when > I was testing I wanted to be sure to run the ISA that used ByteOrderDiffers, > but then I discovered that there wasn't one and spun up the second patch. > > The place it's used is here: > http://reviews.m5sim.org/r/858/ > > It's interesting to note there that the code was incorrectly checking the > *host* order to decide what the instruction should do instead of checking the > guest order. In any case it should have been using the constants defined in > byteswap.hh instead of the macros. > > Steve Reinhardt wrote: > Ah, yes, I'd forgotten about the situation where the ISA might not > determine the endianness directly. BTW, how do we compile for this > currently? I don't recall seeing a MIPSEL_SE build. > > It's probably worth mentioning in a comment here that this macro is used > to distinguish the two MIPS variants, so that other people don't have to go > searching like you did. > > Gabe Black wrote: > The endianness is statically configured like it is for the other ISAs. It > would be nice to have a convenient way to be able to pick a variant, but we > don't have that right now. There isn't any macro being used here, so I'm not > sure what you're talking about as far as that goes. There's a macro that's > used in byteswap.hh to figure out what the host is and hence when swapping is > necessary, but it doesn't (and shouldn't) have any interaction with the ISAs > themselves.
By "statically configured" it looks like you mean "you have to edit mips/isa_traits.hh", right? Mostly I'm just wondering how/whether this stuff gets tested. I meant "constant" not "macro", sorry; I was specifically referring to GuestByteOrder. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/854/#review1529 ----------------------------------------------------------- On 2011-09-11 19:03:55, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/854/ > ----------------------------------------------------------- > > (Updated 2011-09-11 19:03:55) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Endianness: Make it easier to check the compiled in guest endianness. > > It was technically possible but clumsy to determine what endianness a guest > was configured with using the state in byteswap.hh. This change makes that > information available more directly. > > > Diffs > ----- > > src/sim/byteswap.hh 2862c39f66f8 > > Diff: http://reviews.m5sim.org/r/854/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
