I looked at Alpha's ISA description briefly, and I didn't see anywhere
an instruction did anything with the fault returned by a read/write in
initateAcc other than return it. Do have an example where that isn't
true? The only place I can think of where that would be useful is
prefetches, but I think we already have a separate mechanism that
handles that.
Gabe
Quoting Gabe Black <gbl...@eecs.umich.edu>:
On 2011-01-17 00:28:39, Gabe Black wrote:
> This change seems to have some dead functionality in it. The
"delay" member added to translations is never used, the "delay()"
pure virtual function is never used and not defined for any other
ISAs (which I think will break them all), and the
translationDelayed variable is never used. Unnecessary copyright
changes should also be rolled back when removing that dead code.
>
> Are there any instructions that actually expect to get a valid
fault when performing initiateAcc? I could imagine there might be
since it used to be something you could always do for the most
part, but getting rid of those instances would help simplify this
code I think. The code that prepares a request for the memory
system could follow from the finishTranslation function no matter
if it happened immediately or after a table walk. Then an
instruction would (hopefully) only require one pass since it's work
would be done and it would either be ready to commit (a store) or
ready for completeAcc (a load). The actual load/store queue would
have to wait since the address might not be ready, but that might
be a pretty simple extension on top of waiting for initiateAcc to
happen.
Ali Saidi wrote:
I pointed out where delay() is used and it's not defined in
BaseTLB, it's in Translation. All the regressions pass just fine.
If you search for translationDelayed in the diff you'll see it is
used too.
Yes, Alpha still returns a fault on initiateAcc() since there
is no table walk, the tlb lookup is known immediately.
This is quite a substantial patch, so I would prefer to commit
it close to as is and then work on patch on top of it to rework
finishTranslation if it's possible.
Ok, yeah, I see where translationDelayed is used. I see where
delay() is called, but I don't see where the value it sets (member
variable delay) is actually used for anything. There is a lot of
code here, so I might have missed it. My biggest concern about this
patch is that it's adding a lot of code and state (relatively
speaking) to handle the delayed translation case, and O3 is already
really complicated. That may just be what's necessary, though. It
would be nice to go into Alpha and adjust its semantics as far as
expecting faults in initiateAcc. When I get a chance I'll look at
them to see how easy that might be. I'm not sure if or when that
would get done and taking advantage of it would require reworking a
decent bit of this code, so it's not necessarily something you
should wait for.
This is also at least the second time where I've been confused by
nested classes or multiple classes in a file and thought I was
looking at something I wasn't. I need to be more careful about that.
- Gabe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/422/#review741
-----------------------------------------------------------
On 2011-01-12 09:12:18, Ali Saidi wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/422/
-----------------------------------------------------------
(Updated 2011-01-12 09:12:18)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt,
and Nathan Binkert.
Summary
-------
O3: Enhance data address translation by supporting hardware page
table walkers.
Some ISAs (like ARM) relies on hardware page table walkers. For those ISAs,
when a TLB miss occurs, initiateTranslation() can return with
NoFault but with
the translation unfinished.
Instructions experiencing a delayed translation due to a hardware page table
walk are deferred until the translation completes and kept into the IQ. In
order to keep track of them, the IQ has been augmented with a queue of the
outstanding delayed memory instructions. When their translation completes,
instructions are re-executed (only their initiateAccess() was already
executed; their DTB translation is now skipped). The IEW stage has been
modified to support such a 2-pass execution.
Diffs
-----
src/arch/arm/tlb.cc 5d0f62927d75
src/cpu/base_dyn_inst.hh 5d0f62927d75
src/cpu/base_dyn_inst_impl.hh 5d0f62927d75
src/cpu/o3/fetch.hh 5d0f62927d75
src/cpu/o3/iew_impl.hh 5d0f62927d75
src/cpu/o3/inst_queue.hh 5d0f62927d75
src/cpu/o3/inst_queue_impl.hh 5d0f62927d75
src/cpu/o3/lsq_unit_impl.hh 5d0f62927d75
src/cpu/simple/timing.hh 5d0f62927d75
src/cpu/translation.hh 5d0f62927d75
src/sim/tlb.hh 5d0f62927d75
Diff: http://reviews.m5sim.org/r/422/diff
Testing
-------
Thanks,
Ali
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
