1975752 - CodeGenerator: Single vs Double type confusion in StoreToTypedArray on riscv64

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Rong Bao [:csmantle]]

Attachment: Logs from multiple failed test runs

Background

When running jstests entry non262\TypedArray\constructor-iterable-packed-array.js on JIT with riscv64 simulator, the following error surfaced:

D:\Workspace\gecko-dev> ./mach jstests -- -f ./testlist_test262~
## non262\TypedArray\constructor-iterable-packed-array.js: rc = 3, run time = 0.416252
D:\Workspace\gecko-dev\js\src\tests\shell.js:143:13 Error: Exception thrown at index 855: Error: Assertion failed: got NaN, expected 87: mismatch at element 855
Stack:
  assertEqArray@D:\Workspace\gecko-dev\js\src\tests\shell.js:143:13
  @D:\Workspace\gecko-dev\js\src\tests\non262\TypedArray\constructor-iterable-packed-array.js:11:22
REGRESSION - non262\TypedArray\constructor-iterable-packed-array.js

Re-running this entry multiple times leads to a seemingly random mismatch location (see attachment).

Analysis of cause

After dumping the code generation process, the following output is of greatest interest:

[Codegen] # block4 D:\Workspace\gecko-dev\js\src\tests\non262\TypedArray\shell.js:38:37:
[Codegen] .set Llabel 1b06adb38d8 24244
[Codegen]                                 # LIR=LoadUnboxedScalar
[Codegen] 00231e93       slli      t4, t1, 2
[Codegen] 01d50eb3       add       t4, a0, t4
[Codegen] 000ea007       flw       ft0, 0(t4)
[Codegen] a0002ed3       feq.s     t4, ft0, ft0
[Codegen] a0002f53       feq.s     t5, ft0, ft0
[Codegen] 01eefeb3       and       t4, t4, t5
[Codegen] 00100f13       li        t5, 1
[Codegen] .use Llabel 9c8a9fd3c8 on 24272
[Codegen] 01ee9063       bne       t4, t5, 0 -> 0000009C8A9FD0DC
[Codegen] .use Llabel 9c8a9fd558 on 24276
[Codegen] 00000f97       auipc     t6, 0x0
[Codegen] 000f8067       jr        t6
[Codegen] .set Llabel 9c8a9fd3c8 24284
[Codegen] 7fc00eb7       lui       t4, 0x7fc00
[Codegen] f00e8053       fmv.w.x   ft0, t4
[Codegen] .set Llabel 9c8a9fd558 24292
[Codegen]                                 # LIR=KeepAliveObject
[Codegen]                                 # LIR=StoreUnboxedScalar
[Codegen] 40100f53       fcvt.s.d  [RNE] ft10, ft0
[Codegen] 00231e93       slli      t4, t1, 2
[Codegen] 01d68eb3       add       t4, a3, t4
[Codegen] 01eea027       fsw       ft10, 0(t4)
[Codegen]                                 # LIR=KeepAliveObject
[Codegen]                                 # LIR=AddI
[Codegen] 0013039b       addiw     t2, t1, 1
[Codegen]                                 # LIR=MoveGroup
[Codegen] 0003931b       slliw     t1, t2, 0
[Codegen]                                 # LIR=Goto
[Codegen] .use Llabel 1b06adb3898 on 24316
[Codegen] f89ff06f       j         -120 -> 0000009C8A9FD244
[Codegen] --------------------------------

The code is generated from two LIR instructions: LoadUnboxedScalar then StoreUnboxedScalar.

• LoadUnboxedScalar lowers to loading a Float32 from 0(t4) into a 64-bit register ft0. This process undergoes "NaN-boxing" (^1), in which all unassigned high bits in the destination are forced to 1 to form a NaN. Then, the value undergoes a Float32 NaN-checking (the feq.s; feq.s; and sequence) and is loaded with a canonical NaN if necessary.
• StoreUnboxedScalar lowers as follows. The the Float32 is interpreted as a Float64, and then converted back into Float32.

With NaN-boxing taking place in the first LIR's generated code, this sequence is bound to produce a NaN, regardless of the input value. The core fragment of instructions is extracted below:

flw      ft0, 0(t4)
fcvt.s.d [RNE] ft10, ft0
fsw      ft10, 0(t4)

The root of the problem is that StoreUnboxedScalar unconditionally interprets all inputs as Float64 (i.e., Double). As in ^2, void StoreToTypedArray<T>(...) checks whether the destination is floating point. If so, it invokes MacroAssembler::storeToTypedFloatArray<T>(...) with source value specified as ToFloatRegister(value). This ToFloatRegister, however, produces Double-typed registers (^3) by invoking the one-argument overload constructor (^4). That is, the destination is correctly typed according to the LIR's operand, but the source is always interpreted as Double.

Comment 1

[Rong Bao [:csmantle]]

Attachment: Bug 1975752 - [riscv64] Refactor riscv64 FloatRegister representation to allow correct lowering. r=#spidermonkey-reviewers,anba,jandem

Comment 2

[Rong Bao [:csmantle]]

Attachment: trace.codegen.txt.zst: Ion codegen spew of the test case described

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cfcd733f23f4
https://hg.mozilla.org/integration/autoland/rev/3841e2812724
[riscv64] Refactor riscv64 FloatRegister representation to allow correct lowering. r=jandem,anba

Comment 4

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/3841e2812724