[Webkit-unassigned] [Bug 179446] [JSC][MIPS] Use fcsr to check the validity of the result of trunc.w.d

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=179446

--- Comment #5 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 326373
  --> https://bugs.webkit.org/attachment.cgi?id=326373
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326373&action=review

Informally reviewing...

Patch looks good to me as well. There is a small issue where control registers other
than “fcsr” are _not_ available for CPUs older than MIPS V, and we may want to add
some code to handle those for WTF_MIPS_ISA<5. Adding this is not strictly needed for
this one patch (as it only uses “fcsr”) but we may want to add the suggested bits
in a follow-up patch for the sake of correcness.

> Source/JavaScriptCore/assembler/MIPSAssembler.h:116
> +    fcsr = 31

Only “fcsr” is available before MIPS V, according to the manual:

  “MIPS V and MIPS32 introduced the three control registers that access portions
   of FCSR. These registers were not available in MIPS I, II, III, or IV”

The definition for “fir” should be guarded with WTF_MIPS_ISA_AT_LEAST(5).
We may want to have some additional code to always use “fcsr” and emulate
the behavior of the other register (except “fir”) by adding a few extra
instructions to apply bit masks. Read below for more.

> Source/JavaScriptCore/assembler/MIPSAssembler.h:190
> +        case MIPSRegisters::fir:

Ditto, for ISA<5, “fir” should be guarded.

> Source/JavaScriptCore/assembler/MIPSAssembler.h:741
> +        emitInst(0x44400000 | (rt << OP_SH_RT) | (fs << OP_SH_FS));

So here we would do as it is for MIPS V and newer, and add a guard for to
handle the case with ISA<5. Something like the following:

   #if WTF_MIPS_ISA_AT_LEAST(5)
       emitInst(0x44400000 | (rt << OP_SH_RT) | (fs << OP_SH_FS));
   #else
       emitInst(0x44400000 | (rt << OP_SH_RT) | (MIPSRegisters::fcsr << OP_SH_FS));
       switch (fs) {
       case MIPSRegisters::fccr:
           andi(rt, rt, FCSR_FCCR_BITMASK);
           break;
       case MIPSRegisters::fexr:
           andi(rt, rt, FCSR_FEXR_BITMASK);
           break;
       case MIPSRegisters::fenr:
           andi(rt, rt, FCSR_FENR_BITMASK);
           break;
       case MIPSRegisters::fcsr:
           // No mask needed.
           break;
       default:
           RELEASE_ASSERT_NOT_REACHED();
       }
   #endif

Of course with the *_BITMASK constants defined accordingly.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171109/534c4dac/attachment.html>