Presently the top 32-bits of a 64-bit JSImmediate serve as a sign extension of a 31-bit int stored in the low word (shifted left by one, to make room for a tag). In the new format, the top 31-bits serve as a sign extension of a 32-bit int, still shifted left by one.
Created attachment 26355 [details] The patch
Comment on attachment 26355 [details] The patch > + The new benhavior is enabled using a flag in Platform.h, 'WTF_USE_ALTERNATE_JSIMMEDIATE'. Typo, "benhavior". > + When this is set the contatants defining the range of ints allowed to be stored as Typo, "contatants". > + format. This patch updates tehe JIT so that it can also operate with the new format. Typo, "tehe". > -#define CAN_SIGN_EXTEND_8_32(value) (value == ((int)(signed char)value)) > +#define CAN_SIGN_EXTEND_8_32(value) ((int32_t)value == ((int32_t)(signed char)value)) > +#if PLATFORM(X86_64) > +#define CAN_SIGN_EXTEND_32_64(value) ((intptr_t)value == ((intptr_t)(int32_t)value)) > +#define CAN_SIGN_EXTEND_U32_64(value) ((intptr_t)value == ((intptr_t)(uint32_t)value)) > +#endif Can these be inline functions rather than macros? I don't see any benefit to using a macro for this kind of test. > + void compileFastArith_op_add(Instruction* currentInstruction); > + void compileFastArith_op_mul(Instruction* currentInstruction); I think we could omit the argument name "currentInstruction" with no loss of clarity, and so we should. > + void compileFastArithSlow_op_add(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_mul(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter); Ditto. Also we should definitely leave out the name "iter", which adds nothing. > + void compileFastArithSlow_op_mod(unsigned result, unsigned op1, unsigned op2, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_bitand(unsigned result, unsigned op1, unsigned op2, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_lshift(unsigned result, unsigned op1, unsigned op2, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_rshift(unsigned result, unsigned op1, unsigned op2, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_pre_inc(unsigned srcDst, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_pre_dec(unsigned srcDst, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_post_inc(unsigned result, unsigned srcDst, Vector<SlowCaseEntry>::iterator& iter); > + void compileFastArithSlow_op_post_dec(unsigned result, unsigned srcDst, Vector<SlowCaseEntry>::iterator& iter); Please leave out the "iter" here. > +#if USE(ALTERNATE_JSIMMEDIATE) > +#else > +#endif Pleas remove this. > + > +#if USE(ALTERNATE_JSIMMEDIATE) > +#else > +#endif > + > + > + > + > + > #if !ENABLE(JIT_OPTIMIZE_ARITHMETIC) Please remove this. > -static ALWAYS_INLINE uintptr_t asInteger(JSValue* value) > -{ > - return reinterpret_cast<uintptr_t>(value); > -} I like to use these sorts of inline functions to add type safety to the many reinterpret_cast that are needed in the code. I notice that the new code does the casts directly. You should consider the inline function approach. r=me
Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/assembler/MacroAssembler.h Sending JavaScriptCore/assembler/X86Assembler.h Sending JavaScriptCore/jit/JIT.cpp Sending JavaScriptCore/jit/JIT.h Sending JavaScriptCore/jit/JITArithmetic.cpp Sending JavaScriptCore/jit/JITInlineMethods.h Sending JavaScriptCore/runtime/JSImmediate.h Sending JavaScriptCore/wtf/Platform.h Transmitting file data ......... Committed revision 39540.