Integers are not always converted to Smis by a left-shift by one. While we
can
assume that on platform-dependent code (ia32, arm and mips), we avoid using
that
assumption on platform-independent code because on x64 a Smi is converted
by a
left-shift by 32. Code that intend to store integers are Smis should
therefore
use Smi::FromInt() and Smi::cast()->value().
http://codereview.chromium.org/10263002/diff/1006/src/debug.cc
File src/debug.cc (right):
http://codereview.chromium.org/10263002/diff/1006/src/debug.cc#newcode2234
src/debug.cc:2234: const int Debug::FramePaddingLayout::kPaddingValue =
(kInitialSize + 1) * 2;
Use Smi::FromInt to get tagged smi values since x64 tags differently.
Why does the value need to be larger than kInitialSize (smi-tagged)?
http://codereview.chromium.org/10263002/diff/1006/src/debug.h
File src/debug.h (right):
http://codereview.chromium.org/10263002/diff/1006/src/debug.h#newcode487
src/debug.h:487: struct FramePaddingLayout {
we would typically define this as a class deriving from AllStatic (e.g.
class ArgumentsAdaptorFrameConstants : public AllStatic)
http://codereview.chromium.org/10263002/diff/1006/src/debug.h#newcode493
src/debug.h:493: static const int kFrameBaseSize = 4;
This coincides with Debug::kFrameDropperFrameSize, which is also 4. If
they are derived similarly, can't we consistently use only one of the
two?
http://codereview.chromium.org/10263002/diff/1006/src/ia32/debug-ia32.cc
File src/ia32/debug-ia32.cc (right):
http://codereview.chromium.org/10263002/diff/1006/src/ia32/debug-ia32.cc#newcode112
src/ia32/debug-ia32.cc:112: __ push(Immediate(2 *
Debug::FramePaddingLayout::kInitialSize));
I assume multiplying by 2 is for Smi-tagging? Use
Smi::FromInt(Debug::FramePaddingLayout::kInitialSize) instead.
http://codereview.chromium.org/10263002/diff/1006/src/ia32/debug-ia32.cc#newcode176
src/ia32/debug-ia32.cc:176: __ lea(esp, Operand(esp, unused_reg,
times_half_pointer_size, 0));
This is based on the implication that Smis are represented by left shift
by 1 bit. Please add a STATIC_ASSERT(kSmiTagSize == 1) or a comment to
state this implication.
http://codereview.chromium.org/10263002/diff/1006/src/liveedit.cc
File src/liveedit.cc (right):
http://codereview.chromium.org/10263002/diff/1006/src/liveedit.cc#newcode1530
src/liveedit.cc:1530: if (Memory::int_at(padding_pointer) / 2 *
kPointerSize < shortage_bytes) {
I suppose this is a Smi-untagging.
Use Smi::cast(Memory::Object_at())->value() instead would be cleaner,
especially since x64 smi-tags differently.
http://codereview.chromium.org/10263002/diff/1006/src/liveedit.cc#newcode1534
src/liveedit.cc:1534: Memory::int_at(padding_pointer) -= shortage_bytes
/ kPointerSize * 2;
Same here.
http://codereview.chromium.org/10263002/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
