[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH] target-ppc/fpu_helper: Fix efscmp* instructions h
From: |
Mark Cave-Ayland |
Subject: |
Re: [Qemu-ppc] [PATCH] target-ppc/fpu_helper: Fix efscmp* instructions handling |
Date: |
Tue, 31 May 2016 08:06:25 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.8.0 |
On 31/05/16 06:28, David Gibson wrote:
> On Fri, May 27, 2016 at 08:43:15AM +0100, Mark Cave-Ayland wrote:
>> On 27/05/16 02:36, David Gibson wrote:
>>> On Thu, May 26, 2016 at 07:54:46AM +0100, Mark Cave-Ayland wrote:
>>>> On 19/05/16 13:11, Talha Imran wrote:
>>>>
>>>> (Adding CC to David as maintainer)
>>>>
>>>>> With specification at hand from the reference manual from Freescale
>>>>> http://cache.nxp.com/files/32bit/doc/ref_manual/SPEPEM.pdf , I have found
>>>>> a fix
>>>>> to efscmp* instructions handling in QEMU.
>>>>>
>>>>> efscmp* instructions in QEMU set crD (Condition Register nibble) values
>>>>> as
>>>>> (0b0100 << 2) = 0b10000 (consider the HELPER_SINGLE_SPE_CMP macro which
>>>>> left
>>>>> shifts the value returned by efscmp* handler by 2 bits). A value of
>>>>> 0b10000 is
>>>>> not correct according the to the reference manual.
>>>>>
>>>>> The reference manual expects efscmp* instructions to return a value of
>>>>> 0bx1xx.
>>>>> Please find attached a patch which disables left shifting in
>>>>> HELPER_SINGLE_SPE_CMP macro. This macro is used by efscmp* and efstst*
>>>>> instructions only. efstst* instruction handlers, in turn, call efscmp*
>>>>> handlers
>>>>> too.
>>>>>
>>>>> *Explanation:*
>>>>> Traditionally, each crD (condition register nibble) consist of 4 bits,
>>>>> which is
>>>>> set by comparisons as follows:
>>>>> crD = W X Y Z
>>>>> where
>>>>> W = Less than
>>>>> X = Greater than
>>>>> Y = Equal to
>>>>>
>>>>> However, efscmp* instructions being a special case return a binary result.
>>>>> (efscmpeq will set the crD = 0bx1xx iff when op1 == op2 and 0bx0xx
>>>>> otherwise;
>>>>> i.e. there is no notion of different crD values based on Less than,
>>>>> Greater
>>>>> than and Equal to).
>>>>>
>>>>> This effectively means that crD will store a "Greater than" comparison
>>>>> result
>>>>> iff efscmp* instruction comparison is TRUE. Compiler exploits this
>>>>> feature by
>>>>> checking for "Branch if Less than or Equal to" (ble instruction) OR
>>>>> "Branch if
>>>>> Greater than" (bgt instruction) for Branch if FALSE OR Branch if TRUE
>>>>> respectively after an efscmp* instruction. This can be seen in a assembly
>>>>> code
>>>>> snippet below:
>>>>>
>>>>> 27 if (__real__ x != 3.0f || __imag__ x != 4.0f)
>>>>> 10000498: lwz r10,8(r31)
>>>>> 1000049c: lis r9,16448
>>>>> 100004a0: efscmpeq cr7,r10,r9
>>>>> 100004a4: ble- cr7,0x100004b8 <bar+60> //jump to abort() call
>>>>> 100004a8: lwz r10,12(r31)
>>>>> 100004ac: lis r9,16512
>>>>> 100004b0: efscmpeq cr7,r10,r9
>>>>> 100004b4: bgt- cr7,0x100004bc <bar+64> //skip abort() call
>>>>> 28 abort ();
>>>>> 100004b8: bl 0x10000808 <abort>
>>>>>
>>>>> Signed-off-by: Talha Imran <address@hidden>
>>>>> ---
>>>>> target-ppc/fpu_helper.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
>>>>> index b67ebca..6fd56a8 100644
>>>>> --- a/target-ppc/fpu_helper.c
>>>>> +++ b/target-ppc/fpu_helper.c
>>>>> @@ -1442,7 +1442,7 @@ static inline uint32_t efststeq(CPUPPCState *env,
>>>>> uint32_t op1, uint32_t op2)
>>>>> #define HELPER_SINGLE_SPE_CMP(name) \
>>>>> uint32_t helper_e##name(CPUPPCState *env, uint32_t op1, uint32_t
>>>>> op2) \
>>>>> { \
>>>>> - return e##name(env, op1, op2) << 2; \
>>>>> + return e##name(env, op1, op2); \
>>>>> }
>>>>> /* efststlt */
>>>>> HELPER_SINGLE_SPE_CMP(fststlt);
>>>>>
>>>>
>>>>
>>>> ATB,
>>>
>>> Sorry, I'm not sure what that means.
>>
>> Oh that's just my standard signature - no need to worry about that
>> :)
>
> Ah, ok.
>
> Can I get a R-b from you on this one, or is this not your bailiwick?
This one is a bit beyond my area of knowledge really - I just tend to
fire up various Mac images from time-to-time for OpenBIOS testing and
I'm not even sure that these instructions are available on the Mac CPUs?
ATB,
Mark.
Re: [Qemu-ppc] [PATCH] target-ppc/fpu_helper: Fix efscmp* instructions handling, David Gibson, 2016/05/26