Made the changes. I like the byte operations so I went with that, the math
to
calculate which byte and which bit within it is in object.h, close to where
the
smi packing for 64 bit is explained (it also accounts for smi tag). The
codegen
looks much cleaner.
Sorry about the presubmit.
Martin
http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):
http://codereview.chromium.org/6524006/diff/1/src/ia32/builtins-ia32.cc#newcode752
src/ia32/builtins-ia32.cc:752: kSmiTagSize)));
On 2011/02/15 08:50:12, Mads Ager wrote:
On 2011/02/15 08:46:55, Lasse Reichstein wrote:
> You can test directly against memory, without loading into ecx
first, i.e.,
> test(FieldOperand(ecx, SharedFunctionInfo::kCompilerHintsOffset),
> Immediate(...));
>
> You might also want to use a byte-test
> ASSERT_EQ(8, SharedFunctionInfo::kStrictModeFunction);
> test_b(FieldOperand(ecx, ... + 1), Immediate((1 << (...)) >> 8);
> It would allow an 8-bit immediate instead of a 32-bit one.
> (Ditto for X64, with the appropriate differences for encoding).
If you do use the byte version, please introduce some named constants
to use in
the immediate to make it readable. :-)
Done.
http://codereview.chromium.org/6524006/diff/1/src/objects.h
File src/objects.h (right):
http://codereview.chromium.org/6524006/diff/1/src/objects.h#newcode4390
src/objects.h:4390: public:
On 2011/02/15 08:09:31, Mads Ager wrote:
I would rather make Builtins a friend or make them all public.
Done.
http://codereview.chromium.org/6524006/diff/4001/src/objects.h#newcode4391
src/objects.h:4391: // Used from builtins-<arch>.cc
Generate_Function(Call/Apply).
Turns out similar trick for byte-wide operations is used for
kConstructionCount. I followed that for endian-ness and while I was at
it, included the smi shifting here, in the proximity of the comment that
explains the 64 bit packing.
The codegen looks quite clean this way.
http://codereview.chromium.org/6524006/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
