1810688 - Correctly implement MacroAssembler::branchTruncateDoubleToInt32 on ARM64

Status: RESOLVED FIXED

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

Description

[André Bargull [:anba]]

Correctly implement MacroAssembler::branchTruncateDoubleToInt32 on ARM64 and some additional (mostly ARM64 specific) clean-ups.

Comment 1

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 1: Use Fcmp with double constant. r=jandem!

The zero constant 0.0 doesn't need to be loaded in a separate temp register,
but instead can be encoded directly in the Fcmp instruction.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 2: Add digit separators to some large constants. r=jandem!

This makes it a bit easier to read the constants, because missing digits can
be more easy detected.

Depends on D166993

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 3: Correctly implement branchTruncateDoubleToInt32 on ARM64. r=jandem!

The previous code didn't actually implement truncation, so any double with a
fractional part was triggering the failure path.

Depends on D166994

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 4: Use Fjcvtzs for branchTruncateDoubleMaybeModUint32 when available. r=jandem!

Fjcvtzs wasn't actually used for branchTruncateDoubleMaybeModUint32.

Depends on D166995

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 5: Add some comments to MacroAssembler::branchTruncateDouble. r=jandem!

The Add, Cmp, and B instruction sequence tests for INT64_MIN and
INT64_MAX, but that's not easy to see right away.

Depends on D166996

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 6: Use non-signaling Add when no overflows are possible. r=jandem!

If overflows aren't possible, we can use Add instead of Adds.

Depends on D166997

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 7: Remove unused MacroAssembler methods. r=jandem!

These methods aren't used anymore.

Depends on D166998

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 8: Remove Nun-boxing specific MacroAssembler methods from 64-bit platforms. r=jandem!

ToPayload and ToValue are neither used nor useful when nun-boxing isn't used.

Depends on D166999

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 9: Disallow ImmPtr(0) in favour of ImmPtr(nullptr) or ImmWord(0). r=jandem!

Depends on D167000

Comment 10

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 10: Move ARM64 MacroAssembler::{floor,floorf,ceil,ceilf} into their single callers. r=jandem!

Inline the methods into their only callers to simplify the code.

Depends on D167001

Comment 11

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 11: Handle a trivial FIXME note about possibly static functions. r=jandem!

Depends on D167002

Comment 12

[Jan de Mooij [:jandem]]

It looks like Loong64 needs a similar fix to part 3.

Comment 13

[Zhao Jiazhong]

