1430161 - Generalize ARM disassembler and implement for ARM64

Status: RESOLVED FIXED

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

Description

[Lars T Hansen [:lth]]

Attachment: arm-disasm-generalization.patch

The ARM disassembler is fairly general and only requires that the back-end supply a simple "disassemble one instruction" method; it then takes care of label management, and has utilities for handling pc-relative literal loads and similar gunk.

It turns out to be reasonably painless to factor out the ARM disassembler and then implement it for ARM64, see the commit message for technical information.

Comment 1

[Lars T Hansen [:lth]]

Attachment: bug1430161-arm-disasm-generalization-v2.patch

This update fixes some bugs that were caused by the disassembler always being enabled for simulator builds, even Release builds.  As a consequence a number of the ifdefs were not quite right, and some (no-op) methods had to be provided for Release builds.

To be more resilient, this version disables the disassemblers in Simulator release builds; this requires a slight adjustment to the ARM simulator, which had a dependency on the disassembler for some instruction decoding (probably code I wrote).

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8942959 [details] [diff] [review]
bug1430161-arm-disasm-generalization-v2.patch

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

::: js/src/jit/arm64/vixl/MozBaseAssembler-vixl.h
@@ +127,5 @@
> +#ifdef JS_DISASM_ARM64
> +  virtual void spew(const vixl::Instruction* instr) = 0;
> +  virtual void spewBranch(const vixl::Instruction* instr, const LabelDoc& l) = 0;
> +  virtual void spewLiteralLoad(const vixl::Instruction* instr, const LiteralDoc& l) = 0;
> +  virtual LabelDoc refLabel(js::jit::Label* label) = 0;

Q: Could we remove the virtual / override, by moving the spew_ field to the MozBaseAssembler?
Why is the MozBaseAssembler under the vixl directory anyway?

::: js/src/jit/shared/Disassembler-shared.h
@@ +103,5 @@
> +            float    f32;
> +            double   f64;
> +        } value;
> +        LiteralDoc() : type(Type::Patchable) {}
> +        LiteralDoc(int32_t v) : type(Type::I32) { value.i32 = v; }

nit: explicit keyword.

Comment 3

[Lars T Hansen [:lth]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> Comment on attachment 8942959 [details] [diff] [review]
> bug1430161-arm-disasm-generalization-v2.patch
> 
> Review of attachment 8942959 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit/arm64/vixl/MozBaseAssembler-vixl.h
> @@ +127,5 @@
> > +#ifdef JS_DISASM_ARM64
> > +  virtual void spew(const vixl::Instruction* instr) = 0;
> > +  virtual void spewBranch(const vixl::Instruction* instr, const LabelDoc& l) = 0;
> > +  virtual void spewLiteralLoad(const vixl::Instruction* instr, const LiteralDoc& l) = 0;
> > +  virtual LabelDoc refLabel(js::jit::Label* label) = 0;
> 
> Q: Could we remove the virtual / override, by moving the spew_ field to the
> MozBaseAssembler?

That worked very well, thank you.

> Why is the MozBaseAssembler under the vixl directory anyway?

Oh, I have a lot of questions about why things are the way they are in the ARM64 backend, but I fear we'll have to ask Sean or Marty about that.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/045ded11d3f8
Factor ARM disassembler, implement for ARM64.  r=nbp

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/045ded11d3f8