First round of comments.
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) Don't forget SetDateField. https://chromiumcodereview.appspot.com/9117034/diff/1/src/hydrogen-instructions.h#newcode4514 src/hydrogen-instructions.h:4514: class HDateField: public HUnaryOperation { This instruction has two arguments, so HBinaryOperation or HTemplateInstruction<2> seem more appropriate. https://chromiumcodereview.appspot.com/9117034/diff/1/src/hydrogen-instructions.h#newcode4518 src/hydrogen-instructions.h:4518: set_representation(Representation::Tagged()); Is it possible to use Int32 representation and deoptimize if NaN? 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); As discussed offline, it's better to crash if DateField is called not on JSDate. 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. Is it possible/faster to do VisitForAccumulatorValue(args->at(0)) and then mov ebx, eax? 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); As discussed offline, it's better to crash if DateField is called not on JSDate. 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, Can we avoid the write barrier by making sure that each field is set to either a smi or NaN? 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); See comments in full-codegen-ia32.cc 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", ms is a three digit number, so %04d should be %03d. 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 { 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. https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects.h#newcode5863 src/objects.h:5863: // [value]: the time value. Transitional, will disappear. As we discussed offline, the [value] should stay since it's difficult to reconstruct it. https://chromiumcodereview.appspot.com/9117034/diff/1/src/objects.h#newcode5871 src/objects.h:5871: // [day]: caches hours. [day] -> [hour] in the comment. https://chromiumcodereview.appspot.com/9117034/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
