Quoting Ali Saidi <sa...@umich.edu>:
On 2011-02-24 14:56:09, Gabe Black wrote:
> This is actually better than my initial impression suggested. I
think I saw all the lines from the Ra->URa change and got confused
since those didn't have anything to do with RFE. I jumped to the
conclusion that you guys had some big, elaborate, round about
implementation for this when really you have two changes stuck
together.
>
> I'm not 100% sure you're safe with out the PC change even if you
rearrange things this way. What's going to happen is the NPC is
going to be forced to PC + size of the instruction after every
instruction is generated, even if that's wrong and is corrected by
a branch mispredict. The problem manifests itself functionally when
you have a mispredict and don't immediately move the NPC into the
PC, preserving the correct control flow. That happened here because
you were staying on the same inst to do more microops. I think most
of the time if you put the branching microop last you'll get away
with it, but I have a creeping doubt that there's some other corner
case where it'll get you in trouble. I do agree that the ARM PC
state is pretty complicated and it would be nice to slim it down
somehow, or at least not make it worse.
There is always that case, but there is also the possibility that
we'll accidentally introduce other weird bugs in the process. Right
now we know this devil and there aren't many left. This is one of
the last changes until the O3 cpu boots Linux/ARM.
Good to hear. If you guys are comfortable with it and you know the
risks, then it's ok with me. We should put a big fat comment someplace
so nobody unwittingly reintroduces the problem. I'm not sure where to
put it where it's guaranteed to be seen, though.
On 2011-02-24 14:56:09, Gabe Black wrote:
> src/arch/arm/isa/operands.isa, line 231
> <http://reviews.m5sim.org/r/474/diff/1/?file=10293#file10293line231>
>
> There isn't anything inherently wrong with changing these
names, but it doesn't belong in this patch. It causes a lot of code
to change that doesn't have anything to do with RFE.
This patch made it clear that the names had to be changed, although
it can be done in a separate patch. There was some conflict with one
of the names that led to an invalid substitution.
Yeah, it makes sense to put that in a patch preceding this one then.
Those sound like two distinct fixes.
Gabe
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
