On 09/14/11 07:28, Steve Reinhardt wrote: > >> 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
Yeah, in isa_traits.hh. There's only one option being used, but the code looked like it intended to support both cases. Unfortunately it was checking the host endianness and not the guests, but I wanted to preserve its intention and keep it more flexible. Gabe _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
