1976699 - riscv64: ABIStackAlignment does not equal mandated value in psABI standard

Status: RESOLVED FIXED

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

Description

[Rong Bao [:csmantle]]

Constants ABIStackAlignment and JitStackAlignment (8) does not match the values mandated by the psABI (16).

• https://searchfox.org/mozilla-central/rev/1eb4f27ece7cb96ac94b635ad0dc95c00d1443db/js/src/jit/riscv64/Assembler-riscv64.h#92-94
• https://riscv-non-isa.github.io/riscv-elf-psabi-doc/#integer-cc

The stack grows downwards (towards lower addresses) and the stack pointer shall be aligned to a 128-bit boundary upon procedure entry. The first argument passed on the stack is located at offset zero of the stack pointer on function entry; following arguments are stored at correspondingly higher addresses.

Comment 1

[Rong Bao [:csmantle]]

Directly changing these two values at once would make all jit-tests to fail. Would you mind providing me with some pointers on where to start?

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Rong Bao [:csmantle] from comment #1)

Directly changing these two values at once would make all jit-tests to fail. Would you mind providing me with some pointers on where to start?

We use 16 also on arm64 and x64 so code should mostly be able to do the right thing. Are we failing the checkStackAlignment breakpoint here? In that case we have to find out where that happens and fix the alignment. Maybe the generateEnterJIT implementation for RISCV needs to be fixed to align the stack to 16 bytes?

Comment 3

[Rong Bao [:csmantle]]

I pinpointed this issue down to one assertion in function EmitBaselineLeaveStubFrame at https://searchfox.org/mozilla-central/rev/162a9137664040430d01d15c4a9be41c331c09ac/js/src/jit/riscv64/SharedICHelpers-riscv64.h#54.

Curiously, some other architectures has this assertions while some do not:

• x86: No.
• x64: No.
• riscv64: Yes.
• mips-shared: No.
• loong64: No.
• arm64: Yes.
• arm: No.

Comment 4

[Rong Bao [:csmantle]]

Cross-referencing this to bug 1975932.

Comment 5

[Rong Bao [:csmantle]]

Attachment: Bug 1976699 - [riscv64] Part 1: Fix JitRuntime::generateEnterJIT stack alignment on an odd number of arguments and notOsr code path. r=jandem

Comment 6

[Rong Bao [:csmantle]]

Attachment: Bug 1976699 - [riscv64] Part 2: Remove erroneous stack alignment assert in EmitBaselineLeaveStubFrame. r=jandem

Comment 7

[Rong Bao [:csmantle]]

Attachment: Bug 1976699 - [riscv64] Part 3: Match ABIStackAlignment and JitStackAlignment with RISC-V psABI's 128-bit boundary. r=jandem

Comment 8

[Rong Bao [:csmantle]]

In diff D256932, I chose to remove this assert to match four 128-bit aligned architectures' choice: loong64, mips64, x86, and x64. May I get some review on this? Thanks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/40c70a15bb5a
https://hg.mozilla.org/integration/autoland/rev/8e7eb9353156
[riscv64] Part 1: Fix JitRuntime::generateEnterJIT stack alignment on an odd number of arguments and notOsr code path. r=jandem
https://github.com/mozilla-firefox/firefox/commit/876588071f64
https://hg.mozilla.org/integration/autoland/rev/5ef9841219f5
[riscv64] Part 2: Remove erroneous stack alignment assert in EmitBaselineLeaveStubFrame. r=jandem
https://github.com/mozilla-firefox/firefox/commit/10a56804791b
https://hg.mozilla.org/integration/autoland/rev/dbb99e019406
[riscv64] Part 3: Match ABIStackAlignment and JitStackAlignment with RISC-V psABI's 128-bit boundary. r=jandem

Comment 11

[Rong Bao [:csmantle]]

Removing redundant See Also refs.

Comment 12

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/8e7eb9353156
https://hg.mozilla.org/mozilla-central/rev/5ef9841219f5
https://hg.mozilla.org/mozilla-central/rev/dbb99e019406