Thanks for your review Alexander and Tom. All your proposition looks fine
to me.
Xilinx Virtex5 is not ISA compliant and does not support unsigned
conversions, that's why I left all unsigned version as it was but you are
right, PPC_FLOAT_64 is such a bad name !
Adding this flag to all CPU seems ok since it shall result in looking those
that defined the PPC_64B flag.
Will repost a patch in a few days taking in account your comments.
Pierre

On Wed, Sep 10, 2014 at 7:15 PM, Tom Musta <tommusta@gmail.com> wrote:

> On 9/10/2014 4:20 AM, Alexander Graf wrote:
> >
> >
> > On 10.09.14 07:03, Pierre Mallard wrote:
> >> This patch series enable floating point instruction in 440x5 CPUs
> >> which have the capabilities to have optional APU FPU.
> >>
> >> 1) Add floating point standard insns flag to 440x5 in case there is an
> apu fpu.
> >> 2) Define a new floating point insns flag for operation
> >> previously reserved to 64 bits proc (fcfid, fctid, fctidz)
> >> 3) Apply this new flag to fcfid, fctid, fctidz and move TARGET_PPC64
> >> restrictions
> >
> > I've looked through the patches mostly from a stylistic point of view.
> > As for whether the changes are technically correct and fully adhere to
> > the specs, I haven't verified anything and would leave that part to Tom
> :).
> >
>
> I went back to some old (paper) versions of the ISA circa 1998 and the
> Floating Convert To/From Doubleword instructions all have this clause:
>
> "This instruction is defined only for 64-bit implementations.  Using it on
> a 32-bit implementation will cause the system illegal instruction error
> handler to be invoked."
>
> I believe this view of things was in play for the 60x and PowerMAC era
> 32-bit CPUs.  Which is consistent with the existing QEMU implementation.
>
> The next revision of the spec that I have is Power ISA 2.03 (2006) and the
> clause is gone.  Furthermore, the instructions are *NOT* in the "64"
> category.
>
> To complicate matters more, the unsigned integer versions were added in
> ISA 2.06 (fcfidu, fctidu, fctiduz).  QEMU deals with these via the
> PPC2_FP_CVT_ISA206 flag.
>
> My interpretation is that all of the fc[tf]id[*] instructions are a
> required part of any Power floating point implementation -- 32-bit or
> 64-bit is irrelevant.
>
> Based on all of this, I think it would make sense to do the following in
> this patch series:
>
> (1) Eliminate the TARGET_PPC64 checks for all six FP Doubleword Integer
> Conversion instructions.
>
> (2) Defined a new flag for FP Signed Doubleword Conversion instructions
> (PPC2_FP_CVT_S64).  Use this flag exclusively when defining the opcode
> tables, e.g.
>
> +/* fctidz */
> +GEN_FLOAT_B(ctidz, 0x0F, 0x19, 0, PPC2_FP_CVT_S64);
>
>
> (3) You would have to add the flag to all existing 64-bit CPUs that
> support floating point.  And of course, to your new 440-w-fpu CPU.
>