WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
22867
Add support to X86Assembler emitting instructions that access all 16 registers on x86-64.
https://bugs.webkit.org/show_bug.cgi?id=22867
Summary
Add support to X86Assembler emitting instructions that access all 16 register...
Gavin Barraclough
Reported
2008-12-15 12:33:33 PST
Add a new formating class, that is reponsible for both emitting the opcode bytes and the ModRm bytes of an instruction in a single call; this can insert the REX byte as necessary before the opcode, but has access to the register numbers to build the REX.
Attachments
The patch
(106.21 KB, patch)
2008-12-15 12:34 PST
,
Gavin Barraclough
ggaren
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Gavin Barraclough
Comment 1
2008-12-15 12:34:26 PST
Created
attachment 26028
[details]
The patch
Geoffrey Garen
Comment 2
2008-12-15 13:42:40 PST
Comment on
attachment 26028
[details]
The patch
> + (JSC::X86Assembler::X86InstructionFormater::prefix):
"Formater" is a misspelling for "formatter." You should do a global find/replace to change to X86InstructionFormatter, m_formatter, formatter, etc.
> + // FIXME: this won't work on 64-bit! > + // need to make those 'xchgl_rr's be 'xchgq_rr's (we don't care about the top bits of the shift value, > + // but ecx might have contained a pointer! > void lshift32(RegisterID shift_amount, RegisterID dest) > { > // On x86 we can only shift by ecx; if asked to shift by another register we'll
Yikes?
> - void subl_mr(int offset, RegisterID base, RegisterID dst) > + void cmpl_im_force32(int imm, int offset, RegisterID base)
Can we just call this cmpl_i32m, since the normal operation is now called cmpl_im?
> + > +#define REX(r, x, b) (PRE_REX | ((r>>3)<<2) | ((x>>3)<<1) | (b>>3)) > +#if PLATFORM(X86_64) > +#define EMIT_REX_W(r, x, b) m_buffer.putByteUnchecked(REX(r, x, b) | 8) > +#define EMIT_REX_IF(c, r, x, b) if (c) m_buffer.putByteUnchecked(REX(r, x, b)) > +#else > +#define EMIT_REX_IF(c, r, x, b) > +#endif > +#define IS_REG_R8_UP(r) (r >= X86::r8) > +#define IS_REG_SIL_UP(r) (r >= X86::esp) > +#define EMIT_REX_IF_ANY_ARE_R8_UP(r, x, b) EMIT_REX_IF(IS_REG_R8_UP(r)|IS_REG_R8_UP(x)|IS_REG_R8_UP(b), r, x, b)
These should be inline functions. It's OK to have an empty inline function in the non-64 case.
> + // Immediates: > + // > + // An immedaite should be appended where appropriate after an op has been emitted. > + // The writes are unchecked since the opcode formaters above will have ensured space. > + > + void instructionImmediate8(int imm) > + { > + m_buffer.putByteUnchecked(imm); > + } > + > + void instructionImmediate32(int imm) > + { > + m_buffer.putIntUnchecked(imm); > + } > + > + JmpSrc instructionRel32() > + { > + m_buffer.putIntUnchecked(0); > + return JmpSrc(m_buffer.size()); > + }
I think you can remove the "instruction" prefix from these function names. Implicitly, all Formatter operations deal with instructions.
> +#define MODRM(type, reg, rm) ((type << 6) | ((reg & 7) << 3) | (rm & 7)) > +#define SIB(type, reg, rm) MODRM(type, reg, rm)
These should be inline functions. What's up with lshift32? r=me other than that.
Gavin Barraclough
Comment 3
2008-12-15 15:39:01 PST
cmpl_im_force32 function name will change in a later patch, as agreed on irc. Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/assembler/AssemblerBuffer.h Sending JavaScriptCore/assembler/MacroAssembler.h Sending JavaScriptCore/assembler/X86Assembler.h Sending JavaScriptCore/jit/JIT.cpp Sending JavaScriptCore/jit/JITArithmetic.cpp Sending JavaScriptCore/jit/JITCall.cpp Sending JavaScriptCore/jit/JITPropertyAccess.cpp Transmitting file data ........ Committed revision 39316.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug