IonMonkey: Add fstp instruction to the macro assembler

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: ejpbruel, Assigned: ejpbruel)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
In order to extend callWithABI to work with doubles (see bug 709423), we need a way to fetch the result of an ABI call from the floating point stack on x86. The fstp instruction allows us to pop a value of the floating point stack and put it into memory. From there on, we can move it into xmm0 (where it is put by default on x64), from where it can be accessed.
(Assignee)

Comment 1

6 years ago
Created attachment 587837 [details] [diff] [review]
Initial proposal

The enum OneByteOpcodeID is used to identify the primary opcode. For opcodes that have an opcode extension in the reg field of the Mod R/M byte, the value of OneByteOpcodeID is of the form OP_GROUP<n>_... and the enum GroupOpcodeID is used to identify the opcode extension.

For floating point instructions, there are 8 such opcode groups, with primary opcodes ranging from 0xD8 to 0xDF (see http://ref.x86asm.net/geek32.html). The fstp instruction has primary opcode 0xDD, and is thus in the 6th opcode group. Based on this, and in an attempt to maintain consistency with the existing naming scheme, I've introduced the value OP_FPU6 to the enum OneByteOpcodeID and the value FPU6_OP_FSTP to the enum GroupOpcodeID. Together, these two values identify the fstp instruction.

The addressing scheme for fstp follows basic Mod R/M addressing, except that general purpose registers cannot be addressed (they are replaced with the floating point stack registers). The Nitro assembler is smart enough to recognize that it has to generate an SIB byte if we want to address memory via the ESP register (that is, fstp [ESP]), but it only does so if we also pass an offset (if this offset is 0, no displacement byte is generated).
Attachment #587837 - Flags: review?(dvander)
Attachment #587837 - Flags: review?(dvander) → review+
Sorry for not thinking of that earlier, http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/assembler/X86Assembler.h#L1006.

We should really start sharing macro assembler, between JSC and use, I am going to try reaching out to them tomorrow.
(Assignee)

Updated

6 years ago
Assignee: general → ejpbruel
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.