On Tue, Dec 6, 2011 at 4:51 PM, Gabriel Michael Black <[email protected] > wrote:
> >> 2. SE/FS convergence. I generally hesitate to push people to incorporate >> orthogonal code cleanup into unrelated changes, but this seems like a >> really great opportunity to push toward unifying the two TranslatingPort >> classes. At the very least I think it would be good to rename both >> SETranslatingPort and FSTranslatingPort to just TranslatingPort, with the >> one you get depending on your compilation mode. Then in other parts of the >> code you wouldn't really have a distinction... for example, see my comment >> on that snippet in remote_gdb.cc. Also renaming the SE-mode getMemProxy() >> to getVirtProxy() to be parallel with FS mode would help. Since you're >> renaming things anyway it seems like we should do it right. Of course this >> needs to be coordinated with what Gabe is doing; how are you dealing with >> the VirtualPort/TranslatingPort issue, Gabe? >> > > With FULL_SYSTEM, we were choosing to include or exclude things but > generally not to choose between two implementations of the same thing. With > the ISAs things are significantly different. In my merged repository, I > have both translating ports and virtual ports at the same time, although > only one or the other will get used. If we're going to go so far as to > actually combine the two types of ports that would be great, but we > shouldn't use the preprocessor and FULL_SYSTEM to decide which > implementation to use since that gets ugly when the preprocessor can't be > used any more. > My goal is that we eventually actually combine the two types of ports. As I think you have already pointed out, Gabe, there's no reason to have two different ports that do effectively the same thing with different names. I seem to recall this even being somewhat of a poster child for where SE and FS had evolved to be gratuitously different that our SE/FS merger was going to address. I agree that as a general policy we shouldn't have two different objects with the same name and use FULL_SYSTEM to select between them at compile time, but I see this as an exception because this would be an intermediate step toward the eventual goal of unifying them into a single object. In particular, it would let us take some pieces of code (like that remote_gdb.cc snippet) and have a single piece of code there that works in both SE and FS modes. The potential drawback is that it would force us to actually combine the two classes before we're able to get rid of FULL_SYSTEM as a preprocessor symbol. Depending on your perspective that might not be a bad thing, though, as it would also prevent us from putting that piece of cleanup off indefinitely. Or another way to look at it is that if we're going to do it eventually, why not now. (Not as a part of this specific set of patches, but perhaps soon afterward.) Steve _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
