Quoting Steve Reinhardt <[email protected]>:
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
I agree with what you've saying, but this will force us to merge the
two classes before I can move my code back into the main repository.
I've eliminated FULL_SYSTEM entirely and just have a run time
FullSystem (bad caps, I know) which chooses the behavior. As the merge
gets delayed, it will be harder and harder to keep things in sync.
This would be a good forcing function, except I'd absorb the force,
and I'm already sufficiently motivated :-).
This gets into the AddressSpace object idea I'd had before where the
port has an object that does translation for it through whatever means
is necessary. For FullSystem I think it uses a function (vtophys?) to
look through the guests paging structures to do the translation, and
in SE I think it uses the fake page table object we maintain. Since
these are somewhat different, it's not entirely trivial to merge them.
It's still not super hard, but it will take a little work. I was
hoping to do that after merging back in.
And actually speaking of merging back in, I should probably merge with
the new commits in the main repo and put the great big diff from the
merge commit up on review board.
I'd like to characterize how performance changes when FS/SE is a
runtime thing, but as I mentioned I'd have to run that on my laptop
and that would be slow and inconvenient since I'd have to leave it
alone to get reliable results. If somebody wanted to play around with
it for me, I can tell you what to do. There's basically just one patch
that tips it from one to the other, ignoring the follow on changes
that merge the build_opts and regression stuff.
Gabe
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
