Closed Bug 1183842 Opened 8 years ago Closed 8 years ago

ARM64: Use InvertCondition to expand the range of conditional branches


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox42 --- fixed


(Reporter: sstangl, Assigned: sstangl)




(1 file, 1 obsolete file)

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.
Attachment #8633693 - Flags: review?(hv1989)
Comment on attachment 8633693 [details] [diff] [review]

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?
Attachment #8633693 - Flags: review?(hv1989)
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.
Attachment #8633693 - Attachment is obsolete: true
Attachment #8642644 - Flags: review?(hv1989)
Assignee: nobody → sstangl
Attachment #8642644 - Flags: review?(hv1989) → review+
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.