1697846 - Arm64 simulator misexecutes fcvtn

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Yury Delendik (:yury)]

Per Aarch64 instruction description: "the FCVTN instruction writes the vector to the lower half of the destination register and clears the upper half". The https://searchfox.org/mozilla-central/source/js/src/jit/arm64/vixl/Logic-vixl.cpp#4303 does not include logic for the latter part.

I run the following "assertion" code:

 {
    ScratchSimd128Scope scratch(*this);
    vixl::UseScratchRegisterScope temps(this);

    loadConstantSimd128(SimdConstant::SplatX2(333333333333333333.0),
                                    scratch);
    Fcvtn(Simd2S(scratch), Simd2D(scratch));
    const ARMRegister scratch2 = temps.AcquireX();
    Mov(scratch2, Simd2D(scratch), 1);
    Cmp(scratch2, Operand(0));
    Label done;
    branch(Assembler::Zero, &done);
    breakpoint();
    bind(&done);
  }

It fails on the simulator, but not on the hardware.

Comment 1

[Lars T Hansen [:lth]]

cc nbp

Comment 2

[Lars T Hansen [:lth]]

lots of bugs in the armsim. the upstream sources sometimes fix these, see file added by nbp only yesterday for pointer.

Comment 3

[Nicolas B. Pierron [:nbp]]

Reducing severity to the lowest, as the simulator is not used when running Firefox.

Comment 4

[Yury Delendik (:yury)]

Attachment: Bug 1697846 - Fix ARM64 simulator execution for FCVTN. r?nbp

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Yury Delendik (:yury) from comment #0)

Per Aarch64 instruction description: "the FCVTN instruction writes the vector to the lower half of the destination register and clears the upper half".

The AArch64 documentation I have does not state this. Do you have a link to this documentation?

Comment 6

[Yury Delendik (:yury)]

(In reply to Nicolas B. Pierron [:nbp] from comment #5)

(In reply to Yury Delendik (:yury) from comment #0)

Per Aarch64 instruction description: "the FCVTN instruction writes the vector to the lower half of the destination register and clears the upper half".

The AArch64 documentation I have does not state this. Do you have a link to this documentation?

page 1571 of "DDI0487Fc_armv8_arm.pdf" book has this text

I reported the issue to the https://git.linaro.org/arm/vixl.git/about/ -- so far they agree with the report and fix is being tested.

Comment 7

[Yury Delendik (:yury)]

The fix is available at https://github.com/Linaro/vixl/commit/5319db71bd868385fc61bfbd57867453aab2b116 . Which of the fixes we want to use?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5123b27b3f7e
Fix ARM64 simulator execution for FCVTN. r=nbp

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5123b27b3f7e