Closed Bug 1168807 Opened 4 years ago Closed 4 years ago

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


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox41 --- fixed


(Reporter: nbp, Assigned: nbp)


(Blocks 1 open bug)



(1 file)

Each MacroAssembler redefine the same framePushed logic.  We should move the frame logic into the generic MacroAssembler.
Assignee: nobody → nicolas.b.pierron
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.
Attachment #8611241 - Flags: review?(jdemooij)
Comment on attachment 8611241 [details] [diff] [review]
Move MacroAssemblerSpecific::framePushed_ fields to the generic MacroAssembler.

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


::: 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.
Attachment #8611241 - Flags: review?(jdemooij) → review+

 - Moved CodeGeneratorSpecific::ToOperand to CodeGeneratorShared::ToOperand (somehow, I did not noticed that before)
 - framePushed_ is now private.
 - MacroAssembler::freeStack is no longer per-arch.
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Depends on: 1176633
You need to log in before you can comment on or make changes to this bug.