1168807 - Clean-up: Move MacroAssemblerSpecific::framePushed to the generic MacroAssembler.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp]]

Each MacroAssembler redefine the same framePushed logic.  We should move the frame logic into the generic MacroAssembler.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Move MacroAssemblerSpecific::framePushed_ fields to the generic MacroAssembler.

Sorry, this is quite a big patch.

The goal of this patch is to move all framePushed_ references to the generic
MacroAssembler.  This implies creating a jit/MacroAssembler-inl.h file, as
these functions are used quite frequently.

Also, as we cannot include "-inl.h" in any ".h" header, and reference any
asMasm() implementation, we have to either move some functions to the
respective "-inl.h" file or to the ".cpp" file.  This last constraint make
this patch quite verbose compared to the expected modification.

I tested this patch locally against x64, x86, arm, mips(*), and with Ion disabled.

(*) Mips backend does not build, but I removed all errors related to the
current modifications.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 8611241 [details] [diff] [review]
Move MacroAssemblerSpecific::framePushed_ fields to the generic MacroAssembler.

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

Nice!

::: js/src/jit/MacroAssembler.h
@@ +281,5 @@
> +    // This field is used to statically (at compilation time) emulate a frame
> +    // pointer by keeping track of stack manipulations.
> +    //
> +    // It is maintained by all stack manipulation functions below.
> +    uint32_t framePushed_;

This field should be private or protected.

::: js/src/jit/x86/MacroAssembler-x86.cpp
@@ +599,5 @@
> +
> +void
> +MacroAssembler::freeStack(Register amount)
> +{
> +    addl(amount, StackPointer);

I wonder if this could be |addPtr(amount, StackPointer);| and shared between platforms. Doesn't matter too much though.

Comment 3

[Nicolas B. Pierron [:nbp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ade1289c095a

Delta:
 - Moved CodeGeneratorSpecific::ToOperand to CodeGeneratorShared::ToOperand (somehow, I did not noticed that before)
 - framePushed_ is now private.
 - MacroAssembler::freeStack is no longer per-arch.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/06ca9c794fd0

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/06ca9c794fd0