New version uploaded
http://codereview.chromium.org/6682026/diff/1/src/x64/assembler-x64.cc File src/x64/assembler-x64.cc (right): http://codereview.chromium.org/6682026/diff/1/src/x64/assembler-x64.cc#newcode1603 src/x64/assembler-x64.cc:1603: movq(dst, reinterpret_cast<int64_t>(value), RelocInfo::NONE); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Please consider using LoadSmi instead. It tries to optimize small smis
by lea's
on kSmiConstantRegister, and otherwise defaults to loading it verbatim
like
this.
Used LoadSmiConstant http://codereview.chromium.org/6682026/diff/1/src/x64/code-stubs-x64.cc File src/x64/code-stubs-x64.cc (right): http://codereview.chromium.org/6682026/diff/1/src/x64/code-stubs-x64.cc#newcode2254 src/x64/code-stubs-x64.cc:2254: Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR)); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
You can use Cmp(const Operand* dst, Handle<Object> source) here. If
you don't
want to make a handle, consider renaming this to just Cmp(..Operand..,
Smi*).
The "SmiX" operations all work on operands that are both smis.
Renamed to Cmp http://codereview.chromium.org/6682026/diff/1/src/x64/code-stubs-x64.cc#newcode4562 src/x64/code-stubs-x64.cc:4562: __ nop(); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Why the nop?
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/codegen-x64.cc File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/6682026/diff/1/src/x64/codegen-x64.cc#newcode2249 src/x64/codegen-x64.cc:2249: __ cmpq(left_side.reg(), right_side.reg()); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Why not SmiCompare? You are assuming the layout of smis outside of Smi
code,
please try to avoid that (if it really seems necessary, maybe it's
reason to
introduce another Smi-macro). Only use cmpq for pointer-equality test,
not any
of greater/less than.
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/codegen-x64.cc#newcode6810 src/x64/codegen-x64.cc:6810: deferred->Branch(below_equal); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Do use SmiCompare!
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/full-codegen-x64.cc File src/x64/full-codegen-x64.cc (right): http://codereview.chromium.org/6682026/diff/1/src/x64/full-codegen-x64.cc#newcode3035 src/x64/full-codegen-x64.cc:3035: __ cmpq(temp, index_2); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Do use SmiCompare.
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.cc File src/x64/macro-assembler-x64.cc (left): http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.cc#oldcode841 src/x64/macro-assembler-x64.cc:841: cmpq(dst, src); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Don't remove the SmiCompare function. Do use it when you compare smis
for
anything but equality! In general, don't make assumptions (in genrated code) about the
representation
of smis outside of the MacroAssembler smi code.
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.cc File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.cc#newcode865 src/x64/macro-assembler-x64.cc:865: } On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Adding asserts: Good!
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.cc#newcode893 src/x64/macro-assembler-x64.cc:893: cmpq(dst, kScratchRegister); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
This is wasteful, since it doesn't use the kSmiConstantRegister for
the values
where it can be used. It's better to use: Register const_reg = GetSmiConstant(kScratchRegister, src); cmpq(dst, const_reg); as in SmiCompare(Register,Smi*).
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.h File src/x64/macro-assembler-x64.h (right): http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.h#newcode281 src/x64/macro-assembler-x64.h:281: // Simple comparison of Smis. On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Actually, I believe we try to write "smi" as lower-case when referring
to the
concept, not the Class.
Done. http://codereview.chromium.org/6682026/diff/1/src/x64/macro-assembler-x64.h#newcode288 src/x64/macro-assembler-x64.h:288: void SmiCompareWithObject(const Operand& dst, Smi* src); On 2011/03/14 08:51:01, Lasse Reichstein wrote:
Rename them to Cmp and group them with the Cmp functions instead.
Done. http://codereview.chromium.org/6682026/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
