Re: [Qemu-devel] [PATCH] SH: Add prefi, icbi, synco

[Paul Mundt]

On Mon, Oct 20, 2008 at 08:31:19PM +0400, Vladimir Prus wrote:
> On Monday 20 October 2008 20:27:00 Paul Brook wrote:
> > > > I disagree. This is something that should be done right from the start.
> > > > Trying to fix it up later is a real pain.  Doing fine grained features
> > > > isn't that hard. MIPS, sparc, arm, ppc, m68k and sparc already do this.
> > > > IIRC binutils is only complicated because it tried to create a strict
> > > > hieracy of features, rather than using feature bits.
> > >
> > > When we do it does not matter, but it is completely unrelated from this
> > > patch, in that there are already plenty of instructions that are specific
> > > to a certain CPU family that we don't perform an illegal instruction
> > > exception for. Trying to force the prefi/icbi/synco cases to rework all 
> > > of the existing instructions that aren't universal doesn't make a lot of
> > > sense, as it is a clear incremental change of existing behaviour, rather
> > > than a situation caused purely by the addition of these new instructions.
> > 
> > The only cpu we currently claim to support is SH4. When adding support for 
> > other cores these should be properly conditionalized.  Unconditionally 
> > implementing additional instructions is a regression. I don't consider 
> > "we'll 
> > fix this at some undefined point in the future" to be a good enough answer. 
> 
> Can you outline what changes should I make to implement proper 
> conditionalization?
> 
The issue comes with how the ISA is versioned. It changes across CPU
families, with different families borrowing off of each other but not
being strict supersets of previous generations. It is a complicated mess
in binutils, and if someone has a better idea of how to do that cleanly
in qemu, then that is certainly something that should be experimented
with.

Regardless, as Paul seemed to completely ignore, we already have non-SH4
instructions in the tree already, so the illegal instruction behaviour is
already bogus when we hit SH-4A instructions on SH-4. There are also
extra registers that are not handled under the SH-4 CPU definition
in-tree.

Going be a set of features is possible to some extent, but there are
fundamental architectural differences in SH-4A compared to SH-4, so the
feature thing is rather misleading. The existing target-sh4 thing really
ought to be renamed to target-sh and generalized. Adding in more SH-4A
logic is going to create more deviation, and it is already going to be
helpful to start isolating the SH-4 and up parts so that parts that
diverged from before that point can be integrated. Note that even though
SH-4 and up excluded SH-2 and SH-3 at the time, SH-2A borrows quite
heavily from SH-4 and SH-4A also, in which case the feature tests would
be helpful.

In addition to the CPU subtype, it is necessary to track the CPU family
that it falls under also, as this is the only way to know properly how
the instructions have been inherited. The feature bits will have to be
fairly fine grained, as there are CPUs that implement the same features
in different ways. SH-5 for example supports synco/prefi/icbi in two
different ways, depending on which instruction set mode it is in.