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 <tommu...@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. >
