I removed all type checks in generated code, except in debug mode. Removed write
barriers except for the value slot. Also added SetDateField instruction to
Hydrogen instead of going through the more expensive generic StoreNamed.



https://chromiumcodereview.appspot.com/9117034/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/9117034/diff/1/src/hydrogen-instructions.h#newcode183
src/hydrogen-instructions.h:183: V(DateField)
On 2012/01/25 13:08:48, ulan wrote:
Don't forget SetDateField.

There was no HSetDateField instruction. ;) However, it exists now.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/hydrogen-instructions.h#newcode4514
src/hydrogen-instructions.h:4514: class HDateField: public
HUnaryOperation {
On 2012/01/25 13:08:48, ulan wrote:
This instruction has two arguments, so HBinaryOperation or
HTemplateInstruction<2> seem more appropriate.

No, it's a unary instruction. The index is merely a static attribute.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/hydrogen-instructions.h#newcode4518
src/hydrogen-instructions.h:4518:
set_representation(Representation::Tagged());
On 2012/01/25 13:08:48, ulan wrote:
Is it possible to use Int32 representation and deoptimize if NaN?

Then I would need to implement the untagging in the instruction. I don't
see what the win would be. The integers cannot be untagged on the object
itself.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

https://chromiumcodereview.appspot.com/9117034/diff/1/src/ia32/full-codegen-ia32.cc#newcode2888
src/ia32/full-codegen-ia32.cc:2888: __ JumpIfSmi(eax, &done,
Label::kNear);
On 2012/01/25 13:08:48, ulan wrote:
As discussed offline, it's better to crash if DateField is called not
on JSDate.

I removed all (non-debug-mode) type checks, and checked through all
call-sites. Actually uncovered a hidden bug in the existing Date.UTC.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/ia32/full-codegen-ia32.cc#newcode2951
src/ia32/full-codegen-ia32.cc:2951: VisitForStackValue(args->at(0));  //
Load the object.
On 2012/01/25 13:08:48, ulan wrote:
Is it possible/faster to do
VisitForAccumulatorValue(args->at(0)) and then mov ebx, eax?

No, the second Visit can clobber any register. Keep in mind that the
full code-gen implements a stack machine.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/ia32/full-codegen-ia32.cc#newcode2957
src/ia32/full-codegen-ia32.cc:2957: __ JumpIfSmi(ebx, &done,
Label::kNear);
On 2012/01/25 13:08:48, ulan wrote:
As discussed offline, it's better to crash if DateField is called not
on JSDate.

Done.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/ia32/full-codegen-ia32.cc#newcode2969
src/ia32/full-codegen-ia32.cc:2969: __ RecordWriteField(ebx,
JSDate::kYearOffset + kPointerSize * index,
On 2012/01/25 13:08:48, ulan wrote:
Can we avoid the write barrier by making sure that each field is set
to either a
smi or NaN?

Done.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

https://chromiumcodereview.appspot.com/9117034/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode1289
src/ia32/lithium-codegen-ia32.cc:1289: __ JumpIfSmi(input, &done,
Label::kNear);
On 2012/01/25 13:08:48, ulan wrote:
See comments in full-codegen-ia32.cc

Done.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects-printer.cc
File src/objects-printer.cc (right):

https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects-printer.cc#newcode655
src/objects-printer.cc:655: PrintF(out, " - time = %04d/%02d/%02d
%02d:%02d:%02d.%04d\n",
On 2012/01/25 13:08:48, ulan wrote:
ms is a three digit number, so %04d should be %03d.

Done.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects.h#newcode5860
src/objects.h:5860: class JSDate: public JSObject {
On 2012/01/25 13:08:48, ulan wrote:
As we discussed offline, all these fields could be packed into few
SMIs and we
could designate one bit in each SMI as "the date is a NaN" indicator.
A possible
layout would be:

[value]: the time value.

[year_month_day]: SMI that caches the year in bits[31:12], the month
in
bits[11:8], the day in bits[7:3], the is_cache_filled flag in bit[2],
and the
is_nan flag in bit[1].

[hour_min_sec_ms]: SMI that caches the components similar to
[year_month_day].

Advantages of this layout:
- the caches can be filled in lazily;
- less memory usage and CPU cache pressure.

Disadvantage:
- need to check is_nan flag in the getters.

Yeah, I don't want to optimize this prematurely. The whole patch is for
benchmark speed, so I'd like to avoid the extra arithmetic for now and
see where we stand.  Frankly, I don't think space usage matters much for
dates anyway, not on benchmarks and even less so in reality. But let's
see.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects.h#newcode5863
src/objects.h:5863: // [value]: the time value. Transitional, will
disappear.
On 2012/01/25 13:08:48, ulan wrote:
As we discussed offline, the [value] should stay since it's difficult
to
reconstruct it.

Done.

https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects.h#newcode5871
src/objects.h:5871: // [day]: caches hours.
On 2012/01/25 13:08:48, ulan wrote:
[day] -> [hour] in the comment.

Done. (And for month above.)

https://chromiumcodereview.appspot.com/9117034/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to