1514404 - ARM64: Implement MTableSwitch Handling

Status: RESOLVED FIXED

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

Description

[Sean Stangl [:sstangl]]

Attachment: 0001-ARM64-handling-for-MTableSwitch.-r-nbp.patch

A significant amount of jit-tests are blocked on ARM64 having an MTableSwitch implementation. The attached patch provides that implementation.

There is a fair amount of subtlety in this patch because it introduces a new literal-patching mechanism. The concept of teaching Labels how they should later be bound was introduced for MIPS64 and expanded here to ARM64, such that literal loads can mark themselves with CodeLabel::MoveImmediate, and the corresponding call to Bind() will realize that a literal pool is being changed.

There is one ICache area marked "FIXME", because I'm not sure how to flush the DCache at this moment (and, specifically, if I did implement that it would need testing on Windows), so I just left a comment.

Passes basic/testTableSwitch* with "--ion-eager --ion-offthread-compile=off" (so that Ion is actually used).

Comment 1

[Nicolas B. Pierron [:nbp]]

Comment on attachment 9031618 [details] [diff] [review]
0001-ARM64-handling-for-MTableSwitch.-r-nbp.patch

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

::: js/src/jit/arm64/Assembler-arm64.cpp
@@ +490,5 @@
> +  MOZ_ASSERT(inst0->IsLDR());
> +  uint64_t* literal = inst0->LiteralAddress<uint64_t*>();
> +  *literal = value;
> +  // FIXME: This isn't an instruction, can we flush the DCache?
> +  AutoFlushICache::flush(uintptr_t(literal), 8);

I do not think there is any way to flush the Data Cache, as this is implicit that it is always changing.

::: js/src/jit/arm64/CodeGenerator-arm64.cpp
@@ +827,5 @@
> +  // Load the target from the jump table and branch to it.
> +  vixl::UseScratchRegisterScope temps(&masm.asVIXL());
> +  const ARMRegister scratch64 = temps.AcquireX();
> +  masm.loadPtr(pointer, scratch64.asUnsized());
> +  masm.Br(scratch64);

nit: Move these 4 lines to https://searchfox.org/mozilla-central/source/js/src/jit/arm64/MacroAssembler-arm64-inl.h#1528

Comment 2

[Sean Stangl [:sstangl]]

Attachment: 0001-Bug-1514404-ARM64-handling-for-MTableSwitch.-r-nbp.patch

Comment 3

[Andreea Pavel [:apavel]]

This got landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/e6859652b27a90086a81698d7aa8f1b24b2abd24

Comment 4

[Arthur Iakab [arthur_iakab]]

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