To me there's actually a spectrum of ISA dependence, incompletely
described below:
1. If ISA == suchandsuch do this, otherwise do that.
2. If ISA has characteristic such and such, do this, otherwise do that.
3. Here, ISA, you take care of this.
4. ISA data parameterizing non-ISA stuff.
Number 1 is the worst since it's hard to maintain, can be cumbersome
to specify, and it isn't always clear -why- ISA suchandsuch needs a
particular behavior.
I'd say number 2 is second worst, and that's what this is an example
of. It's better since it's obvious why the code is separated out and
there can be sharing between CPU models, etc., but at the same time it
increases the CPU's awareness of the ISA on it and partially breaks
down the barriers of abstraction. It also sets up special case code
paths where, for instance, only x86 on the timing CPU would possibly
exhibit a certain bug. If someone changes things for ARM and
everything seems to work, they could be subtley breaking x86 which
they aren't familiar with and weren't thinking about when they made
their change.
3. Three is better in some ways and worse in others. It's clear what's
happening, there's a lot of flexibility, and the CPU isn't actually
aware of -how- something is being done, just that, say, this would be
a good time to check for interrupts, whatever that means. It's not as
great because you have more complex interactions between ISAs and
CPUs, and you have to do a bit more work in the ISA. It can also be
hard to make some of this functionality work sensibly in order, out of
order, single cycle, multi cycle, timing mode, atomic mode, etc. etc.
4. This one is the best when you can get away with it. This is where
you, say, make your integer register file 32 registers because the ISA
says that's how many it needs. Basically the only draw back to this is
that behavior changes a little based on each ISA, but if you can get
away with it this is the safest.
I think having a multi ISA simulator that will be modified by its end
users, especially one with as large a cross product of options as
ours, needs to try to be safe and simple before being as absolutely
fast as it can be. You know what they say about premature
optimization. Ideally we should design things so the big, unnecessary
pieces of code just aren't part of the equation because they're in the
ISA defined objects, control just doesn't go that way when it wouldn't
make sense, etc. And if, for instance, a pointer should always be 0,
the code should still behave correctly. The code should do its job
correctly no matter -why- it's being asked to do it.
I think it's bad news to have a big list of yes or no checkboxes in
each ISA directory which turn on and off behaviors. This is especially
true when it's ambiguous whether to say yes or no, if the behavior
changes based on circumstances, etc., and if/when the checkbox is
interpretted subtley (or not so subtley) differently by the consumer.
In this particular limitted case it seems relatively ok although not
necessarily advisable, but it's a slippery slope I don't consider us
have a completely clean history with.
Gabe
Quoting Steve Reinhardt <[email protected]>:
In generalI think this is the kind of ISA hook we should be using...
in the sense that checking TheISA::HasUnalignedMemAcc is much better
than "(TheISA == x86 || TheISA == Power)". I think it's useful not
only to avoid the overhead of a dynamic check for an ISA that doesn't
need it, but also to clarify that this is code that never gets
executed on those ISAs. Maybe for a little one-liner like this it's
not a big deal either way, but for bigger hunks of code I think that
clarification is potentially useful.
Steve
On Thu, Jul 22, 2010 at 1:10 PM, Gabriel Michael Black
<[email protected]> wrote:
Yes it does, and that sounds reasonable to me. I'd still like to see us use
ISA hooks as minimally as possible, but this seems ok.
Gabe
Quoting Timothy M Jones <[email protected]>:
Oh, ok, I see where you're going with that. However, the main idea of
having TheISA::HasUnalignedMemAcc was that it is a constant
specific to each
ISA. Therefore, the compiler should really recognise this and optimise it
away wherever it's used. In this case, for ISAs that don't have unaligned
memory accesses the whole 'if' block should disappear. For ISAs that do
have them then the condition should be reduced to just checking sreqLow.
Therefore it's better for the first set of ISAs for this to be kept in.
Does that make sense?
Tim
On Thu, 22 Jul 2010 14:30:56 -0400, Gabe Black <[email protected]>
wrote:
I think you missed my point. If the check of TheISA::HasUnalignedMemAcc
is redundant, we shouldn't be checking it at all. It's a free, though
very small, performance bump, but more significantly it removes a direct
dependence on the ISA.
Gabe
Timothy M. Jones wrote:
changeset 3bd51d6ac9ef in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=3bd51d6ac9ef
description:
O3CPU: Fix a bug where stores in the cpu where never marked as
split.
diffstat:
src/cpu/o3/lsq_unit.hh | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diffs (16 lines):
diff -r 02b471d9d400 -r 3bd51d6ac9ef src/cpu/o3/lsq_unit.hh
--- a/src/cpu/o3/lsq_unit.hh Thu Jul 22 18:47:52 2010 +0100
+++ b/src/cpu/o3/lsq_unit.hh Thu Jul 22 18:52:02 2010 +0100
@@ -822,6 +822,12 @@
storeQueue[store_idx].sreqLow = sreqLow;
storeQueue[store_idx].sreqHigh = sreqHigh;
storeQueue[store_idx].size = sizeof(T);
+
+ // Split stores can only occur in ISAs with unaligned memory
accesses. If
+ // a store request has been split, sreqLow and sreqHigh will be
non-null.
+ if (TheISA::HasUnalignedMemAcc && sreqLow) {
+ storeQueue[store_idx].isSplit = true;
+ }
assert(sizeof(T) <= sizeof(storeQueue[store_idx].data));
T gData = htog(data);
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
--
Timothy M. Jones
http://homepages.inf.ed.ac.uk/tjones1
The University of Edinburgh is a charitable body, registered in
Scotland, with registration number SC005336.
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev