OK, that seems like a good explanation, but (1) as Ali said, it really
needs to go in a doxygen comment where these accessors are defined in
the base PCState class and (2) I seem to recall some specific comments
where substitutions were made that didn't necessarily make sense wrt
this comment, so it would be good to double-check those, and possibly
add some explanatory comments to the less obvious uses as well.


I'll go back and look. I was going through so much code I'd occasionally forget if I was in an ISA dependent or independent file. I think I got it right most of the time.

The PCs were previously managed entirely by the CPU which had to know about PC semantics, try to figure out which dimension to increment the PC in, what to
set NPC/NNPC, etc. These decisions are best left to the ISA in conjunction
with the PC type itself. Because most of the information about how to
increment the PC (mainly what type of instruction it refers to) is contained in the instruction object, a new advancePC virtual function was added to the StaticInst class. Subclasses provide an implementation that moves around the right element of the PC with a minimal amount of decision making. In ISAs like Alpha, the instructions always simply assign NPC to PC without having to worry about micropcs, nnpcs, etc. The added cost of a virtual function call should be outweighed by not having to figure out as much about what to do with the
PCs and mucking around with the extra elements.


Does it make sense to go through an ISA-specific function, for example:

inline
TheISA::advancePC(PCState &pcState, StaticInstPtr inst)
{
    pcState.advance();
OR
    inst->advancePC(pcState);
}


As far as pcState.advance(), no. The idea is that the instruction knows
what would follow it (a microop or a new instruction) and advances the
PC in the right dimension. As far as inst->advancePC that would
functionally work, but I'm not sure how it would be useful. Wouldn't
that just obscure what's going on?

I'm not sure we're communicating here... you're talking about how in
some ISAs (like x86) you need to know the current instruction to know
how to advance the PC, and in other ISAs (like Alpha) you don't, and
always calling inst->advancePC(pcState) costs you a virtual function
call even in the latter case.  My suggestion is that this ISA-specific
advancePC function (where the "OR" is a compile-time decision based on
which ISA you're using) would allow you to avoid the virtual function
call when it's not needed.


Ok.

ISA parser:

To support the new API, all PC related operand types were removed from the
parser and replaced with a PCState type. There are two warts on this
implementation. First, as with all the other operand types, the PCState still has to have a valid operand type even though it doesn't use it. Second, using syntax like PCS.npc(target) doesn't work for two reasons, this looks like the syntax for operand type overriding, and the parser can't figure out if you're
reading or writing. Instructions that use the PCS operand (which I've
consistently called it) need to first read it into a local variable,
manipulate it, and then write it back out.


See my comments below... I don't see why this change is necessary or even desirable as opposed to just making the existing PC/NPC/NNPC/UPC etc. operands do the right thing, at least in the common cases.


I would have really like to keep the old syntax since I completely agree
this new version is a bit clunky and round about. The basic problem is
that unlike in the previous system, there aren't accessors for reading
and writing parts of the PC since what those parts are isn't defined
external to the ISA. Because of that, an instruction that wants to
modify the PC needs to bring in a copy of the whole thing, do any
modifications locally, and then send the whole revised version back.
This could be done locally to update each operand, but then you'd have a
lot of wasteful copying of the PCState structure and calls to get/set it.

One possible solution would be to keep track of whether -any- member of
the PC was being accessed, read it in once, use it, and if any member
was written to write it out at the end. Then the individual components
could be accessed with operands.

I think I understand everything you're saying up to here.  I think we
agree that you could take an ISA description statement like
    NNPC = foo;
and automatically rewrite it to something like
    PCState _tmp_pc = tc->pc();
    _tmp_pc.nnpc(foo);
    tc->pc(_tmp_pc);
if necessary, or perhaps something a little simpler if possible.

Your concern seems to be that if we have multiple statements like
  PC = foo;
  NPC =bar;
then a naive translation would do two read-modify-writes on the pcState.

Yeah.


I agree that's a potential issue, but how many times does that really
happen?  In particular, it seems like there are a lot of places where
we used to have two updates like
    PC = foo;
    NPC = foo + 4;
where we've already replaced it (or could replace it) with a more
simple call like setPC(foo).  Does it even make sense in most ISAs to
update PC without updating NPC?  I feel like for both the ISA code and
for the regular C++ code that manipulates the PC we should be able to
identify the common patterns (like the way that setting the PC
typically updates NPC to PC+sizeof(inst)) and make those the default
behaviors, and then in 90+% of the cases we are at most only deleting
redundant lines like setting NPC rather than actually changing the
sequence of how things get set.


The accessors sound like a good idea by themselves since it would potentially simplify the instruction implementations a little. More below.

This could be a little limitting,
though, because the names, quantities, arguments, etc. of the accessors
and raw members aren't fixed. We'd have to have a syntax for sort of
dynamically building up an operand type on a per ISA basis to give
access to all the bits of the PC. This would all be possible, but I
think is outside the scope of this already very large change.

You lost me here...

If you have ISA X and it needs to set the PC in some weird way y, then you need an method for Y on the PC object. You also need an operand Y that knows how to use y to manage the PC. The ISA needs to be able to define that operand type, or the parser needs to have it built in. The second is a bad idea, I think. The first might work, but it makes the workings of the description more elaborately obfuscate the end result. This also assumes whatever you're doing can be parameterized one dimensionally.

I think what you're proposing might work, but honestly I'm really tired of this change. If we're going to add whole new features to the parser, lets do that as a follow up.

Gabe
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to