(In reply to Jan de Mooij [:jandem] from comment #12)

It looks like Loong64 needs a similar fix to part 3.

Thanks for your information, I have submited loong64 fix patch to bug 1810728.

Comment 14

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 12: Consistently use Uxtw to zero the upper 32 bits on ARM64. r=jandem!

Depends on D167003

Comment 15

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 13: Add more comments to MacroAssembler::{floor,ceil}{Float32,Double}ToInt32. r=jandem!

Drive-by fix:

• Remove unnecessary scratch register use in ceilDoubleToInt32.

Depends on D167063

Comment 16

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 14: Merge NaN with negative zero check in floor{Float32,Double}ToInt32. r=jandem!

We can combine the NaN with the existing negative zero check, which avoids the
additional Fcmp instruction.

Depends on D167064

Comment 17

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 15: Merge NaN with negative zero check in trunc{Float32,Double}ToInt32. r=jandem!

Depends on D167065

Comment 18

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 16: Avoid unnecessary failures in trunc{Double,Float32}ToInt32 for ARM64. r=jandem!

Convert the input to a 64-bit instead of 32-bit integer, similar to the floor
and ceil methods. This ensures we don't repeatedly fail for inputs like
Math.trunc(0x7fff_ffff + 0.5).

Depends on D167066

Comment 19

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 17: Directly fail for values in [-0.5,-0) in round{Double,Float32}ToInt32. r=jandem!

Instead of adding 0.5 for values in [-0.5, -0) and later always taking the
failure branch, because the result is always 0, directly take the failure
path when the input is in [-0.5, -0).

Depends on D167067

Comment 20

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 18: Avoid unnecessary failures in round{Double,Float32}ToInt32 for ARM64. r=jandem!

Changes similar to part 16 to avoid repeatedly failing for inputs like Math.round(0x7fff_ffff + 0.1).

Depends on D167068

Comment 21

[André Bargull [:anba]]

Attachment: Bug 1810688 - Part 19: Directly fail for values in [-0.5,-0) in round{Double,Float32}ToInt32 for x86-shared. r=jandem!

Port the changes from part 17 to x86-shared.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92a6cbe11ba1
Part 1: Use Fcmp with double constant. r=jandem
https://hg.mozilla.org/integration/autoland/rev/5a51bab18af1
Part 2: Add digit separators to some large constants. r=jandem
https://hg.mozilla.org/integration/autoland/rev/091bc4e096d4
Part 3: Correctly implement branchTruncateDoubleToInt32 on ARM64. r=jandem
https://hg.mozilla.org/integration/autoland/rev/828041e46623
Part 4: Use Fjcvtzs for branchTruncateDoubleMaybeModUint32 when available. r=jandem
https://hg.mozilla.org/integration/autoland/rev/c620790821e8
Part 5: Add some comments to MacroAssembler::branchTruncateDouble. r=jandem
https://hg.mozilla.org/integration/autoland/rev/124475b37249
Part 6: Use non-signaling Add when no overflows are possible. r=jandem
https://hg.mozilla.org/integration/autoland/rev/61316566a8c2
Part 7: Remove unused MacroAssembler methods. r=jandem
https://hg.mozilla.org/integration/autoland/rev/e3ecd2e00465
Part 8: Remove Nun-boxing specific MacroAssembler methods from 64-bit platforms. r=jandem
https://hg.mozilla.org/integration/autoland/rev/bf7c143ca420
Part 9: Disallow ImmPtr(0) in favour of ImmPtr(nullptr) or ImmWord(0). r=jandem
https://hg.mozilla.org/integration/autoland/rev/e0b1f6b79c01
Part 10: Move ARM64 MacroAssembler::{floor,floorf,ceil,ceilf} into their single callers. r=jandem
https://hg.mozilla.org/integration/autoland/rev/8854d2912c9c
Part 11: Handle a trivial FIXME note about possibly static functions. r=jandem
https://hg.mozilla.org/integration/autoland/rev/7b75cfc88295
Part 12: Consistently use Uxtw to zero the upper 32 bits on ARM64. r=jandem
https://hg.mozilla.org/integration/autoland/rev/2cc9d573aa5e
Part 13: Add more comments to MacroAssembler::{floor,ceil}{Float32,Double}ToInt32. r=jandem
https://hg.mozilla.org/integration/autoland/rev/b1758f40e0b7
Part 14: Merge NaN with negative zero check in floor{Float32,Double}ToInt32. r=jandem
https://hg.mozilla.org/integration/autoland/rev/ea06cba70d99
Part 15: Merge NaN with negative zero check in trunc{Float32,Double}ToInt32. r=jandem
https://hg.mozilla.org/integration/autoland/rev/65989e5b9ae1
Part 16: Avoid unnecessary failures in trunc{Double,Float32}ToInt32 for ARM64. r=jandem
https://hg.mozilla.org/integration/autoland/rev/634ea72f2f8f
Part 17: Directly fail for values in [-0.5,-0) in round{Double,Float32}ToInt32. r=jandem
https://hg.mozilla.org/integration/autoland/rev/2022d4e30361
Part 18: Avoid unnecessary failures in round{Double,Float32}ToInt32 for ARM64. r=jandem
https://hg.mozilla.org/integration/autoland/rev/0bdaddbae382
Part 19: Directly fail for values in [-0.5,-0) in round{Double,Float32}ToInt32 for x86-shared. r=jandem

Comment 24

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/92a6cbe11ba1
https://hg.mozilla.org/mozilla-central/rev/5a51bab18af1
https://hg.mozilla.org/mozilla-central/rev/091bc4e096d4
https://hg.mozilla.org/mozilla-central/rev/828041e46623
https://hg.mozilla.org/mozilla-central/rev/c620790821e8
https://hg.mozilla.org/mozilla-central/rev/124475b37249
https://hg.mozilla.org/mozilla-central/rev/61316566a8c2
https://hg.mozilla.org/mozilla-central/rev/e3ecd2e00465
https://hg.mozilla.org/mozilla-central/rev/bf7c143ca420
https://hg.mozilla.org/mozilla-central/rev/e0b1f6b79c01
https://hg.mozilla.org/mozilla-central/rev/8854d2912c9c
https://hg.mozilla.org/mozilla-central/rev/7b75cfc88295
https://hg.mozilla.org/mozilla-central/rev/2cc9d573aa5e
https://hg.mozilla.org/mozilla-central/rev/b1758f40e0b7
https://hg.mozilla.org/mozilla-central/rev/ea06cba70d99
https://hg.mozilla.org/mozilla-central/rev/65989e5b9ae1
https://hg.mozilla.org/mozilla-central/rev/634ea72f2f8f
https://hg.mozilla.org/mozilla-central/rev/2022d4e30361
https://hg.mozilla.org/mozilla-central/rev/0bdaddbae382