1183842 - ARM64: Use InvertCondition to expand the range of conditional branches

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Sean Stangl [:sstangl]]

Attachment: 0001-Use-InvertCondition-to-expand-the-range-of-condition.patch

ARM64 branch-to-offset instructions take different widths: unconditional branches have 26 bits of signed offset, but conditional branches only have 19 bits. We run out of bits pretty easily on some longer tests -- Baseline binds a label at the beginning of the code, then outputs conditional branches to that throughout.

Instead of using a branch to the literal pool, this works around the problem by inverting the condition, which allows using an unconditional branch to encode the far-off location.

The place where literal pools need to come into play is marked with a TODO, but that will be a larger patch and is best left to when some test case requires it (fuzzing will definitely generate many).

Please keep in mind that the VIXL files use VIXL style. The functions introduced here are imported from the newest VIXL version.

Fixes auto-regress/bug690650.js with --ion-eager and a bunch of others.

Comment 1

[Hannes Verschore [:h4writer]]

Comment on attachment 8633693 [details] [diff] [review]
0001-Use-InvertCondition-to-expand-the-range-of-condition.patch

Review of attachment 8633693 [details] [diff] [review]:
-----------------------------------------------------------------

Do we need to special-case Always and Never in B() ?
Currently we assert for those, but we are not guaranteed they aren't Always/Never.
In one change we actually use Always?!

::: js/src/jit/arm64/MacroAssembler-arm64.h
@@ +1562,3 @@
>      }
>      void j(Label* dest) {
> +        B(dest, Always);

If I understand correctly B(xxx, Always) will assert?

Comment 2

[Sean Stangl [:sstangl]]

Attachment: 0001-Use-InvertCondition-to-expand-the-range-of-condition.patch

It looks like that code came from someone copying the equivalent ARM code without thinking. The invalid j() function is never referenced, so I just removed it.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d18e3a6d5149

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d18e3a6d5149