Closed Bug 1275994 Opened 4 years ago Closed 4 years ago

Document x86/x64 opcode operands

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
Unspecified
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: nbp, Assigned: nbp)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attachment #8757403 - Flags: review?(bbouvier)
Comment on attachment 8757403 [details] [diff] [review]
Document how x86 opcodes are named.

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

Nice, thanks. I think there are a few more to add though.

How about also adding a reference to http://ref.x86asm.net/geek.html, next to the top most comment? This is where I've tended to retrieve the mnemonics, to not scratch my head for too long.

::: js/src/jit/x86-shared/Encoding-x86-shared.h
@@ +15,5 @@
>  namespace X86Encoding {
>  
>  static const size_t MaxInstructionSize = 16;
>  
> +// See chapter A.2 for naming enumerated values:

Can you tell which volume? (it's volume 2)

@@ +21,5 @@
> +// E = general reg
> +// G = memory
> +// V = xmm
> +// W = xmm/mem64
> +// I = immediate

Can you add to this list:
- O for e.g. OP_MOV_EAXOv
- U for e.g. OP2_PSLLD_UdqIb

@@ +30,5 @@
> +// d = double
> +// ss = scalar float 32 (for xmm)
> +// ps = packed float 32 (for xmm)
> +// sd = scalar double (for xmm)
> +// pd = packed double (for xmm)

Can you add:
- dq (e.g. OP2_PADDD_VdqWdq)
- p (e.g. OP2_CMOVZ_GvqpEvqp)
- z (e.g. OP_PUSH_Iz)
Attachment #8757403 - Flags: review?(bbouvier)
Attachment #8757944 - Flags: review?(bbouvier)
Comment on attachment 8757944 [details] [diff] [review]
Document how x86 opcodes are named.

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

Thank you! (fwiw, you can commit this with DONTBUILD in the changeset message)

::: js/src/jit/x86-shared/Encoding-x86-shared.h
@@ +16,5 @@
>  
>  static const size_t MaxInstructionSize = 16;
>  
> +// These enumerated values are following the Intel documentation Volume 2C [1],
> +// Appendix A.2 and Append A.3.

nit: Append => Appendix

@@ +41,5 @@
> +// pd = packed double (xmm)
> +// z = 16/32/64-bit
> +// vqp = (*)
> +//
> +// (*) Some website [2], provide a convenient list of all instructions, but be

nit: remove the comma and "provide" => "provides"
Attachment #8757944 - Flags: review?(bbouvier) → review+
https://hg.mozilla.org/mozilla-central/rev/4d0006f1b696
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Attachment #8757403 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.