Re: [Qemu-devel] [2.4 PATCH] target-mips: add Config5.FRE support allowing Status.FR=0 emulation

[James Hogan <james.hogan@imgtec.com>]

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

On 17/03/15 13:08, Leon Alrae wrote:
> Hi James,
> 
> On 17/03/2015 10:55, James Hogan wrote:
>> Hi Leon,
>>
>> On 17/03/15 09:56, Leon Alrae wrote:
>>> +    case 5:
>>> +        /* FRE Support - clear Config5.FRE bit */
>>> +        if (!((env->active_fpu.fcr0 & (1 << FCR0_FREP)) && (rt == 0))) {
>>> +            return;
>>> +        }
>>
>> If rt != 0, is it really desired for a Config5 bit (which is privileged
>> state) to be modified? Assuming these behave similarly to UFR/UNFR, the
>> behaviour is architecturally UNPREDICTABLE when rt != $0, not UNDEFINED
>> (and at least UNFR is required to produce an RI exception in r5
>> implementations).
> 
> Hmm, I believe the code above is correct and is doing exactly what you
> described :) Note that "&& (rt == 0)" is inside parenthesis following
> the logical NOT operator. It is no-op if rt != 0.

Yep, my apologies. Stupid brackets :-).

>>>  /* Floating point register moves. */
>>> -static void gen_load_fpr32(TCGv_i32 t, int reg)
>>> +static void gen_load_fpr32(DisasContext *ctx, TCGv_i32 t, int reg)
>>>  {
>>> +    if (ctx->hflags & MIPS_HFLAG_FRE) {
>>> +        generate_exception(ctx, EXCP_RI);
>>
>> Maybe return to avoid generating dead code?
> 
> The reason is to avoid leaving the TCG temp marked as TEMP_VAL_DEAD
> which would cause assertion failures in TCG as we still generate code
> using this temp after returning from gen_load_fpr().

Okay, fair enough.

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]